GNU bug report logs - #74090
31.0.50; Problems with dabbrev-expand

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74090 in the body.
You can then email your comments to 74090 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; Problems with dabbrev-expand
Date: Tue, 29 Oct 2024 18:06:18 +0100

[Message part 1 (text/plain, inline)]

If an expansion found by dabbrev-expand is located in a different buffer
than the buffer from which dabbrev-expand was invoked, the following
kinds of problems can occur:

First problem:
0. emacs -Q
1. Visit (for example) the INSTALL file in the Emacs source tree.
2. Switch to a new buffer (either empty or at least not containing any
   text that can be a dabbrev expansion), type into it the string "Ind"
   and then type `M-/' (dabbrev-expand), which expands "Ind" to "Indic".
3. Type `SPC M-/'
=> This does not expand the buffer substring "Indic and" but to
   "Indicmacs", which is not a string in any live buffer.

A variant of this problem:
After switching to a new buffer replace the rest of step 2 by the
following:
2a. Type `Indic SPC M-/'.  This correctly expands to "Indic and".
3. Type `SPC M-/'
=> This does not expand the buffer substring "Indic and Khmer" but to
   "Indic and Installation", which is not a string in any live buffer.

Another variant of this problem:
After visiting INSTALL, type `M-x find-library RET dabbrev RET' to visit
the dabbrev.el source file, then switch to a new buffer, e.g. `C-x b a'
and type `M-x emacs-lisp-mode'.  Now continue:
2b. Type `Ind SPC M-/'.  This expands to "Indicate" and displays the
    message "Expansion found in ‘dabbrev.el’".
3b. Type `M-/' to replace the expansion.  This now expands to "Indic"
    and displays the message "Expansion found in ‘INSTALL’".
4. Type `SPC M-/'
=> This expands as above to "Indicmacs", which is not a string in any
   live buffer.

Second problem:
0. emacs -Q
1. Visit (for example) the INSTALL file in the Emacs source tree.
2. Type `C-s Ind M-a C-SPC M-} C-x n n' and at the prompt type SPC to
   narrow the buffer to the paragraph containing "Indic".
3. As in step 2 of the first problem, switch to a new buffer, e.g. `C-x
   b a' and then type `Ind M-/'.  As above, this expands to "Indic".
4. Type `SPC M-/'.
=> This raises the error "Args out of range: #<buffer INSTALL>, #<marker
   at 6 in a>, 4886".


These problems also occur if, instead of invoking dabbrev-expand from an
ordinary buffer, you invoke a command that accepts text input in the
minibuffer, e.g `M-%', and then invoke dabbrev-expand from there:

- For the first case, typing `M-% Ind M-/' in the minibuffer expands to
  "Indic", but now typing `SPC M-/' expands the string to "Indicon",
  which is not a string in INSTALL or any other live buffer.

- For the first variant of the first case, typing `M-% Indic SPC M-/' in
  the minibuffer correctly expands to "Indic and", but typing `SPC M-/'
  again expands the string to "Indic anduide", which is not a string in
  INSTALL or any other live buffer.

- For the second variant of the first case, after visiting INSTALL and
  dabbrev.el, in the latter buffer first typing `M-% Ind M-/' in the
  minibuffer expands to "Indicate", then typing `M-/' replaces the
  expansion with "Indic", and now typing `SPC M-/' expands string to
  "Indicon", which is not a string in any live buffer.

- For the second case, after narrowing buffer INSTALL as above, typing
  `M-% Ind M-/' expands to "Indic" and then typing SPC M-/' results in
  the error "Args out of range: #<buffer INSTALL>, #<marker at 31 in
  *Minibuf-1*>, 4886".

(For the record, I'm pretty sure I've encountered the first problem and
its variants in my normal use of Emacs, though I can't recall specific
instances.  I don't recall encountering problems with dabbrev-expand
when using query-replace (but I probably seldom use dabbrev-expand with
query-replace); however, a number of times I have hit the
args-out-of-range error on invoking dabbrev-expand in the minibuffer in
todo-mode, and that is what motivated me to debug the problem, which led
to finding the other dabbrev-expand problems and variants.  I used
query-replace instead of todo-mode in the above recipes to keep them
simpler by not having to include the additional steps needed to use
todo-mode.)

