bug#51335: 29.0.50; Use warnings facility for reporting Gnus errors

[Eric Abrahamsen <eric@ericabrahamsen.net>]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> I haven't changed how `gnus-verbose' works, so that filter does the same
>> thing it always has. The only real difference is that messages/errors go
>> into a dedicated buffer, and the user has the option to keep the buffer
>> hidden (the default), pop it up with a command, or fit it into their
>> existing Gnus window configuration however they like.
>
> Sounds good.

Well, I'm about 75% happy with this. I've attached both the patch I'm
running, and a screenshot of my *Group* configuration with the logging
window visible (I've set `gnus-verbose' to 10). I will suppress the urge
to apologize for my *Group* buffer appearance, I'm in the middle of some
home improvements.

The rightmost blob is the warning category: the "(gnus)". With fuller
adoption it might look like "(gnus nntp)" or "(gnus draft)" or "(gnus
search)", etc. I've made it so `gnus-add-timestamp-to-message' also adds
a timestamp here, though that's not activated in the screenshot.

The main awkwardness come from the fact that warnings.el is kind of set
up for you to incorporate it into other packages, but kind of not. You
can opt to split warnings off into your own buffer, which is great. But
in this case, whether and how warnings are logged and/or displayed
should be mostly controlled by Gnus' existing mechanisms (window
configuration and `gnus-verbose'), and I'm fighting warnings.el on that:

- `warning-suppress-types' should belong solely to the user, but I have
  to push `(gnus)' onto it to shut off automatic display, and move
  display control to Gnus's own knobs.
- There's no :info warning level, and it's unclear if `warning-levels'
  is fair game for packages to fool with. Instructions are given, but
  pushing your own values there will mess up eg `warning-numeric-level',
  and it's hard to know if it's really okay.
- The biggest eyesore in the screenshot is the acres of "disable showing
  disable logging" buttons, which are hardcoded. Again, Gnus has its own
  knobs for these.

Lastly, the messages coming from Gnus are very much set up for regular
message display, in particular the "<message>...done" pattern, which
doesn't work with the warnings setup. If we only use delayed warnings
then each time the warnings are output it runs `delayed-warnings-hook',
which runs `collapse-delayed-warnings', which might be an okay place to
collapse the "...done" lines into one. That hook is supposed to run as
part of the `post-command-hook', though, and it doesn't always run, and
I haven't figured out why yet.

