unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* improve "next locus from <buffer>" messages
@ 2019-04-03 17:50 Stephen Leake
  2019-04-03 18:20 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Leake @ 2019-04-03 17:50 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

Currently, next-error always outputs a message "next locus from
<buffer>". In the vast majority of cases, <buffer> does not change from
one message to the next, so this message is just annoying.

The attached patch changes it so the message is only output when the
locus changes.

It also gets rid of the "first/current/previous" options; what matters
is where the _next_ error will come from.

Comments?

-- 
-- Stephe

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: error_locus.diff --]
[-- Type: text/x-patch, Size: 2870 bytes --]

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 5bfb0bf901..1d5b932ba1 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1825,6 +1825,8 @@ compilation-start
 	(goto-char (point-max))))
 
     ;; Make it so the next C-x ` will use this buffer.
+    (when (not (eq outbuf next-error-last-buffer))
+      (message "next locus from %s" outbuf))
     (setq next-error-last-buffer outbuf)))
 
 (defun compilation-set-window-height (window)
diff --git a/lisp/simple.el b/lisp/simple.el
index 306df96766..832c62ffb3 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -312,34 +312,31 @@ next-error
       ;; We know here that next-error-function is a valid symbol we can funcall
       (with-current-buffer buffer
         (funcall next-error-function (prefix-numeric-value arg) reset)
-        (next-error-found buffer (current-buffer))
-        (message "%s locus from %s"
-                 (cond (reset                             "First")
-                       ((eq (prefix-numeric-value arg) 0) "Current")
-                       ((< (prefix-numeric-value arg) 0)  "Previous")
-                       (t                                 "Next"))
-                 next-error-last-buffer)))))
+        (next-error-found buffer (current-buffer))))))
 
 (defun next-error-internal ()
   "Visit the source code corresponding to the `next-error' message at point."
   (let ((buffer (current-buffer)))
     ;; We know here that next-error-function is a valid symbol we can funcall
     (funcall next-error-function 0 nil)
-    (next-error-found buffer (current-buffer))
-    (message "Current locus from %s" next-error-last-buffer)))
+    (next-error-found buffer (current-buffer))))
 
 (defun next-error-found (&optional from-buffer to-buffer)
   "Function to call when the next locus is found and displayed.
 FROM-BUFFER is a buffer from which next-error navigated,
 and TO-BUFFER is a target buffer."
-  (setq next-error-last-buffer (or from-buffer (current-buffer)))
-  (when to-buffer
-    (with-current-buffer to-buffer
-      (setq next-error-buffer from-buffer)))
-  (when next-error-recenter
-    (recenter next-error-recenter))
-  (funcall next-error-found-function from-buffer to-buffer)
-  (run-hooks 'next-error-hook))
+  (let ((prev next-error-last-buffer)
+        (next (or from-buffer (current-buffer))))
+    (when (not (eq prev next))
+      (message "next locus from %s" next))
+    (setq next-error-last-buffer next)
+    (when to-buffer
+      (with-current-buffer to-buffer
+        (setq next-error-buffer from-buffer)))
+    (when next-error-recenter
+      (recenter next-error-recenter))
+    (funcall next-error-found-function from-buffer to-buffer)
+    (run-hooks 'next-error-hook)))
 
 (defun next-error-select-buffer (buffer)
   "Select a `next-error' capable BUFFER and set it as the last used.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-03 17:50 improve "next locus from <buffer>" messages Stephen Leake
@ 2019-04-03 18:20 ` Eli Zaretskii
  2019-04-03 20:53   ` Stephen Leake
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2019-04-03 18:20 UTC (permalink / raw)
  To: Stephen Leake; +Cc: emacs-devel

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Wed, 03 Apr 2019 09:50:20 -0800
> 
> Currently, next-error always outputs a message "next locus from
> <buffer>". In the vast majority of cases, <buffer> does not change from
> one message to the next, so this message is just annoying.
> 
> The attached patch changes it so the message is only output when the
> locus changes.
> 
> It also gets rid of the "first/current/previous" options; what matters
> is where the _next_ error will come from.
> 
> Comments?

Comment: why change long-time default behavior based on personal
preferences?  If indeed some people are annoyed by these simple
messages, maybe a defcustom is in order, but changing the default
behavior unconditionally due to such reasons sounds gross to me.

Thanks.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-03 18:20 ` Eli Zaretskii
@ 2019-04-03 20:53   ` Stephen Leake
  2019-04-03 21:22     ` Stephen Leake
  2019-04-04 12:54     ` Eli Zaretskii
  0 siblings, 2 replies; 14+ messages in thread
From: Stephen Leake @ 2019-04-03 20:53 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Currently, next-error always outputs a message "next locus from
>> <buffer>". In the vast majority of cases, <buffer> does not change from
>> one message to the next, so this message is just annoying.
>> 
>> The attached patch changes it so the message is only output when the
>> locus changes.
>> 
>> It also gets rid of the "first/current/previous" options; what matters
>> is where the _next_ error will come from.
>> 
>> Comments?
>
> Comment: why change long-time default behavior based on personal
> preferences?  

These messages are not in emacs-26, so I don't think they count as
"long-time".

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-03 20:53   ` Stephen Leake
@ 2019-04-03 21:22     ` Stephen Leake
  2019-04-04  9:26       ` Felician Nemeth
  2019-04-04 12:54     ` Eli Zaretskii
  1 sibling, 1 reply; 14+ messages in thread
From: Stephen Leake @ 2019-04-03 21:22 UTC (permalink / raw)
  To: emacs-devel

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Currently, next-error always outputs a message "next locus from
>>> <buffer>". In the vast majority of cases, <buffer> does not change from
>>> one message to the next, so this message is just annoying.
>>> 
>>> The attached patch changes it so the message is only output when the
>>> locus changes.
>>> 
>>> It also gets rid of the "first/current/previous" options; what matters
>>> is where the _next_ error will come from.
>>> 
>>> Comments?
>>
>> Comment: why change long-time default behavior based on personal
>> preferences?  
>
> These messages are not in emacs-26, so I don't think they count as
> "long-time".

If you are refering to the "first/current/previous", they were not
always accurate; often the first error from a locus prints "next error
from ...", because next-error is invoked without reset.

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-03 21:22     ` Stephen Leake
@ 2019-04-04  9:26       ` Felician Nemeth
  2019-04-04 13:41         ` Stephen Leake
  0 siblings, 1 reply; 14+ messages in thread
From: Felician Nemeth @ 2019-04-04  9:26 UTC (permalink / raw)
  To: Stephen Leake; +Cc: emacs-devel

>>>> The attached patch changes it so the message is only output when the
>>>> locus changes.

>>>> Comments?

I'm not completely sure, but the proposed patch seems to assume that
next-error is only called from compilation-start.  xref doesn't use the
compilation (mirror) mode, but it uses the next-error functionality.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-03 20:53   ` Stephen Leake
  2019-04-03 21:22     ` Stephen Leake
@ 2019-04-04 12:54     ` Eli Zaretskii
  2019-04-04 13:55       ` Stephen Leake
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2019-04-04 12:54 UTC (permalink / raw)
  To: Stephen Leake; +Cc: emacs-devel

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Wed, 03 Apr 2019 12:53:41 -0800
> 
> > Comment: why change long-time default behavior based on personal
> > preferences?  
> 
> These messages are not in emacs-26, so I don't think they count as
> "long-time".

You are right, sorry.  It's just that the changes which introduced
them were discussed quite some time ago.

But the main issue still stands: why change something based on
personal preferences?  This function is called from many other places:
xref, multi-occur, even some Dired commands (AFAIR), and it can be
called intermittently in different contexts.  I think displaying the
context in these situations is generally a good idea.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-04  9:26       ` Felician Nemeth
@ 2019-04-04 13:41         ` Stephen Leake
  2019-04-04 14:09           ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Leake @ 2019-04-04 13:41 UTC (permalink / raw)
  To: emacs-devel

Felician Nemeth <felician.nemeth@gmail.com> writes:

>>>>> The attached patch changes it so the message is only output when the
>>>>> locus changes.
>
>>>>> Comments?
>
> I'm not completely sure, but the proposed patch seems to assume that
> next-error is only called from compilation-start.  

No, that assumption is not made. I added the message in
compilation-start because otherwise it does not print a message when the
locus is changed.

I did not search the rest of emacs for places that set
next-error-last-buffer, so there may be other places that need a
message.

Yes; xref--xref-buffer-mode sets next-error-last-buffer without a
message.

It makes sense to print a message only when the error locus
change might be unexpected. If the user has started a compilation, they
expect error messages to come from *compilation*; similarly for *xref*.
The original discussion included things like flymake, which is invoked
in the background.

I'll take out the message in compilation-start.

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-04 12:54     ` Eli Zaretskii
@ 2019-04-04 13:55       ` Stephen Leake
  2019-04-07 16:55         ` Stephen Leake
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Leake @ 2019-04-04 13:55 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stephen Leake <stephen_leake@stephe-leake.org>
>> Date: Wed, 03 Apr 2019 12:53:41 -0800
>> 
>> > Comment: why change long-time default behavior based on personal
>> > preferences?  
>> 
>> These messages are not in emacs-26, so I don't think they count as
>> "long-time".
>
> You are right, sorry.  It's just that the changes which introduced
> them were discussed quite some time ago.

Yes. I've only started using master for development recently, so I only
noticed this now.

> But the main issue still stands: why change something based on
> personal preferences? This function is called from many other places:
> xref, multi-occur, even some Dired commands (AFAIR), and it can be
> called intermittently in different contexts. I think displaying the
> context in these situations is generally a good idea.

I agree, but I don't think it makes sense to print the same message over
and over again, when nothing has changed. Isn't there a general
rule to not fill *Messages* with noise?

xref and compile are the only places that change next-error-last-buffer
directly (in the core emacs sources; EPLA packages may also do this).

All of those other places still print the message when the locus has
changed. However, with this patch they only print "next error locus"; I
guess in come cases the "first/current/previous" messages might make
sense.

I'll put back the "first/current/previous", and add a custom option to
control this.

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-04 13:41         ` Stephen Leake
@ 2019-04-04 14:09           ` Dmitry Gutov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Gutov @ 2019-04-04 14:09 UTC (permalink / raw)
  To: Stephen Leake, emacs-devel; +Cc: Juri Linkov

On 04.04.2019 16:41, Stephen Leake wrote:

> It makes sense to print a message only when the error locus
> change might be unexpected. If the user has started a compilation, they
> expect error messages to come from *compilation*; similarly for *xref*.

That sounds silly. Why not do that in the next-error feature itself?

Also, it seems Yuri is missing from this discussion.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-04 13:55       ` Stephen Leake
@ 2019-04-07 16:55         ` Stephen Leake
  2019-04-08 19:21           ` Juri Linkov
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Leake @ 2019-04-07 16:55 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

Stephen Leake <stephen_leake@stephe-leake.org> writes:


> I'll put back the "first/current/previous", and add a custom option to
> control this.

Updated patch attached. I added a NEWS entry; I'm not clear if the new
defcustom should be mentioned in the Emacs manual at (emacs) Compilation
Mode.

-- 
-- Stephe

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: error_locus.diff --]
[-- Type: text/x-patch, Size: 2972 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 26c761ae01..59a97b6b9c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -303,6 +303,10 @@ and directory-local variables.
 'with-connection-local-profiles'.  No argument 'profiles' needed any
 longer.
 
+---
+** next-error-suppress-locus-message controls when `next-error'
+   outputs a message about the error locus.
+
 \f
 * Editing Changes in Emacs 27.1
 
diff --git a/lisp/simple.el b/lisp/simple.el
index 306df96766..4bb4116842 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -110,6 +110,15 @@ next-error-hook
   :type 'hook
   :group 'next-error)
 
+(defcustom next-error-suppress-locus-message nil
+  "If nil, `next-error' always outputs the current error buffer.
+If non-nil, the message is output only when the error buffer
+changes."
+  :group 'next-error
+  :type 'boolean
+  :safe #'booleanp
+  :version "27.1")
+
 (defvar next-error-highlight-timer nil)
 
 (defvar next-error-overlay-arrow-position nil)
@@ -312,21 +321,27 @@ next-error
       ;; We know here that next-error-function is a valid symbol we can funcall
       (with-current-buffer buffer
         (funcall next-error-function (prefix-numeric-value arg) reset)
-        (next-error-found buffer (current-buffer))
-        (message "%s locus from %s"
-                 (cond (reset                             "First")
-                       ((eq (prefix-numeric-value arg) 0) "Current")
-                       ((< (prefix-numeric-value arg) 0)  "Previous")
-                       (t                                 "Next"))
-                 next-error-last-buffer)))))
+        (let ((prev next-error-last-buffer))
+          (next-error-found buffer (current-buffer))
+          (when (or (not next-error-suppress-locus-message)
+                    (not (eq prev next-error-last-buffer)))
+            (message "%s locus from %s"
+                     (cond (reset                             "First")
+                           ((eq (prefix-numeric-value arg) 0) "Current")
+                           ((< (prefix-numeric-value arg) 0)  "Previous")
+                           (t                                 "Next"))
+                     next-error-last-buffer)))))))
 
 (defun next-error-internal ()
   "Visit the source code corresponding to the `next-error' message at point."
   (let ((buffer (current-buffer)))
     ;; We know here that next-error-function is a valid symbol we can funcall
     (funcall next-error-function 0 nil)
-    (next-error-found buffer (current-buffer))
-    (message "Current locus from %s" next-error-last-buffer)))
+    (let ((prev next-error-last-buffer))
+      (next-error-found buffer (current-buffer))
+      (when (or (not next-error-suppress-locus-message)
+                (not (eq prev next-error-last-buffer)))
+        (message "Current locus from %s" next-error-last-buffer)))))
 
 (defun next-error-found (&optional from-buffer to-buffer)
   "Function to call when the next locus is found and displayed.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: improve "next locus from <buffer>" messages
  2019-04-07 16:55         ` Stephen Leake
@ 2019-04-08 19:21           ` Juri Linkov
  2019-04-08 22:16             ` [SPAM UNSURE] " Stephen Leake
  0 siblings, 1 reply; 14+ messages in thread
From: Juri Linkov @ 2019-04-08 19:21 UTC (permalink / raw)
  To: Stephen Leake; +Cc: emacs-devel

> +---
> +** next-error-suppress-locus-message controls when `next-error'
> +   outputs a message about the error locus.

Is this feature really expected to be used often to deserve the new
defcustom or a simple defvar would be enough?

In any case I recommend to rename it to next-error-verbosity.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [SPAM UNSURE] Re: improve "next locus from <buffer>" messages
  2019-04-08 19:21           ` Juri Linkov
@ 2019-04-08 22:16             ` Stephen Leake
  2019-04-13 20:56               ` Juri Linkov
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Leake @ 2019-04-08 22:16 UTC (permalink / raw)
  To: emacs-devel

Juri Linkov <juri@linkov.net> writes:

>> +---
>> +** next-error-suppress-locus-message controls when `next-error'
>> +   outputs a message about the error locus.
>
> Is this feature really expected to be used often to deserve the new
> defcustom or a simple defvar would be enough?

If it's a defcustom, it is easier for people to find, if they are
bothered by the verbosity.

> In any case I recommend to rename it to next-error-verbosity.

ok.

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [SPAM UNSURE] Re: improve "next locus from <buffer>" messages
  2019-04-08 22:16             ` [SPAM UNSURE] " Stephen Leake
@ 2019-04-13 20:56               ` Juri Linkov
  2019-04-14 16:25                 ` [SPAM UNSURE] " Stephen Leake
  0 siblings, 1 reply; 14+ messages in thread
From: Juri Linkov @ 2019-04-13 20:56 UTC (permalink / raw)
  To: Stephen Leake; +Cc: emacs-devel

>>> +---
>>> +** next-error-suppress-locus-message controls when `next-error'
>>> +   outputs a message about the error locus.
>>
>> Is this feature really expected to be used often to deserve the new
>> defcustom or a simple defvar would be enough?
>
> If it's a defcustom, it is easier for people to find, if they are
> bothered by the verbosity.
>
>> In any case I recommend to rename it to next-error-verbosity.
>
> ok.

Sorry, I realized only after your commit that a better name would be
next-error-verbose.  The word “verbosity” usually applies to
customization of the level of verbosity, but I see that you defined
a boolean variable for which “verbose” would be more appropriate.

Also this makes clear that the meaning of its value should be reversed -
to show verbose messages when the value is non-nil.  But I have no opinion
about its default value, if you want you can change it from nil to t.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [SPAM UNSURE] Re: [SPAM UNSURE] Re: improve "next locus from <buffer>" messages
  2019-04-13 20:56               ` Juri Linkov
@ 2019-04-14 16:25                 ` Stephen Leake
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Leake @ 2019-04-14 16:25 UTC (permalink / raw)
  To: emacs-devel

Juri Linkov <juri@linkov.net> writes:

>>>> +---
>>>> +** next-error-suppress-locus-message controls when `next-error'
>>>> +   outputs a message about the error locus.
>>>
>>> Is this feature really expected to be used often to deserve the new
>>> defcustom or a simple defvar would be enough?
>>
>> If it's a defcustom, it is easier for people to find, if they are
>> bothered by the verbosity.
>>
>>> In any case I recommend to rename it to next-error-verbosity.
>>
>> ok.
>
> Sorry, I realized only after your commit that a better name would be
> next-error-verbose.  

Makes sense

> Also this makes clear that the meaning of its value should be reversed -
> to show verbose messages when the value is non-nil.  

Yes, I realized that when I went to set it.

> But I have no opinion about its default value, if you want you can
> change it from nil to t.

On the principle of least bother to others, I'll make the default be the
verbose setting.

Thanks for your help.

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-04-14 16:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-03 17:50 improve "next locus from <buffer>" messages Stephen Leake
2019-04-03 18:20 ` Eli Zaretskii
2019-04-03 20:53   ` Stephen Leake
2019-04-03 21:22     ` Stephen Leake
2019-04-04  9:26       ` Felician Nemeth
2019-04-04 13:41         ` Stephen Leake
2019-04-04 14:09           ` Dmitry Gutov
2019-04-04 12:54     ` Eli Zaretskii
2019-04-04 13:55       ` Stephen Leake
2019-04-07 16:55         ` Stephen Leake
2019-04-08 19:21           ` Juri Linkov
2019-04-08 22:16             ` [SPAM UNSURE] " Stephen Leake
2019-04-13 20:56               ` Juri Linkov
2019-04-14 16:25                 ` [SPAM UNSURE] " Stephen Leake

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).