From my debugging of these problems, I think they arise because the code
in dabbrev-expand that sets up looking for an expansion (either
directly, or after a space, or as a replacement for the current
expansion) wrongly using positions in the buffer from which
dabbrev-expand was invoked instead of the buffer in which the expansion
is found.  The attached patch fixes these problems, according to my
tests.  The patch includes comments justifying or at least motivating
the changes.

The patch also includes new tests for dabbrev-tests.el, both for the
cases described above as well as for the same operations but confined to
a single buffer, for which the current code yields correct results (the
tests for the problematic cases fail with the current code, of course).
After applying my fix to dabbrev.el all tests pass with `make check' as
well as in a batch run.  However, the four "other-buffer" tests fail
when manually running ert in the dabbrev-tests.el buffer, and I don't
know why; from the ERT output it looks like dabbrev-expand is treating
the test file buffer as the current buffer when manually running these
tests, but I don't see how that happens.  Another unresolved issue is
with the test dabbrev-expand-test-minibuffer-3: I could only get this to
pass by invoking `dabbrev--reset-global-variables', and I don't know why
that is needed here but not in the other tests.  (The patch omits the
two new resource files used in the tests, which are copies of parts of
INSTALL and dabbrev.el from the Emacs sources).


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.43, cairo version 1.18.2) of 2024-10-29 built on strobelfssd
Repository revision: 9aa186592634212fcdb2dbafdfd0c52a2475ba96
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Linux From Scratch r12.2-17-systemd

Configured using:
 'configure -C 'CFLAGS=-Og -g3' PKG_CONFIG_PATH=/opt/qt6/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

[Message part 2 (text/x-patch, attachment)]

Message #8 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Tue, 29 Oct 2024 20:17:18 +0200

> The attached patch fixes these problems, according to my tests.

Maybe your patch also fixes bug#36516
where we failed to find a solution.
Or maybe these are separate cases.

Message #11 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Tue, 29 Oct 2024 19:57:54 +0100

On Tue, 29 Oct 2024 20:17:18 +0200 Juri Linkov <juri <at> linkov.net> wrote:

>> The attached patch fixes these problems, according to my tests.
>
> Maybe your patch also fixes bug#36516
> where we failed to find a solution.
> Or maybe these are separate cases.

I was unaware of that bug, thanks for the pointer.  But unfortunately,
my patch does not fix it.  My changes only affect cases where the buffer
in which dabbrev-expand is called is different from the buffer where it
finds the expected expansion (at least I only tried to fix such cases
and not alter the behavior of the same buffer cases).  I'll try taking a
closer look at this case, but judging by the discussion in that bug
thread, it doesn't look related, or easy to fix.  But as was noted in
that thread, at least you can get the expected expected by repeatedly
typing `M-/', while in the cases I tried (hopefully successfully) to
fix, without my patch either you cannot get the expected expansion or
you get an error, so bug#36516 seems less serious.

Steve Berman

Message #14 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Wed, 30 Oct 2024 09:32:17 +0200

>>> The attached patch fixes these problems, according to my tests.
>>
>> Maybe your patch also fixes bug#36516
>> where we failed to find a solution.
>> Or maybe these are separate cases.
>
> I was unaware of that bug, thanks for the pointer.  But unfortunately,
> my patch does not fix it.  My changes only affect cases where the buffer
> in which dabbrev-expand is called is different from the buffer where it
> finds the expected expansion (at least I only tried to fix such cases
> and not alter the behavior of the same buffer cases).  I'll try taking a
> closer look at this case, but judging by the discussion in that bug
> thread, it doesn't look related, or easy to fix.  But as was noted in
> that thread, at least you can get the expected expected by repeatedly
> typing `M-/', while in the cases I tried (hopefully successfully) to
> fix, without my patch either you cannot get the expected expansion or
> you get an error, so bug#36516 seems less serious.

Indeed, bug#36516 reports just a logical inconsistency,
whereas your patch fixes a plain bug.  I've looked at your patch,
and everything looks right, and the comprehensive test coverage
will ensure no breakages, thanks.

