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

Cannot bind <Ctrl-Up>/<Ctrl-Down> #1046

Closed
mgedmin opened this issue Nov 3, 2020 · 18 comments
Closed

Cannot bind <Ctrl-Up>/<Ctrl-Down> #1046

mgedmin opened this issue Nov 3, 2020 · 18 comments

Comments

@mgedmin
Copy link

mgedmin commented Nov 3, 2020

I'm trying to use these mappings in my .tigrc

# Scroll with mouse wheel in gnome-terminal (it sends Up/Down key events when in alt screen)
bind diff <Up> move-up
bind diff <Down> move-down

# but I want some way to move to next/previous commit
bind diff <Ctrl-Up> previous
bind diff <Ctrl-Down> next

The tigrc manual page led me to expect this to be valid syntax:

Key values

Key values should never be quoted. Use either an ASCII or UTF8-encoded character or one of the following symbolic key names. Symbolic key names are case insensitive and starts with "<" and ends with ">". Use <Hash> to bind to the # key, since the hash mark is used as a comment character. Use <LessThan> to bind to the < key.

<Enter>, <Space>, <Backspace>, <Tab>, <Escape> or <Esc>, <Left>, <Right>, <Up>, <Down>, <Insert> or <Ins>,
<Delete> or <Del>, <Hash>, <LessThan> or <LT>, <Home>, <End>, <PageUp> or <PgUp>, <PageDown> or <PgDown>,
<ScrollBack> or <SBack>, <ScrollFwd> or <SFwd>, <ShiftTab> or <BackTab>, <ShiftLeft>, <ShiftRight>,
<ShiftDelete> or <ShiftDel>, <ShiftHome>, <ShiftEnd>, <SingleQuote>, <DoubleQuote>, <F1> ... <F19>

To define key mappings with the Ctrl key, use <Ctrl-key>. In addition, key combos consisting of an initial Escape key followed by a normal key value can be bound using <Esc>key.

also there are no errors printed by tig on startup.

Still, when I press ctrl-up, I get a "Unknown key, press h for help", and when I press ctrl-down, nothing happens at all. Also, these bindings don't show up in the help page shown by h.

I've tig version 2.4.1 from Ubuntu 20.10 (ncursesw version 6.2.20200212, readline version 8.0).

@mgedmin
Copy link
Author

mgedmin commented Nov 3, 2020

Attempts to work around this with

bind generic <Esc>[1;5A previous
bind generic <Esc>[1;5B next

were not successful either.

@mckellygit
Copy link
Contributor

mckellygit commented Nov 3, 2020

Hi,
I couldn't get mappings like that to work either.
I used <ScrollFwd> which is Shift-Down and <ScrollBack> (Shift-Up) for this. Perhaps that works for you ?
It would be awesome if we could map these type of Ctrl sequences.
thx,
-m

@koutcher
Copy link
Collaborator

koutcher commented Nov 3, 2020

Have you tried:

bind generic    <Esc>[A previous
bind generic    <Esc>[B next

@mgedmin, set mouse = yes might do what you expect without altering the Up/Down mapping.

@mgedmin
Copy link
Author

mgedmin commented Nov 3, 2020

<Esc>[A/B is for regular Up/Down; I want to map Ctrl-Up/Down which is <Esc>[1;5A/B (and I tried that, and it didn't work).

(Wow, I didn't know about set mouse!)

@koutcher
Copy link
Collaborator

koutcher commented Nov 4, 2020

With Putty, <Esc>[A/B works for Ctrl-Up/Down but, indeed, it doesn't work with xterm or gnome-terminal. With xterm or gnome-terminal, Tig doesn't receive <Esc>[1;5A/B either because ncurses transforms Ctrl-Up/Down in an obscure keycode with a value > KEY_MAX. Hence, I believe, the statement in #381 that bindings such as Ctrl-Down are not supported.

@mckellygit
Copy link
Contributor

mckellygit commented Nov 4, 2020

@koutcher hi. Thanks. But can we alter the check for KEY_MAX (or KEY_MAX) for this ? And add these new values.
Or is that just too much of a break from the ncurses model ?
Does keyname() provide any good info ? I'll try some things.

@mckellygit
Copy link
Contributor

Perhaps allow an ncurses key value directly in .tigrc bindings ? Or extra foo for special values->names ...
Anyway thanks for the info and I will add something to my fork for it. I already have a few other mods I find useful (in PRs here).
thx so much,
-m

@mckellygit
Copy link
Contributor

We can check 511+ to find the values for names kDN5 and kUP5 and use those as new CtrlDown and CtrlUp keys.
Or perhaps remove these capabilities at startup somehow to get the esc codes to come through ?

@koutcher
Copy link
Collaborator

koutcher commented Nov 4, 2020

Maybe there is some hope with neomutt/neomutt@aa64ad7...

@mckellygit
Copy link
Contributor

Yea, cool, that would be the way to get them all.
I made a ~10 line hack to get the values for kDN5/kUP5 and check for those in display.c and it works well for several different terminals. But this would get all the sequences that ncurses returns a value for that are not defined.
Should we put together a PR with this ?

@koutcher
Copy link
Collaborator

koutcher commented Nov 4, 2020

Some have been waiting for this since 2015 and it would make the impossible possible, so that doesn't sound like a bad idea.

@mckellygit
Copy link
Contributor

ok, I'll work on something this weekend that adds an extended_key_mappings[] and searches that also.

@mckellygit
Copy link
Contributor

ok, first attempt is here #1048

@mckellygit
Copy link
Contributor

mckellygit commented Nov 11, 2020

An easier way might be to just add -

    int i;
    for (i=KEY_MAX; i<2000; i++)
        keyok(i, false);

at the end of init_display(). And then use the explicit mappings:

bind generic   <Esc>[1;5B    scroll-line-down
bind generic   <Esc>[1;5A    scroll-line-up

@mgedmin
Copy link
Author

mgedmin commented Nov 11, 2020

As a user I'd much prefer being able to specify <Ctrl-Up> instead of <Esc>[1;whatwasitagain?.

@mckellygit
Copy link
Contributor

@mgedmin sure, and I agree. It's just that sometimes a simple code change is accepted when a larger one is not and I just thought I should mention it.
It sort of makes sense in that there are extended key codes that are there, but not fully supported as-is, and so it breaks the esc-code binding method and the above removes the extended key codes. We would really just need to make sure the version of curses used has keyok() with a check at configure/compile time.
But we can wait and see what others think of the original PR and if anything grows from that :-)
thx,
-m

@mckellygit
Copy link
Contributor

I've also added PR #1049 for the much more simple way.
thx,
-m

@koutcher
Copy link
Collaborator

Indeed #1048 looks a bit complex... Let start with #1049 and we can improve it later if needed.

koutcher pushed a commit that referenced this issue Dec 22, 2020
This makes it possible to use esc-codes in mappings for Shift/Ctrl/Alt
Up/Down/Left/Right/Home/End/Insert/Delete/PgUp/PgDn.

[tk: minor tweaks and NCURSES_EXT_FUNCS was only made visible in ncurses
 v5_6_20071117, so use NCURSES_VERSION instead for compatibility.]

References #1046
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

No branches or pull requests

3 participants