Skip to content

fix: allow ArrowUp/ArrowDown/Space during filtering in multiselect#164

Merged
jdx merged 1 commit into
jdx:mainfrom
gaojunran:main
May 17, 2026
Merged

fix: allow ArrowUp/ArrowDown/Space during filtering in multiselect#164
jdx merged 1 commit into
jdx:mainfrom
gaojunran:main

Conversation

@gaojunran
Copy link
Copy Markdown
Contributor

@gaojunran gaojunran commented May 17, 2026

pitchfork relies on this fix!

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enables navigation and item toggling within the filtering mode of the MultiSelect component. Feedback highlights a potential panic risk due to unclamped cursor indices when the filtered list size changes, and a UX conflict where using the Space key for toggling prevents multi-word search queries. It is recommended to use the Tab key for toggling and to ensure the cursor is properly clamped.

Comment thread src/multiselect.rs Outdated
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 17, 2026

Greptile Summary

This PR enables ArrowUp, ArrowDown, and the toggle key (defaulting to Space) during filtering in MultiSelect, and introduces a configurable toggle_key builder so callers can choose a different key when they need Space for filter queries.

  • Navigation during filtering: handle_down/handle_up are now routed from the filtering match branch, and handle_toggle is reached when toggle_key is pressed while the filter is active.
  • Stale-cursor safety: self.cursor is reset to 0 in both handle_filter_key and handle_filter_backspace so narrowing the list never leaves the cursor pointing past the visible results; handle_toggle also clamps with .min(visible_options.len().saturating_sub(1)) to guard against any remaining out-of-bounds access.
  • Help text: The filtering-mode help bar now lists ↑/↓, the toggle key label, esc, and enter, so the new bindings are discoverable.

Confidence Score: 5/5

Safe to merge — all previously flagged defects are addressed and no new issues were found.

The stale-cursor bug, the out-of-bounds access in handle_toggle, and the missing help-text updates are all resolved in this diff. The toggle_key guard arm is correctly ordered ahead of the Char wildcard so Space interception works as designed. No regressions were found in the existing rendering or paging logic.

No files require special attention.

Important Files Changed

Filename Overview
src/multiselect.rs Adds ArrowUp/ArrowDown/Space navigation during filtering, configurable toggle_key, cursor-reset on filter change, OOB-safe handle_toggle, and updated help text — all previous P1 issues resolved

Reviews (5): Last reviewed commit: "fix(multiselect): support filtering navi..." | Re-trigger Greptile

@gaojunran gaojunran marked this pull request as draft May 17, 2026 11:36
@gaojunran
Copy link
Copy Markdown
Contributor Author

@greptileai

@gaojunran
Copy link
Copy Markdown
Contributor Author

@greptileai

@gaojunran gaojunran marked this pull request as ready for review May 17, 2026 11:55
@jdx jdx merged commit ad9aaf6 into jdx:main May 17, 2026
6 checks passed
@jdx jdx mentioned this pull request May 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants