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

Add ctrl n, p shortcuts, don't do anything with unknown control sequences #330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ayroblu
Copy link

@ayroblu ayroblu commented Sep 4, 2021

No description provided.

@posva
Copy link

posva commented Jul 20, 2022

This would be really nice to have as ctrl+n and ctrl+p are common shortcuts in terminals (and vim) to cycle through options without having to reach out for arrow keys. It's also more common than #365 (BTW fzf also works with ctrl+n and ctrl+p).

It also ignores ctrl sequences which currently add invisible characters to the prompt that must be deleted by pressing backspace so that's a great addition too.

@terkelg is there anything missing in this PR for you?

@posva
Copy link

posva commented Feb 1, 2023

Hey @joeykilpatrick, do you know if the author of this package is still active? I noticed you merged the last PR but it hasn't been released

@joeykilpatrick
Copy link
Contributor

I don't want to speak for @terkelg, but I think part of the hesitation for merging these nice-to-have PRs is that

  1. they add opinionated functionality that is not customizable
  2. there are no regression tests for these features, and there isn't currently an easy way to write those tests

For #1, this package needs a solution for adding this type of custom feature to a prompt type without having to include it in this package. One thought would be to implement something like enquirer's custom prompts feature, which uses ES6 classes that can be extended to change the prompt's base functionality.

For #2, this package needs a solution for testing, both for this package and for dependent packages. There was a test branch to begin implementing this feature, but development has stalled.

There is an old 3.0 milestone that includes some of these solutions. I think this package needs to develop a roadmap for a 3.0 release that includes solutions to these issues, and more. I have expressed a willingness to dedicate time to developing these features.

@posva
Copy link

posva commented Feb 1, 2023

Thanks for taking the time to explain the situation. I understand the reluctance to merge new features without tests. Regarding this functionality being opinionated, I could be wrong but it seems to be a standard among shells to use ctrl p for previous and n for next (differently from using ctrl j or k which is a vim thing). Currently, doing a ctrl sequence still types that invisible ctrl sequence and backspace removes it. It should probably be ignored

posva added a commit to posva/prompts that referenced this pull request Apr 7, 2023
@posva
Copy link

posva commented Apr 7, 2023

I needed this so I released in a fork in the meantime: https://github.com/posva/prompts

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.

3 participants