bug#70937: 30.0.50; todo-mode item insertion bug

[Stephen Berman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

0. emacs -Q  (If you don't use todo-mode and don't want to change
   ~/.emacs.d by carrying out the following recipe, you can start Emacs
   with e.g. `HOME=/tmp/user emacs -Q'.)

1. Type `M-x todo-show'.  If you have not already created a todo file in
   todo-mode, respond to the successive prompts to enter a todo file
   name, a category, and an item, e.g. "todo RET test RET test RET".
   If you do already have a todo file, by typing `M-x todo-show' you are
   now visiting it in todo-mode.
   
2. Now in the current todo category type `i' to initiate an item
   insertion command.  This displays the following prompt in the echo
   area:

   Press a key (so far ‘i’):  { i=>default p=>copy } { y=>diary k=>nonmarking } { c=>calendar d=>date n=>dayname } t=>time { h=>here r=>region }

   You build an item insertion command by successively typing a key from
   one or more of the groups in the prompt (details are in the Todo
   manual).  You can skip a group but you cannot enter a key out of the
   given order.  So e.g. `i y c t h' and `i k r' are valid item
   insertion commands, but `i c y' and `i t k' are invalid.

3. If you do enter `i c y' you get the error message "y is undefined"
   and the item insertion command construction is broken off.  That's
   fine, because `y' has been entered out of order for item insertion
   and is otherwise not a key for a todo-mode command.
   
4. Now comes the bug: If you enter `i t k', after pressing `k' the item
   at point is highlighted and the echo area displays the prompt
   "Permanently delete this item? (y or no)".  This is because `k' is
   also bound in todo-mode to the command for deleting an item; it can
   only be part of a item insertion command when it is entered before
   any item insertion key from a later group (thus, `i k t k' is also
   invalid and has the same effect as `i t k').  Likewise, typing `i t
   p', `i t n' or `i t d' breaks off the item insertion command and
   executes a non-insertion todo-mode command.  And if you mistakenly
   type a key that is not an item insertion key but is bound in
   todo-mode, e.g. `m' in the sequence `i m', then the todo-mode command
   it is bound to is executed.  Such unintended command execution is at
   least confusing, and in the worst case can result in data loss
   (e.g. with `k' if the user is inattentive).

The attached patch prevents the unintended execution of a todo-mode
command due to mistaken or out-of-order item insertion key input.  In
that case a message is displayed, e.g. "‘k’ is not a valid remaining
item insertion key" (the word "remaining" is meant to indicate that the
key is not valid at this point in the sequence), and the construction of
the item insertion command breaks off.

Instead of breaking off the key input, a more user-friendly fix might be
to continue prompting to enter the item insertion keys as long as an
invalid key is entered, but I haven't found a way to do that within the
current implemention, which uses set-transient-map, and changing the
implemention seems non-trivial and trying to do that just to slightly
increase the user-friendliness is probably more effort than it's worth.
But if someone sees a straightforward way to do that, I'm all ears.

Otherwise, if there are no objections within a few days I will install
the attached patch to master (the bug has existed since the use of
set-transient-map (then still called set-temporary-overlay-map) was
added to todo-mode.el in commit f3a66082).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: todo-mode.el patch --]
[-- Type: text/x-patch, Size: 1364 bytes --]

diff --git a/lisp/calendar/todo-mode.el b/lisp/calendar/todo-mode.el
index 12287299a7f..36f1982f447 100644
--- a/lisp/calendar/todo-mode.el
+++ b/lisp/calendar/todo-mode.el
@@ -5794,7 +5794,24 @@ todo-insert-item--next-param
               (apply #'todo-insert-item--basic (nconc arg parlist)))))
          ;; Operate on a copy of the parameter list so the original is
          ;; not consumed, thus available for the next key typed.
-         (params0 params))
+         (params0 params)
+         (tm-keys (let (l)
+                    (map-keymap (lambda (key _binding)
+                                  (push key l))
+                                todo-mode-map)
+                    l)))
+    ;; Initially assign each key in todo-mode-map a function identifying
+    ;; it as invalid for item insertion, thus preventing mistakenly
+    ;; pressing a key from executing an unwanted different todo-mode
+    ;; command (bug#xxxx); the actual item insertion keys are redefined
+    ;; when looping over the item insertion parameters.
+    (dolist (k tm-keys)
+      (when (characterp k)
+        (define-key map (string k)
+          (lambda ()
+            (interactive)
+            (message (concat "`%s' is not a valid remaining item insertion key")
+                     (string k))))))
     (when last
       (if (memq last '(default copy))
 	  (progn

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



In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-05-08 built on strobelfs
Repository revision: e020f4e9ce5d98438033fea098d943c311b0fa3d
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Linux From Scratch r12.1-53

Configured using:
 'configure -C 'CFLAGS=-Og -g3' PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG
RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER
WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB