Skip to content

Conversation

@lukhnos
Copy link
Collaborator

@lukhnos lukhnos commented Dec 17, 2025

Shift+[1-9] keys were not handled when Shift+Enter was disabled for associated phrases. With the v2 associated phrases always prompting when such phrases are available, those keys need to be handled.

Shift+[1-9] keys were not handled when Shift+Enter was disabled for
associated phrases. With the v2 associated phrases always prompting when
such phrases are available, those keys need to be handled.
Copilot AI review requested due to automatic review settings December 17, 2025 17:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where Shift+[1-9] keys were not being handled for associated phrases when the Shift+Enter feature was disabled. With v2 associated phrases always prompting when available, these selection keys need to be handled regardless of the Shift+Enter configuration setting.

  • Removes the dependency on config_.shiftEnterEnabled.value() from the Shift key handling logic
  • Updates the comment to accurately reflect all three conditions that trigger Shift key handling

Copy link

@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

The change correctly enables Shift+[1-9] key handling for associated phrases, aligning with the PR's goal. However, this change also brings a pre-existing bug with space key handling to the forefront, which I've detailed in a specific comment. Addressing this would significantly improve the user experience for the v2 associated phrases feature.

@zonble zonble merged commit db6e8f1 into openvanilla:master Dec 18, 2025
20 checks passed
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