[Emacs-diffs] emacs-26 f930963 18/39: Simplify Flymake logging and erroring

[João Távora]

branch: emacs-26
commit f930963dd48e8c912a7623e204315b02433866cd
Author: João Távora <address@hidden>
Commit: João Távora <address@hidden>

    Simplify Flymake logging and erroring
    
    Use display-warning and a dedicated *Flymake log* buffer.
    
    To ease readability, flymake log messages are now prefixed with a
    common prefix and the buffer that originated them.
    
    Some situations of over-zealous logging are fixed.
    
    Use byte-compiler info, if available, to determine whence the
    flymake-related log message is coming.
    
    * lisp/progmodes/flymake-proc.el
    (flymake-proc--diagnostics-for-pattern): Improve log message.
    (flymake-proc--panic): Always flymake-log an error
    (flymake-proc--safe-delete-file)
    (flymake-proc--safe-delete-directory):
    Downgrade warning
    (flymake-proc-start-syntax-check): Simplify slightly.
    (flymake-proc--start-syntax-check-process): Simplify.
    (flymake-proc--init-find-buildfile-dir)
    (flymake-proc--init-create-temp-source-and-master-buffer-copy):
    No need to warn twice.
    
    * lisp/progmodes/flymake.el (flymake-log): Convert to macro.
    (flymake--log-1): New helper.
    (flymake-log-level): Deprecate.
    (flymake-error): New helper.
    (flymake-ler-make-ler, flymake--handle-report, flymake-mode):
    Use flymake-error.
    (flymake-on-timer-event)
    (flymake--handle-report, flymake--disable-backend)
    (flymake--run-backend, flymake-start, flymake-mode-on)
    (flymake-mode-off, flymake-after-change-function)
    (flymake-after-save-hook, flymake-find-file-hook): Adjust
    flymake-log calls.
    
    * test/lisp/progmodes/flymake-tests.el
    (flymake-tests--call-with-fixture): Only log errors.
---
 lisp/progmodes/flymake-proc.el       | 29 +++++------
 lisp/progmodes/flymake.el            | 96 +++++++++++++++++++++++-------------
 test/lisp/progmodes/flymake-tests.el |  3 +-
 3 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 55f0095..1028d9a 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -441,7 +441,7 @@ Create parent directories as needed."
                       (guess-type flymake-proc-diagnostic-type-pred message)
                       message)))
          else
-         do (flymake-log 2 "No buffer found for diagnosed file %s" fname))
+         do (flymake-log 2 "Reference to file %s is out of scope" fname))
       (error
        (flymake-log 1 "Error parsing process output for pattern %s: %s"
                     pattern err)
@@ -532,7 +532,7 @@ May only be called in a dynamic environment where
            flymake-proc--report-fn)
       (funcall flymake-proc--report-fn :panic
                :explanation (format "%s: %s" problem explanation))