All in all, it nearly works :( Probably there would need to be some
small design changes to warnings.el to make it nice. I still think this
or something like it is worth doing.

Eri


[-- Attachment #2: screenshot.png --]
[-- Type: image/png, Size: 189248 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-WIP-on-using-warnings-in-Gnus.patch --]
[-- Type: text/x-patch, Size: 7773 bytes --]

From c56bc92f9b41df5b302606458697cd49a0f98762 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Fri, 22 Oct 2021 10:44:26 -0700
Subject: [PATCH] WIP on using warnings in Gnus

---
 lisp/gnus/gnus-start.el  |  1 +
 lisp/gnus/gnus-util.el   | 61 ++++++++++++++++++++++++++--------------
 lisp/gnus/gnus-win.el    |  1 +
 lisp/gnus/gnus.el        |  7 ++---
 lisp/gnus/mail-source.el | 21 ++++++--------
 lisp/gnus/nnmail.el      |  2 +-
 6 files changed, 55 insertions(+), 38 deletions(-)

diff --git a/lisp/gnus/gnus-start.el b/lisp/gnus/gnus-start.el
index 606bd3a39a..ffbe656c14 100644
--- a/lisp/gnus/gnus-start.el
+++ b/lisp/gnus/gnus-start.el
@@ -1603,6 +1603,7 @@ gnus-get-unread-articles
 	 (type-cache nil)
 	 (gnus-agent-article-local-times 0)
 	 (archive-method (gnus-server-to-method "archive"))
+         (warning-series t)
 	 info group active method cmethod
 	 method-type method-group-list entry)
     (gnus-message 6 "Checking new news...")
diff --git a/lisp/gnus/gnus-util.el b/lisp/gnus/gnus-util.el
index a777157f89..915ef5aee8 100644
--- a/lisp/gnus/gnus-util.el
+++ b/lisp/gnus/gnus-util.el
@@ -35,6 +35,7 @@
 (eval-when-compile (require 'cl-lib))
 
 (require 'seq)
+(require 'warnings)
 (require 'time-date)
 (require 'text-property-search)
 
@@ -440,6 +441,12 @@ gnus-add-timestamp-to-message
 		       :value t)
 		 (const :tag "No timestamp" nil)))
 
+(defcustom gnus-log-all-messages nil
+  "When non-nil, echo `gnus-message' messages to \"*Gnus Log*\"."
+  :version "29.1"
+  :group 'gnus-start
+  :type 'boolean)
+
 (eval-when-compile
   (defmacro gnus-message-with-timestamp-1 (format-string args)
     (let ((timestamp '(format-time-string "%Y%m%dT%H%M%S.%3N> " time)))
@@ -474,7 +481,18 @@ gnus-add-timestamp-to-message
 	       (t
 		(apply #'message ,format-string ,args)))))))
 
-(defvar gnus-action-message-log nil)
+(defun gnus-setup-logging ()
+  ;; We suppress the display of all Gnus warnings: they go to a
+  ;; separate buffer, and should only be displayed as part of Gnus'
+  ;; own window display routines (or explicitly with
+  ;; `gnus-display-log').
+  (cl-pushnew '(gnus) warning-suppress-types :test #'equal)
+  (cl-pushnew '(:gnus-info "%s ") warning-levels :test #'equal))
+
+(defun gnus-log-with-timestamp (_severity warning-entry)
+  (when gnus-add-timestamp-to-message
+    (insert (format-time-string "%Y%m%dT%H%M%S> " (current-time))))
+  warning-entry)
 
 (defun gnus-message-with-timestamp (format-string &rest args)
   "Display message with timestamp.  Arguments are the same as `message'.
@@ -493,33 +511,22 @@ gnus-message
       (let ((message
 	     (if gnus-add-timestamp-to-message
 		 (apply #'gnus-message-with-timestamp args)
-	       (apply #'message args))))
-	(when (and (consp gnus-action-message-log)
-		   (<= level 3))
-	  (push message gnus-action-message-log))
-	message)
+	       (apply #'message args)))
+            (warning-prefix-function #'gnus-log-with-timestamp))
+	(when gnus-log-all-messages
+          (display-warning '(gnus) message :gnus-info "*Gnus Log*"))
+        message)
     ;; We have to do this format thingy here even if the result isn't
     ;; shown - the return value has to be the same as the return value
     ;; from `message'.
     (apply #'format-message args)))
 
-(defun gnus-final-warning ()
-  (when (and (consp gnus-action-message-log)
-	     (setq gnus-action-message-log
-		   (delete nil gnus-action-message-log)))
-    (message "Warning: %s"
-	     (mapconcat #'identity gnus-action-message-log "; "))))
-
 (defun gnus-error (level &rest args)
-  "Beep an error if LEVEL is equal to or less than `gnus-verbose'.
+  "Log an error if LEVEL is equal to or less than `gnus-verbose'.
 ARGS are passed to `message'."
-  (when (<= (floor level) gnus-verbose)
-    (apply #'message args)
-    (ding)
-    (let (duration)
-      (when (and (floatp level)
-		 (not (zerop (setq duration (* 10 (- level (floor level)))))))
-	(sit-for duration))))
+  (when (<= level gnus-verbose)
+    (let ((warning-prefix-function #'gnus-log-with-timestamp))
+      (delay-warning '(gnus) (apply #'message args) :error "*Gnus Log*")))
   nil)
 
 (defun gnus-split-references (references)
@@ -1252,6 +1259,18 @@ gnus-create-info-command
     (setq gnus-info-buffer (current-buffer))
     (gnus-configure-windows 'info)))
 
+(defun gnus-display-log ()
+  "Pop up a window displaying the *Gnus Log* buffer."
+  (interactive)
+  ;; We just use plain `display-buffer' here.  If the user wants to
+  ;; compose the log buffer as part of a Gnus window configuration,
+  ;; they can do that in `gnus-buffer-configuration'.  If they want to
+  ;; control how this display works, they can configure
+  ;; `display-buffer-alist'.
+  (if (gnus-buffer-live-p "*Gnus Log*")
+      (display-buffer "*Gnus Log*")
+    (message "No log to display")))
+
 (defun gnus-not-ignore (&rest _args)
   t)
 
diff --git a/lisp/gnus/gnus-win.el b/lisp/gnus/gnus-win.el
index 8ac4e39fa5..9fafe88bcf 100644
--- a/lisp/gnus/gnus-win.el
+++ b/lisp/gnus/gnus-win.el
@@ -188,6 +188,7 @@ gnus-window-to-buffer
     (score-trace . "*Score Trace*")
     (split-trace . "*Split Trace*")
     (info . gnus-info-buffer)
+    (log . "*Gnus Log*")
     (category . gnus-category-buffer)
     (article-copy . gnus-article-copy)
     (draft . gnus-draft-buffer)
diff --git a/lisp/gnus/gnus.el b/lisp/gnus/gnus.el
index dbbbb71e57..6f420b7bd8 100644
--- a/lisp/gnus/gnus.el
+++ b/lisp/gnus/gnus.el
@@ -48,7 +48,6 @@ gnus-spam-resend-to
 (defvar gnus-ham-resend-to)
 (defvar gnus-spam-process-newsgroups)
 
-
 (defgroup gnus nil
   "The coffee-brewing, all singing, all dancing, kitchen sink newsreader."
   :group 'news
@@ -4203,9 +4202,9 @@ gnus
 	      (subr-native-elisp-p (symbol-function 'gnus)))
     (message "You should compile Gnus")
     (sit-for 2))
-  (let ((gnus-action-message-log (list nil)))
-    (gnus-1 arg dont-connect child)
-    (gnus-final-warning)))
+  (gnus-setup-logging)
+  (let ((warning-series t))
+    (gnus-1 arg dont-connect child)))
 
 (declare-function debbugs-gnu "ext:debbugs-gnu"
 		  (severities &optional packages archivedp suppress tags))
diff --git a/lisp/gnus/mail-source.el b/lisp/gnus/mail-source.el
index af0a198376..c2ec48cc86 100644
--- a/lisp/gnus/mail-source.el
+++ b/lisp/gnus/mail-source.el
@@ -554,18 +554,15 @@ mail-source-fetch
 		 (condition-case err
 		     (funcall function source callback)
 		   (error
-		    (if (and (not mail-source-ignore-errors)
-			     (not
-			      (yes-or-no-p
-			       (format "Mail source %s error (%s).  Continue? "
-				       (if (memq ':password source)
-					   (let ((s (copy-sequence source)))
-					     (setcar (cdr (memq ':password s))
-						     "********")
-					     s)
-					 source)
-				       (cadr err)))))
-		      (error "Cannot get new mail"))
+		    (unless mail-source-ignore-errors
+		      (error "Mail source %s error (%s)"
+                             (if (memq ':password source)
+                                 (let ((s (copy-sequence source)))
+                                   (setcar (cdr (memq ':password s))
+	                                   "********")
+                                   s)
+                               source)
+                             (cadr err)))
 		    0)))))))))
 
 (declare-function gnus-message "gnus-util" (level &rest args))
diff --git a/lisp/gnus/nnmail.el b/lisp/gnus/nnmail.el
index bcf01cfa9e..a9a778f458 100644
--- a/lisp/gnus/nnmail.el
+++ b/lisp/gnus/nnmail.el
@@ -1842,7 +1842,7 @@ nnmail-get-new-mail-1
 							    src)))
 			      ansym))))
 		      ((error quit)
-		       (message "Mail source %s failed: %s" source cond)
+		       (gnus-error 5 cond)
 		       0)))
 	  (cl-incf total new)
 	  (cl-incf i)))
-- 
2.33.1