Message #17 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Wed, 30 Oct 2024 15:03:17 +0200

> Date: Tue, 29 Oct 2024 18:06:18 +0100
> From:  Stephen Berman via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> >From my debugging of these problems, I think they arise because the code
> in dabbrev-expand that sets up looking for an expansion (either
> directly, or after a space, or as a replacement for the current
> expansion) wrongly using positions in the buffer from which
> dabbrev-expand was invoked instead of the buffer in which the expansion
> is found.  The attached patch fixes these problems, according to my
> tests.  The patch includes comments justifying or at least motivating
> the changes.

Thanks, feel free to install on the emacs-30 branch.

> +                 ;; Comparing with point only makes sense in the buffer
> +                 ;; where we called dabbrev-exand, but if that differs
                                       ^^^^^^^^^^^^^
Typo.

Message #20 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 11:00:17 +0100

On Wed, 30 Oct 2024 15:03:17 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:

>> Date: Tue, 29 Oct 2024 18:06:18 +0100
>> From:  Stephen Berman via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>
>> >From my debugging of these problems, I think they arise because the code
>> in dabbrev-expand that sets up looking for an expansion (either
>> directly, or after a space, or as a replacement for the current
>> expansion) wrongly using positions in the buffer from which
>> dabbrev-expand was invoked instead of the buffer in which the expansion
>> is found.  The attached patch fixes these problems, according to my
>> tests.  The patch includes comments justifying or at least motivating
>> the changes.
>
> Thanks, feel free to install on the emacs-30 branch.
>
>> +                 ;; Comparing with point only makes sense in the buffer
>> +                 ;; where we called dabbrev-exand, but if that differs
>                                        ^^^^^^^^^^^^^
> Typo.

Thanks, typo corrected and pushed as commit f6c359cb66a -- but to
master, I just now noticed that you said emacs-30.  I tried reverting
but I don't know how.  What should I do?  Sorry for the snafu.

Steve Berman

Message #23 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 11:01:04 +0100

On Wed, 30 Oct 2024 09:32:17 +0200 Juri Linkov <juri <at> linkov.net> wrote:

