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

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

[Basil L. Contovounesios]

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

Severity: minor
Tags: patch

In some cases the tutorial can falsely claim that a user's modified key
bindings no longer correspond to the tutorial's text:

0. emacs -Q
1. M-x winner-mode RET
2. C-h t

The user is then presented with the following:

 NOTICE: The main purpose of the Emacs tutorial is to teach you
 the most important standard Emacs commands (key bindings).
 However, your Emacs has been customized by changing some of
 these basic editing commands, so it doesn't correspond to the
 tutorial.  We have inserted colored notices where the altered
 commands have been introduced. [More]

Yet no such coloured notices are to be found in TUTORIAL, since
winner-mode binds only 'C-c <left>' and 'C-c <right>', neither of which
is mentioned in the tutorial.

Pushing the "More" button gives the following *Help* buffer contents:

--8<---------------cut here---------------start------------->8---
The following key bindings used in the tutorial have been changed
from the Emacs default:

   Standard Key   Command                     In Your Emacs   
   C-c 		  mode-specific-command-prefix M-x mode-specific-command-prefix more info

It is OK to change key bindings, but changed bindings do not
correspond to what the tutorial says.
--8<---------------cut here---------------end--------------->8---

Pushing the "more info" button in turn gives the following *Help* text:

--8<---------------cut here---------------start------------->8---
Your Emacs customizations override the default binding for this key:

The default Emacs binding for the key C-c is the command
‘mode-specific-command-prefix’.  However, your customizations have
rebound it to the command ‘(keymap (keymap (right . winner-redo) (left
. winner-undo)) mode-specific-command-prefix)’.
--8<---------------cut here---------------end--------------->8---

The following patch fixes this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-handling-of-changed-prefix-keys-in-tutorial.patch --]
[-- Type: text/x-diff, Size: 1337 bytes --]

From 1d607f275458e14d36d00cccdcfb7b7bae55933b Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sat, 28 Mar 2020 22:26:25 +0000
Subject: [PATCH] Fix handling of changed prefix keys in tutorial

* lisp/tutorial.el (tutorial--find-changed-keys): Use keymapp to
detect prefix definitions rather than hard-coding them.  A notable
omission from the hard-coded list was mode-specific-command-prefix,
whose subcommands are often rebound.
---
 lisp/tutorial.el | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lisp/tutorial.el b/lisp/tutorial.el
index d07737e333..a906af3b5a 100644
--- a/lisp/tutorial.el
+++ b/lisp/tutorial.el
@@ -427,11 +427,9 @@ tutorial--find-changed-keys
 	       ;; Handle prefix definitions specially
 	       ;; so that a mode that rebinds some subcommands
 	       ;; won't make it appear that the whole prefix is gone.
-	       (key-fun (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))))
+               (key-fun (if (keymapp def-fun)
+                            (lookup-key global-map key)
+                          (key-binding key)))
 	       (where (where-is-internal (if rem-fun rem-fun def-fun)))
 	       cwhere)
 
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 3365 bytes --]


I don't think this qualifies as urgent enough for inclusion in emacs-27,
but I think the fix is harmless enough if desired.  WDYT?

Thanks,

-- 
Basil

In GNU Emacs 27.0.91 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2020-04-17 built on thunk
Repository revision: c36c5a3dedbb2e0349be1b6c3b7567ea7b594f1c
Repository branch: emacs-27
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux bullseye/sid

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Winner mode enabled
Preparing tutorial ...

Configured using:
 'configure 'CC=ccache gcc' 'CFLAGS=-O0 -g3 -ggdb -gdwarf-4'
 --config-cache --prefix=/home/blc/.local --program-suffix=27
 --enable-checking=yes,glyphs --enable-check-lisp-object-type
 --with-x-toolkit=lucid --with-file-notification=yes --with-x'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11 XDBE XIM MODULES THREADS
LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_IE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  winner-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs text-property-search time-date subr-x seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils tutorial help-mode
easymenu cl-loaddefs cl-lib cus-start cus-load winner ring tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 53148 6793)
 (symbols 48 7187 1)
 (strings 32 18206 1076)
 (string-bytes 1 550231)
 (vectors 16 9603)
 (vector-slots 8 128508 8846)
 (floats 8 22 33)
 (intervals 56 228 0)
 (buffers 1000 12))

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Mon, 20 Apr 2020 00:31:25 +0100
> 
> 0. emacs -Q
> 1. M-x winner-mode RET
> 2. C-h t
> 
> The user is then presented with the following:
> 
>  NOTICE: The main purpose of the Emacs tutorial is to teach you
>  the most important standard Emacs commands (key bindings).
>  However, your Emacs has been customized by changing some of
>  these basic editing commands, so it doesn't correspond to the
>  tutorial.  We have inserted colored notices where the altered
>  commands have been introduced. [More]
> 
> Yet no such coloured notices are to be found in TUTORIAL, since
> winner-mode binds only 'C-c <left>' and 'C-c <right>', neither of which
> is mentioned in the tutorial.
> 
> Pushing the "More" button gives the following *Help* buffer contents:
> 
> --8<---------------cut here---------------start------------->8---
> The following key bindings used in the tutorial have been changed
> from the Emacs default:
> 
>    Standard Key   Command                     In Your Emacs   
>    C-c 		  mode-specific-command-prefix M-x mode-specific-command-prefix more info
> 
> It is OK to change key bindings, but changed bindings do not
> correspond to what the tutorial says.
> --8<---------------cut here---------------end--------------->8---
> 
> Pushing the "more info" button in turn gives the following *Help* text:
> 
> --8<---------------cut here---------------start------------->8---
> Your Emacs customizations override the default binding for this key:
> 
> The default Emacs binding for the key C-c is the command
> ‘mode-specific-command-prefix’.  However, your customizations have
> rebound it to the command ‘(keymap (keymap (right . winner-redo) (left
> . winner-undo)) mode-specific-command-prefix)’.
> --8<---------------cut here---------------end--------------->8---
> 
> The following patch fixes this:

Isn't the patch too general?  How do we distinguish the case where
_all_ of the subcommands were rebound, for example?  Also, don't we
have some prefixes that for the purposes of the tutorial must not have
_any_ of its subcommands rebound?

> I don't think this qualifies as urgent enough for inclusion in emacs-27,
> but I think the fix is harmless enough if desired.  WDYT?

The release branch should at this point only accept changes that are
really urgent (and documentation fixes).  This one isn't, IMO.

Thanks.

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

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: 40725@debbugs.gnu.org
> Date: Mon, 20 Apr 2020 23:19:46 +0100
> 
> The function in question, tutorial--find-changed-keys, is only ever
> passed the defconst tutorial--default-keys as argument.

Yes, and one of the aspects I thought about was whether this change
could make use less future-proof, if more keys are added.

> 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.

AFAIU from the code, the main consideration with C-c is when the user
turns on the CUA mode, not because it's a common prefix.  So maybe we
should narrow the test to only make sure CUA rebindings get caught?

> > 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?

Well, we are trying to improve the current logic, aren't we?

> > 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.

So maybe we should add that, to make the test more thorough?

> 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.

I just want to make sure we don't do anything that could cause subtle
problems.  Bugs while reading the tutorial are the worst kind, for
obvious reasons.

Thanks.

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

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: 40725@debbugs.gnu.org
>> Date: Mon, 20 Apr 2020 23:19:46 +0100
>> 
>> The function in question, tutorial--find-changed-keys, is only ever
>> passed the defconst tutorial--default-keys as argument.
>
> Yes, and one of the aspects I thought about was whether this change
> could make use less future-proof, if more keys are added.

I don't think the proposed patch makes the code any less future-proof,
but then my crystal ball isn't the best, and tutorial.el could use some
love regardless, as this bug has shown.

>> 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.
>
> AFAIU from the code, the main consideration with C-c is when the user
> turns on the CUA mode, not because it's a common prefix.  So maybe we
> should narrow the test to only make sure CUA rebindings get caught?

Good point.  I hadn't thought of that and I'll look into it.

>> > 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?
>
> Well, we are trying to improve the current logic, aren't we?

You drive a hard bargain. ;) I thought I'd suggest the current small
patch before attacking tutorial.el wholesale, but I can do both in one
go after the end of April when I'll have more free time.

While we're on the topic of improving the manual - it's on my todo to
eventually help with a Greek and possibly even Hungarian translation.
Is there anything more to it than posting a patch to
bug-gnu-emacs/emacs-devel for review, such as getting a GNU translation
team involved or anything like that?

