GNU bug report logs -
#72995
31.0.50; widget-move fails when widget starts at the second character in the buffer
Previous Next
Reported by: Dale <dale <at> codefu.org>
Date: Tue, 3 Sep 2024 04:38:01 UTC
Severity: normal
Found in version 31.0.50
Done: Stephen Berman <stephen.berman <at> gmx.net>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Your message dated Sat, 14 Sep 2024 12:47:13 +0200
with message-id <8734m2ikb2.fsf <at> gmx.net>
and subject line Re: bug#72995: 31.0.50; widget-move fails when widget starts at the second character in the buffer
has caused the debbugs.gnu.org bug report #72995,
regarding 31.0.50; widget-move fails when widget starts at the second character in the buffer
to be marked as done.
(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)
--
72995: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72995
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
I think changes in commit 94dec95 (bug#69943) broke `widget-move' in a customize buffer when trying to move to the first widget in a buffer when that first widget starts at the second character in the buffer. Here's some code to reproduce (tested in IELM):
(let* ((first-wid (progn (widget-forward 1) (point))))
(print (format "First widget at %S" first-wid))
(cl-assert (and (numberp first-wid) (>= first-wid 1)))
(with-current-buffer (customize-group 'editing)
(narrow-to-region (1- first-wid) (point-max))
(goto-char (point-min))
(widget-forward 1)
(print (format "Expected to be at %S, point=%S" first-wid (point)))))
On my Emacs I get:
"First widget at 33"
"Expected to be at 33, point=32"
I think this happens because of this code near the end of `widget-move' (which is called by `widget-forward'):
(let ((new (widget-tabable-at)))
(while (and (eq (widget-tabable-at) new) (not (bobp)))
(backward-char)))
(unless (bobp) (forward-char)))
In my test case, as we enter the while loop point is at the start of the first widget (AKA "new"). We are not yet at beginning of buffer, so it moves point back one character. Now we are at beginning of buffer, but that doesn't matter: the `eq' test fails first, and the loop ends.
However, the `forward-char' never runs because we are indeed at beginning of buffer now. I think this `forward-char' should have been run to put point back on the start of the widget.
Bug#70594 also recently modified code around here, but I don't *think* that's relevant.
In case you're wondering, this comes up because I use link-hint[1], which narrows a customize buffer in exactly the way shown above.
[1]: https://github.com/noctuid/link-hint.el
Please let me know if I can provide any more information!
Best regards,
Dale
In GNU Emacs 31.0.50 (build 1, aarch64-apple-darwin23.6.0, NS
appkit-2487.70 Version 14.6.1 (Build 23G93)) of 2024-09-01 built on
mymacRepository revision: 0f98515cec1298bc82c589b5f8565503900b806d
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2487
System Description: macOS 14.6.1
Configured using:
'configure --without-x --with-xwidgets --with-json --with-tree-sitter --without-imagemagick --with-xpm --with-jpeg --with-tiff --with-gif --with-png --with-rsvg --with-webp --with-sqlite3 --with-cairo --with-xml2 --with-gnutls --with-zlib --with-modules --with-threads --with-native-compilation --with-ns --enable-ns-self-contained 'CFLAGS= -D_DARWIN_UNLIMITED_SELECT=1 -DFD_SETSIZE=10240''
[Message part 3 (message/rfc822, inline)]
On Sat, 14 Sep 2024 10:44:42 +0300 Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Cc: 72995 <at> debbugs.gnu.org
>> Date: Tue, 03 Sep 2024 13:11:40 +0200
>> From: Stephen Berman via "Bug reports for GNU Emacs,
>> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> On Mon, 2 Sep 2024 23:36:35 -0500 Dale <dale <at> codefu.org> wrote:
>>
>> > I think changes in commit 94dec95 (bug#69943) broke `widget-move' in a
>> > customize buffer when trying to move to the first widget in a buffer when that
>> > first widget starts at the second character in the buffer. Here's some code
>> > to reproduce (tested in IELM):
>> >
>> > (let* ((first-wid (progn (widget-forward 1) (point))))
>> > (print (format "First widget at %S" first-wid))
>> > (cl-assert (and (numberp first-wid) (>= first-wid 1)))
>> > (with-current-buffer (customize-group 'editing)
>> > (narrow-to-region (1- first-wid) (point-max))
>> > (goto-char (point-min))
>> > (widget-forward 1)
>> > (print (format "Expected to be at %S, point=%S" first-wid (point)))))
>> >
>> > On my Emacs I get:
>> >
>> > "First widget at 33"
>> >
>> > "Expected to be at 33, point=32"
>> >
>> > I think this happens because of this code near the end of `widget-move' (which
>> > is called by `widget-forward'):
>> >
>> > (let ((new (widget-tabable-at)))
>> > (while (and (eq (widget-tabable-at) new) (not (bobp)))
>> > (backward-char)))
>> > (unless (bobp) (forward-char)))
>> >
>> > In my test case, as we enter the while loop point is at the start of the first
>> > widget (AKA "new"). We are not yet at beginning of buffer, so it moves point
>> > back one character. Now we are at beginning of buffer, but that doesn't
>> > matter: the `eq' test fails first, and the loop ends.
>> >
>> > However, the `forward-char' never runs because we are indeed at beginning of
>> > buffer now. I think this `forward-char' should have been run to put point
>> > back on the start of the widget.
>> >
>> > Bug#70594 also recently modified code around here, but I don't *think* that's
>> > relevant.
>> >
>> > In case you're wondering, this comes up because I use link-hint[1], which
>> > narrows a customize buffer in exactly the way shown above.
>> >
>> > [1]: https://github.com/noctuid/link-hint.el
>> >
>> > Please let me know if I can provide any more information!
>>
>> Thanks for the bug report, which indeed shows a use case that the change
>> in commit 94dec95 fails to account for. The reason that commit
>> conditionalized the call to forward-char was to ensure that tabbing puts
>> point at the start of the widget, but your case shows using just (bobp)
>> as the condition is insufficient (I used that because the existing unit
>> test for widget-move has the first widget starting at BOB). I think the
>> following patch closes this gap:
>>
>> diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
>> index e7e6351fcab..7eec06297ce 100644
>> --- a/lisp/wid-edit.el
>> +++ b/lisp/wid-edit.el
>> @@ -1336,7 +1336,7 @@ widget-move
>> (let ((new (widget-tabable-at)))
>> (while (and (eq (widget-tabable-at) new) (not (bobp)))
>> (backward-char)))
>> - (unless (bobp) (forward-char)))
>> + (unless (and (widget-tabable-at) (bobp)) (forward-char)))
>> (unless suppress-echo
>> (widget-echo-help (point)))
>> (run-hooks 'widget-move-hook))
>>
>> Can you confirm that this fixes your use case? If so, I think it should
>> go into emacs-30, since that's where the faulty commit first appeared.
>> I'll also add a unit test for this case.
>
> Steve, do you think this should be installed on the emacs-30 branch
> now, even though we had no response from Dale?
Yes, and I intended to ask today if I should do that, but you beat me to
the punch :). The reason I wanted confirmation is that I cannot
reproduce the problem via ielm as in the OP, so perhaps there's more
involved. But with `M-x customize-group RET editing' and evaluating the
sexp in the Customize buffer, the problem is apparent, and is
reproducible without Customize but just by using simple widgets (that's
what the test I added does). So it is definitely a regression and
should be fixed in emacs-30.
> If so, please
> install, and thanks.
Done in commit d509a356997 to emacs-30 and closing the bug. (If there's
something I missed having to do with ielm, we can reopen the bug.)
Steve Berman
This bug report was last modified 302 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.