GNU bug report logs - #40919
27.0.91; next-error-select-buffer does not always behave as documented

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 40919 in the body.
You can then email your comments to 40919 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: Trevor Spiteri <tspiteri <at> ieee.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.91; next-error-select-buffer does not always behave as documented
Date: Tue, 28 Apr 2020 03:51:09 +0200

The next-error-select-buffer documentation states that “the selected
buffer becomes the source of locations for the subsequent invocation of
‘next-error’ or ‘previous-error’.” However, it is not the case for the
following:

1. Go in a fresh next-error capable buffer (not *grep*).
2. Grep for something.
3. M-x next-error-select-buffer *grep*
4. M-x next-error

The buffer of 1 (not *grep*) is the source of locations instead of
the expected *grep*.

This is because although next-error-select-buffer sets the variable
next-error-last-buffer, it is not used in this case: When next-error
calls next-error-find-buffer, next-error-buffer has no buffer-local
value yet, so condition 2. in next-error-find-buffer (that
next-error-buffer has no buffer-local value and the current buffer is a
next-error capable buffer) is true, and the function never even checks
next-error-last-buffer.



In GNU Emacs 27.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.13, cairo version 1.16.0)
of 2020-04-23 built on desktop
Repository revision: ba6104d1e8db4e8db2f12acaebf092ef579c6632
Repository branch: emacs-27

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

From: Trevor Spiteri <tspiteri <at> ieee.org>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 28 Apr 2020 13:37:23 +0200

On 28/04/2020 03:51, Trevor Spiteri wrote:
> 1. Go in a fresh next-error capable buffer (not *grep*).
> 2. Grep for something.
> 3. M-x next-error-select-buffer *grep*
> 4. M-x next-error

And I just realized, this is also a regression from Emacs 26 if step 3
is skipped, as step 2 itself also sets next-error-last-buffer .

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

From: Juri Linkov <juri <at> linkov.net>
To: Trevor Spiteri <tspiteri <at> ieee.org>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Wed, 29 Apr 2020 02:40:20 +0300

> The next-error-select-buffer documentation states that “the selected
> buffer becomes the source of locations for the subsequent invocation of
> ‘next-error’ or ‘previous-error’.” However, it is not the case for the
> following:
>
> 1. Go in a fresh next-error capable buffer (not *grep*).
> 2. Grep for something.
> 3. M-x next-error-select-buffer *grep*
> 4. M-x next-error
>
> The buffer of 1 (not *grep*) is the source of locations instead of
> the expected *grep*.
>
> This is because although next-error-select-buffer sets the variable
> next-error-last-buffer, it is not used in this case: When next-error
> calls next-error-find-buffer, next-error-buffer has no buffer-local
> value yet, so condition 2. in next-error-find-buffer (that
> next-error-buffer has no buffer-local value and the current buffer is a
> next-error capable buffer) is true, and the function never even checks
> next-error-last-buffer.

Thanks for the report.  Do you think the problem is in implementation,
or only in documentation?  IOW, do you think its behavior is correct,
but the documentation should be fixed to describe more clearly what
next-error was intended to do in this situation?

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

From: Trevor Spiteri <tspiteri <at> ieee.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Wed, 29 Apr 2020 02:13:22 +0200

>> The next-error-select-buffer documentation states that “the selected
>> buffer becomes the source of locations for the subsequent invocation of
>> ‘next-error’ or ‘previous-error’.” However, it is not the case for the
>> following:
>>
>> 1. Go in a fresh next-error capable buffer (not *grep*).
>> 2. Grep for something.
>> 3. M-x next-error-select-buffer *grep*
>> 4. M-x next-error
>>
>> The buffer of 1 (not *grep*) is the source of locations instead of
>> the expected *grep*.
>>
>> This is because although next-error-select-buffer sets the variable
>> next-error-last-buffer, it is not used in this case: When next-error
>> calls next-error-find-buffer, next-error-buffer has no buffer-local
>> value yet, so condition 2. in next-error-find-buffer (that
>> next-error-buffer has no buffer-local value and the current buffer is a
>> next-error capable buffer) is true, and the function never even checks
>> next-error-last-buffer.
> Thanks for the report.  Do you think the problem is in implementation,
> or only in documentation?  IOW, do you think its behavior is correct,
> but the documentation should be fixed to describe more clearly what
> next-error was intended to do in this situation?

I think the error is in the implementation. In fact I added a later
comment to the bug report.


> And I just realized, this is also a regression from Emacs 26 if step 3
> is skipped, as step 2 itself also sets next-error-last-buffer .

As a use case, let's say I'm in a buffer that has next-error
capabilities because of say flycheck, and I grep or compile; I want to
start going through the new errors immediately. That is why
compilation-start finishes  with (setq next-error-last-buffer outbuf)
and that's how Emacs 26 works (without step 3 as
next-error-select-buffer is new). In Emacs 27 not only does that break,
but even using the new function has no effect.

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

From: Juri Linkov <juri <at> linkov.net>
To: Trevor Spiteri <tspiteri <at> ieee.org>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Wed, 29 Apr 2020 23:38:59 +0300

> I think the error is in the implementation.

Then I see no way other than for next-error-select-buffer to say:
"the current buffer was visited from next-error-last-buffer".
Yes, this is a lie, but a white lie with good intentions, so
next-error-find-buffer will trust this misinformation and leave
the buffer alone.  Is this patch morally acceptable?

diff --git a/lisp/simple.el b/lisp/simple.el
index b5ba05426f..b5f148b7d5 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -379,7 +379,8 @@ next-error-select-buffer
    (list (get-buffer
           (read-buffer "Select next-error buffer: " nil nil
                        (lambda (b) (next-error-buffer-p (cdr b)))))))