>> > 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.
>
> So maybe we should add that, to make the test more thorough?

Sure.

>> 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.
>
> I just want to make sure we don't do anything that could cause subtle
> problems.  Bugs while reading the tutorial are the worst kind, for
> obvious reasons.

Agreed.

-- 
Basil

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: 40725@debbugs.gnu.org
> Date: Wed, 22 Apr 2020 23:26:53 +0100
> 
> > AFAIU from the code, the main consideration with C-c is when the user
> > turns on the CUA mode, not because it's a common prefix.  So maybe we
> > should narrow the test to only make sure CUA rebindings get caught?
> 
> Good point.  I hadn't thought of that and I'll look into it.

Thanks.

> >> > 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?
> >
> > Well, we are trying to improve the current logic, aren't we?
> 
> You drive a hard bargain. ;) I thought I'd suggest the current small
> patch before attacking tutorial.el wholesale, but I can do both in one
> go after the end of April when I'll have more free time.

It's fine to do it piecemeal.  But if you are not sure you will get to
that soon, perhaps leave some FIXME somewhere, so that we don't forget.

> While we're on the topic of improving the manual - it's on my todo to
> eventually help with a Greek and possibly even Hungarian translation.

You mean the tutorial, right?

> Is there anything more to it than posting a patch to
> bug-gnu-emacs/emacs-devel for review, such as getting a GNU translation
> team involved or anything like that?

No, I think posting it and letting people comment is good enough.

Thanks.

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

[Juri Linkov]

> While we're on the topic of improving the manual - it's on my todo to
> eventually help with a Greek and possibly even Hungarian translation.

The problem is that a Greek tutorial when added to a file with the .el
extension in etc/tutorials/TUTORIAL.el will be visited in Emacs Lisp mode.

Out of curiosity, I checked other tutorials, and indeed the problem exists:
the Swedish tutorial etc/tutorials/TUTORIAL.sv is visited in Verilog mode.

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  40725@debbugs.gnu.org
> Date: Fri, 24 Apr 2020 00:52:52 +0300
> 
> The problem is that a Greek tutorial when added to a file with the .el
> extension in etc/tutorials/TUTORIAL.el will be visited in Emacs Lisp mode.
> 
> Out of curiosity, I checked other tutorials, and indeed the problem exists:
> the Swedish tutorial etc/tutorials/TUTORIAL.sv is visited in Verilog mode.

File-local variables should be able to fix that, don't they?

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > While we're on the topic of improving the manual - it's on my todo to
  > > eventually help with a Greek and possibly even Hungarian translation.

  > The problem is that a Greek tutorial when added to a file with the .el
  > extension in etc/tutorials/TUTORIAL.el will be visited in Emacs Lisp mode.

  > Out of curiosity, I checked other tutorials, and indeed the problem exists:
  > the Swedish tutorial etc/tutorials/TUTORIAL.sv is visited in Verilog mode.

This is funny.

I guess we should change the names of those files, perhaps by adding
.txt at the end.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  40725@debbugs.gnu.org
> Date: Fri, 24 Apr 2020 00:52:52 +0300
> 
> The problem is that a Greek tutorial when added to a file with the .el
> extension in etc/tutorials/TUTORIAL.el will be visited in Emacs Lisp mode.
> 
> Out of curiosity, I checked other tutorials, and indeed the problem exists:
> the Swedish tutorial etc/tutorials/TUTORIAL.sv is visited in Verilog mode.

I've now tried that, and I think you saw that when visiting the
tutorial as any other file, with "C-x C-f", right?  Because starting a
Swedish tutorial with "C-u C-h t Swedish RET" visits the file in
Fundamental mode.  So I don't think the problem is serious; if we want
to cover the "C-x C-f" use case, we can always use file-local
variables.

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

[Juri Linkov]

> I've now tried that, and I think you saw that when visiting the
> tutorial as any other file, with "C-x C-f", right?  Because starting a
> Swedish tutorial with "C-u C-h t Swedish RET" visits the file in
> Fundamental mode.  So I don't think the problem is serious; if we want
> to cover the "C-x C-f" use case, we can always use file-local
> variables.

Indeed, this problem exists only for tutorial writers, not readers.
And for writers we could set a file-local variable -*- mode: text -*-

