bug#28864: 25.3.50; next-error-no-select does select

[Dmitry Gutov <dgutov@yandex.ru>]

On 10/29/17 11:42 PM, Juri Linkov wrote:

> I thought maybe context diff is easier to read in this case,
> but here is unified diff below.

This one is better, thanks. Lots of practice reading unified diffs.

> We need the global value as well to get the last next-error buffer
> when there is no buffer-local value yet in the current buffer.

Doesn't sound very elegant: we accumulate hidden state as the time 
passes, and the user calls 'next-error' with compilations, grep, and so 
on. But this is better than nothing, of course.

Does it also mean that the effect of next-error-select-buffer will also 
be buffer-local?

>>> 4. moves window-on-frame-visibility code to separate function
>>>      that can be used to customize for backward-compatibility
>>
>> Do we get a built-in alternative to this value? If we just provide
>> a customization point, that nice, but not a significant improvement.
> 
> You mean to remove next-error-buffer-on-selected-frame?
> Or maybe to do the other way: add more predefined functions like
> a function to use window-local navigation as an alternative.

Now I see it: you have moved the visibility-based search to the custom 
variable and disabled it by default. So the default behavior is changed 
(for the better, IMO), and I withdraw the question, thanks. :)

>>> 5. adds next-error-select-buffer to manually switch to another
>>>      next-error capable buffer
>>
>> Not having tried the patch yet... Will the user have to do that after
>> semi-accidentally opening a changelog-mode buffer, in order not to switch
>> to its error list?
> 
> When the user visits a ChangeLog buffer from *grep*, then
> next-error continues the *grep* navigation.  When the user
> visit a ChangeLog file by any other command, then next-error
> navigates entries in that ChangeLog buffer.

Does this also work with next-error, not just next-error-no-select?

I'm guessing this part helps with that:

+  (let ((next-error-buffer (next-error-find-buffer)))
+    (when next-error-buffer
+      ;; we know here that next-error-function is a valid symbol we can 
funcall
+      (with-current-buffer next-error-last-buffer
+        (funcall next-error-function (prefix-numeric-value arg) reset)
+        ;; next-error-function might change the value of
+        ;; next-error-last-buffer, so set it later
+        (setq next-error-last-buffer next-error-buffer)
+        (setq-default next-error-last-buffer next-error-last-buffer)

So we ignore the next-error-last-buffer change during a call to 
next-error-last-function, but not in any other circumstances? Like 
visiting a ChangeLog file manually. Maybe that's okay...

>>> 6. message to show which next-error buffer is currently used
>>
>> Every time we call `next-error'?
> 
> That's right.

That might be a bit excessive. But more importantly, opaque to an 
average user.

How about a message like this:

"Showing next/last/previous error from %s"

?