promptui icon indicating copy to clipboard operation
promptui copied to clipboard

Adding key for quit in SelectKeys

Open LuciferInLove opened this issue 4 years ago • 12 comments

Adding key for quit (default is 'q') in SelectKeys

LuciferInLove avatar Jul 02 '20 20:07 LuciferInLove

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 02 '20 20:07 CLAassistant

@blast-hardcheese, thanks for your comment. I've added variable for an optional value that can be returned on exit.

LuciferInLove avatar Jul 08 '20 01:07 LuciferInLove

what's blocking this from getting merged?

blast-hardcheese avatar Oct 04 '20 00:10 blast-hardcheese

I like this feature and I hope this PR would be merged soon. :) thank you.


Additionally, I want to leave some comments below. :)

I doubt both os.Exit(0) and q as default value is good decision.

  1. I believe it would be better to return errors like ErrSelectionQuitted because it gives users freedom of choices. Currently, if a user want to emit os.Exit(1) when quitted, there's nothing users can do about it.

  2. I am wondering that ESC can be a better choice comparing with q because Select have search feature.

HurSungYun avatar Jun 29 '21 09:06 HurSungYun

@HurSungYun, thanks for your comment.

  1. I believe it would be better to return errors like ErrSelectionQuitted because it gives users freedom of choices. Currently, if a user want to emit os.Exit(1) when quitted, there's nothing users can do about it.

There is an ExitValue. You can use it instead of exit, then intercept the value in your main module and emit whatever you want. I hope it'll help. Maybe, os.Exit(0) from this module isn't a good idea, and I'm waiting for advice from reviewers.

2. I am wondering that ESC can be a better choice comparing with q because Select have search feature.

readline.CharEsc and readline.CharEscapeEx don't work on my Linux laptop at all... You could try to redefine promptui.SelectKeys in your main module like this:

prompt := promptui.Select{
		Label:        "Select an item (or press \"Esc\" for exit)",
		Items:        items,
		Templates:    &templates,
		Size:         20,
		Searcher:     searcher,
		HideSelected: true,
		Keys: &promptui.SelectKeys{
			Next: promptui.Key{
				Code:    readline.CharNext,
				Display: "↓",
			},
			Prev: promptui.Key{
				Code:    readline.CharPrev,
				Display: "↑",
			},
			PageUp: promptui.Key{
				Code:    readline.CharForward,
				Display: "→",
			},
			PageDown: promptui.Key{
				Code:    readline.CharBackward,
				Display: "←",
			},
			Search: promptui.Key{
				Code:    '/',
				Display: "/",
			},
			Exit: promptui.Key{
				Code:    readline.CharEsc,
				Display: "Esc",
			},
		},
	}

LuciferInLove avatar Jun 29 '21 19:06 LuciferInLove

Happy early first birthday for this PR!

blast-hardcheese avatar Jun 29 '21 23:06 blast-hardcheese

@LuciferInLove Thanks for replying!

There is an ExitValue. You can use it instead of exit, then intercept the value in your main module and emit whatever you want. I hope it'll help. Maybe, os.Exit(0) from this module isn't a good idea, and I'm waiting for advice from reviewers.

As you said, ExitValue would be helpful if user want to intercept cancel signal, but I believe it could be a better interface to return some kind of error (ErrSelectionQuitted) instead of ExitValue because ExitValue shares same parameter with normal results.

I think waiting for advice from reviewers is a good idea as well :)

readline.CharEsc and readline.CharEscapeEx don't work on my Linux laptop at all... You could try to redefine promptui.SelectKeys in your main module like this:

Oh, I didn't consider that there are keyboards w/o escape key. I believe there are plenty of people having keyboard w/o escape key, so q as default value might be better :) thank you for letting me know.

HurSungYun avatar Jun 30 '21 02:06 HurSungYun

Hi Folks; Not sure if this project is active and/or maintained (over a year. This PR for example is super useful, but has been sitting here. I ended up forking the project and implementing a similar thing, before I stumbled upon this.

Any ideas why PR velocity is slow. Thanks folks!

1xyz avatar Jun 30 '21 20:06 1xyz

@jbowes You seem to be the only person maintaining this project at the moment, is this an orphaned project that would better off be forked so that popular PRs like this that have been waiting for over a year can be merged, or is the project going to be maintained?

jroper avatar Nov 11 '21 07:11 jroper

this is a super useful feature which can improve the user experience a lot, hope it can be merged soon.

rr-nick-tan avatar Aug 04 '22 19:08 rr-nick-tan

@jbowes , second to @jroper 's question, is this project still maintained? otherwise, can you add some collaborators to keep this project going? or shall we just fork it?

rr-nick-tan avatar Aug 04 '22 19:08 rr-nick-tan

I hope this project not be abandoned as well. 🙏 This project is really useful and helps lots of other CLI projects.

HurSungYun avatar Aug 07 '22 09:08 HurSungYun