Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add <C-p> and <C-n> to the select component for moving the cursor up/down #779

Merged
merged 1 commit into from
Mar 6, 2024
Merged

Conversation

hankertrix
Copy link
Contributor

@hankertrix hankertrix commented Mar 5, 2024

Particularly, Ctrl + n and Ctrl + p to move down and up the completion menu respectively.

Closes #778

@sxyazi
Copy link
Owner

sxyazi commented Mar 6, 2024

Thank you for the PR!

C-n and C-p look good to me, but I'm not keen on adding q for the following reasons:

  • We already have Esc and Ctrl-q (which will be changed to Ctrl-c in the next major version), so it doesn't seem necessary to introduce another key.
  • It's not always available, which adds to the user's cognitive load. For example, as you mentioned, it can't be used in input insert mode. That's one reason I introduced C-q besides Esc. I wanted a key that could provide a unified and consistent operating experience, while Esc is used to enter normal mode and cannot exit input in insert mode.

@hankertrix
Copy link
Contributor Author

Ah I see, that makes a lot of sense.

I'll remove the commit for having q close the selection menu.

These 2 key binds are for moving the cursor down and up respectively in
the completion menu.
@sxyazi sxyazi changed the title feat: add a few more sensible key binds feat: add <C-p> and <C-n> to the select component for moving the cursor up/down Mar 6, 2024
@sxyazi
Copy link
Owner

sxyazi commented Mar 6, 2024

All looks good to me now, let me merge it!

@sxyazi sxyazi merged commit b6e458f into sxyazi:main Mar 6, 2024
5 checks passed
@sxyazi sxyazi mentioned this pull request Mar 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a few more sensible key bindings
2 participants