OTOH, to avoid false grep hits, files could be renamed after fixing vc-git.el
to handle renames better.

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

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>>> 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.
>>
>> AFAIU from the code, the main consideration with C-c is when the user
>> turns on the CUA mode, not because it's a common prefix.  So maybe we
>> should narrow the test to only make sure CUA rebindings get caught?
>
> Good point.  I hadn't thought of that and I'll look into it.

If I'm skimming this thread, the discussion then turned to (possibly)
renaming some tutorial files that have file names that end with things
like .el, which gives us the wrong modes.

However, the original patch, which seemed to fix a concrete issue,
doesn't seem to have been applied (because Eli thought it could be fixed
in a better way)?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Basil L. Contovounesios]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> If I'm skimming this thread, the discussion then turned to (possibly)
> renaming some tutorial files that have file names that end with things
> like .el, which gives us the wrong modes.

I haven't revisited this thread recently, but IIRC renaming files is to
be avoided in general, especially if there is an alternative solution,
such as using file variables.

> However, the original patch, which seemed to fix a concrete issue,
> doesn't seem to have been applied (because Eli thought it could be fixed
> in a better way)?

The patch can be applied as is, but I didn't do it yet because I want to
extend its scope following Eli's suggestions.  I'll try to get back to
it soon™, but don't hold your breath. :)

-- 
Basil

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

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> The patch can be applied as is, but I didn't do it yet because I want to
> extend its scope following Eli's suggestions.  I'll try to get back to
> it soon™, but don't hold your breath. :)

:-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> The patch can be applied as is, but I didn't do it yet because I want to
>> extend its scope following Eli's suggestions.  I'll try to get back to
>> it soon™, but don't hold your breath. :)
>
> :-)

*phew*  This was a year ago -- any progress here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2021-11-12 09:28 +0100] wrote:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>>
>>> The patch can be applied as is, but I didn't do it yet because I want to
>>> extend its scope following Eli's suggestions.  I'll try to get back to
>>> it soon™, but don't hold your breath. :)
>>
>> :-)
>
> *phew*  This was a year ago -- any progress here?

Sorry, I started looking at it again recently but didn't get very far.
Starting tomorrow I'm going to be AFK for a few months, so feel free to
apply the original patch as-is, and I'll continue auditing the key
bindings when I'm back.

Thanks,

-- 
Basil

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

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Sorry, I started looking at it again recently but didn't get very far.
> Starting tomorrow I'm going to be AFK for a few months, so feel free to
> apply the original patch as-is, and I'll continue auditing the key
> bindings when I'm back.

OK; I've now pushed the original patch to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Mattias Engdegård]

>> Out of curiosity, I checked other tutorials, and indeed the problem exists:
>> the Swedish tutorial etc/tutorials/TUTORIAL.sv is visited in Verilog mode. 
> 
> File-local variables should be able to fix that, don't they?

What about renaming them all to TUTORIALS-xx instead? Would that inconvenience anyone?

I'm frequently scanning all .el files in the Emacs tree with various tools, and having ones that don't contain Elisp code would make the file selection pattern awkward and ad-hoc.

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

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 24 Apr 2020 10:46:15 +0200
> Cc: 40725@debbugs.gnu.org
> 
> >> Out of curiosity, I checked other tutorials, and indeed the problem exists:
> >> the Swedish tutorial etc/tutorials/TUTORIAL.sv is visited in Verilog mode. 
> > 
> > File-local variables should be able to fix that, don't they?
> 
> What about renaming them all to TUTORIALS-xx instead? Would that inconvenience anyone?

I don't think it will inconvenient someone, but renaming files causes
complications in VCS history forensics, so I'd prefer to avoid
renaming as much as possible.

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

[Mattias Engdegård]

24 apr. 2020 kl. 12.20 skrev Eli Zaretskii <eliz@gnu.org>:

> I don't think it will inconvenient someone, but renaming files causes
> complications in VCS history forensics, so I'd prefer to avoid
> renaming as much as possible.

Since there is a clear benefit from avoiding file names that clash with well-established conventions, version control is a very minor concern and we should not be held hostage by it. Git has a lot less trouble with renames than earlier systems, and here we are talking about straight renames (not file splits), thus there is a one-to-one correspondence. Finally, these files are not changed at great frequency.

