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

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

[Michael Heerdegen]

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

Stefan Monnier via Mailing list for Emacs changes <emacs-diffs@gnu.org>
writes:

> 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.
> [...]
> 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))))

Since you explicitly allow FORMAT to be nil (standing for a default
format string), and using this is quite handy, how about documenting the
nil semantics?  E.g. like


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-with-demoted-errors-document-FORMAT-arg-nil.patch --]
[-- Type: text/x-diff, Size: 1062 bytes --]

From e83a00ddc1eede6d51cbc94862f4fa89c887509f Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Thu, 3 Mar 2022 01:47:17 +0100
Subject: [PATCH] WIP: with-demoted-errors: document FORMAT arg nil

---
 lisp/subr.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index eb9af0b36d..bb077267f8 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -4541,8 +4541,9 @@ 'condition-case-no-debug

 (defmacro with-demoted-errors (format &rest body)
   "Run BODY and demote any errors to simple messages.
-FORMAT is a string passed to `message' to format any error message.
-It should contain a single %-sequence; e.g., \"Error: %S\".
+FORMAT is a string passed to `message' to format any error message, or nil.
+If it is a string, it should contain a single %-sequence.  If it is
+nil, \"Error: %S\" is used.

 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
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]



TIA,

Michael.

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

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Thu, 03 Mar 2022 02:02:44 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
> 
> Since you explicitly allow FORMAT to be nil (standing for a default
> format string), and using this is quite handy, how about documenting the
> nil semantics?  E.g. like
> 
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -4541,8 +4541,9 @@ 'condition-case-no-debug
> 
>  (defmacro with-demoted-errors (format &rest body)
>    "Run BODY and demote any errors to simple messages.
> -FORMAT is a string passed to `message' to format any error message.
> -It should contain a single %-sequence; e.g., \"Error: %S\".
> +FORMAT is a string passed to `message' to format any error message, or nil.
> +If it is a string, it should contain a single %-sequence.  If it is
> +nil, \"Error: %S\" is used.
                      ^^^^^^^
Passive tense alert!

Thanks.  (This should also be documented in the manual and in NEWS.)