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

Suggestion: use <space> to scroll in view mode #2721

Closed
benjaminrich opened this issue Jun 9, 2022 · 7 comments · Fixed by #2803
Closed

Suggestion: use <space> to scroll in view mode #2721

benjaminrich opened this issue Jun 9, 2022 · 7 comments · Fixed by #2803
Labels
A-keymap Area: Keymap and keybindings C-enhancement Category: Improvements

Comments

@benjaminrich
Copy link
Contributor

One thing that's always bugged me is that while h, j, k, l are so ergonomic, <c-f>, <c-b> and <c-d>, <c-u> are just the opposite; they feel characteristically "un-modal", and I've never understood why I should want to use them.

It's pretty ubiquitous in pagers, viewers and even web browsers to use <space> and <s-space> to scroll forwards and backwards page-wise, and this is very comfortable. Now, obviously <space> has a different usage in normal mode, but in view mode (and in particular 'sticky' view mode) it could be used for scrolling. That would give you the best of both worlds, it seems.

@benjaminrich benjaminrich added the C-enhancement Category: Improvements label Jun 9, 2022
@the-mikedavis the-mikedavis added the A-keymap Area: Keymap and keybindings label Jun 9, 2022
@the-mikedavis
Copy link
Member

I think this is a good idea. If you'd like to submit a PR, the default keymap for Z lives here:

"Z" => { "View" sticky=true
"z" | "c" => align_view_center,
"t" => align_view_top,
"b" => align_view_bottom,
"m" => align_view_middle,
"k" | "up" => scroll_up,
"j" | "down" => scroll_down,
"C-b" | "pageup" => page_up,
"C-f" | "pagedown" => page_down,
"C-u" => half_page_up,
"C-d" => half_page_down,
},

You could bind space to page_down or maybe half_page_down might be smoother. I'm not sure it's possible to bind S-space because of either terminal or crossterm limitations. Using the event-read example from crossterm:

Event: Key(KeyEvent { code: Char(' '), modifiers: NONE })
Event: Key(KeyEvent { code: Char(' '), modifiers: NONE })

both space and shift+space register as just space.

@benjaminrich
Copy link
Contributor Author

I will try. Please give me a bit of time, as I have not contributed before. Thanks.

@benjaminrich
Copy link
Contributor Author

benjaminrich commented Jun 18, 2022

Sorry for the naive question, because I am not a real dev and have never worked with rust before, but if I make a change to the code base and want to test it, what do I do? I mean, what are the commands to compile and run my modified version of the code?

EDIT: what I did is just run cargo run and that seems to work. :)

benjaminrich added a commit to benjaminrich/helix that referenced this issue Jun 18, 2022
@the-mikedavis the-mikedavis linked a pull request Jun 20, 2022 that will close this issue
@CptPotato
Copy link
Contributor

I'm not sure it's possible to bind S-space because of either terminal or crossterm limitations.
[...]
both space and shift+space register as just space.

FWIW, ShiftSpace does appear to work on Windows on all terminal emulators I've tried (Wezterm nightly, CMD, Powershell, ConEmu).

@the-mikedavis
Copy link
Member

Finally a key combination that works on Windows and not Linux 😄

For me the shift modified isn't being recognized on either Kitty or WezTerm

@CptPotato
Copy link
Contributor

Well, if the platform support is kinda hit-or-miss, it's probably a good idea to use a different keybind 👀

@benjaminrich
Copy link
Contributor Author

Yes. It would be quite a useful key combination, but you cannot rely on it.

archseer pushed a commit that referenced this issue Jul 1, 2022
* Make view mode more pager-like

Addresses #2721

* Remove view mode bindings for J and K
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-keymap Area: Keymap and keybindings C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants