Scroll through all fuzzy-find results, with a preference toggle - #84
Open
charlesangus wants to merge 5 commits into
Open
charlesangus wants to merge 5 commits into
charlesangus wants to merge 5 commits into
Conversation
NodeModel.rowCount() returned min(num_items, len(_items)), so a search matching more anchors than the popup's 18 rows silently dropped the rest with no way to reach them. Introduce NodeModel.max_items — how many matched rows the model retains — separately from num_items, which stays what the fixed-height popup is sized to show. With scrolling enabled max_items rises to DEFAULT_SCROLL_MAX_ITEMS; with it disabled it equals num_items, reproducing the old behaviour exactly. Native QListView scrolling (wheel, scrollbar, and arrow keys via setCurrentIndex's autoScroll) does the rest once rowCount() exceeds the viewport, so the vertical scrollbar policy and per-pixel scroll mode are all the widget needs. The popup window itself is unchanged in size. set_scroll_enabled() lets a cached widget pick up a preference change, and scroll_enabled is threaded through launch()/preload() to keep this vendored core's entry points in step with tabtabtab-nuke's.
Stores picker_scroll_enabled in anchors_prefs.json, defaulting to on so the pickers show every match out of the box. It is an anchors preference rather than one followed from tabtabtab-nuke: the existing "Use tabtabtab-nuke preferences" checkbox is documented as governing the space-prefix mapping alone, and widening it would quietly change what an already-saved choice means. All three picker widgets are constructed with the preference, and the two cached ones have it re-applied on the reuse path — mirroring _apply_space_mode_order — so a change takes effect on the next picker rather than the next Nuke session.
Follows the existing plain-checkbox pattern, sits under the Space-prefix search modes group since both govern the fuzzy-find pickers, and is flushed on OK like the other toggles. Nothing to validate and nothing to grey out — unlike the mapping above it, this preference is always the user's own.
The core is loaded from source under purpose-built Qt stubs, since conftest.py replaces tabtabtab_anchors with a stub for every other test file. That lets the retention rules be asserted against the real NodeModel: what survives the cap with scrolling on and off, that the cap is raised rather than removed, and that re-applying an unchanged preference costs no re-render. The rest is wiring: the popup is still sized from num_items and not from the retained row count, the preference round-trips through anchors_prefs.json and shrugs off a corrupt value, OK flushes the checkbox, and all three pickers — plus both cached reuse paths — are handed the preference.
Adds the toggle to both preference lists and a short "Scrolling through the results" section to the user guide, next to the space-prefix modes it sits beside in the dialog. Regenerated the committed PDF with `make pdf`. No screenshot changes: nothing in docs/img/ depicts the preferences dialog, and the picker screenshots are unaffected — the popup keeps its fixed size, and the example comp the pickers load has far fewer anchors than fill it, so no scrollbar appears.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The user guide Markdown was updated but the committed generated PDF (docs/anchors-user-guide.pdf) is not updated in this PR, leaving the published artifact out of sync.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes fuzzy-find pickers silently dropping matches beyond the first 18 results by separating row retention from popup sizing, enabling native QListView scrolling, and adding a user preference toggle (picker_scroll_enabled, default on) that applies to newly opened and cached pickers.
Changes:
- Update the vendored fuzzy-find core so
NodeModelretains up tomax_items(default 300 when scrolling is enabled) while the popup remains sized bynum_items. - Add
picker_scroll_enabledpreference persistence + a Preferences dialog checkbox, and plumb the setting through all picker construction and cached reuse paths. - Add a comprehensive new test suite covering model retention, UI setup, preference persistence/validation, dialog flushing, and picker wiring.
File summaries
| File | Description |
|---|---|
tabtabtab_anchors.py |
Adds scroll-enabled retention cap (max_items), enables QListView vertical scrolling, and re-applies scrolling on cached-widget reuse paths. |
prefs.py |
Adds picker_scroll_enabled default, load-time type validation, and persistence to anchors_prefs.json. |
colors.py |
Adds a Preferences dialog checkbox for the scroll setting and flushes it on OK. |
anchor.py |
Passes scroll_enabled into all picker constructions and reapplies it for cached widgets. |
tests/test_picker_scroll.py |
New test coverage for scrolling behavior, preference persistence/validation, dialog flushing, and picker wiring. |
README.md |
Documents the new preference and its behavior. |
docs/user-guide.md |
Adds preference documentation and a new “Scrolling through the results” section. |
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #82
Adopts the scrollable-results behaviour and preference that tabtabtab-nuke PR
#19 adds, applied to our
copy of the fuzzy-find widget.
The problem
NodeModel.rowCount()returnedmin(self.num_items, len(self._items)).num_itemsis 18 — the number of rows the popup is sized to show — so on a script with more
than eighteen matching anchors, every match past the eighteenth was silently
dropped. The
A/Alt+Apickers had no vertical scrollbar and no way to reachthem.
The change
Row retention is now separate from window sizing:
NodeModel.max_itemsdecides how many matched rows the model keeps.num_itemskeeps its old job of sizing the fixed-height popup, which does notchange size.
max_itemsrises toDEFAULT_SCROLL_MAX_ITEMS(300). With itoff,
max_items == num_items, reproducing the old behaviour exactly.QListViewscrolling does the rest oncerowCount()exceeds the viewport— mouse wheel, scrollbar, and arrow keys via
setCurrentIndex'sautoScroll—so the widget only needed the vertical scrollbar policy and a per-pixel scroll
mode.
A
picker_scroll_enabledpreference (default on) is stored inanchors_prefs.jsonand surfaced as a checkbox in the preferences dialog. Allthree pickers are built with it, and the two cached ones have it re-applied on the
reuse path via
_apply_scroll_enabled— mirroring_apply_space_mode_order— so achange lands on the next picker rather than the next Nuke session.
Notes on two judgement calls
Why not follow tabtabtab-nuke's own
scroll_enabled? The existing Usetabtabtab-nuke preferences checkbox is documented (README, user guide) as
governing the space-prefix mapping alone. Widening it would quietly change what an
already-saved choice means, so this is an anchors preference of its own.
Why not port upstream's lazy colour resolution? Upstream needed it because its
update()resolved colour only for the visible window, and raising the retentioncap would have widened that cost. Our copy already resolves colour for every
matched candidate inside the scoring loop, so raising the cap adds no colour cost
here and nothing regresses. Left out as unrelated scope.
Testing
tests/test_picker_scroll.py(24 new tests): retention under both settings, thecap being raised rather than removed, an unchanged preference costing no
re-render, the popup still being sized from
num_items, preference persistenceand corrupt-value rejection, the dialog flushing the checkbox, and all three
pickers plus both cached reuse paths receiving the preference.
Docs
Both preference lists and a new "Scrolling through the results" section in the user
guide;
docs/anchors-user-guide.pdfregenerated withmake pdf. No screenshotschanged — nothing in
docs/img/depicts the preferences dialog, and the pickerscreenshots are unaffected since the popup keeps its fixed size and the example
comp has far fewer anchors than fill it.