-  (setq next-error-last-buffer buffer))
+  (setq next-error-last-buffer buffer)
+  (setq next-error-buffer buffer))
 
 (defalias 'goto-next-locus 'next-error)
 (defalias 'next-match 'next-error)

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

From: Trevor Spiteri <tspiteri <at> ieee.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 30 Apr 2020 00:40:35 +0200

On 29/04/2020 22:38, Juri Linkov wrote:
>> I think the error is in the implementation.
> Then I see no way other than for next-error-select-buffer to say:
> "the current buffer was visited from next-error-last-buffer".
> Yes, this is a lie, but a white lie with good intentions, so
> next-error-find-buffer will trust this misinformation and leave
> the buffer alone.  Is this patch morally acceptable?
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index b5ba05426f..b5f148b7d5 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -379,7 +379,8 @@ next-error-select-buffer
>     (list (get-buffer
>            (read-buffer "Select next-error buffer: " nil nil
>                         (lambda (b) (next-error-buffer-p (cdr b)))))))
> -  (setq next-error-last-buffer buffer))
> +  (setq next-error-last-buffer buffer)
> +  (setq next-error-buffer buffer))
>  
>  (defalias 'goto-next-locus 'next-error)
>  (defalias 'next-match 'next-error)
>
I think this would work for next-error-select-buffer. Then to fix the
other issue about new compilations, compilation-start can be modified to
use next-error-select-buffer. That way the change in compilation-start
is morally unambiguous :)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 455f181f50..41e77007c6 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1910,7 +1910,7 @@ compilation-start
     (goto-char (point-max))))
 
     ;; Make it so the next C-x ` will use this buffer.
-    (setq next-error-last-buffer outbuf)))
+    (next-error-select-buffer outbuf)))
 
 (defun compilation-set-window-height (window)
   "Set the height of WINDOW according to `compilation-window-height'."

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

From: Juri Linkov <juri <at> linkov.net>
To: Trevor Spiteri <tspiteri <at> ieee.org>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 30 Apr 2020 23:14:09 +0300

>> @@ -379,7 +379,8 @@ next-error-select-buffer
>>     (list (get-buffer
>>            (read-buffer "Select next-error buffer: " nil nil
>>                         (lambda (b) (next-error-buffer-p (cdr b)))))))
>> -  (setq next-error-last-buffer buffer))
>> +  (setq next-error-last-buffer buffer)
>> +  (setq next-error-buffer buffer))
>>  
>>  (defalias 'goto-next-locus 'next-error)
>>  (defalias 'next-match 'next-error)
>>
> I think this would work for next-error-select-buffer. Then to fix the
> other issue about new compilations, compilation-start can be modified to
> use next-error-select-buffer. That way the change in compilation-start
> is morally unambiguous :)

I'm not sure if this is morally right, because users might want to
continue to navigate an old next-error buffer, even after creating
a new next-error buffer.

OTOH, by using next-error-select-buffer they explicitly say that
they want to switch to the new navigation.  Do you think it's right
to implicitly assume the user's intention?

We should base the decision not on precedence set by older versions,
but on expectations of most users.

> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index 455f181f50..41e77007c6 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -1910,7 +1910,7 @@ compilation-start
>      (goto-char (point-max))))
>  
>      ;; Make it so the next C-x ` will use this buffer.
> -    (setq next-error-last-buffer outbuf)))
> +    (next-error-select-buffer outbuf)))
>  
>  (defun compilation-set-window-height (window)
>    "Set the height of WINDOW according to `compilation-window-height'."

There are more places that set next-error-last-buffer: vc-git-grep, occur-1,
xref.el...

But I'm still not sure if this is a preferable behavior for most users.
Maybe this needs a user option?

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

From: Trevor Spiteri <tspiteri <at> ieee.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Fri, 1 May 2020 01:18:49 +0200

>> I think this would work for next-error-select-buffer. Then to fix the
>> other issue about new compilations, compilation-start can be modified to
>> use next-error-select-buffer. That way the change in compilation-start
>> is morally unambiguous :)
> I'm not sure if this is morally right, because users might want to
> continue to navigate an old next-error buffer, even after creating
> a new next-error buffer.
>
> OTOH, by using next-error-select-buffer they explicitly say that
> they want to switch to the new navigation.  Do you think it's right
> to implicitly assume the user's intention?
>
> We should base the decision not on precedence set by older versions,
> but on expectations of most users.
>
>> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
>> index 455f181f50..41e77007c6 100644
>> --- a/lisp/progmodes/compile.el
>> +++ b/lisp/progmodes/compile.el
>> @@ -1910,7 +1910,7 @@ compilation-start
>>      (goto-char (point-max))))
>>  
>>      ;; Make it so the next C-x ` will use this buffer.
>> -    (setq next-error-last-buffer outbuf)))
>> +    (next-error-select-buffer outbuf)))
>>  
>>  (defun compilation-set-window-height (window)
>>    "Set the height of WINDOW according to `compilation-window-height'."
> There are more places that set next-error-last-buffer: vc-git-grep, occur-1,
> xref.el...
>
> But I'm still not sure if this is a preferable behavior for most users.
> Maybe this needs a user option?

I thought that the change was unintentional. If it's intentional that's
another thing. If no one else complains then maybe most users prefer the
new behavior. I've already added advice to compilation-start so it's not
hard for me that I prefer the old behavior.

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

