unify generic keybindings across panes

[miraclx]

In reference to conversation https://github.com/tarkah/tickrs/pull/93#issuecomment-785456170, https://github.com/tarkah/tickrs/pull/93#issuecomment-785460109 and https://github.com/tarkah/tickrs/pull/93#issuecomment-785465182.

This PR:

• unifies ctrl+c on all panes
• unifies ? across panes with exceptional behavior on the help pane
• unifies q across panes with exceptional behavior on the help, addstock, and options pane
• allows useful toggles like c, v, p on all panes except help and addstock (especially including the options pane)
• allows x on all panes except help, addstock and summary (especially including the options pane)

keymap (top-down in match order)

bindings	where	action
ctrl+c	Mode::*	exit app
enter	Mode::AddStock	add stock
[char]	Mode::AddStock	type character in stock bar
backspace	Mode::AddStock	delete character in stock bar
esc	Mode::AddStock	close stock bar
esc | q | ?	Mode::Help	close help window
q	Mode::* (if not Mode::DisplayOptions)	exit app
?	Mode::*	open help window
c	Mode::*	toggle chart type
v	Mode::*	toggle volumes on/off
p	Mode::*	toggle pre-post
Left	Mode::DisplaySummary	select timeframe to the left
Right	Mode::DisplaySummary	select timeframe to the right
Up	Mode::DisplaySummary	scroll up
Down	Mode::DisplaySummary	scroll down
s	Mode::DisplaySummary	switch to stock view
/	Mode::DisplaySummary	add new stock
x	Mode::*	toggle x-axis labels
esc | o | q	Mode::DisplayOptions	close options pane
tab	Mode::DisplayOptions	toggle call/puts
Up	Mode::DisplayOptions	navigate up
Down	Mode::DisplayOptions	navigate down
Left	Mode::DisplayOptions	navigate left
Right	Mode::DisplayOptions	navigate right
ctrl+Left	Mode::DisplayStock	move stock left
ctrl+Right	Mode::DisplayStock	move stock right
Left	Mode::DisplayStock	select timeframe to the left
Right	Mode::DisplayStock	select timeframe to the left
/	Mode::DisplayStock	add new stock
k	Mode::DisplayStock	remove selected stock
s	Mode::DisplayStock	switch to summary view
o	Mode::DisplayStock	open options pane
tab	Mode::DisplayStock	select tab to the left
shift+tab	Mode::DisplayStock	select tab to the right

[tarkah]

Thanks for this! I really like your breakdown in the table above, I may add a wiki entry for that.

The new handle_key_bindings function is a little hard to reason about being that there are 3 match fields and some additional filter logic. I'm not sure if if modifiers sections are really necessary? For example with add_stock, we already matched on Ctrl-C which is the ONLY keybinding that should take precedence over it. I have no issue with 100% of everything else being captured by that mode. There's no reason to need to use ctrl+< or ctrl+> while in this mode.

Same with capturing key events while in the Help mode. I have no issue with this mode capturing 100% of the key events past Ctrl-C. We don't want 'v' to toggle volumes behind the scenes while Help screen is displayed. --EDIT: Bad example here, as you are only matching on a few keycodes. But I still think it's cleaner if we have the match just capture ALL keycodes / modifiers vs the logic that's in there currently.

So I think we can clean up the logic a bit if that makes sense?

[miraclx]

I'm not sure if if modifiers sections are really necessary? For example with add_stock, we already matched on Ctrl-C which is the ONLY keybinding that should take precedence over it. I have no issue with 100% of everything else being captured by that mode. There's no reason to need to use ctrl+< or ctrl+> while in this mode.

I agree with you, that's why the match is structured in order of capture precedence.

We first capture ctrl+c because it should have effect everywhere, we can say it has highest precedence.

And thereafter, AddStock captures every key except those with non-SHIFT modifiers. So, it never actually gets any ctrl sent to it.

And thereafter, Help captures esc, ? and q for special uses.

etc.. that's how the order of capture precedence is structured.

At this point, the implementation 99.5% acts out the previous logic with extra capabilities of being able to use the c, v, x and p toggles while in the options pane.

[miraclx]

I have no issue with this mode capturing 100% of the key events past Ctrl-C. We don't want 'v' to toggle volumes behind the scenes while Help screen is displayed. --EDIT: Bad example here, as you are only matching on a few keycodes. But I still think it's cleaner if we have the match just capture ALL keycodes / modifiers vs the logic that's in there currently.

Okay, I see, I'll fix this right away

---

For others (if any), this means, because the current code is loose with matching modifiers

https://github.com/tarkah/tickrs/blob/adc0b8a047481d0f9c0d5db2f6ea57072920d4d7/src/event.rs#L257-L260

ctrl+p has the same effect as p on toggling pre-post on all panes

At this point, the only "safe" (predictable) binding is ctrl+c | c

[miraclx]

I'll use KeyModifiers::NONE instead of if modifiers.is_empty() to fix this.

[miraclx]

Done. Please test.

[tarkah]

We first capture ctrl+c because it should have effect everywhere, we can say it has highest precedence.

And thereafter, AddStock captures every key except those with non-SHIFT modifiers. So, it never actually gets any ctrl sent to it.

I get this, I guess it just comes down to preference for how to code reads. It seems pointless to have any unnecessary logic on the modifiers when not necessary, as it just increases perceived complexity of how things get resolved. Since AddStock mode should just capture EVERYTHING past the Ctrl+C, I'd prefer to see it as:

(Mode::AddStock, _, keycode) => {
    handle_keys_add_stock(keycode, app)
}

vs current

(Mode::AddStock, modifiers, keycode) => {
    if modifiers.is_empty() || modifiers == KeyModifiers::SHIFT {
        handle_keys_add_stock(keycode, app)
    }
}

The latter reads to me as if there might be an instance where another Ctrl+key sequence might need to be captured for the AddStock mode later in the order of things

[miraclx]

Yeah, that makes sense. But, correct me if I'm wrong, it seems, that's in the context of AddStock alone. It seemed trivial to me but I'll add a patch for that.

Although this means ctrl+p enters p in the AddStock text entry. And so too with holding alt and typing hey enters hey in the AddStock text entry.

The current impl would simply ignore it and not type anything, which I think, should be the default behaviour.

[tarkah]

Although this means ctrl+p enters p in the AddStock text entry. And so too with holding alt and typing hey enters hey in the AddStock text entry.

Good points. I forgot you aren't pattern matching on Modifier within the handler for this one. I'm good with how it is, definitely an overall improvement and reduction in redundancy! I'll give this PR a good final once over later today but I think we should be set. Thanks for all your work on this.

[miraclx]

Glad to have helped! Thanks!