Re: Fixing numerous `message' bugs..

[Dave Goel]

David Kastrup <address@hidden> writes:

> D.  Goel <address@hidden> writes:
>>
>> (let ((arg ((rest-of-the-code))))
>>      (if (null arg)
>>      (message nil)
>>      (message "%s" arg)))
>
> (message (unless foo (if bar "abc" "def%s")) filename)

This alternative fix relies on going deep inside developer's
"rest-of-the-code".  This is a wrong solution.  What will you do if
that rest-of-the-code relies on strings constructed in other
functions?  (That is just a simple example. Emacs is full of very
creative codes, constructed in macros and many other beautiful things,
and strings read from assoc lists...) Change the code in those other
functions?  What if the other functions are used elsewhere? To be
safe, I should perhaps duplicate the 10000-char assoc-list into an
assoc-list-called-by-message and make your fix there?  Rather than
duplicate many such solutions and add thousands of lines of code,
isn't it easier to add one small function `msg'?  Even otherwise, the
(let ((arg "rest-of-the-code"..)))  solution itself would add a lot of
extra verbiage, much more so than a simple function `msg'.

This is also an impractical solution, 
(a) My semi-automated code-fixers cannot handle this.
(b) This involves a lot of studying of code.

I mentioned these bugs in calls to `message' and `error' were
identified probably more than a year ago; I have fixed a lot of these
bugs, but something like 90% of these bugs remain unfixed... the
problem was so bad that edebug itself had these bugs, so that
edebugging itself broke mysteriously at times.  If I can introduce
(msg), I can fix all these bugs right now.



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

sure.

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

nice.

> and then it is simple enough that it is not worth bothering introducing
> a separate function.  

No, it isn't.  You identify the problem below yourself.

> Of course, arg should not involve a complex calculation.

And, of course, they do.  They also involve calls to other functions,
etc.

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

Yeah, which is the case a lot of those times. And, that is the
problem. 

> (which may or may not contain percentages), but a message will
> pretty much never contain _exclusively_ non-fixed material.  

huh? 

> So a good solution will almost always involve more than a trivial
> "%s" format.

If you really want something more than %s, say, %% [1], no one stops you
from still using message correctly, as follows:

(message "%%")  

or even
(msg (format "%%"))

Most of the time a developer calls (message), he calls it with one
arg, and he already pre-formats his arg.  Almost always when the
developer calls (message) with one arg, he is actually looking for
(msg)[2].   And, if he isn't, he is still welcome to use (message) the
correct way. 


[1] though I assure you, each of the thousand times I have seen a
(message) bug, the developer has always meant that "trivial" %s.

[2] Once again, in the thousands of cases I have examined, nowhere
have I seen (message) called with just one non-nil argument, where
(message "%s" arg) fails to do what the developer wanted.

---

Please note that I am *not* saying I want to replace every call to
(message) in the source code with (msg).  *Only* the buggy calls; and
only when I have verified carefully that (msg) works correctly for the
code at hand.