>>>> The attached patch fixes these problems, according to my tests.
>>>
>>> Maybe your patch also fixes bug#36516
>>> where we failed to find a solution.
>>> Or maybe these are separate cases.
>>
>> I was unaware of that bug, thanks for the pointer.  But unfortunately,
>> my patch does not fix it.  My changes only affect cases where the buffer
>> in which dabbrev-expand is called is different from the buffer where it
>> finds the expected expansion (at least I only tried to fix such cases
>> and not alter the behavior of the same buffer cases).  I'll try taking a
>> closer look at this case, but judging by the discussion in that bug
>> thread, it doesn't look related, or easy to fix.  But as was noted in
>> that thread, at least you can get the expected expected by repeatedly
>> typing `M-/', while in the cases I tried (hopefully successfully) to
>> fix, without my patch either you cannot get the expected expansion or
>> you get an error, so bug#36516 seems less serious.
>
> Indeed, bug#36516 reports just a logical inconsistency,
> whereas your patch fixes a plain bug.  I've looked at your patch,
> and everything looks right, and the comprehensive test coverage
> will ensure no breakages, thanks.

Thanks for the review!

Steve Berman

Message #26 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 12:09:04 +0200

> From: Stephen Berman <stephen.berman <at> gmx.net>
> Cc: 74090 <at> debbugs.gnu.org
> Date: Thu, 31 Oct 2024 11:00:17 +0100
> 
> > Thanks, feel free to install on the emacs-30 branch.
> >
> >> +                 ;; Comparing with point only makes sense in the buffer
> >> +                 ;; where we called dabbrev-exand, but if that differs
> >                                        ^^^^^^^^^^^^^
> > Typo.
> 
> Thanks, typo corrected and pushed as commit f6c359cb66a -- but to
> master, I just now noticed that you said emacs-30.  I tried reverting
> but I don't know how.  What should I do?  Sorry for the snafu.

Instead of reverting, simply cherry-pick it to emacs-30.

Message #29 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 11:20:30 +0100

On Thu, 31 Oct 2024 12:09:04 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Stephen Berman <stephen.berman <at> gmx.net>
>> Cc: 74090 <at> debbugs.gnu.org
>> Date: Thu, 31 Oct 2024 11:00:17 +0100
>>
>> > Thanks, feel free to install on the emacs-30 branch.
>> >
>> >> +                 ;; Comparing with point only makes sense in the buffer
>> >> +                 ;; where we called dabbrev-exand, but if that differs
>> >                                        ^^^^^^^^^^^^^
>> > Typo.
>>
>> Thanks, typo corrected and pushed as commit f6c359cb66a -- but to
>> master, I just now noticed that you said emacs-30.  I tried reverting
>> but I don't know how.  What should I do?  Sorry for the snafu.
>
> Instead of reverting, simply cherry-pick it to emacs-30.

I don't want to screw up again; do I enter exactly the following in the
shell (from the emacs-30 directory)?

git cherry-pick master

Steve Berman

Message #32 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 11:39:08 +0100

On Thu, 31 Oct 2024 11:20:30 +0100 Stephen Berman <stephen.berman <at> gmx.net> wrote:

> On Thu, 31 Oct 2024 12:09:04 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>>> From: Stephen Berman <stephen.berman <at> gmx.net>
>>> Cc: 74090 <at> debbugs.gnu.org
>>> Date: Thu, 31 Oct 2024 11:00:17 +0100
>>>
>>> > Thanks, feel free to install on the emacs-30 branch.
>>> >
>>> >> +                 ;; Comparing with point only makes sense in the buffer
>>> >> +                 ;; where we called dabbrev-exand, but if that differs
>>> >                                        ^^^^^^^^^^^^^
>>> > Typo.
>>>
>>> Thanks, typo corrected and pushed as commit f6c359cb66a -- but to
>>> master, I just now noticed that you said emacs-30.  I tried reverting
>>> but I don't know how.  What should I do?  Sorry for the snafu.
>>
>> Instead of reverting, simply cherry-pick it to emacs-30.
>
> I don't want to screw up again; do I enter exactly the following in the
> shell (from the emacs-30 directory)?
>
> git cherry-pick master

My commit to master is no longer the most recent, so should I use this
instead?

git cherry-pick master f6c359cb66a

I looked at the git-cherry-pick man page but it's not clear to me.  I
almost exclusively use VC.

Steve Berman

Message #35 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 12:47:49 +0200

> From: Stephen Berman <stephen.berman <at> gmx.net>
> Cc: 74090 <at> debbugs.gnu.org
> Date: Thu, 31 Oct 2024 11:39:08 +0100
> 
> On Thu, 31 Oct 2024 11:20:30 +0100 Stephen Berman <stephen.berman <at> gmx.net> wrote:
> 
> > On Thu, 31 Oct 2024 12:09:04 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> >>> From: Stephen Berman <stephen.berman <at> gmx.net>
> >>> Cc: 74090 <at> debbugs.gnu.org
> >>> Date: Thu, 31 Oct 2024 11:00:17 +0100
> >>>
> >>> > Thanks, feel free to install on the emacs-30 branch.
> >>> >
> >>> >> +                 ;; Comparing with point only makes sense in the buffer
> >>> >> +                 ;; where we called dabbrev-exand, but if that differs
> >>> >                                        ^^^^^^^^^^^^^
> >>> > Typo.
> >>>
> >>> Thanks, typo corrected and pushed as commit f6c359cb66a -- but to
> >>> master, I just now noticed that you said emacs-30.  I tried reverting
> >>> but I don't know how.  What should I do?  Sorry for the snafu.
> >>
> >> Instead of reverting, simply cherry-pick it to emacs-30.
> >
> > I don't want to screw up again; do I enter exactly the following in the
> > shell (from the emacs-30 directory)?
> >
> > git cherry-pick master
> 
> My commit to master is no longer the most recent, so should I use this
> instead?
> 
> git cherry-pick master f6c359cb66a

Just make sure emacs-30 is the current branch, and then type

  $ git cherry-pick -x f6c359cb66a

Then "git show" to eyeball the change, and "git push" to push
upstream.

You cannot screw up as long as you examine the changes before you push
after cherry-picking, because any the problems, such as they are, are
only present in your local clone, and you can always undo with

  $ git reset --hard HEAD^

Thanks.

Message #40 received at 74090-done <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74090-done <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Thu, 31 Oct 2024 12:17:06 +0100

On Thu, 31 Oct 2024 12:47:49 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Stephen Berman <stephen.berman <at> gmx.net>
>> Cc: 74090 <at> debbugs.gnu.org
>> Date: Thu, 31 Oct 2024 11:39:08 +0100
>>
>> On Thu, 31 Oct 2024 11:20:30 +0100 Stephen Berman <stephen.berman <at> gmx.net> wrote:
>>
>> > On Thu, 31 Oct 2024 12:09:04 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:
>> >
>> >>> From: Stephen Berman <stephen.berman <at> gmx.net>
>> >>> Cc: 74090 <at> debbugs.gnu.org
>> >>> Date: Thu, 31 Oct 2024 11:00:17 +0100
>> >>>
>> >>> > Thanks, feel free to install on the emacs-30 branch.
>> >>> >
>> >>> >> +                 ;; Comparing with point only makes sense in the buffer
>> >>> >> +                 ;; where we called dabbrev-exand, but if that differs
>> >>> >                                        ^^^^^^^^^^^^^
>> >>> > Typo.
>> >>>
>> >>> Thanks, typo corrected and pushed as commit f6c359cb66a -- but to
>> >>> master, I just now noticed that you said emacs-30.  I tried reverting
>> >>> but I don't know how.  What should I do?  Sorry for the snafu.
>> >>
>> >> Instead of reverting, simply cherry-pick it to emacs-30.
>> >
>> > I don't want to screw up again; do I enter exactly the following in the
>> > shell (from the emacs-30 directory)?
>> >
>> > git cherry-pick master
>>
>> My commit to master is no longer the most recent, so should I use this
>> instead?
>>
>> git cherry-pick master f6c359cb66a
>
> Just make sure emacs-30 is the current branch, and then type
>
>   $ git cherry-pick -x f6c359cb66a
>
> Then "git show" to eyeball the change, and "git push" to push
> upstream.

Thanks, that worked.  And with that I'm closing the bug.

Steve Berman

Message #47 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Fri, 29 Nov 2024 09:51:57 +0200

> I don't remember seeing such backtrace before this change.
> But now occasionally an error is raised on typing 'M-/':
>
> Debugger entered--Lisp error: (error "Selecting deleted buffer")
>   set-buffer(#<killed buffer>)
>   (save-current-buffer (set-buffer buf) (if (and (or (eq (current-buffer) dabbrev--last-buffer) ...
>   dabbrev-expand(nil)
>   funcall-interactively(dabbrev-expand nil)
>   command-execute(dabbrev-expand)
>
> because can't set the killed buffer 'buf' here:
>
>       (with-current-buffer buf
>         (if (and (or (eq (current-buffer) dabbrev--last-buffer)
> 		     (null dabbrev--last-buffer)
>                      (buffer-live-p dabbrev--last-buffer))
> 	         (numberp dabbrev--last-expansion-location)
> 	         (and (> dabbrev--last-expansion-location (point))))
> 	    (setq dabbrev--last-expansion-location
> 		  (copy-marker dabbrev--last-expansion-location))))
>
> It's difficult to debug because the buffer is already killed at this point.
> But looking at the logic of backtrace, here is a reproducible test case:
>
> 0. emacs-29 -Q
> 1. C-x b foo RET
> 2. Type: abc SPC abd
> 3. C-x b *scratch* RET
> 4. Type: ab M-/
> 5. C-x k foo RET
> 6. Type: SPC ab M-/

Sorry, I meant emacs-30, not emacs-29.

Message #50 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Fri, 29 Nov 2024 16:38:43 +0100

[Message part 1 (text/plain, inline)]

On Fri, 29 Nov 2024 09:51:57 +0200 Juri Linkov <juri <at> linkov.net> wrote:

>> I don't remember seeing such backtrace before this change.
>> But now occasionally an error is raised on typing 'M-/':
>>
>> Debugger entered--Lisp error: (error "Selecting deleted buffer")
>>   set-buffer(#<killed buffer>)
>>   (save-current-buffer (set-buffer buf) (if (and (or (eq (current-buffer) dabbrev--last-buffer) ...
>>   dabbrev-expand(nil)
>>   funcall-interactively(dabbrev-expand nil)
>>   command-execute(dabbrev-expand)
>>
>> because can't set the killed buffer 'buf' here:
>>
>>       (with-current-buffer buf
>>         (if (and (or (eq (current-buffer) dabbrev--last-buffer)
>> 		     (null dabbrev--last-buffer)
>>                      (buffer-live-p dabbrev--last-buffer))
>> 	         (numberp dabbrev--last-expansion-location)
>> 	         (and (> dabbrev--last-expansion-location (point))))
>> 	    (setq dabbrev--last-expansion-location
>> 		  (copy-marker dabbrev--last-expansion-location))))
>>
>> It's difficult to debug because the buffer is already killed at this point.
>> But looking at the logic of backtrace, here is a reproducible test case:
>>
>> 0. emacs-29 -Q
>> 1. C-x b foo RET
>> 2. Type: abc SPC abd
>> 3. C-x b *scratch* RET
>> 4. Type: ab M-/
>> 5. C-x k foo RET
>> 6. Type: SPC ab M-/
>
> Sorry, I meant emacs-30, not emacs-29.

Thanks, I can reproduce it.  With the attached patch (against emacs-30)
I don't get the error; instead, at step 6 "ab" expands to "abc", which I
suppose is what's expected, and then typing `M-/' again shows the
message "No further dynamic expansion for ‘ab’ found", which also seems
as expected.  And with the patch, all current dabbrev regression tests
pass with ert-run-tests-batch-and-exit.  Does anyone see a problem with
the patch?

