emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Fixing numerous `message' bugs..


From: David Kastrup
Subject: Re: Fixing numerous `message' bugs..
Date: Thu, 06 Dec 2007 11:56:25 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.50 (gnu/linux)

D.  Goel <address@hidden> writes:

> The emacs source code is littered with thousands of buggy calls to
> `message'.  Here is an example of such a structure:
>
>
> (message 
>       (if foo
>               nil
>               (if bar "abc"
>                       (concat "def" filename))))
>
> This example will lead to an error if the filename has %s in it. 
>
> At the same time, the simplistic fix of replacing (message by (message
> "%s" would be incorrect because if foo is true, the coder wanted
> (message nil), and not (message "%s" nil).
>
>
> The appropriate fix to this would be
>
> (let ((arg ((rest-of-the-code))))
>       (if (null arg)
>       (message nil)
>       (message "%s" arg)))

(message (unless foo (if bar "abc" "def%s")) filename)

Note that it is perfectly fine to have spurious trailing arguments to
message.

>
> ^^^ This fix will have to be repeated thousands of time as I go
> through the source code.
>
> It would rather make much more sense to code this fix into a little
> function:
>
> (defun msg (arg)
>       (if (null arg)
>       (message nil)
>       (message (format "%s" arg))))
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Uh, no.  You mean (message "%s" arg) here, and anyway:

(defun msg (arg)
  (message (and arg "%s") arg))

and then it is simple enough that it is not worth bothering introducing
a separate function.  Of course, arg should not involve a complex
calculation.

> .. this new function can be further improved to be more general, so
> that develepors can simply start preferring the new msg if they like -
>
> (defun msg (&rest args)
>      (cond
>       ((null args) (message nil))
>       ((null (car args) (message nil)))
>       ((= (length args) 1) (message "%s" (car args)))
>       (apply 'message args)))

I think that is nonsensical.  Problems occur only when a message
contains non-fixed material (which may or may not contain percentages),
but a message will pretty much never contain _exclusively_ non-fixed
material.  So a good solution will almost always involve more than a
trivial "%s" format.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum




reply via email to

[Prev in Thread] Current Thread [Next in Thread]