-    (error "Trouble telling flymake-ui about problem %s(%s)"
+    (flymake-error "Trouble telling flymake-ui about problem %s(%s)"
                    problem explanation)))
 
 (defun flymake-proc-reformat-err-line-patterns-from-compile-el (original-list)
@@ -676,13 +676,13 @@ expression. A match indicates `:warning' type, otherwise
 (defun flymake-proc--safe-delete-file (file-name)
   (when (and file-name (file-exists-p file-name))
     (delete-file file-name)
-    (flymake-log 1 "deleted file %s" file-name)))
+    (flymake-log 2 "deleted file %s" file-name)))
 
 (defun flymake-proc--safe-delete-directory (dir-name)
   (condition-case nil
       (progn
        (delete-directory dir-name)
-       (flymake-log 1 "deleted dir %s" dir-name))
+       (flymake-log 2 "deleted dir %s" dir-name))
     (error
      (flymake-log 1 "Failed to delete dir %s, error ignored" dir-name))))
 
@@ -758,15 +758,11 @@ expression. A match indicates `:warning' type, otherwise
                      default-directory)
         process)
     (error
-     (let* ((err-str
-             (format-message
-              "Failed to launch syntax check process `%s' with args %s: %s"
-              cmd args (error-message-string err)))
-            (source-file-name buffer-file-name)
-            (cleanup-f        (flymake-proc--get-cleanup-function 
source-file-name)))
-       (flymake-log 0 err-str)
-       (funcall cleanup-f)
-       (flymake-proc--panic :make-process-error err-str)))))
+     (flymake-proc--panic :make-process-error
+                          (format-message
+                           "Failed to launch syntax check process `%s' with 
args %s: %s"
+                           cmd args (error-message-string err)))
+     (funcall (flymake-proc--get-cleanup-function buffer-file-name)))))
 
 (defun flymake-proc-stop-all-syntax-checks (&optional reason)
   "Kill all syntax check processes."
@@ -917,7 +913,6 @@ Return full-name.  Names are real, not patched."
                                         (file-name-directory 
source-file-name))))
     (if buildfile-dir
         (setq flymake-proc--base-dir buildfile-dir)
-      (flymake-log 1 "no buildfile (%s) for %s" buildfile-name 
source-file-name)
       (flymake-proc--panic
        "NOMK" (format "No buildfile (%s) found for %s"
                       buildfile-name source-file-name)))))