Steve Berman

[Message part 2 (text/x-patch, attachment)]

Message #53 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Sat, 30 Nov 2024 19:51:38 +0200

>>> 1. C-x b foo RET
>>> 2. Type: abc SPC abd
>>> 3. C-x b *scratch* RET
>>> 4. Type: ab M-/
>>> 5. C-x k foo RET
>>> 6. Type: SPC ab M-/
>>
>> Sorry, I meant emacs-30, not emacs-29.
>
> Thanks, I can reproduce it.  With the attached patch (against emacs-30)
> I don't get the error; instead, at step 6 "ab" expands to "abc", which I
> suppose is what's expected, and then typing `M-/' again shows the
> message "No further dynamic expansion for ‘ab’ found", which also seems
> as expected.  And with the patch, all current dabbrev regression tests
> pass with ert-run-tests-batch-and-exit.  Does anyone see a problem with
> the patch?

Thanks, I confirm it works now.  Would it be nice
to have a new test that covers this case?

Message #56 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: stephen.berman <at> gmx.net, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Sat, 30 Nov 2024 22:07:27 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  74090 <at> debbugs.gnu.org
> Date: Sat, 30 Nov 2024 19:51:38 +0200
> 
> >>> 1. C-x b foo RET
> >>> 2. Type: abc SPC abd
> >>> 3. C-x b *scratch* RET
> >>> 4. Type: ab M-/
> >>> 5. C-x k foo RET
> >>> 6. Type: SPC ab M-/
> >>
> >> Sorry, I meant emacs-30, not emacs-29.
> >
> > Thanks, I can reproduce it.  With the attached patch (against emacs-30)
> > I don't get the error; instead, at step 6 "ab" expands to "abc", which I
> > suppose is what's expected, and then typing `M-/' again shows the
> > message "No further dynamic expansion for ‘ab’ found", which also seems
> > as expected.  And with the patch, all current dabbrev regression tests
> > pass with ert-run-tests-batch-and-exit.  Does anyone see a problem with
> > the patch?
> 
> Thanks, I confirm it works now.

