Skip to content

fix: Enter in filtering mode now confirms selection in MultiSelect#165

Merged
jdx merged 1 commit into
jdx:mainfrom
gaojunran:fix/multiselect-enter-filtering
May 18, 2026
Merged

fix: Enter in filtering mode now confirms selection in MultiSelect#165
jdx merged 1 commit into
jdx:mainfrom
gaojunran:fix/multiselect-enter-filtering

Conversation

@gaojunran
Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR fixes filtering mode in MultiSelect so that pressing Enter immediately confirms the current selection rather than merely saving the filter text and returning to normal mode. It achieves this by extracting the existing inline validation logic into a reusable try_confirm() method and calling it from both the filtering-mode and normal-mode Key::Enter arms, and updates the help hint from "save filter" to "confirm".

  • try_confirm() collects the currently-selected labels, validates them against self.min/self.max, sets self.err on failure, and returns None — consistent with the pre-existing non-filtering Enter behaviour.
  • The filtering-mode Key::Enter arm now calls try_confirm() and, on success, runs the same cleanup + render + return path that normal mode uses; the non-filtering arm is identically refactored to remove the old inlined validation.

Confidence Score: 5/5

The change is safe to merge; it correctly unifies two code paths around a shared helper and the guard that previously silenced Enter in filtering mode has been removed.

The extraction of try_confirm() is accurate and both call sites produce identical behaviour. No logic was lost and the help-hint mismatch noted in earlier review rounds is addressed in this diff.

No files require special attention.

Important Files Changed

Filename Overview
src/multiselect.rs Extracts try_confirm() helper and wires it into the filtering-mode Enter key, so pressing Enter while filtering now confirms the whole selection instead of just saving the filter text; help hint updated from "save filter" to "confirm".

Reviews (4): Last reviewed commit: "fix: Enter in filtering mode now confirm..." | Re-trigger Greptile

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 refactors the MultiSelect component by extracting selection validation into a new try_confirm method, reducing duplication in the Key::Enter handling for both filtering and non-filtering states. The reviewer suggests further deduplication by moving terminal cleanup and rendering logic into a dedicated confirm helper method. Additionally, an unnecessary guard in the filtering branch was identified that could prevent users from confirming valid selections when no options match the current filter.

Comment thread src/multiselect.rs Outdated
Comment thread src/multiselect.rs
Comment thread src/multiselect.rs
@gaojunran gaojunran force-pushed the fix/multiselect-enter-filtering branch from ae324bd to cfe0003 Compare May 18, 2026 11:56
@gaojunran
Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/multiselect.rs Outdated
Comment thread src/multiselect.rs Outdated
@gaojunran gaojunran force-pushed the fix/multiselect-enter-filtering branch from cfe0003 to 16908b4 Compare May 18, 2026 12:04
@gaojunran
Copy link
Copy Markdown
Contributor Author

@greptileai

@gaojunran gaojunran marked this pull request as ready for review May 18, 2026 12:10
@jdx jdx merged commit da7ade9 into jdx:main May 18, 2026
6 checks passed
@jdx jdx mentioned this pull request May 18, 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