From: Juri Linkov <juri <at> linkov.net>
To: Trevor Spiteri <tspiteri <at> ieee.org>
Cc: 40919 <at> debbugs.gnu.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Sun, 03 May 2020 02:38:10 +0300

>> But I'm still not sure if this is a preferable behavior for most users.
>> Maybe this needs a user option?
>
> I thought that the change was unintentional. If it's intentional that's
> another thing. If no one else complains then maybe most users prefer the
> new behavior. I've already added advice to compilation-start so it's not
> hard for me that I prefer the old behavior.

I think the problem occurs only because compilation/grep pop up their
output buffer in another window - this creates ambiguity whether to use
navigation from the current buffer or from another (new) buffer.

It seems there is no right answer suitable for all users.  So I propose
to add more options.

Eli, do you agree with adding more options to `next-error-find-buffer-function'
in emacs-27?

Currently it has one option that defines one of possible behaviors existed
in older versions.  Now we need to add other option for compatibility
with pre-27 versions.

Then users of emacs-27 could decide whether they want to keep
the old behavior or use the new.

I'm not sure about the default value: should it default to pre-27 behavior.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91;
 next-error-select-buffer does not always behave as documented
Date: Sun, 03 May 2020 05:40:22 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Sun, 03 May 2020 02:38:10 +0300
> Cc: 40919 <at> debbugs.gnu.org
> 
> It seems there is no right answer suitable for all users.  So I propose
> to add more options.
> 
> Eli, do you agree with adding more options to `next-error-find-buffer-function'
> in emacs-27?
> 
> Currently it has one option that defines one of possible behaviors existed
> in older versions.  Now we need to add other option for compatibility
> with pre-27 versions.
> 
> Then users of emacs-27 could decide whether they want to keep
> the old behavior or use the new.

I think the time for adding new options to Emacs 27 has come and gone
long ago.  We should defer that to future releases.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Mon, 04 May 2020 01:36:43 +0300