Would you prefer the rename to be done on master, or in emacs-27 (since the bug was reported against that version)?

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

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 24 Apr 2020 12:41:03 +0200
> Cc: juri@linkov.net, contovob@tcd.ie, 40725@debbugs.gnu.org
> 
> 24 apr. 2020 kl. 12.20 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > I don't think it will inconvenient someone, but renaming files causes
> > complications in VCS history forensics, so I'd prefer to avoid
> > renaming as much as possible.
> 
> Since there is a clear benefit from avoiding file names that clash with well-established conventions, version control is a very minor concern and we should not be held hostage by it. Git has a lot less trouble with renames than earlier systems, and here we are talking about straight renames (not file splits), thus there is a one-to-one correspondence. Finally, these files are not changed at great frequency.
> 
> Would you prefer the rename to be done on master, or in emacs-27 (since the bug was reported against that version)?

Like I said: I would prefer not to rename files at all, if we can
solve the mode issue in some other way.

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

[Mattias Engdegård]

24 apr. 2020 kl. 13.21 skrev Eli Zaretskii <eliz@gnu.org>:

> Like I said: I would prefer not to rename files at all, if we can
> solve the mode issue in some other way.

I agree, let's not rename files without a good reason. Here we have multiple good reasons.
The file names cause more inconvenience than just the mode problem. Git handles renames gracefully.

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

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 24 Apr 2020 13:35:52 +0200
> Cc: juri@linkov.net, contovob@tcd.ie, 40725@debbugs.gnu.org
> 
> 24 apr. 2020 kl. 13.21 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > Like I said: I would prefer not to rename files at all, if we can
> > solve the mode issue in some other way.
> 
> I agree, let's not rename files without a good reason. Here we have multiple good reasons.

I didn't say "a good reason", I said "if we can solve the issue in
some other way".

> The file names cause more inconvenience than just the mode problem.

What problems are those?  You mentioned grepping the files, which can
be easily handled by excluding the tutorials directory from the
search.  Are there any other problems we should consider?

> Git handles renames gracefully.

"Gracefully" is in the eyes of the beholder.  It's true the support
for renames improved in the recent years, but there are still commands
that either fails or need special invocation methods to work across
renames.  So it's still a source of inconvenience and occasional
failure or mistaken conclusions, and I'd like to avoid that if
possible.

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

[Mattias Engdegård]

24 apr. 2020 kl. 14.13 skrev Eli Zaretskii <eliz@gnu.org>:

> I didn't say "a good reason", I said "if we can solve the issue in
> some other way".

Why does renaming files, a quite uncomplicated operation, have to be the last resort?

>> The file names cause more inconvenience than just the mode problem.
> 
> What problems are those?  You mentioned grepping the files, which can
> be easily handled by excluding the tutorials directory from the
> search.  Are there any other problems we should consider?

I didn't mention grepping (scanning files can be done in multiple ways), but why would I or anyone else need to perform such contortions every time to compensate for a problem that is so easily solved once and for all by simply giving the files better names?

>> Git handles renames gracefully.
> 
> "Gracefully" is in the eyes of the beholder.  It's true the support
> for renames improved in the recent years, but there are still commands
> that either fails or need special invocation methods to work across
> renames.  So it's still a source of inconvenience and occasional
> failure or mistaken conclusions, and I'd like to avoid that if
> possible.

You are making a mountain out of a molehill. It's not going to be a problem; this is not CVS. Let's not invent difficulties.
Nowadays, the bar for renaming files is much lower, and there is no reason whatsoever to shy away from it because of bad experiences in the past with other version control systems.

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

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 24 Apr 2020 14:47:44 +0200
> Cc: juri@linkov.net, contovob@tcd.ie, 40725@debbugs.gnu.org
> 
> 24 apr. 2020 kl. 14.13 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > I didn't say "a good reason", I said "if we can solve the issue in
> > some other way".
> 
> Why does renaming files, a quite uncomplicated operation, have to be the last resort?

I thought I explained why.

> > What problems are those?  You mentioned grepping the files, which can
> > be easily handled by excluding the tutorials directory from the
> > search.  Are there any other problems we should consider?
> 
> I didn't mention grepping (scanning files can be done in multiple ways), but why would I or anyone else need to perform such contortions every time to compensate for a problem that is so easily solved once and for all by simply giving the files better names?

Because renaming comes with a price I'd rather not pay.

