master d52c929e31: (with-demoted-errors): Warn on missing `format` arg

[Stefan Monnier]

branch: master
commit d52c929e31f60ff0462371bfe27ebd479e3e82bd
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    (with-demoted-errors): Warn on missing `format` arg
    
    The `format` arg has been mandatory for a while, but the backward
    compatibility code that handled the case of a missing `format` arg
    made it hard to notice when using the old calling convention.
    
    * lisp/subr.el (with-demoted-errors): Warn on missing `format` arg.
    
    * lisp/emacs-lisp/smie.el (smie-indent--separator-outdent): Don't abuse
    `with-demoted-errors`.
    (smie-indent-line, smie-auto-fill):
    * test/lisp/emacs-lisp/ert-tests.el (ert-test-with-demoted-errors):
    * lisp/vc/vc-hooks.el (vc-refresh-state):
    * lisp/vc/vc-annotate.el (vc-annotate-background-mode):
    * lisp/vc/diff-mode.el (diff-syntax-fontify-hunk):
    * lisp/textmodes/reftex-toc.el (reftex-re-enlarge):
    * lisp/progmodes/sh-script.el (sh-smie-sh-rules):
    * lisp/progmodes/octave.el (inferior-octave-startup):
    * lisp/pcmpl-gnu.el (pcmpl-gnu-make-all-targets):
    * lisp/org/org-refile.el (org-refile):
    * lisp/org/org-capture.el (org-capture-store-last-position):
    * lisp/nxml/nxml-mode.el (nxml-mode):
    * lisp/notifications.el (notifications-notify):
    * lisp/gnus/mm-view.el (mm-display-inline-fontify):
    * lisp/finder.el (finder-unload-function):
    * lisp/files.el (safe-local-variable-p, backup-buffer-copy
    * lisp/autorevert.el (auto-revert-notify-handler):
    Pass `format` arg to `with-demoted-errors`.
---
 lisp/autorevert.el                |  2 +-
 lisp/emacs-lisp/smie.el           | 10 +++++-----
 lisp/files.el                     | 12 +++++++-----
 lisp/finder.el                    |  3 ++-
 lisp/gnus/mm-view.el              | 22 +++++++++++-----------
 lisp/net/tramp.el                 |  1 +
 lisp/notifications.el             |  2 +-
 lisp/nxml/nxml-mode.el            |  3 ++-
 lisp/org/org-capture.el           |  3 ++-
 lisp/org/org-refile.el            |  8 ++++----
 lisp/pcmpl-gnu.el                 |  2 +-
 lisp/progmodes/octave.el          |  3 ++-
 lisp/progmodes/sh-script.el       |  2 +-
 lisp/subr.el                      | 26 ++++++++++++++------------
 lisp/textmodes/reftex-toc.el      |  2 +-
 lisp/vc/diff-mode.el              |  3 ++-
 lisp/vc/vc-annotate.el            |  2 +-
 lisp/vc/vc-hooks.el               |  7 ++++---
 test/lisp/emacs-lisp/ert-tests.el |  2 +-
 19 files changed, 63 insertions(+), 52 deletions(-)

diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index 97a122b7bc..918c0c7f19 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -692,7 +692,7 @@ system.")
 
 (defun auto-revert-notify-handler (event)
   "Handle an EVENT returned from file notification."
-  (with-demoted-errors
+  (with-demoted-errors "Error while auto-reverting: %S"
     (let* ((descriptor (car event))
           (action (nth 1 event))
           (file (nth 2 event))
diff --git a/lisp/emacs-lisp/smie.el b/lisp/emacs-lisp/smie.el
index b2283e66e4..2bab131913 100644
--- a/lisp/emacs-lisp/smie.el
+++ b/lisp/emacs-lisp/smie.el
@@ -1301,9 +1301,9 @@ Only meaningful when called from within 
`smie-rules-function'."
   (let ((afterpos (save-excursion
                     (let ((tok (funcall smie-forward-token-function)))
                       (unless tok
-                        (with-demoted-errors
-                          (error "smie-rule-separator: Can't skip token %s"
-                                 smie--token))))
+                        (funcall (if debug-on-error #'error #'message)
+                                 "smie-rule-separator: Can't skip token %s"
+                                 smie--token)))
                     (skip-chars-forward " ")
                     (unless (eolp) (point)))))
     (or (and afterpos
@@ -1820,7 +1820,7 @@ to which that point should be aligned, if we were to 
reindent it.")
   "Indent current line using the SMIE indentation engine."
   (interactive)
   (let* ((savep (point))
-        (indent (or (with-demoted-errors
+        (indent (or (with-demoted-errors "SMIE Error: %S"
                        (save-excursion
                          (forward-line 0)
                          (skip-chars-forward " \t")
@@ -1846,7 +1846,7 @@ to which that point should be aligned, if we were to 
reindent it.")
                            (move-to-column fc)
                            (syntax-ppss))))
         (while
-            (and (with-demoted-errors
+            (and (with-demoted-errors "SMIE Error: %S"
                    (save-excursion
                      (let ((end (point))
                            (bsf nil)    ;Best-so-far.
diff --git a/lisp/files.el b/lisp/files.el
index 247579efb4..cfa1a5972c 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4061,7 +4061,8 @@ It is safe if any of these conditions are met:
         (and (functionp safep)
              ;; If the function signals an error, that means it
              ;; can't assure us that the value is safe.
-             (with-demoted-errors (funcall safep val))))))
+             (with-demoted-errors "Local variable error: %S"
+               (funcall safep val))))))
 
 (defun risky-local-variable-p (sym &optional _ignored)
   "Non-nil if SYM could be dangerous as a file-local variable.
@@ -4937,7 +4938,7 @@ BACKUPNAME is the backup file name, which is the old file 
renamed."
        nil)))
   ;; If set-file-extended-attributes fails, fall back on set-file-modes.
   (unless (and extended-attributes
-              (with-demoted-errors
+              (with-demoted-errors "Error setting attributes: %S"
                 (set-file-extended-attributes to-name extended-attributes)))
     (and modes
         (set-file-modes to-name (logand modes #o1777) nofollow-flag)))))
@@ -5558,7 +5559,8 @@ Before and after saving the buffer, this function runs
                     (goto-char (point-max))
                     (insert ?\n))))
            ;; Don't let errors prevent saving the buffer.
-           (with-demoted-errors (run-hooks 'before-save-hook))
+           (with-demoted-errors "Before-save hook error: %S"
+             (run-hooks 'before-save-hook))
             ;; Give `write-contents-functions' a chance to
             ;; short-circuit the whole process.
            (unless (run-hook-with-args-until-success 'write-contents-functions)
@@ -5606,7 +5608,7 @@ Before and after saving the buffer, this function runs
                  (condition-case ()
                      (progn
                        (unless
-                           (with-demoted-errors
+                           (with-demoted-errors "Error setting file modes: %S"
                                (set-file-modes buffer-file-name (car 
setmodes)))
                          (set-file-extended-attributes buffer-file-name
                                                        (nth 1 setmodes))))
@@ -5721,7 +5723,7 @@ Before and after saving the buffer, this function runs
               ;; If set-file-extended-attributes fails, fall back on
               ;; set-file-modes.
               (unless
-                  (with-demoted-errors
+                  (with-demoted-errors "Error setting attributes: %s"
                     (set-file-extended-attributes buffer-file-name
                                                   (nth 1 setmodes)))
                 (set-file-modes buffer-file-name
diff --git a/lisp/finder.el b/lisp/finder.el
index 5a6fe45192..a40f8c64f2 100644
--- a/lisp/finder.el
+++ b/lisp/finder.el
@@ -454,7 +454,8 @@ Quit the window and kill all Finder-related buffers."
 
 (defun finder-unload-function ()
   "Unload the Finder library."
-  (with-demoted-errors (unload-feature 'finder-inf t))
+  (with-demoted-errors "Error unloading finder: %S"
+    (unload-feature 'finder-inf t))
   ;; continue standard unloading
   nil)
 
diff --git a/lisp/gnus/mm-view.el b/lisp/gnus/mm-view.el
index c40c38a95f..57ce36a944 100644
--- a/lisp/gnus/mm-view.el
+++ b/lisp/gnus/mm-view.el
@@ -519,17 +519,17 @@ If MODE is not set, try to find mode automatically."
         ;; setting now, but it seems harmless and potentially still useful.
        (setq-local font-lock-mode-hook nil)
         (setq buffer-file-name (mm-handle-filename handle))
-       (with-demoted-errors
-           (if mode
-                (save-window-excursion
-                  ;; According to Katsumi Yamaoka <yamaoka@jpl.org>, org-mode
-                  ;; requires the buffer to be temporarily displayed here, but
-                  ;; I could not reproduce this problem.  Furthermore, if
-                  ;; there's such a problem, we should fix org-mode rather than
-                  ;; use switch-to-buffer which can have undesirable
-                  ;; side-effects!
-                  ;;(switch-to-buffer (current-buffer))
-                 (funcall mode))
+       (with-demoted-errors "Error setting mode: %S"
+         (if mode
+              (save-window-excursion
+                ;; According to Katsumi Yamaoka <yamaoka@jpl.org>, org-mode
+                ;; requires the buffer to be temporarily displayed here, but
+                ;; I could not reproduce this problem.  Furthermore, if
+                ;; there's such a problem, we should fix org-mode rather than
+                ;; use switch-to-buffer which can have undesirable
+                ;; side-effects!
+                ;;(switch-to-buffer (current-buffer))
+               (funcall mode))
            (let ((auto-mode-alist
                   (delq (rassq 'doc-view-mode-maybe auto-mode-alist)
                         (copy-sequence auto-mode-alist))))
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 121ede42c4..126badca3e 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5023,6 +5023,7 @@ Mostly useful to protect BODY from being interrupted by 
timers."
           ,@body)
        (tramp-flush-connection-property ,proc "locked"))))
 
+;; FIXME: This call is redundant in current Emacsen.
 (font-lock-add-keywords
  'emacs-lisp-mode '("\\<with-tramp-locked-connection\\>"))
 
diff --git a/lisp/notifications.el b/lisp/notifications.el
index 5ad64ff73b..b58a1a0211 100644
--- a/lisp/notifications.el
+++ b/lisp/notifications.el
@@ -202,7 +202,7 @@ This function returns a notification id, an integer, which 
can be
 used to manipulate the notification item with
 `notifications-close-notification' or the `:replaces-id' argument
 of another `notifications-notify' call."
-  (with-demoted-errors
+  (with-demoted-errors "Notification error: %S"
     (let ((bus (or (plist-get params :bus) :session))
          (title (plist-get params :title))
          (body (plist-get params :body))
diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index b8f6cb5ad3..171b7088c1 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -566,7 +566,8 @@ Many aspects this mode can be customized using
           (font-lock-syntactic-face-function
            . sgml-font-lock-syntactic-face)))
 
-  (with-demoted-errors (rng-nxml-mode-init)))
+  (with-demoted-errors "RNG NXML error: %S"
+    (rng-nxml-mode-init)))
 
 (defun nxml--buffer-substring-filter (string)
   ;; The `rng-state' property is huge, so don't copy it to the kill ring.
diff --git a/lisp/org/org-capture.el b/lisp/org/org-capture.el
index d3c5094b46..2fd9a9c74d 100644
--- a/lisp/org/org-capture.el
+++ b/lisp/org/org-capture.el
@@ -1453,7 +1453,8 @@ Of course, if exact position has been required, just put 
it there."
       (org-with-point-at pos
        (when org-capture-bookmark
          (let ((bookmark (plist-get org-bookmark-names-plist :last-capture)))
-           (when bookmark (with-demoted-errors (bookmark-set bookmark)))))
+           (when bookmark (with-demoted-errors "Bookmark set error: %S"
+                            (bookmark-set bookmark)))))
        (move-marker org-capture-last-stored-marker (point))))))
 
 (defun org-capture-narrow (beg end)
diff --git a/lisp/org/org-refile.el b/lisp/org/org-refile.el
index 8e1ab7439e..f76ebefe7b 100644
--- a/lisp/org/org-refile.el
+++ b/lisp/org/org-refile.el
@@ -566,16 +566,16 @@ prefix argument (`C-u C-u C-u C-c C-w')."
               (let ((bookmark-name (plist-get org-bookmark-names-plist
                                               :last-refile)))
                 (when bookmark-name
-                  (with-demoted-errors
-                      (bookmark-set bookmark-name))))
+                  (with-demoted-errors "Bookmark set error: %S"
+                    (bookmark-set bookmark-name))))
               ;; If we are refiling for capture, make sure that the
               ;; last-capture pointers point here
               (when (bound-and-true-p org-capture-is-refiling)
                 (let ((bookmark-name (plist-get org-bookmark-names-plist
                                                 :last-capture-marker)))
                   (when bookmark-name
-                    (with-demoted-errors
-                        (bookmark-set bookmark-name))))
+                    (with-demoted-errors "Bookmark set error: %S"
+                      (bookmark-set bookmark-name))))
                 (move-marker org-capture-last-stored-marker (point)))
               (when (fboundp 'deactivate-mark) (deactivate-mark))
               (run-hooks 'org-after-refile-insert-hook)))
diff --git a/lisp/pcmpl-gnu.el b/lisp/pcmpl-gnu.el
index d0ae9390e3..3c9bf1ec9d 100644
--- a/lisp/pcmpl-gnu.el
+++ b/lisp/pcmpl-gnu.el
@@ -134,7 +134,7 @@ Return the new list."
   "Add to TARGETS the list of target names in MAKEFILE and files it includes.
 Return the new list."
   (with-temp-buffer
-    (with-demoted-errors                       ;Could be a directory or 
something.
+    (with-demoted-errors "Error inserting makefile: %S"
         (insert-file-contents makefile))
 
     (let ((filenames (when pcmpl-gnu-makefile-includes 
(pcmpl-gnu-make-includes))))
diff --git a/lisp/progmodes/octave.el b/lisp/progmodes/octave.el
index ecc9386cae..7b7c675873 100644
--- a/lisp/progmodes/octave.el
+++ b/lisp/progmodes/octave.el
@@ -879,7 +879,8 @@ startup file, `~/.emacs-octave'."
     (set-process-filter proc 'comint-output-filter)
     ;; Just in case, to be sure a cd in the startup file won't have
     ;; detrimental effects.
-    (with-demoted-errors (inferior-octave-resync-dirs))
+    (with-demoted-errors "Octave resync error: %S"
+      (inferior-octave-resync-dirs))
     ;; Generate a proper prompt, which is critical to
     ;; `comint-history-isearch-backward-regexp'.  Bug#14433.
     (comint-send-string proc "\n")))
diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 3ad0f0182f..0a2ec348c1 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1973,7 +1973,7 @@ May return nil if the line should not be treated as 
continued."
          (cons 'column (smie-indent-keyword ";"))
        (smie-rule-separator kind)))
     (`(:after . ,(or ";;" ";&" ";;&"))
-     (with-demoted-errors
+     (with-demoted-errors "SMIE rule error: %S"
        (smie-backward-sexp token)
        (cons 'column
              (if (or (smie-rule-bolp)
diff --git a/lisp/subr.el b/lisp/subr.el
index a1eb6fe3af..0b546c0e0b 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4531,19 +4531,21 @@ It should contain a single %-sequence; e.g., \"Error: 
%S\".
 
 If `debug-on-error' is non-nil, run BODY without catching its errors.
 This is to be used around code that is not expected to signal an error
-but that should be robust in the unexpected case that an error is signaled.
-
-For backward compatibility, if FORMAT is not a constant string, it
-is assumed to be part of BODY, in which case the message format
-used is \"Error: %S\"."
+but that should be robust in the unexpected case that an error is signaled."
   (declare (debug t) (indent 1))
-  (let ((err (make-symbol "err"))
-        (format (if (and (stringp format) body) format
-                  (prog1 "Error: %S"
-                    (if format (push format body))))))
-    `(condition-case-unless-debug ,err
-         ,(macroexp-progn body)
-       (error (message ,format ,err) nil))))
+  (let* ((err (make-symbol "err"))
+         (orig-body body)
+         (format (if (and (stringp format) body) format
+                   (prog1 "Error: %S"
+                     (if format (push format body)))))
+         (exp
+          `(condition-case-unless-debug ,err
+               ,(macroexp-progn body)
+             (error (message ,format ,err) nil))))
+    (if (eq orig-body body) exp
+      ;; The use without `format' is obsolete, let's warn when we bump
+      ;; into any such remaining uses.
+      (macroexp-warn-and-return format "Missing format argument" exp))))
 
 (defmacro combine-after-change-calls (&rest body)
   "Execute BODY, but don't call the after-change functions till the end.
diff --git a/lisp/textmodes/reftex-toc.el b/lisp/textmodes/reftex-toc.el
index 4ba3c2193e..f6f72cec4f 100644
--- a/lisp/textmodes/reftex-toc.el
+++ b/lisp/textmodes/reftex-toc.el
@@ -381,7 +381,7 @@ SPC=view TAB=goto RET=goto+hide [q]uit [r]escan [l]abels 
[f]ollow [x]r [?]Help
                 (- (or reftex-last-window-height (window-height))
                    (window-height)))))
     (when (> count 0)
-      (with-demoted-errors           ;E.g. the window might be the root window!
+      (with-demoted-errors "Enlarge window error: %S"
         (enlarge-window count reftex-toc-split-windows-horizontally)))))
 
 (defun reftex-toc-dframe-p (&optional frame error)
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 731d1e8256..0bf7899246 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2678,7 +2678,8 @@ When OLD is non-nil, highlight the hunk from the old 
source."
          ;; Trim a trailing newline to find hunk in diff-syntax-fontify-props
          ;; in diffs that have no newline at end of diff file.
          (text (string-trim-right
-                (or (with-demoted-errors (diff-hunk-text hunk (not old) nil))
+                (or (with-demoted-errors "Error getting hunk text: %S"
+                      (diff-hunk-text hunk (not old) nil))
                     "")))
         (line (if (looking-at "\\(?:\\*\\{15\\}.*\n\\)?[-@* 
]*\\([0-9,]+\\)\\([ acd+]+\\([0-9,]+\\)\\)?")
                   (if old (match-string 1)
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index bd4ff3e015..4a511f1f68 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -57,7 +57,7 @@ is applied to the background."
   :set (lambda (symbol value)
         (set-default symbol value)
         (when (boundp 'vc-annotate-color-map)
-          (with-demoted-errors
+          (with-demoted-errors "VC color map error: %S"
             ;; Update the value of the dependent variable.
             (custom-reevaluate-setting 'vc-annotate-color-map))))
   :version "25.1"
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 9c49e94781..bd2ea337b1 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -799,9 +799,10 @@ In the latter case, VC mode is deactivated for this 
buffer."
     (add-hook 'vc-mode-line-hook #'vc-mode-line nil t)
     (let (backend)
       (cond
-        ((setq backend (with-demoted-errors (vc-backend buffer-file-name)))
-         ;; Let the backend setup any buffer-local things he needs.
-         (vc-call-backend backend 'find-file-hook)
+       ((setq backend (with-demoted-errors "VC refresh error: %S"
+                        (vc-backend buffer-file-name)))
+        ;; Let the backend setup any buffer-local things he needs.
+        (vc-call-backend backend 'find-file-hook)
        ;; Compute the state and put it in the mode line.
        (vc-mode-line buffer-file-name backend)
        (unless vc-make-backup-files
diff --git a/test/lisp/emacs-lisp/ert-tests.el 
b/test/lisp/emacs-lisp/ert-tests.el
index 270cca1c2e..dd12e3764c 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -865,7 +865,7 @@ This macro is used to test if macroexpansion in `should' 
works."
 (ert-deftest ert-test-with-demoted-errors ()
   "Check that ERT correctly handles `with-demoted-errors'."
   :expected-result :failed  ;; FIXME!  Bug#11218
-  (should-not (with-demoted-errors (error "Foo"))))
+  (should-not (with-demoted-errors "FOO: %S" (error "Foo"))))
 
 (ert-deftest ert-test-fail-inside-should ()
   "Check that `ert-fail' inside `should' works correctly."