>> It seems there is no right answer suitable for all users.  So I propose
>> to add more options.
>>
>> Eli, do you agree with adding more options to `next-error-find-buffer-function'
>> in emacs-27?
>>
>> Currently it has one option that defines one of possible behaviors existed
>> in older versions.  Now we need to add other option for compatibility
>> with pre-27 versions.
>>
>> Then users of emacs-27 could decide whether they want to keep
>> the old behavior or use the new.
>
> I think the time for adding new options to Emacs 27 has come and gone
> long ago.  We should defer that to future releases.

To do nothing in emacs-27 is one variant, indeed.  Another variant would be
to add an optional choice to the existing option that allows to restore
the old behavior and doesn't affect the current behavior in any way.
A third variant is to explain in the documentation what defadvice
the users could add to their init files to restore the old behavior.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 19 May 2020 04:48:51 +0300

On 04.05.2020 01:36, Juri Linkov wrote:
> Another variant would be
> to add an optional choice to the existing option that allows to restore
> the old behavior and doesn't affect the current behavior in any way.

FWIW, I'd consider that more of a documentation change.

More importantly, and certainly only for Emacs 28, Juri could you remind 
me what we'll be losing by removing the case no. 2 from 
next-error-find-buffer?

The ability to switch to an arbitrary Grep buffer and start using it 
with 'M-x next-error'? E.g. if there are several of them. That's more of 
a backward compatibility concern, right? Or do you have scenarios in 
mind where this will really save on keystrokes?

On another note, perhaps we could add a message to 
next-error-select-buffer that would be shown if we suspect this command 
will not have the expected result for the user.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Wed, 20 May 2020 01:21:35 +0300

>> Another variant would be
>> to add an optional choice to the existing option that allows to restore
>> the old behavior and doesn't affect the current behavior in any way.
>
> FWIW, I'd consider that more of a documentation change.
>
> More importantly, and certainly only for Emacs 28, Juri could you remind me
> what we'll be losing by removing the case no. 2 from
> next-error-find-buffer?

It is used to handle such cases when navigating one next-error list
visits a buffer that contains another next-error list - visiting
such buffer should not switch the navigation, it should continue
the initial navigation.

But I'm not proud of the case no. 2, so you can drop it :)

Or better move it to a separate function and provide this function
as an option in next-error-find-buffer-function.

> The ability to switch to an arbitrary Grep buffer and start using it with
> 'M-x next-error'? E.g. if there are several of them. That's more of
> a backward compatibility concern, right? Or do you have scenarios in
> mind where this will really save on keystrokes?

The ability to visit an arbitrary Grep buffer and use 'next-error' in it
to switch navigation should always work as the most reliable and implicit
way to switch navigation.  This is the case no. 3 in next-error-find-buffer.

> On another note, perhaps we could add a message to next-error-select-buffer
> that would be shown if we suspect this command will not have the expected
> result for the user.

Or maybe ask the user using the minibuffer to resolve ambiguity.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Fri, 22 May 2020 02:57:37 +0300

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

On 20.05.2020 01:21, Juri Linkov wrote:
>>> Another variant would be
>>> to add an optional choice to the existing option that allows to restore
>>> the old behavior and doesn't affect the current behavior in any way.
>>
>> FWIW, I'd consider that more of a documentation change.
>>
>> More importantly, and certainly only for Emacs 28, Juri could you remind me
>> what we'll be losing by removing the case no. 2 from
>> next-error-find-buffer?
> 
> It is used to handle such cases when navigating one next-error list
> visits a buffer that contains another next-error list - visiting
> such buffer should not switch the navigation, it should continue
> the initial navigation.

Didn't we fix changelog-mode so this doesn't happen anymore? I think as 
long as file major modes don't set next-error-last-buffer (and minor 
modes either, on initialization), we should be safe.

Can you reproduce a problematic scenario with the attached patch?

> But I'm not proud of the case no. 2, so you can drop it :)
> 
> Or better move it to a separate function and provide this function
> as an option in next-error-find-buffer-function.

Patch attached. Comments welcome.

>> The ability to switch to an arbitrary Grep buffer and start using it with
>> 'M-x next-error'? E.g. if there are several of them. That's more of
>> a backward compatibility concern, right? Or do you have scenarios in
>> mind where this will really save on keystrokes?
> 
> The ability to visit an arbitrary Grep buffer and use 'next-error' in it
> to switch navigation should always work as the most reliable and implicit
> way to switch navigation.  This is the case no. 3 in next-error-find-buffer.

Not if there are, say, two Compilation buffers (or Grep and Occur, etc), 
and you switch to the least recently used one, and press M-g n.

It's probably fine, though.

>> On another note, perhaps we could add a message to next-error-select-buffer
>> that would be shown if we suspect this command will not have the expected
>> result for the user.
> 
> Or maybe ask the user using the minibuffer to resolve ambiguity.

Resolve it how? I think we can safely guess their intention, it's just 
not trivial to execute it.

Anyway, this problem should go away with the attached patch.

[next-error-extract-case-2.diff (text/x-patch, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Sun, 24 May 2020 01:24:51 +0300

>> But I'm not proud of the case no. 2, so you can drop it :)
>> Or better move it to a separate function and provide this function
>> as an option in next-error-find-buffer-function.
>
> Patch attached. Comments welcome.

Is this patch for master?  So there is enough time for thorough testing?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Sun, 24 May 2020 02:30:27 +0300

On 24.05.2020 01:24, Juri Linkov wrote:
> Is this patch for master?  So there is enough time for thorough testing?

I think it's for master. Though if we put it into emacs-27 and changed 
the default to the name of the new function, the behavior shouldn't 
change, and yet it would be easier to "fix" this bug through user 
configuration... (ʘ‿ʘ)

But your question seems to confirm that it's for master.

Suggestions for a better (shorter?) name for the new function welcome, 
by the way.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Mon, 25 May 2020 00:48:53 +0300

>> Is this patch for master?  So there is enough time for thorough testing?
>
> I think it's for master. Though if we put it into emacs-27 and changed the
> default to the name of the new function, the behavior shouldn't change, and
> yet it would be easier to "fix" this bug through user
> configuration... (ʘ‿ʘ)

Yes, this will fix the reported problem of customizability.
Maybe Eli will agree to fix this in emacs-27.

> Suggestions for a better (shorter?) name for the new function welcome, by
> the way.

Maybe next-error-find-keep-navigation.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Mon, 25 May 2020 04:58:05 +0300

On 25.05.2020 00:48, Juri Linkov wrote:
>>> Is this patch for master?  So there is enough time for thorough testing?
>> I think it's for master. Though if we put it into emacs-27 and changed the
>> default to the name of the new function, the behavior shouldn't change, and
>> yet it would be easier to "fix" this bug through user
>> configuration... (ʘ‿ʘ)
> Yes, this will fix the reported problem of customizability.
> Maybe Eli will agree to fix this in emacs-27.

I can post the corresponding patch, if it helps.

>> Suggestions for a better (shorter?) name for the new function welcome, by
>> the way.
> Maybe next-error-find-keep-navigation.

Short, but kinda cryptic (why "find"?). Not bad, but some other options:

next-error-maybe-current-keep-navigation
next-error-no-navigation-try-current

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91;
 next-error-select-buffer does not always behave as documented
Date: Mon, 25 May 2020 18:17:58 +0300

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Mon, 25 May 2020 04:58:05 +0300
> Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
> 
> On 25.05.2020 00:48, Juri Linkov wrote:
> >>> Is this patch for master?  So there is enough time for thorough testing?
> >> I think it's for master. Though if we put it into emacs-27 and changed the
> >> default to the name of the new function, the behavior shouldn't change, and
> >> yet it would be easier to "fix" this bug through user
> >> configuration... (ʘ‿ʘ)
> > Yes, this will fix the reported problem of customizability.
> > Maybe Eli will agree to fix this in emacs-27.
> 
> I can post the corresponding patch, if it helps.

It's okay to add a defcustom and a new function, but the other part of
the patch changes the default behavior, and that is less okay.  Can we
do one without the other?

(Btw, the textual descriptions of the options both in the patch and
those already in the code are confusingly obscure, so much so that I
don't think I could understand what each one does.)

All in all, I feel (for quite some time) that this area is
over-engineered and keeps bumping into more and more unintended
consequences.  Maybe it's time to take a step back and rethink the
entire subject?  (But definitely not on the release branch.)

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 26 May 2020 02:17:34 +0300

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

On 25.05.2020 18:17, Eli Zaretskii wrote:

>>> Yes, this will fix the reported problem of customizability.
>>> Maybe Eli will agree to fix this in emacs-27.
>>
>> I can post the corresponding patch, if it helps.
> 
> It's okay to add a defcustom and a new function, but the other part of
> the patch changes the default behavior, and that is less okay.  Can we
> do one without the other?

The defcustom already exists. It currently defaults to #'ignore (do 
nothing). It is called in case #1.

The attached patch moves case #2 into a new function and makes it the 
default value of the said defcustom (thus case #2 effectively moves into 
case #1). As a result, the default behavior doesn't change, but the user 
will have a much easier time turning off case #2.

> (Btw, the textual descriptions of the options both in the patch and
> those already in the code are confusingly obscure, so much so that I
> don't think I could understand what each one does.)

Knowing the subject matter somewhat, I think the descriptions are 
meaningful enough, but to make sense of them one has to understand how 
the whole feature comes together. E.g. at what times 
next-error-find-buffer is called.

> All in all, I feel (for quite some time) that this area is
> over-engineered and keeps bumping into more and more unintended
> consequences.  Maybe it's time to take a step back and rethink the
> entire subject?  (But definitely not on the release branch.)

That's what we're doing here.

If the attached patch is accepted for emacs-27, then on master we'll 
change next-error-find-buffer-function's default back to #'ignore (thus 
reverting to the previous path that I sent). And see if we find problems 
with how the result is working.

So in the end, next-error-find-buffer-function will have three possible 
values:

- Pre Emacs 27 behavior.
- Emacs 27 behavior (extracted in the attached patch).
- Simplified behavior with less buffer-local state (Emacs 28, hopefully).

[next-error-extract-case-2-keep-current-behavior.diff (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 26 May 2020 19:06:53 +0300

> Cc: juri <at> linkov.net, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Tue, 26 May 2020 02:17:34 +0300
> 
> The attached patch moves case #2 into a new function and makes it the 
> default value of the said defcustom (thus case #2 effectively moves into 
> case #1). As a result, the default behavior doesn't change, but the user 
> will have a much easier time turning off case #2.

Maybe I don't understand something, but it looks to me that this
changes the behavior if next-error-find-buffer-function has a
non-default value: previously what's now
next-error-no-navigation-try-current would be executed after calling
next-error-find-buffer-function, now it isn't.  Is that really
necessary?

> > (Btw, the textual descriptions of the options both in the patch and
> > those already in the code are confusingly obscure, so much so that I
> > don't think I could understand what each one does.)
> 
> Knowing the subject matter somewhat, I think the descriptions are 
> meaningful enough, but to make sense of them one has to understand how 
> the whole feature comes together. E.g. at what times 
> next-error-find-buffer is called.

I think I know something about the subject matter, and still the text
is quite impenetrable for me.

> > All in all, I feel (for quite some time) that this area is
> > over-engineered and keeps bumping into more and more unintended
> > consequences.  Maybe it's time to take a step back and rethink the
> > entire subject?  (But definitely not on the release branch.)
> 
> That's what we're doing here.

Sigh.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 26 May 2020 19:20:50 +0300

On 26.05.2020 19:06, Eli Zaretskii wrote:
>> Cc: juri <at> linkov.net, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Tue, 26 May 2020 02:17:34 +0300
>>
>> The attached patch moves case #2 into a new function and makes it the
>> default value of the said defcustom (thus case #2 effectively moves into
>> case #1). As a result, the default behavior doesn't change, but the user
>> will have a much easier time turning off case #2.
> 
> Maybe I don't understand something, but it looks to me that this
> changes the behavior if next-error-find-buffer-function has a
> non-default value:

Yes. I only claimed that the _default_ behavior doesn't change. The user 
option was only added in Emacs 27, so this doesn't seem to be a big problem.

> previously what's now
> next-error-no-navigation-try-current would be executed after calling
> next-error-find-buffer-function, now it isn't.  Is that really
> necessary?

It seems to be the best and safest option at the moment. I imagine that 
if someone customized the variable they either used the only available 
non-#'ignore value and thus want the Emacs 26 behavior (so taking out 
case #2 would only make them happier), or wrong their own function, in 
which case they might need to update that function.

>>> (Btw, the textual descriptions of the options both in the patch and
>>> those already in the code are confusingly obscure, so much so that I
>>> don't think I could understand what each one does.)
>>
>> Knowing the subject matter somewhat, I think the descriptions are
>> meaningful enough, but to make sense of them one has to understand how
>> the whole feature comes together. E.g. at what times
>> next-error-find-buffer is called.
> 
> I think I know something about the subject matter, and still the text
> is quite impenetrable for me.

Perhaps they could be improved. Still, the situation is probably better 
than it was before.

Do the 'next-error' entries in NEWS.27 make sense to you, BTW?

>>> All in all, I feel (for quite some time) that this area is
>>> over-engineered and keeps bumping into more and more unintended
>>> consequences.  Maybe it's time to take a step back and rethink the
>>> entire subject?  (But definitely not on the release branch.)
>>
>> That's what we're doing here.
> 
> Sigh.

Also see the related discussion in emacs-devel (one that stems from 2018).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 26 May 2020 19:33:54 +0300

> Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Tue, 26 May 2020 19:20:50 +0300
> 
> > Maybe I don't understand something, but it looks to me that this
> > changes the behavior if next-error-find-buffer-function has a
> > non-default value:
> 
> Yes. I only claimed that the _default_ behavior doesn't change. The user 
> option was only added in Emacs 27, so this doesn't seem to be a big problem.

OK.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 26 May 2020 23:39:30 +0300

On 26.05.2020 19:33, Eli Zaretskii wrote:
>> Cc:40919 <at> debbugs.gnu.org,tspiteri <at> ieee.org,juri <at> linkov.net
>> From: Dmitry Gutov<dgutov <at> yandex.ru>
>> Date: Tue, 26 May 2020 19:20:50 +0300
>>
>>> Maybe I don't understand something, but it looks to me that this
>>> changes the behavior if next-error-find-buffer-function has a
>>> non-default value:
>> Yes. I only claimed that the_default_  behavior doesn't change. The user
>> option was only added in Emacs 27, so this doesn't seem to be a big problem.
> OK.

Thank you.

(I'll take this to mean that the patch is OK for emacs-27.)

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919-done <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Wed, 27 May 2020 22:18:06 +0300

Version: 27.1

Aand pushed.

Trevor, you can now try different values of 
next-error-find-buffer-function (in particular, #'ignore) and see which 
behaves more to your liking.

Thanks all, closing.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Fri, 29 May 2020 02:07:17 +0300

On 24.05.2020 01:24, Juri Linkov wrote:
>>> But I'm not proud of the case no. 2, so you can drop it:)
>>> Or better move it to a separate function and provide this function
>>> as an option in next-error-find-buffer-function.
>> Patch attached. Comments welcome.
> Is this patch for master?  So there is enough time for thorough testing?

And it's now in master.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Sun, 31 May 2020 01:29:07 +0300

>>> Suggestions for a better (shorter?) name for the new function welcome, by
>>> the way.
>> Maybe next-error-find-keep-navigation.
>
> Short, but kinda cryptic (why "find"?). Not bad, but some other options:

Sorry, a typo, I meant next-error-keep-navigation.

> next-error-maybe-current-keep-navigation
> next-error-no-navigation-try-current

Not bad, I guess it's difficult to find a name that it both short and self-describing.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 02 Jun 2020 01:41:39 +0300

>>>> But I'm not proud of the case no. 2, so you can drop it:)
>>>> Or better move it to a separate function and provide this function
>>>> as an option in next-error-find-buffer-function.
>>> Patch attached. Comments welcome.
>> Is this patch for master?  So there is enough time for thorough testing?
>
> And it's now in master.

Thanks.  Tho I think in master next-error-find-buffer-function
should support a list of functions, so the user could customize
their order to arrange their priorities.  Then they could be called
with run-hook-with-args-until-success.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Tue, 2 Jun 2020 02:04:39 +0300

On 02.06.2020 01:41, Juri Linkov wrote:
> Tho I think in master next-error-find-buffer-function
> should support a list of functions, so the user could customize
> their order to arrange their priorities.  Then they could be called
> with run-hook-with-args-until-success.

I suppose.

We should perhaps wait and see if people do customize that variable at 
all. But the idea is solid.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 11 Jun 2020 02:03:11 +0300

>> next-error-maybe-current-keep-navigation
>> next-error-no-navigation-try-current
>
> Not bad, I guess it's difficult to find a name that it both short and self-describing.

I found a better name:

next-error-buffer-unnavigated-current

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 11 Jun 2020 02:05:39 +0300

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

>> Tho I think in master next-error-find-buffer-function
>> should support a list of functions, so the user could customize
>> their order to arrange their priorities.  Then they could be called
>> with run-hook-with-args-until-success.
>
> I suppose.
>
> We should perhaps wait and see if people do customize that variable at
> all. But the idea is solid.

Actually, people already want to customize it to a list (at least, I know one such user :)
So here is a patch:

[next-error-find-buffer-function-repeat.patch (text/x-diff, inline)]

diff --git a/lisp/simple.el b/lisp/simple.el
index 0fe8a1025c..646236879c 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -199,7 +199,7 @@ next-error-buffer-p
 	   (and extra-test-inclusive
 		(funcall extra-test-inclusive))))))
 
-(defcustom next-error-find-buffer-function #'ignore
+(defcustom next-error-find-buffer-function '(ignore)
   "Function called to find a `next-error' capable buffer.
 This functions takes the same three arguments as the function
 `next-error-find-buffer', and should return the buffer to be
@@ -208,12 +208,13 @@ next-error-find-buffer-function
 If the function returns nil, `next-error-find-buffer' will
 try to use the buffer it used previously, and failing that
 all other buffers."
