bug#66994: 30.0.50; Emacs hangs in Todo mode when moving an item to another todo file

bug#66994: 30.0.50; Emacs hangs in Todo mode when moving an item to another todo file

[Stephen Berman]

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

If you have at least two files created using todo-mode, you can move an
item from a category in one todo file to a category in another todo file
by typing `C-u m' with point on the item to be moved.  But if the file
the item is to be moved to is not yet visiting a buffer in todo-mode
when the command is invoked, this results in an infinite loop.  The
patch below prevents this and allows the move to succeed.  For those
interested, an analysis of the bug follows.

The call chain leading to the hang is this: todo-move-item ->
todo-set-item-priority -> todo-marked-item-p -> todo-get-overlay ->
todo-item-start.  The latter function tests conditions under which point
is not on an item; if these tests fail, a loop does a search backwards
in the file to find the item start.  But the tests overlook the case at
hand, where a todo file is loaded into a buffer but todo-mode is not yet
set: then point is at the start of the file, whose first line contains
metadata that is hidden in todo-mode.  Since this case is not tested and
the other tests fail, the backwards search begins, but since point is
already at position 1, it is impossible to find an item start, so the
loop is never exited.

Looking through the commit history, it seems this bug has existed since
I first added the code, which was before my rewrite of todo-mode.el was
merged into Emacs.  Evidently I had never tested precisely this use
case.  (One reason for that may be that it is also possible to move an
item between files by typing just `m', when the goal file's name is in
the list `todo-category-completions-files'; but in this case the file is
set to todo-mode before the movement, so the condition triggering the
hang is not met.  I have often used this functionality, but evidently
not the movement with `C-u m'.)  But I now encountered this bug while
testing changes in todo-mode.el to support changing the format of item
date headers, as announced in bug#66395.

Given that it's an old bug that has apparently never been triggered in
normal use of todo-mode, I have no problem committing the fix to master.
But since an infinite loop is always nasty and the fix is
straightforward and AFAICT safe, I think it's suitable for the release
branch.  I'll wait for the maintainers' decision.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.18.0) of 2023-11-06 built on strobelfs2
Repository revision: bf81706988f6b1b9d6e8033c8227f0129e04ef03
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Linux From Scratch r12.0-63

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: todo-item-start patch --]
[-- Type: text/x-patch, Size: 680 bytes --]

diff --git a/lisp/calendar/todo-mode.el b/lisp/calendar/todo-mode.el
index 093ea0e22b6..86bf3afbce7 100644
--- a/lisp/calendar/todo-mode.el
+++ b/lisp/calendar/todo-mode.el
@@ -5277,7 +5277,9 @@ todo-item-start
            ;; Point is on done items separator.
            (save-excursion (beginning-of-line) (looking-at todo-category-done))
 	   ;; Buffer is widened.
-	   (looking-at (regexp-quote todo-category-beg)))
+	   (looking-at (regexp-quote todo-category-beg))
+           ;; With `C-u m' to a file that is then loaded into a buffer.
+	   (= (point) 1))
     (goto-char (line-beginning-position))
     (while (not (looking-at todo-item-start))
       (forward-line -1))

bug#66994: 30.0.50; Emacs hangs in Todo mode when moving an item to another todo file

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Wed, 08 Nov 2023 00:13:36 +0100
> 
> Given that it's an old bug that has apparently never been triggered in
> normal use of todo-mode, I have no problem committing the fix to master.
> But since an infinite loop is always nasty and the fix is
> straightforward and AFAICT safe, I think it's suitable for the release
> branch.  I'll wait for the maintainers' decision.

I'm okay with installing this on the emacs-29 branch, but please
improve the comment to the line you added, so that it explains better
why this condition is needed and how it is related to "C-u m":

> -	   (looking-at (regexp-quote todo-category-beg)))
> +	   (looking-at (regexp-quote todo-category-beg))
> +           ;; With `C-u m' to a file that is then loaded into a buffer.
> +	   (= (point) 1))

Also, should this test point-min instead of literally 1?

Thanks.

bug#66994: 30.0.50; Emacs hangs in Todo mode when moving an item to another todo file

[Stephen Berman]

On Wed, 08 Nov 2023 14:27:33 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Wed, 08 Nov 2023 00:13:36 +0100
>>
>> Given that it's an old bug that has apparently never been triggered in
>> normal use of todo-mode, I have no problem committing the fix to master.
>> But since an infinite loop is always nasty and the fix is
>> straightforward and AFAICT safe, I think it's suitable for the release
>> branch.  I'll wait for the maintainers' decision.
>
> I'm okay with installing this on the emacs-29 branch, but please
> improve the comment to the line you added, so that it explains better
> why this condition is needed and how it is related to "C-u m":

Done.

>> -	   (looking-at (regexp-quote todo-category-beg)))
>> +	   (looking-at (regexp-quote todo-category-beg))
>> +           ;; With `C-u m' to a file that is then loaded into a buffer.
>> +	   (= (point) 1))
>
> Also, should this test point-min instead of literally 1?

No, the infinite loop happens when point is at the start of the file.
Todo mode uses narrowing to display categories, and in that case the
problem does not arise, because then point-min is the location of the
start of the first item in the category.

Fixed in commit b7871cefe7b to emacs-29.  Thanks.

Steve Berman