Re: [PATCH] Fix a bug that `bind -x '"metachar":command'' fails when rl-variable `convert-meta' is `on'

[Chet Ramey]

On 2/11/19 1:57 AM, Koichi Murase wrote:

> Bash Version: 5.0
> Patch Level: 0
> Release Status: release
> 
> Description:
> 
>   When readline variable `convert-meta' is set to `on', `bind -x'
> including meta characters in its keyseq fails to bind.
> 
>   For an actual example, see the next `Repeat-By' section. An
> inconsistency comes from the condition whether to contain ESC (from
> meta chars) in the keyseq stored in keymaps. For example, in
> `rl_generic_bind' (lib/readline/bind.c), ESC is only contained when
> the keymap already has a child ISKMAP entry at ESC as quoted below:
> 
>     if (META_CHAR (ic) && _rl_convert_meta_chars_to_ascii)
>       {
>         ic = UNMETA (ic);
>         if (map[ESC].type == ISKMAP) /* <-- this condition */
>           {
>             prevmap = map;
>             map = FUNCTION_TO_KEYMAP (map, ESC);
>           }
>       }
> 
>   In addition, the condition in `rl_translate_keyseq'
> (lib/readline/bind.c) always refers to the current top-level keymap
> `_rl_keymap' rather than the corresponding keymap entry in the target
> keymap tree, which appears to be unreasonable:
> 
>     if (_rl_convert_meta_chars_to_ascii && _rl_keymap[ESC].type == ISKMAP)
>         array[l++] = ESC; /* ESC is meta-prefix */
> 
>   `bind -x' stores data in two different keymaps: the selected keymap
> and the keymap `cmd_xmap' defined in `bashline.c'. When one has a
> child ISKMAP entry at ESC and the other doesn't have, the inconsisty
> arises.
> 
> Repeat-By:
> 
>   The readline variable `convert-meta' is enabled by default when Bash
> starts with `LANG=C'.
> 
>     $ LANG=C bash-5.0 --norc
>     $ bind -v | grep convert-meta
>     set convert-meta on
> 
>   `bind -x' whose keyseq contains meta characters stores the command
> string to wrong places in `cmd_xmap'. In the following example, the
> command string for `\M-c' is stored to the placeholder for `c'.
> 
>     $ bind -x '"c": "echo hello"'
>     $ bind -x $'"\xE3": "echo world"'
>     world # <-- somehow the binding for `\M-c' is invoked by typing `c'
>     $
>     bash-5.0: bash_execute_unix_command: cannot find keymap for
> command # <-- error is produced by typing `\M-c'
>     $ bind -X
>     "c": "echo world"
>     $
> 
>   For reference, usual macro key bindings work as expected.
> 
>     $ bind '"c": "hello"'
>     $ bind $'"\xE3": "world"'
>     $ echo helloworld # <-- `hello' and `world' is inserted by typing
> `c' and `\M-c'
>     helloworld
>     $ bind -s
>     "\ec": "world"
>     "c": "hello"
> 
> Fix:
> 
>   Actually I'm not sure if there is a reason why stored keyseqs
> contains ESC only when there is already ESC entry, but a possible
> solution would be to create a new ISKMAP entry when there is no ISKMAP
> entry at ESC. I attach a patch
> `0001-translate-meta-characters-when-convert-meta-is-set.patch'. I
> have changed `rl_bind_key' (lib/readeline/bind.c) so that the ISKMAP
> entry is explicitly created. I have changed `rl_translate_keyseq'
> (lib/readeline/bind.c) so that meta characters `c' are always
> translated to `ESC' + `UNMETA (c)' when `convert-meta' is set. In this
> way, the array `keys' in `rl_generic_bind' (lib/readeline/bind.c)
> would never contain meta characters when `convert-meta' is set, so the
> codes to check meta characters have been removed from
> `rl_generic_bind'.

I agree that this is a problem. I think an easier solution for rl_bind_key
is to simply create a new key sequence in KEYSEQ, with the first character
as ESC, then pass the whole thing off to rl_generic_bind, which will handle
creating the appropriate shadow keymaps if necessary (your second patch
tries to do this inline, it appears).

The other patches are good. Thanks for the detailed report. Please take a
look at the next devel branch push and make sure it fixes your issues.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    chet@case.edu    http://tiswww.cwru.edu/~chet/