> > "Gracefully" is in the eyes of the beholder.  It's true the support
> > for renames improved in the recent years, but there are still commands
> > that either fails or need special invocation methods to work across
> > renames.  So it's still a source of inconvenience and occasional
> > failure or mistaken conclusions, and I'd like to avoid that if
> > possible.
> 
> You are making a mountain out of a molehill.

It isn't a molehill for me.  I need to search VCS history quite a lot.

> It's not going to be a problem; this is not CVS. Let's not invent difficulties.

I'm not inventing anything, the problems are real.

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

[Mattias Engdegård]

24 apr. 2020 kl. 15.32 skrev Eli Zaretskii <eliz@gnu.org>:

> I thought I explained why.

You didn't, but now you have. Thank you Eli, I understand perfectly.

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > You are making a mountain out of a molehill.

  > It isn't a molehill for me.  I need to search VCS history quite a lot.

If we rename a file, can't we rename its master file too, so that the
history before the rename is include in what you search?

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: mattiase@acm.org, contovob@tcd.ie, 40725@debbugs.gnu.org,
> 	juri@linkov.net
> Date: Fri, 24 Apr 2020 23:33:22 -0400
> 
>   > > You are making a mountain out of a molehill.
> 
>   > It isn't a molehill for me.  I need to search VCS history quite a lot.
> 
> If we rename a file, can't we rename its master file too, so that the
> history before the rename is include in what you search?

With Git (and most modern VCS) there's no master file, as there were
with RCS and CVS.  Changes are tracked on the entire tree level, and
stored as binary blobs in a special subdirectory (under the .git
directory in the case of Git).  I don't think it's possible to "edit"
those blobs post-mortem to do the equivalent of the renaming you
mention, but even if it were possible, it's a bad idea, because of at
least 2 reasons: (a) it changes history, i.e. pretends that this file
was always called by that new name; and (b) in modern VCS systems each
commit's changeset is hashed and its hash is stored, so changing even
one byte of that would cause the hash be incorrect, and the result
will be a corrupted repository, AFAIU.

So I think this kind of history rewriting is impossible, let alone not
a good idea.

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

It is too bad that the option of renaming the file's whole history in
the repo has been lost.  It has been useful.  I disagree with the idea
that there is anything wrong about doing so.

The change history is not meant as evidence for a court case.  It is a
log of past changes to the code, to help us understand how to change
it in the future.  What happened in the past is not a crucial question
in its own right; rather, it matters because it can help answer
questions such as why something fails now and how to change it.

If a source file (let's say, flles.el) had a different name in the
past, that makes little difference to maintaining the code in that
file, as long as the renaming didn't change when and where the file is
loaded.  So it would be a significant help, and little loss, to put
all the records for that file's code under the name "files.el",
including records from when the file had another name.  This is not a
kind of tampering.  It is filing the info under the name where we
would look for it.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: contovob@tcd.ie, mattiase@acm.org, 40725@debbugs.gnu.org,
> 	juri@linkov.net
> Date: Sun, 26 Apr 2020 22:18:21 -0400
> 
> It is too bad that the option of renaming the file's whole history in
> the repo has been lost.  It has been useful.  I disagree with the idea
> that there is anything wrong about doing so.

With modern distributed VCSes, there's no single p-lace where the
history is held, so changing it make a problem for many people whose
clones are now divergent, and cannot be updated.

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

[Andreas Schwab]

On Apr 26 2020, Richard Stallman wrote:

> It is too bad that the option of renaming the file's whole history in
> the repo has been lost.  It has been useful.

It has never been useful.

> I disagree with the idea that there is anything wrong about doing so.

Renaming a file after the fact will break all historical references.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > It is too bad that the option of renaming the file's whole history in
  > > the repo has been lost.  It has been useful.

  > It has never been useful.

How can you be so sure?

  > > I disagree with the idea that there is anything wrong about doing so.

  > Renaming a file after the fact will break all historical references.

Which historical references, and in what sense would it "break" them?

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Andreas Schwab]

On Apr 27 2020, Richard Stallman wrote:

> Which historical references, and in what sense would it "break" them?

Any place that mentions the file name in any point of history.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > Which historical references, and in what sense would it "break" them?

  > Any place that mentions the file name in any point of history.

It is hard to understand what those words mean.  I am compelled to
resort to guessing.  But I think the statement is not correct.

