bug#40725: 27.0.91; Tutorial reports false positive key rebindings

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

> Isn't the patch too general?

I don't think so - apart from avoiding the false warning in the OP, it
should be equivalent to the current logic.

The function in question, tutorial--find-changed-keys, is only ever
passed the defconst tutorial--default-keys as argument.

The only elements of tutorial--default-keys whose car satisfies keymapp
are:

  ((ESC-prefix                   [27])
   (Control-X-prefix             [24])
   (mode-specific-command-prefix [3]))

Currently, tutorial--find-changed-keys hard-codes the check for the
first two:

  (if (eq def-fun 'ESC-prefix)
      (lookup-key global-map [27])
    (if (eq def-fun 'Control-X-prefix)
        (lookup-key global-map [24])
      (key-binding key)))

Therefore changing this to:

  (if (keymapp def-fun)
      (lookup-key global-map key)
    (key-binding key))

Has the same effect as:

  (if (eq def-fun 'ESC-prefix)
      (lookup-key global-map [27])
    (if (eq def-fun 'Control-X-prefix)
        (lookup-key global-map [24])
      (if (eq def-fun 'mode-specific-command-prefix)
          (lookup-key global-map [3])
        (key-binding key))))

which I think is correct, since I don't see how C-c is any different to
C-x or ESC in the context of this function.  In fact, the tutorial
doesn't mention C-c at all, but apparently it's included in
tutorial--default-keys just because it's an otherwise common prefix.

> How do we distinguish the case where _all_ of the subcommands were
> rebound, for example?

I don't think the current logic tries to handle that either, does it?
Besides, mode-specific-command-prefix is an empty keymap by default, the
tutorial makes no mention of it, and tutorial--default-keys already
tries (and fails, see below) to list all the pertinent keys under the
ESC and C-x prefixes.

FWIW, tutorial--find-changed-keys rightly detects and warns about the
following situation both with and without my patch:

  (define-key global-map "\C-c" #'ignore)

> Also, don't we have some prefixes that for the purposes of the
> tutorial must not have _any_ of its subcommands rebound?

Hm, I don't know.  Did you have any examples in mind?  The only prefixes
I see used in the tutorial are C-x, C-h, and Meta/ESC.

AFAICT if a command-binding pair isn't listed in tutorial--default-keys,
then C-h t won't complain about it being rebound.  For example, you can
rebind C-x k (which IS mentioned in the tutorial) and C-h t won't notice
at all.

I can open another bug report for extending tutorial--default-keys to
detect changes to all default key bindings used in the tutorial, but for
now I think the proposed patch fixes the issue at hand without making
things worse.

-- 
Basil