@@ -933,8 +928,10 @@ Return full-name.  Names are real, not patched."
 
     (if (not master-and-temp-master)
        (progn
-         (flymake-log 1 "cannot find master file for %s" source-file-name)
-          (flymake-proc--panic "NOMASTER" "")  ; NOMASTER
+          (flymake-proc--panic
+           "NOMASTER"
+           (format-message "cannot find master file for %s"
+                           source-file-name))
           nil)
       (setq flymake-proc--master-file-name (nth 0 master-and-temp-master))
       (setq flymake-proc--temp-master-file-name (nth 1 
master-and-temp-master)))))
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index a3cec8d..242c83c 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -34,7 +34,7 @@
 
 (require 'cl-lib)
 (require 'thingatpt) ; end-of-thing
-(require 'warnings) ; warning-numeric-level
+(require 'warnings) ; warning-numeric-level, display-warning
 (eval-when-compile (require 'subr-x)) ; when-let*, if-let*
 
 (defgroup flymake nil
@@ -106,10 +106,11 @@ See `flymake-error-bitmap' and `flymake-warning-bitmap'."
   :type 'boolean)
 
 (defcustom flymake-log-level -1
-  "Logging level, only messages with level lower or equal will be logged.
--1 = NONE, 0 = ERROR, 1 = WARNING, 2 = INFO, 3 = DEBUG"
-  :group 'flymake
+  "Obsolete and ignored variable."
   :type 'integer)
+(make-obsolete-variable 'flymake-log-level
+                       "it is superseded by `warning-minimum-log-level.'"
+                        "26.1")
 
 (defvar-local flymake-timer nil
   "Timer for starting syntax check.")
@@ -120,15 +121,45 @@ See `flymake-error-bitmap' and `flymake-warning-bitmap'."
 (defvar-local flymake-check-start-time nil
   "Time at which syntax check was started.")
 
-(defun flymake-log (level text &rest args)
-  "Log a message at level LEVEL.
-If LEVEL is higher than `flymake-log-level', the message is
-ignored.  Otherwise, it is printed using `message'.
-TEXT is a format control string, and the remaining arguments ARGS
-are the string substitutions (see the function `format')."
-  (if (<= level flymake-log-level)
-      (let* ((msg (apply #'format-message text args)))
-       (message "%s" msg))))
+(defun flymake--log-1 (level sublog msg &rest args)
+  "Do actual work for `flymake-log'."
+  (let (;; never popup the log buffer
+        (warning-minimum-level :emergency)
+        (warning-type-format
+         (format " [%s %s]"
+                 (or sublog 'flymake)
+                 (current-buffer))))
+    (display-warning (list 'flymake sublog)
+                     (apply #'format-message msg args)
+                     (if (numberp level)
+                         (or (nth level
+                                  '(:emergency :error :warning :debug :debug) )
+                             :error)
+                       level)
+                     "*Flymake log*")))
+
+;;;###autoload
+(defmacro flymake-log (level msg &rest args)
+  "Log, at level LEVEL, the message MSG formatted with ARGS.
+LEVEL is passed to `display-warning', which is used to display
+the warning.  If this form is included in a byte-compiled file,
+the generated warning contains an indication of the file that
+generated it."
+  (let* ((compile-file (and (boundp 'byte-compile-current-file)
+                            (symbol-value 'byte-compile-current-file)))
+         (sublog (if (and
+                      compile-file
+                      (not load-file-name))
+                     (intern
+                      (file-name-nondirectory
+                       (file-name-sans-extension compile-file))))))
+    `(flymake--log-1 ,level ',sublog ,msg ,@args)))
+
+(defun flymake-error (text &rest args)
+  "Format TEXT with ARGS and signal an error for flymake."
+  (let ((msg (apply #'format-message text args)))
+    (flymake-log :error msg)
+    (error (concat "[Flymake] " msg))))
 
 (cl-defstruct (flymake--diag
                (:constructor flymake--diag-make))
@@ -147,7 +178,7 @@ description of the problem detected in this region."
 (defun flymake-ler-make-ler (file line type text &optional full-file)
   (let* ((file (or full-file file))
          (buf (find-buffer-visiting file)))
-    (unless buf (error "No buffer visiting %s" file))
+    (unless buf (flymake-error "No buffer visiting %s" file))
     (pcase-let* ((`(,beg . ,end)
                   (with-current-buffer buf
                     (flymake-diag-region line nil))))
@@ -241,8 +272,7 @@ Or nil if the region is invalid."
               (let* ((beg (fallback-bol))
                      (end (fallback-eol beg)))
                 (cons beg end))))))
-    (error (flymake-log 4 "Invalid region for diagnostic %s")
-           nil)))
+    (error (flymake-error "Invalid region line=%s col=%s" line col))))
 
 (defvar flymake-diagnostic-functions nil
   "List of flymake backends i.e. sources of flymake diagnostics.
@@ -427,7 +457,7 @@ If TYPE doesn't declare PROP in either
                     flymake-no-changes-timeout))
 
        (setq flymake-last-change-time nil)
-       (flymake-log 3 "starting syntax check as more than 1 second passed 
since last change")
+       (flymake-log :debug "starting syntax check after no changes for some 
time")
        (flymake--start-syntax-check)))))
 
 (define-obsolete-function-alias 'flymake-display-err-menu-for-current-line
@@ -456,7 +486,7 @@ If TYPE doesn't declare PROP in either
                         (cl-count-if #'flymake--diag-errorp diagnostics)
                         (cl-count-if-not #'flymake--diag-errorp diagnostics)))
          (choice (x-popup-menu event (list title (cons "" menu)))))
-    (flymake-log 3 "choice=%s" choice)
+    (flymake-log :debug "choice=%s" choice)
     ;; FIXME: What is the point of going to the problem locus if we're
     ;; certainly already there?
     ;;
@@ -492,14 +522,14 @@ A backend is disabled if it reported `:panic'.")
 
 (defun flymake--disable-backend (backend action &optional explanation)
   (cl-pushnew backend flymake--disabled-backends)
-  (flymake-log 0 "Disabled the backend %s due to reports of %s (%s)"
+  (flymake-log :warning "Disabled the backend %s due to reports of %s (%s)"
                backend action explanation))
 
 (cl-defun flymake--handle-report (backend action &key explanation)
   "Handle reports from flymake backend identified by BACKEND."
   (cond
    ((not (memq backend flymake--running-backends))
-    (error "Ignoring unexpected report from backend %s" backend))
+    (flymake-error "Ignoring unexpected report from backend %s" backend))
    ((eq action :progress)
     (flymake-log 3 "Backend %s reports progress: %s" backend explanation))
    ((eq :panic action)
@@ -573,10 +603,10 @@ non-nil."
         (setq flymake-check-start-time (float-time))
         (dolist (backend flymake-diagnostic-functions)
           (cond ((memq backend flymake--running-backends)
-                 (flymake-log 2 "Backend %s still running, not restarting"
+                 (flymake-log :debug "Backend %s still running, not restarting"
                               backend))
                 ((memq backend flymake--disabled-backends)
-                 (flymake-log 2 "Backend %s is disabled, not starting"
+                 (flymake-log :debug "Backend %s is disabled, not starting"
                               backend))
                 (t
                  (flymake--run-backend backend))))))
@@ -595,7 +625,7 @@ non-nil."
    (flymake-mode
     (cond
      ((not flymake-diagnostic-functions)
-      (error "flymake cannot check syntax in buffer %s" (buffer-name)))
+      (flymake-error "No backends to check buffer %s" (buffer-name)))
      (t
       (add-hook 'after-change-functions 'flymake-after-change-function nil t)
       (add-hook 'after-save-hook 'flymake-after-save-hook nil t)
@@ -625,29 +655,25 @@ non-nil."
 ;;;###autoload
 (defun flymake-mode-on ()
   "Turn flymake mode on."
-  (flymake-mode 1)
-  (flymake-log 1 "flymake mode turned ON for buffer %s" (buffer-name)))
+  (flymake-mode 1))
 
 ;;;###autoload
 (defun flymake-mode-off ()
   "Turn flymake mode off."
-  (flymake-mode 0)
-  (flymake-log 1 "flymake mode turned OFF for buffer %s" (buffer-name)))
+  (flymake-mode 0))
 
 (defun flymake-after-change-function (start stop _len)
   "Start syntax check for current buffer if it isn't already running."
-  ;;+(flymake-log 0 "setting change time to %s" (float-time))
   (let((new-text (buffer-substring start stop)))
     (when (and flymake-start-syntax-check-on-newline (equal new-text "\n"))
-      (flymake-log 3 "starting syntax check as new-line has been seen")
+      (flymake-log :debug "starting syntax check as new-line has been seen")
       (flymake--start-syntax-check 'deferred))
     (setq flymake-last-change-time (float-time))))
 
 (defun flymake-after-save-hook ()
-  (if (local-variable-p 'flymake-mode (current-buffer))        ; (???) other 
way to determine whether flymake is active in buffer being saved?
-      (progn
-       (flymake-log 3 "starting syntax check as buffer was saved")
-       (flymake--start-syntax-check)))) ; no more mode 3. cannot start check 
if mode 3 (to temp copies) is active - (???)
+  (when flymake-mode
+    (flymake-log :debug "starting syntax check as buffer was saved")
+    (flymake--start-syntax-check))) ; no more mode 3. cannot start check if 
mode 3 (to temp copies) is active - (???)
 
 (defun flymake-kill-buffer-hook ()
   (when flymake-timer
@@ -657,9 +683,9 @@ non-nil."
 ;;;###autoload
 (defun flymake-find-file-hook ()
   (unless (or flymake-mode
-             (null flymake-diagnostic-functions))
+              (null flymake-diagnostic-functions))
     (flymake-mode)
-    (flymake-log 3 "automatically turned ON flymake mode")))
+    (flymake-log :warning "Turned on in `flymake-find-file-hook'")))
 
 (defun flymake-goto-next-error (&optional n interactive)
   "Go to next, or Nth next, flymake error in buffer."
diff --git a/test/lisp/progmodes/flymake-tests.el 
b/test/lisp/progmodes/flymake-tests.el
index c2deb1d..921c2f6 100644
--- a/test/lisp/progmodes/flymake-tests.el
+++ b/test/lisp/progmodes/flymake-tests.el
@@ -46,7 +46,8 @@ SEVERITY-PREDICATE is used to setup
          (visiting (find-buffer-visiting file))
          (buffer (or visiting (find-file-noselect file)))
          (process-environment (cons "LC_ALL=C" process-environment))
-         (i 0))
+         (i 0)
+         (warning-minimum-log-level :error))
     (unwind-protect
         (with-current-buffer buffer
           (save-excursion