Thanks for testing.  Stephen, you have green light for installing this
on the emacs-30 branch.

> Would it be nice to have a new test that covers this case?

Definitely.  But it is less urgent than fixing the bug itself.

Message #59 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74090 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Sat, 30 Nov 2024 23:30:21 +0100

On Sat, 30 Nov 2024 22:07:27 +0200 Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Juri Linkov <juri <at> linkov.net>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  74090 <at> debbugs.gnu.org
>> Date: Sat, 30 Nov 2024 19:51:38 +0200
>> 
>> >>> 1. C-x b foo RET
>> >>> 2. Type: abc SPC abd
>> >>> 3. C-x b *scratch* RET
>> >>> 4. Type: ab M-/
>> >>> 5. C-x k foo RET
>> >>> 6. Type: SPC ab M-/
>> >>
>> >> Sorry, I meant emacs-30, not emacs-29.
>> >
>> > Thanks, I can reproduce it.  With the attached patch (against emacs-30)
>> > I don't get the error; instead, at step 6 "ab" expands to "abc", which I
>> > suppose is what's expected, and then typing `M-/' again shows the
>> > message "No further dynamic expansion for ‘ab’ found", which also seems
>> > as expected.  And with the patch, all current dabbrev regression tests
>> > pass with ert-run-tests-batch-and-exit.  Does anyone see a problem with
>> > the patch?
>> 
>> Thanks, I confirm it works now.

