htop-dev
reintroduce vim_mode
with the goal of implementing vim_mode in a way that everyone can agree on and wont cause unforeseen future bugs, this is what I've come up with so far:
- allow settings to effect action bindings -> easy to stop them overlapping vim binds
- allow settings to effect which keys count as up/down/etc -> easy to add vim movement
- centralize keybind mapping to KeyBinds.c -> easy to expand and add future setting dependent alternate keybinds
I've kept it to hjkl for example purposes because I wanted to know what the thoughts on it as is are, but adding more vim related binds like this should now be easy.
any and all critisim and advice welcome, I've done my best to follow the existing style(where its consistent) and not do things in a too roundabout way but I'm sure theres room for improvement
somewhat related, whats the deal with this piece of code https://github.com/htop-dev/htop/blob/6646030116c325157097cf8f66ec83b118d3c54f/ScreenManager.c#L219-L224
While the patchset itself LGTM I'm not quite happy with the overall approach.
Existing: Each detail window/screen has basically its own key handler that causes keys to be translated directly into actions.
This patchset: Each detail window/screen has its own key handler, but certain actions are specially translated into special bindings before they are handled by the handler executes the operation.
My suggestion: Each window/screen declares a table of actions it can perform alongside appropriate handlers for each action. For each action there is a list of 1...many key bindings that triggers this action. The actual key handling loop subsequently becomes generic across all windows/screens and only receives the active actions for each screen. When a key is pressed this key is checked against all bindings that are bound to listed actions if any of the keys for that binding match.
Example: Let's tka ethe OpenFilesScreen which basically has the following actions:
LIST_DOWN
LIST_UP
LIST_LEFT
LIST_RIGHT
SEARCH
FILTER
REFRESH
CLOSE
For the key bindings there's a list telling e.g.
Classic:
LIST_LEFT [KEY_LEFT, KEY_ALT('H'), …]
LIST_UP [KEY_UP, KEY_ALT('J'), …]
LIST_DOWN [KEY_DOWN, KEY_ALT('K'), …]
LIST_RIGHT [KEY_RIGHT, KEY_ALT('L'), …]
FILTER ['f', …]
KILL ['k', …]
TRACE ['s', …]
QUIT ['q', KEY_ESC]
vim:
LIST_LEFT [KEY_LEFT, 'h', …]
LIST_UP [KEY_UP, 'j', …]
LIST_DOWN [KEY_DOWN, 'k', …]
LIST_RIGHT [KEY_RIGHT, 'l', …]
FILTER ['f', …]
KILL ['d', …]
TRACE ['s', …]
QUIT ['q', KEY_ESC]
Basically similar to what is already the case with defining color schemes. The key binding map contains all bindings, but each screen only tells which actions it supports and an appropriate handler for it.
My suggestion: Each window/screen declares a table of actions it can perform alongside appropriate handlers for each action. For each action there is a list of 1...many key bindings that triggers this action. The actual key handling loop subsequently becomes generic across all windows/screens and only receives the active actions for each screen. When a key is pressed this key is checked against all bindings that are bound to listed actions if any of the keys for that binding match.
I can see this causing a lot of duplicate effort, as "each window/screen" will have a different "action" thats basically just "go down". with such an implementation a usecase as "I wish this button also counted as going down" would require going through all these action -> keybinds lists and adding them where it makes sense, vs what would with this patchset be one change to keybinds.c. which was my main logic of choosing the solution of trying to translate keybinds that had the same purpose everywhere into the same result from one place.
that aside, to see if I'm grasping the suggested alternative correctly. you'd prefer full on different keymaps for everything similar to how the different colorschemes also have to define the full set colors for each scheme? wouldnt this get convoluted with possibly more single keybind changing options and different keymaps? (forcing a new entire keymap for each one button remap option in every possible)
regardless of that, implementation wise of this option. would it look like calling a HandleActions(this->panel, ch) or something along these lines inside, for the sake of sticking with the OpenFileScreen, InfoScreen_Run, which checks all these action handlers to see if a key matches(in the appropriate keymap), running the designated action if it does? which would replace the current switchcase for handling these keybinds.
Implementation-wise yes.
Regarding the Keybinding init: While we could use somewhat static lists as is done with the color schemes there's also the option to keep most things statically defined and supplement for those few bindings which may be dependent on some setting. What matters is the final map of binding -> (list of) keys.
On the note of code duplication: I think this issue might be manageable if we "inherited" views with some list in them from the same "class" that had the necessary information (selected index, max index, horizontal offset) available and used by all those views. Any view with a list could then use this basic mechanic and built upon it.
another option I see, we could also do some form of mix where theres some generic action name in the keybinding lists like:
LEFT [KEY_LEFT, 'h', …]
UP [KEY_UP, 'j', …]
DOWN [KEY_DOWN, 'k', …]
RIGHT [KEY_RIGHT, 'l', …]
... panel specific keys
since the mapping of supported action->function to be called would be in the panel's list of supported actions this could work for a multitude of panels without causing duplicates and while still keeping mostly static keymaps. didnt really think of it like that when you explained it before, this makes it far more reasonable than I initially thought.
for now I'll at least undo the other solution and mark this as draft till I have a form of the suggested solution implemented.
When I described my suggestion above I implicitly thought of reusing common action bindings. Thus having only one LEFT everywhere instead of having MAINVIEW_LEFT, OPENFILES_LEFT, CONFIGNAV_LEFT …
More like having some "common" bindings reused where possible and special bindings for each panel. Reusing the bindings makes the overall UX/feel more consistent.
Is this a good place to request that hjkl work in addition to arrows, and not instead of arrows?
Is this a good place to request that hjkl work in addition to arrows, and not instead of arrows?
There was already some discussion regarding this topic going on and as this is "VIM" mode" instead of "VI mode" I see working arrow keys as a given. :smile_cat:
Please also consider a vim shortcut the expand/collapse actions on a tree. It doesn't have to be vim-specific though (like zr za zm etc.); I think in the vim mode, maybe simply re-mapping the original actions for F6 to TAB would suffice; it works in the sorted mode as changing the sort columns, and in tree-view mode for collapsing and expanding.