I think the term "break" fits only if the name needs to be understood
by a program,  In that case, a program looking at the old file name
would not find the file (unless we fix that program).

However, a human can learn about the rename, and then understand what to
do on seeing the old name.  For this, "broken" is too strong a word to use.

The change history should record the rename.  Then a human who sees a reference
to the old name can search for it and find out that the file was renamed.


-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Tue, 28 Apr 2020 23:25:45 -0400
> Cc: contovob@tcd.ie, mattiase@acm.org, 40725@debbugs.gnu.org, juri@linkov.net
> 
> The change history should record the rename.  Then a human who sees a reference
> to the old name can search for it and find out that the file was renamed.

Not all modern VCSes record renames.  Git doesn't.  Instead, it
guesses the renames by comparing deleted files' contents with that of
files added in the same commit.

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > The change history should record the rename.  Then a human who sees a reference
  > > to the old name can search for it and find out that the file was renamed.

  > Not all modern VCSes record renames.  Git doesn't.

If the change history won't do this automatically, the developers
should write it explicitly in some log entry.  That won't be hard.

Also, it is useful to insert a file's previous names in a comment near
the start of the file.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> Git handles renames gracefully.
>
> "Gracefully" is in the eyes of the beholder.  It's true the support
> for renames improved in the recent years, but there are still commands
> that either fails or need special invocation methods to work across
> renames.  So it's still a source of inconvenience and occasional
> failure or mistaken conclusions, and I'd like to avoid that if
> possible.

Out of curiosity, would it help if VC turned on these "special
invocation methods"[1] by default?

I ask because there are obvious advantages to renaming: Mattias gave
"multiple good reasons" that are specific to this thread; more
generally, better naming helps discoverability and onboarding, which
Emacs could always use more of.

Of course, if the costs of renaming (in terms of inconvenience for
maintainers) cannot be made low enough, it's not worth lingering on the
benefits.  I'm just wondering if there will come a day where The
Technology™ gets good enough that we can afford renames[2], or if it
will forever remain the "bane" of maintainers.



[1] I am thinking of git-log's --follow option and git-diff's
    --find-rename option; I might be missing other knobs.

[2] Dmitry pointed out bug#39044; I don't know how many other such
    issues are lying around, or if there are some "war stories" people
    can readily recount to show shortcomings in the current handling of
    renames.

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

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Mattias Engdegård <mattiase@acm.org>,  contovob@tcd.ie,
>   40725@debbugs.gnu.org,  juri@linkov.net
> Date: Fri, 24 Apr 2020 21:48:02 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Git handles renames gracefully.
> >
> > "Gracefully" is in the eyes of the beholder.  It's true the support
> > for renames improved in the recent years, but there are still commands
> > that either fails or need special invocation methods to work across
> > renames.  So it's still a source of inconvenience and occasional
> > failure or mistaken conclusions, and I'd like to avoid that if
> > possible.
> 
> Out of curiosity, would it help if VC turned on these "special
> invocation methods"[1] by default?

They have limitations, and quite a few are expensive.

> [1] I am thinking of git-log's --follow option and git-diff's
>     --find-rename option; I might be missing other knobs.

"git log"'s --follow only works for single files (and it also has
"--find-renames", which AFAIK is expensive).

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

[Dmitry Gutov]

On 24.04.2020 14:35, Mattias Engdegård wrote:
> The file names cause more inconvenience than just the mode problem. Git handles renames gracefully.

VC, less so. Here's a related bug report: 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39044

It's not insurmountable, of course (I expect we'll fix it for Emacs 28). 
But let's not ignore the problems either.

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

[Eli Zaretskii]

> Cc: contovob@tcd.ie, 40725@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 24 Apr 2020 18:38:29 +0300
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39044
> 
> It's not insurmountable, of course (I expect we'll fix it for Emacs 28). 
> But let's not ignore the problems either.

The problems aren't insurmountable, sure.  They just add
complications.  In most cases, if you are aware a rename was made, you
can tweak the VCS commands to handle that.  But (a) you need to be
aware, and that is not always obvious; and (b) how to tweak the
commands is also not always obvious.  If you fail to notice and/or
fail to tweak the command as required, you might very well see
incorrect information and make the wrong decisions.  It happened to me
several times, so I'd rather we avoided that if possible.