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

vi-like navigation with j & k #21

Closed
wants to merge 1 commit into from

Conversation

Jengah
Copy link

@Jengah Jengah commented Mar 3, 2019

Adds vi-like navigation for feature request in #13. Although the request only asked for changes to the Bullet and Check classes, I also extended it to the ScrollBar class as well, as it feels just as applicable.

This was tested on macOS High Sierra and Ubuntu 16.04 (with a docker image). I don't have the bandwidth to know/test this in Windows, but nothing about this should introduce a breaking change in Windows.

Although I did my best to stifle my auto-formatter/linter, it looks like some trailing spaces were cut off automatically in some places. If this is a problem, I can fix it, but I do agree with and recommend the adoption a code-formatter as mentioned in #17

@Jengah
Copy link
Author

Jengah commented Mar 3, 2019

Didn't notice you recently merged in some refactors to key registration. Will rebase and make necessary changes.

@rcfox
Copy link
Contributor

rcfox commented Mar 4, 2019

This should be easier with #23 :)

Just do something like:

@keyhandler.register(ARROW_UP_KEY)
@keyhandler.register(VIM_UP_KEY)
def moveUp(self):
    #...

@@ -31,3 +31,5 @@
BEEP_CHAR = 7
BACK_SPACE_CHAR = 8
SPACE_CHAR = ord(' ')
VIM_UP_KEY = 107 # k
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use ord('k')?

@bchao1
Copy link
Owner

bchao1 commented Mar 4, 2019

Since PR#23 already supports this (which is more flexible than hard-coding) I will close this PR.

@bchao1 bchao1 closed this Mar 4, 2019
@Jengah
Copy link
Author

Jengah commented Mar 4, 2019

#23 adds the ability to handle multiple keys, but doesn't implement the change in this PR (adding vi-like bindings). I will open in another branch with the actual implementation since the Feature Request wasn't added in #23

@Jengah Jengah mentioned this pull request Mar 4, 2019
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