Thanks!

> Thanks for testing.  Stephen, you have green light for installing this
> on the emacs-30 branch.

Thanks, done in commit bd8a6f70fb9.

>> Would it be nice to have a new test that covers this case?
>
> Definitely.  But it is less urgent than fixing the bug itself.

I've added a test but I'm somewhat dissatisfied with it.  I wanted to
essentially reproduce the behavior I see with the patch, described
above.  However, after killing buffer "foo", the message about no
further expansion is from a user-error in dabbrev-expand, and on hitting
this in the test run, the test immediately finishes.  So I used
should-error with :type 'user-error and the test succeeds, but I cannot
test for the final buffer content nor the message displayed by
user-error.  If anyone has an idea how to do that or otherwise improve
the test, please chime in.

Steve Berman

Message #62 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Mon, 02 Dec 2024 09:33:51 +0200

> I've added a test but I'm somewhat dissatisfied with it.  I wanted to
> essentially reproduce the behavior I see with the patch, described
> above.  However, after killing buffer "foo", the message about no
> further expansion is from a user-error in dabbrev-expand, and on hitting
> this in the test run, the test immediately finishes.  So I used
> should-error with :type 'user-error and the test succeeds, but I cannot
> test for the final buffer content nor the message displayed by
> user-error.  If anyone has an idea how to do that or otherwise improve
> the test, please chime in.

Probably 'condition-case' could help to catch a user-error.
Or maybe better to set a buffer-local or let-bind 'command-error-function'
like e.g. in 'minibuffer-error-initialize'.