-  :type '(choice (const :tag "No default" ignore)
-                 (const :tag "Single next-error capable buffer on selected frame"
-                        next-error-buffer-on-selected-frame)
-                 (const :tag "Current buffer if next-error capable and outside navigation"
-                        next-error-no-navigation-try-current)
-                 (function :tag "Other function"))
+  :type '(repeat
+          (choice (const :tag "No default" ignore)
+                  (const :tag "Single next-error capable buffer on selected frame"
+                         next-error-buffer-on-selected-frame)
+                  (const :tag "Current buffer if next-error capable and outside navigation"
+                         next-error-no-navigation-try-current)
+                  (function :tag "Other function")))
   :group 'next-error
   :version "28.1")
 
@@ -275,9 +276,13 @@ next-error-find-buffer
 that buffer is rejected."
   (or
    ;; 1. If a customizable function returns a buffer, use it.
-   (funcall next-error-find-buffer-function avoid-current
-                                            extra-test-inclusive
-                                            extra-test-exclusive)
+   (or (and (functionp next-error-find-buffer-function)
+            (funcall next-error-find-buffer-function avoid-current
+                     extra-test-inclusive extra-test-exclusive))
+       (and (listp next-error-find-buffer-function)
+            (run-hook-with-args-until-success
+             'next-error-find-buffer-function avoid-current
+             extra-test-inclusive extra-test-exclusive)))
    ;; 2. If next-error-last-buffer is an acceptable buffer, use that.
    (if (and next-error-last-buffer
             (next-error-buffer-p next-error-last-buffer avoid-current

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 11 Jun 2020 02:28:30 +0300

On 11.06.2020 02:03, Juri Linkov wrote:
>>> next-error-maybe-current-keep-navigation
>>> next-error-no-navigation-try-current
>> Not bad, I guess it's difficult to find a name that it both short and self-describing.
> I found a better name:
> 
> next-error-buffer-unnavigated-current

Sounds okay to me.

But... do we rename it on the emacs-27 branch? Or do we keep it and 
introduce an obsolete alias in master?

Considering the above complication, I would just keep the current name. 
But it's up to you (and Eli, I guess).

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 11 Jun 2020 02:32:44 +0300

On 11.06.2020 02:05, Juri Linkov wrote:
> Actually, people already want to customize it to a list (at least, I know one such user:)

Say hello to the user to me. ;-)

And maybe ask a question: what kind of functions do they want to put on 
there?

And/or would they be content to advice-add on 
next-error-find-buffer-function instead?

> -(defcustom next-error-find-buffer-function #'ignore
> +(defcustom next-error-find-buffer-function '(ignore)

                                             ^s, maybe?

> +   (or (and (functionp next-error-find-buffer-function)
> +            (funcall next-error-find-buffer-function avoid-current
> +                     extra-test-inclusive extra-test-exclusive))
> +       (and (listp next-error-find-buffer-function)
> +            (run-hook-with-args-until-success
> +             'next-error-find-buffer-function avoid-current
> +             extra-test-inclusive extra-test-exclusive)))

Looks like run_hook_with_args can deal with the case where the value of 
the hook is a single function.

>      ;; 2. If next-error-last-buffer is an acceptable buffer, use that.
>      (if (and next-error-last-buffer
>               (next-error-buffer-p next-error-last-buffer avoid-current

Should we take the rest of the cases in next-error-find-buffer and move 
them to the default value of the above hook?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, juri <at> linkov.net
Subject: Re: bug#40919: 27.0.91;
 next-error-select-buffer does not always behave as documented
Date: Thu, 11 Jun 2020 16:11:06 +0300

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Thu, 11 Jun 2020 02:28:30 +0300
> Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
> 
> But... do we rename it on the emacs-27 branch? Or do we keep it and 
> introduce an obsolete alias in master?

The latter, I think.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Fri, 12 Jun 2020 01:39:29 +0300

>> I found a better name:
>> next-error-buffer-unnavigated-current
>
> Sounds okay to me.
>
> But... do we rename it on the emacs-27 branch? Or do we keep it and
> introduce an obsolete alias in master?

It makes no sense to introduce an obsolete alias for the function
added two weeks ago.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Fri, 12 Jun 2020 01:43:29 +0300

> what kind of functions do they want to put on there?

Both next-error-buffer-on-selected-frame and next-error-no-navigation-try-current.

> And/or would they be content to advice-add on
> next-error-find-buffer-function instead?

Is it possible to add advice-add by using customization?

>> -(defcustom next-error-find-buffer-function #'ignore
>> +(defcustom next-error-find-buffer-function '(ignore)
>
>                                              ^s, maybe?

Ok, when using as a hook it could be '-functions', but in case
of using advice-add it should be still '-function'.

>> +   (or (and (functionp next-error-find-buffer-function)
>> +            (funcall next-error-find-buffer-function avoid-current
>> +                     extra-test-inclusive extra-test-exclusive))
>> +       (and (listp next-error-find-buffer-function)
>> +            (run-hook-with-args-until-success
>> +             'next-error-find-buffer-function avoid-current
>> +             extra-test-inclusive extra-test-exclusive)))
>
> Looks like run_hook_with_args can deal with the case where the value of the
> hook is a single function.

Ok, if it's impossible to use advice-add then lets simplify the hook case.

>>      ;; 2. If next-error-last-buffer is an acceptable buffer, use that.
>>      (if (and next-error-last-buffer
>>               (next-error-buffer-p next-error-last-buffer avoid-current
>
> Should we take the rest of the cases in next-error-find-buffer and move
> them to the default value of the above hook?

I don't think so, I don't believe someone might want to customize the
rest of the cases.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, dgutov <at> yandex.ru
Subject: Re: bug#40919: 27.0.91;
 next-error-select-buffer does not always behave as documented
Date: Fri, 12 Jun 2020 10:06:26 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Fri, 12 Jun 2020 01:39:29 +0300
> Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
> 
> >> I found a better name:
> >> next-error-buffer-unnavigated-current
> >
> > Sounds okay to me.
> >
> > But... do we rename it on the emacs-27 branch? Or do we keep it and
> > introduce an obsolete alias in master?
> 
> It makes no sense to introduce an obsolete alias for the function
> added two weeks ago.

Then what was the part about renaming it on emacs-27 about?

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, dgutov <at> yandex.ru
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Sun, 14 Jun 2020 01:53:19 +0300

>> >> I found a better name:
>> >> next-error-buffer-unnavigated-current
>> >
>> > Sounds okay to me.
>> >
>> > But... do we rename it on the emacs-27 branch? Or do we keep it and
>> > introduce an obsolete alias in master?
>> 
>> It makes no sense to introduce an obsolete alias for the function
>> added two weeks ago.
>
> Then what was the part about renaming it on emacs-27 about?

If you think that the new name is better than the name added 2 weeks ago,
then it's better to rename it on emacs-27, or not to rename it at all.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Sun, 14 Jun 2020 14:50:33 +0300

On 12.06.2020 01:43, Juri Linkov wrote:
>> what kind of functions do they want to put on there?
> 
> Both next-error-buffer-on-selected-frame and next-error-no-navigation-try-current.
> 
>> And/or would they be content to advice-add on
>> next-error-find-buffer-function instead?
> 
> Is it possible to add advice-add by using customization?

No, or at least not yet. But if we know of only one user that wants this 
setup, surely that's not a problem?

By the way, you were going to evaluate the new default. Do you now think 
that it's problematic somehow (and, for instance, the previous was a 
better default), or do you want to change it as a purely personal 
preference?

>>> -(defcustom next-error-find-buffer-function #'ignore
>>> +(defcustom next-error-find-buffer-function '(ignore)
>>
>>                                               ^s, maybe?
> 
> Ok, when using as a hook it could be '-functions', but in case
> of using advice-add it should be still '-function'.

Yup. These are the two options.

>>>       ;; 2. If next-error-last-buffer is an acceptable buffer, use that.
>>>       (if (and next-error-last-buffer
>>>                (next-error-buffer-p next-error-last-buffer avoid-current
>>
>> Should we take the rest of the cases in next-error-find-buffer and move
>> them to the default value of the above hook?
> 
> I don't think so, I don't believe someone might want to customize the
> rest of the cases.

Well, if you're sure about that.

Having them all on the hook seemed logical to me, but indeed I don't 
know how necessary that is.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Mon, 15 Jun 2020 02:15:30 +0300

>>> what kind of functions do they want to put on there?
>> Both next-error-buffer-on-selected-frame and
>> next-error-no-navigation-try-current.
>>
>>> And/or would they be content to advice-add on
>>> next-error-find-buffer-function instead?
>> Is it possible to add advice-add by using customization?
>
> No, or at least not yet. But if we know of only one user that wants this
> setup, surely that's not a problem?

It's a general problem that hindered the development of other features
that might benefit from customizable advice-add (namely set-multi-message).

> By the way, you were going to evaluate the new default. Do you now think
> that it's problematic somehow (and, for instance, the previous was a 
> better default), or do you want to change it as a purely
> personal preference?

Only personal preference, it seems the default in master is fine
for most users.

>>>>       ;; 2. If next-error-last-buffer is an acceptable buffer, use that.
>>>>       (if (and next-error-last-buffer
>>>>                (next-error-buffer-p next-error-last-buffer avoid-current
>>>
>>> Should we take the rest of the cases in next-error-find-buffer and move
>>> them to the default value of the above hook?
>> I don't think so, I don't believe someone might want to customize the
>> rest of the cases.
>
> Well, if you're sure about that.
>
> Having them all on the hook seemed logical to me, but indeed I don't know
> how necessary that is.

The reason why I think no one might want to customize the rest of the cases
is because I believe that next-error-last-buffer is always non-nil, so
all other cases (i.e. 3, 4, 5, 6) are useless and never used.  Isn't it so?

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org, dgutov <at> yandex.ru
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Mon, 15 Jun 2020 02:17:13 +0300

>>> >> I found a better name:
>>> >> next-error-buffer-unnavigated-current
>>> >
>>> > Sounds okay to me.
>>> >
>>> > But... do we rename it on the emacs-27 branch? Or do we keep it and
>>> > introduce an obsolete alias in master?
>>>
>>> It makes no sense to introduce an obsolete alias for the function
>>> added two weeks ago.
>>
>> Then what was the part about renaming it on emacs-27 about?
>
> If you think that the new name is better than the name added 2 weeks ago,
> then it's better to rename it on emacs-27, or not to rename it at all.

I have to point out that according to the naming convention
the function should have the prefix next-error-buffer-,
so it seems the renaming in emacs-27 is inevitable?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Mon, 15 Jun 2020 10:58:29 +0300

On 15.06.2020 02:15, Juri Linkov wrote:

>>>> And/or would they be content to advice-add on
>>>> next-error-find-buffer-function instead?
>>> Is it possible to add advice-add by using customization?
>>
>> No, or at least not yet. But if we know of only one user that wants this
>> setup, surely that's not a problem?
> 
> It's a general problem that hindered the development of other features
> that might benefit from customizable advice-add (namely set-multi-message).

The Customize UI is definitely not my area, sorry (not as developer, nor 
as a user).

>> By the way, you were going to evaluate the new default. Do you now think
>> that it's problematic somehow (and, for instance, the previous was a
>> better default), or do you want to change it as a purely
>> personal preference?
> 
> Only personal preference, it seems the default in master is fine
> for most users.

Very good.

>> Having them all on the hook seemed logical to me, but indeed I don't know
>> how necessary that is.
> 
> The reason why I think no one might want to customize the rest of the cases
> is because I believe that next-error-last-buffer is always non-nil, so
> all other cases (i.e. 3, 4, 5, 6) are useless and never used.  Isn't it so?

change-log-mode doesn't set it (and it shouldn't). Maybe there will be 
other major modes like that. So #3 should be used sometimes.

#4 doesn't seem very intuitive/useful to me, and I don't understand #5 
(when is AVOID-CURRENT non-nil?)

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 40919 <at> debbugs.gnu.org, tspiteri <at> ieee.org
Subject: Re: bug#40919: 27.0.91; next-error-select-buffer does not always
 behave as documented
Date: Thu, 25 Jun 2020 02:38:45 +0300

>>>>> And/or would they be content to advice-add on
>>>>> next-error-find-buffer-function instead?
>>>> Is it possible to add advice-add by using customization?
>>>
>>> No, or at least not yet. But if we know of only one user that wants this
>>> setup, surely that's not a problem?
>> It's a general problem that hindered the development of other features
>> that might benefit from customizable advice-add (namely set-multi-message).
>
> The Customize UI is definitely not my area, sorry (not as developer, nor as
> a user).

I retract my patch to use a list of next-error functions,
because it's easy to configure this using add-function:

(add-function :override next-error-find-buffer-function
              #'next-error-buffer-on-selected-frame)

(add-function :after-until next-error-find-buffer-function
              #'next-error-buffer-unnavigated-current)

Previous Next

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