[PATCH] For curses display, recognize a few more control keys

Sean Estabrooks posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/CAHyVn3Bh9CRgDuOmf7G7Ngwamu8d4cVozAcB2i4ymnnggBXNmg@mail.gmail.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
ui/curses_keys.h | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH] For curses display, recognize a few more control keys
Posted by Sean Estabrooks 10 months ago
The curses display handles most control-X keys, and translates
them into their corresponding keycode.  Here we recognize
a few that are missing, Ctrl-@ (null), Ctrl-\ (backslash),
Ctrl-] (right bracket), Ctrl-^ (caret), Ctrl-_ (underscore).

Signed-off-by: Sean Estabrooks <sean.estabrooks@gmail.com>
---
 ui/curses_keys.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ui/curses_keys.h b/ui/curses_keys.h
index 71e04acdc7..88a2208ed1 100644
--- a/ui/curses_keys.h
+++ b/ui/curses_keys.h
@@ -210,6 +210,12 @@ static const int _curses2keycode[CURSES_CHARS] = {
     ['N' - '@'] = 49 | CNTRL, /* Control + n */
     /* Control + m collides with the keycode for Enter */

+    ['@' - '@']  =  3 | CNTRL, /* Control + @ */
+    /* Control + [ collides with the keycode for Escape */
+    ['\\' - '@'] = 43 | CNTRL, /* Control + Backslash */
+    [']' - '@']  = 27 | CNTRL, /* Control + ] */
+    ['^' - '@']  =  7 | CNTRL, /* Control + ^ */
+    ['_' - '@']  = 12 | CNTRL, /* Control + Underscore */
 };

 static const int _curseskey2keycode[CURSES_KEYS] = {
-- 
2.40.1
Re: [PATCH] For curses display, recognize a few more control keys
Posted by Peter Maydell 9 months, 3 weeks ago
On Sat, 15 Jul 2023 at 14:31, Sean Estabrooks <sean.estabrooks@gmail.com> wrote:
>
> The curses display handles most control-X keys, and translates
> them into their corresponding keycode.  Here we recognize
> a few that are missing, Ctrl-@ (null), Ctrl-\ (backslash),
> Ctrl-] (right bracket), Ctrl-^ (caret), Ctrl-_ (underscore).
>
> Signed-off-by: Sean Estabrooks <sean.estabrooks@gmail.com>
> ---
>  ui/curses_keys.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/ui/curses_keys.h b/ui/curses_keys.h
> index 71e04acdc7..88a2208ed1 100644
> --- a/ui/curses_keys.h
> +++ b/ui/curses_keys.h
> @@ -210,6 +210,12 @@ static const int _curses2keycode[CURSES_CHARS] = {
>      ['N' - '@'] = 49 | CNTRL, /* Control + n */
>      /* Control + m collides with the keycode for Enter */
>
> +    ['@' - '@']  =  3 | CNTRL, /* Control + @ */
> +    /* Control + [ collides with the keycode for Escape */
> +    ['\\' - '@'] = 43 | CNTRL, /* Control + Backslash */
> +    [']' - '@']  = 27 | CNTRL, /* Control + ] */
> +    ['^' - '@']  =  7 | CNTRL, /* Control + ^ */
> +    ['_' - '@']  = 12 | CNTRL, /* Control + Underscore */
>  };
>
>  static const int _curseskey2keycode[CURSES_KEYS] = {
> --
> 2.40.1

So, there's already some logic in ui/curses.c that
handles control keys generically, and it was put
in (or at least modified) way back in commit d03703c81a202ce
in 2010 with the commit message saying it was for
control-{@[\]^_} :

                if (chr < ' ') {
                    keysym = chr + '@';
                    if (keysym >= 'A' && keysym <= 'Z')
                        keysym += 'a' - 'A';
                    keysym |= KEYSYM_CNTRL;
                } else
                    keysym = chr;

But we only use that code if kbd_layout is set.

So I'm not sure how this should work -- are these
control-key combinations keyboard layout specific and
that's why this is only in that branch of the code?
Or are they generic and we can support them in the
no-keyboard-layout case too? I did a bit of playing
around with my non-standard-keyboard-layout and one
of the ncurses-examples test programs, and I think we
can do this regardless of keyboard layout.

Unfortunately this curses code is (a) pretty old
(b) has no active maintainer (c) is not used much.
This patch looks like it's OK to me, but I'm guessing
a bit. It looks a little odd that we programatically
handle control-X in the with-keyboard-layout case
but do it data-driven via the array in the without
case, but that's the way the code is already written...

So I guess

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

This seems like a safe enough small change, so I'm going
to take it via target-arm.next, unless somebody else
would like more time to review it (or to claim the
vacant maintainership of the code ;-))

thanks
-- PMM