Then you can let-bind 'set-message-function' to catch a message
to check it (but I don't remember if the error calls the message function).

OTOH, I don't understand why the test immediately finishes after 'should-error'?
There are many tests that call 'should-error' sequentially like:

  (ert-deftest abbrev-table-empty-p-test ()
    (should-error (abbrev-table-empty-p 42))
    (should-error (abbrev-table-empty-p "aoeu"))
    (should-error (abbrev-table-empty-p '()))
    (should-error (abbrev-table-empty-p []))

And indeed implementation of 'should-error' already has 'condition-case'.

Message #65 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Mon, 02 Dec 2024 13:15:23 +0100

On Mon, 02 Dec 2024 09:33:51 +0200 Juri Linkov <juri <at> linkov.net> wrote:

>> I've added a test but I'm somewhat dissatisfied with it.  I wanted to
>> essentially reproduce the behavior I see with the patch, described
>> above.  However, after killing buffer "foo", the message about no
>> further expansion is from a user-error in dabbrev-expand, and on hitting
>> this in the test run, the test immediately finishes.  So I used
>> should-error with :type 'user-error and the test succeeds, but I cannot
>> test for the final buffer content nor the message displayed by
>> user-error.  If anyone has an idea how to do that or otherwise improve
>> the test, please chime in.
>
> Probably 'condition-case' could help to catch a user-error.
> Or maybe better to set a buffer-local or let-bind 'command-error-function'
> like e.g. in 'minibuffer-error-initialize'.
>
> Then you can let-bind 'set-message-function' to catch a message
> to check it (but I don't remember if the error calls the message function).
>
> OTOH, I don't understand why the test immediately finishes after 'should-error'?
> There are many tests that call 'should-error' sequentially like:
>
>   (ert-deftest abbrev-table-empty-p-test ()
>     (should-error (abbrev-table-empty-p 42))
>     (should-error (abbrev-table-empty-p "aoeu"))
>     (should-error (abbrev-table-empty-p '()))
>     (should-error (abbrev-table-empty-p []))
>
> And indeed implementation of 'should-error' already has 'condition-case'.

Thanks very much for the feedback.  I was wrong about the test
immediately finishing after `should-error'; I guess I misread or
misunderstood the output of the batch run.  Additionally, there was a
typo in the test of the final buffer content, which caused it to fail,
and that probably confused me further.  As for testing the error
message, I overlooked that `should-error' returns an error description
containing the message, so that seems the simplest way to do it.  With
these changes the test succeeds in batch runs and I think now better
reflects the user experience with manual input, so I went ahead and
pushed the fix (commit 6bca138d60e to emacs-30).  If you agree, feel
free to close the bug again (assuming it needs to be reclosed, since you
unarchived it); otherwise, if you see other problems with the test,
please let me know.

Steve Berman

Message #68 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Tue, 03 Dec 2024 09:35:30 +0200

> With these changes the test succeeds in batch runs and I think now better
> reflects the user experience with manual input, so I went ahead and
> pushed the fix (commit 6bca138d60e to emacs-30).  If you agree, feel
> free to close the bug again (assuming it needs to be reclosed, since you
> unarchived it); otherwise, if you see other problems with the test,
> please let me know.

I see the same test failures that were reported on emacs-devel.

One of them is clear because of quotes mismatch:

      (string= "No further dynamic expansion for `ab' found"
	       "No further dynamic expansion for ‘ab’ found")

But why 4 lines with should-error affect two other tests
is not clear.  Maybe other tests expect the initial contents
in the scratch buffer?  Or the same buffer list?

Generally, this is deficiency of the test-suite design
that tests are not isolated.

Message #71 received at 74090 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74090 <at> debbugs.gnu.org
Subject: Re: bug#74090: 31.0.50; Problems with dabbrev-expand
Date: Tue, 03 Dec 2024 10:39:28 +0100

On Tue, 03 Dec 2024 09:35:30 +0200 Juri Linkov <juri <at> linkov.net> wrote:

>> With these changes the test succeeds in batch runs and I think now better
>> reflects the user experience with manual input, so I went ahead and
>> pushed the fix (commit 6bca138d60e to emacs-30).  If you agree, feel
>> free to close the bug again (assuming it needs to be reclosed, since you
>> unarchived it); otherwise, if you see other problems with the test,
>> please let me know.
>
> I see the same test failures that were reported on emacs-devel.
>
> One of them is clear because of quotes mismatch:
>
>       (string= "No further dynamic expansion for `ab' found"
> 	       "No further dynamic expansion for ‘ab’ found")

Thanks, I've (hopefully) fixed this problem now (or rather avoided it,
since the change is just a workaround) with commit 7b8d12e95de to
emacs-30.

> But why 4 lines with should-error affect two other tests
> is not clear.  Maybe other tests expect the initial contents
> in the scratch buffer?  Or the same buffer list?

Those other tests are two of the four dabbrev-expand-other-buffer* tests
that also fail when manually running ert in the test buffer.  I couldn't
figure out why but since these tests succeed in batch runs and with
`make check', I haven't investigated further, but I'll try to do that.

> Generally, this is deficiency of the test-suite design
> that tests are not isolated.

The macro `with-dabbrev-test' is supposed to ensure that, but maybe it's
failing somehow.

Steve Berman

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.