search

petereon / beaupy

A Python library of interactive CLI elements you have been looking for
https://petereon.github.io/beaupy/
MIT License
200 stars 13 forks source link

feat: Add vi movement keys in selection menu #46

Closed BrewingWeasel closed 1 year ago

BrewingWeasel commented 1 year ago

I often find myself absent mindedly switching between using VI keybinds and the arrow keys in terminal apps. This PR adds using 'j' and 'k' to move up and down and 'g' and 'G' to move to the beginning and end of the selection.

petereon commented 1 year ago

First of all, thanks for the contribution. However I am not sure I can accept it as is.

Despite being an avid vim user myself I'd prefer keeping choice of bindings on downstream user and resorting to only most basic defaults.

As for the single stepping through the options, you can resort to including any key bindings you like in the DefaultKeys.up and DefaultKeys.down lists for your downstream application.

With regards to stepping all the options using a single key: That is a valid use case. However it would need to be implemented package-wide for all functions and it would again have to rely on HOME and END keys coming from the DefaultKeys.home and DefaultKeys.end respectively rather than hard-coded vi bindings.

BrewingWeasel commented 1 year ago

Thanks for the quick response!

I agree that leaving it up to the user is a better approach. The only problem is that right now DefaultKeys.up and DefaultKeys.down are checked before input in prompt(). If you add j and k to defaultkeys, you will be unable to type them into a prompt. That should be a pretty simple fix though.

I'll work on replacing 'g' and 'G' with the default home and end. Thanks for being patient with a noob lol. (Also, unrelated, but any idea why the lint is failing? It passes when I run it - though I might be doing something wrong.)

BrewingWeasel commented 1 year ago

Ok, I removed the vi movement and added home and end in selections. (And tests for those)

petereon commented 1 year ago

Also, unrelated, but any idea why the lint is failing? It passes when I run it - though I might be doing something wrong.

I am looking for a way to fix this but lint will always fail for forks as it requires a SonarCloud secret which is not available in forks. No need to worry about that.

petereon commented 1 year ago

Also, coming back to the idea of supporting vi bindings for navigation; It could be interesting to have prompts support a modal-editing experience akin to vi.

I would not implement this within the prompt itself as I am trying to keep this dead simple, but I can imagine a function like modal_prompt which would allow for alternating between modes, thus supporting vi navigation bindings in COMMAND mode and switching to INSERT mode where it would interpret these as characters.

To be sure, I am not prompting you to implement it, as it is quite a long-shot and would require some long-overdue refactoring of the prompt. Just thinking about the possibilities and day-dreaming.

As for the present changes, looks like the checks have passed. I will do some quick manual testing on all the platforms later today and after that I am going to merge. Thanks again!

BrewingWeasel commented 1 year ago

Cool, thank you!

And yeah I agree that a modal prompt could be super cool. That being said, it seems like it would be really complicated (though probably fun) to implement.