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

classic Classic list List threaded Threaded
32 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
> 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?



Reply | Threaded
Open this post in threaded view
|

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

Trevor Spiteri
>> 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.





Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
> 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)




Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
>> @@ -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?



Reply | Threaded
Open this post in threaded view
|

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

Trevor Spiteri
>> 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.





Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
>> 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.



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Juri Linkov <[hidden email]>
> Date: Sun, 03 May 2020 02:38:10 +0300
> Cc: [hidden email]
>
> 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.



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
>> 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.



Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
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 (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
>> 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?



Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
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.



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
>> 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.



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Dmitry Gutov <[hidden email]>
> Date: Mon, 25 May 2020 04:58:05 +0300
> Cc: [hidden email], [hidden email]
>
> 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.)



Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
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 (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
On 26.05.2020 19:06, Eli Zaretskii wrote:

>> Cc: [hidden email], [hidden email], [hidden email]
>> From: Dmitry Gutov <[hidden email]>
>> 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).



Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
On 26.05.2020 19:33, Eli Zaretskii wrote:

>> Cc:[hidden email],[hidden email],[hidden email]
>> From: Dmitry Gutov<[hidden email]>
>> 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.)



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
In reply to this post by Juri Linkov-2
>>> 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.



Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
In reply to this post by Juri Linkov-2
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.



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
In reply to this post by Juri Linkov-2
>> 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



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
In reply to this post by Dmitry Gutov
>> 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:


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
12