From d52c929e31f60ff0462371bfe27ebd479e3e82bd Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Fri, 4 Feb 2022 19:39:53 -0500 Subject: [PATCH] (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 97a122b7bcf..918c0c7f19d 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 b2283e66e4f..2bab1319132 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 247579efb41..cfa1a5972c8 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 5a6fe451928..a40f8c64f24 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 c40c38a95f9..57ce36a9442 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 , 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 , 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 121ede42c43..126badca3e8 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 '("\\")) diff --git a/lisp/notifications.el b/lisp/notifications.el index 5ad64ff73b6..b58a1a02116 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 b8f6cb5ad36..171b7088c10 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 d3c5094b462..2fd9a9c74da 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 8e1ab7439e6..f76ebefe7b7 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 d0ae9390e31..3c9bf1ec9d2 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 ecc9386cae3..7b7c675873b 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 3ad0f0182f8..0a2ec348c1a 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 a1eb6fe3afb..0b546c0e0ba 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 4ba3c2193ee..f6f72cec4f8 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 731d1e8256f..0bf78992460 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 bd4ff3e015a..4a511f1f688 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 9c49e947810..bd2ea337b16 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 270cca1c2e7..dd12e3764ce 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." -- 2.39.5