(message (format "File %s" file))

(message (format "File %s" file))

[D Goel]

A *lot* of source code contains easy-to-miss bugs like this: 

	(message (format "File: %s" file))

which will lead to erers like these: 

Debugger entered--Lisp error: (error "Not enough arguments for format string")

if the filename contains %.


I plan to go through the entire source code, and only in cases I am
sure, fix these bugs (which mostly involves adding a "%s% after
`message', or getting rid of the unnecessary format call following
(message).

(I will be spotting these bugs with the aid of a regexp search tool).

I have committ access, but this is the first time I am planning to
make any real changes (which happen te be large-scale too), so I
thought I should ask for permission as well as any suggestions before
doing so.  For each fix, I would add a proper changlog entry saying
something like:  message format spec fix.



Here are a few examples:

iswitchb:
      (message (format "no buffer matching `%s'" buf)))))


printing.el:
     (message (error-message-string data)))))


flyspell:
	(message (format "duplicate `%s'" word)))




gud:
   (message (format "Error parsing file %s." file))



I will leave any code alone unless I am absolutely sure of what I am
doing.  



Here's an example of when I am not sure: 

  (message (substitute-command-keys "Press \\[wdired-finish-edit] when
  finished \
	or \\[wdired-abort-changes] to abort changes")))

Can the above code ever lead to errors? 

Either way, isn't it safe to insert a "%s" after message anyways?


====================================================

All this almost makes one wonder if it would make sense to provide a
msg:

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

A lot of times, an author accidentally writes the code with msg in
mind rather than message.  This error is pervasive, especially if you
look at all the add-on code (not part of emacs).  I bet there is aon
average, at least one bug per file.

 


Sincerely,

DG                                 http://gnufans.net/
--

Re: (message (format "File %s" file))

[Stefan Monnier]

> Here's an example of when I am not sure:
>   (message (substitute-command-keys "Press \\[wdired-finish-edit] when
>   finished \
> 	or \\[wdired-abort-changes] to abort changes")))
> Can the above code ever lead to errors?

Yes.

> Either way, isn't it safe to insert a "%s" after message anyways?

The only case where it's not safe is when the existing code already does
%-escaping, in which case adding the "%s" will cause any % sign in the
output string to appear twice.  That's extremely rare and I suspect it only
happens in cases where the sole arg to message is a string constant (which
thus contains a double % sign).


> All this almost makes one wonder if it would make sense to provide a
> msg:

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

> A lot of times, an author accidentally writes the code with msg in
> mind rather than message.  This error is pervasive, especially if you
> look at all the add-on code (not part of emacs).  I bet there is aon
> average, at least one bug per file.

In the past I suggested to make (message ARG) automatically behave like
(message "%s" ARG) and AFAIK the only prolem with that was a few rare case
like (message "Compiling list...( 0%%)") and those problems are easy to fix.


        Stefan

Re: (message (format "File %s" file))

[D Goel]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
> In the past I suggested to make (message ARG) automatically behave like
> (message "%s" ARG) and AFAIK the only prolem with that was a few rare case
> like (message "Compiling list...( 0%%)") and those problems are easy to fix.


ah, nice or it could do this:

(message arg) behaves like (message "%s" arg) only if the arg has no
%%.

that way, one wouldn't have to break backward compatibility...  

.. or one could just provide an extra (msg)



DG                                 http://gnufans.net/
--

Re: (message (format "File %s" file))

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> All this almost makes one wonder if it would make sense to provide a
>> msg:
>
>> (defun msg (arg)
>> 	(message "%s" arg))
>
>> A lot of times, an author accidentally writes the code with msg in
>> mind rather than message.  This error is pervasive, especially if you
>> look at all the add-on code (not part of emacs).  I bet there is aon
>> average, at least one bug per file.
>
> In the past I suggested to make (message ARG) automatically behave like
> (message "%s" ARG) and AFAIK the only prolem with that was a few rare case
> like (message "Compiling list...( 0%%)") and those problems are easy to fix.
>

I think this would be a very good approach -- and trivial to implement.

One problem is that it is not backwards compatible, so code written for
22.x may cause problems when used on older emacsen or other dialects.

I.e. (message "%s") works fine on 22.x, but breaks on 21.x.

Anyway, here is a patch (mostly doc changes in fact)...


*** editfns.c	04 Sep 2005 23:32:29 +0200	1.396
--- editfns.c	05 Sep 2005 16:00:17 +0200	
***************
*** 3115,3125 ****
  The message also goes into the `*Messages*' buffer.
  \(In keyboard macros, that's all it does.)
  
! The first argument is a format control string, and the rest are data
! to be formatted under control of the string.  See `format' for details.
! 
! If the first argument is nil, the function clears any existing message;
! this lets the minibuffer contents show.  See also `current-message'.
  
  usage: (message STRING &rest ARGS)  */)
       (nargs, args)
--- 3115,3128 ----
  The message also goes into the `*Messages*' buffer.
  \(In keyboard macros, that's all it does.)
  
! If called with one string argument, that string is printed literally.
! Otherwise, the first argument is a format control string, and the rest
! are data to be formatted under control of the string.  See `format'
! for details.
! 
! If the first argument is nil or an empty string, the function clears
! any existing message; this lets the minibuffer contents show.
! See also `current-message'.
  
  usage: (message STRING &rest ARGS)  */)
       (nargs, args)
***************
*** 3136,3142 ****
    else
      {
        register Lisp_Object val;
!       val = Fformat (nargs, args);
        message3 (val, SBYTES (val), STRING_MULTIBYTE (val));
        return val;
      }
--- 3139,3148 ----
    else
      {
        register Lisp_Object val;
!       if (nargs == 1 && STRINGP (args[0]))
! 	val = args[0];
!       else
! 	val = Fformat (nargs, args);
        message3 (val, SBYTES (val), STRING_MULTIBYTE (val));
        return val;
      }
***************
*** 3145,3155 ****
  DEFUN ("message-box", Fmessage_box, Smessage_box, 1, MANY, 0,
         doc: /* Display a message, in a dialog box if possible.
  If a dialog box is not available, use the echo area.
- The first argument is a format control string, and the rest are data
- to be formatted under control of the string.  See `format' for details.
  
! If the first argument is nil, clear any existing message; let the
! minibuffer contents show.
  
  usage: (message-box STRING &rest ARGS)  */)
       (nargs, args)
--- 3151,3164 ----
  DEFUN ("message-box", Fmessage_box, Smessage_box, 1, MANY, 0,
         doc: /* Display a message, in a dialog box if possible.
  If a dialog box is not available, use the echo area.
  
! If called with one string argument, that string is printed literally.
! Otherwise, the first argument is a format control string, and the rest
! are data to be formatted under control of the string.  See `format'
! for details.
! 
! If the first argument is nil or an empty string, clear any existing
! message; let the minibuffer contents show.
  
  usage: (message-box STRING &rest ARGS)  */)
       (nargs, args)
***************
*** 3164,3170 ****
    else
      {
        register Lisp_Object val;
!       val = Fformat (nargs, args);
  #ifdef HAVE_MENUS
        /* The MS-DOS frames support popup menus even though they are
  	 not FRAME_WINDOW_P.  */
--- 3173,3182 ----
    else
      {
        register Lisp_Object val;
!       if (nargs == 1 && STRINGP (args[0]))
! 	val = args[0];
!       else
! 	val = Fformat (nargs, args);
  #ifdef HAVE_MENUS
        /* The MS-DOS frames support popup menus even though they are
  	 not FRAME_WINDOW_P.  */
***************
*** 3207,3217 ****
  If this command was invoked with the mouse, use a dialog box if
  `use-dialog-box' is non-nil.
  Otherwise, use the echo area.
- The first argument is a format control string, and the rest are data
- to be formatted under control of the string.  See `format' for details.
  
! If the first argument is nil, clear any existing message; let the
! minibuffer contents show.
  
  usage: (message-or-box STRING &rest ARGS)  */)
       (nargs, args)
--- 3219,3232 ----
  If this command was invoked with the mouse, use a dialog box if
  `use-dialog-box' is non-nil.
  Otherwise, use the echo area.
  
! If called with one string argument, that string is printed literally.
! Otherwise, the first argument is a format control string, and the rest
! are data to be formatted under control of the string.  See `format'
! for details.
! 
! If the first argument is nil or an empty string, clear any existing
! message; let the minibuffer contents show.
  
  usage: (message-or-box STRING &rest ARGS)  */)
       (nargs, args)


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: (message (format "File %s" file))

[Richard M. Stallman]

I do not want to make `message' with one arg inconsistent with the
case of multiple args.  I would rather put in a compiler warning to
detect (message (format ...)).

Re: (message (format "File %s" file))

[Richard M. Stallman]

    I plan to go through the entire source code, and only in cases I am
    sure, fix these bugs (which mostly involves adding a "%s% after
    `message', or getting rid of the unnecessary format call following
    (message).

Thank you for taking the initiative.

Re: (message (format "File %s" file))

[Juri Linkov]

> A *lot* of source code contains easy-to-miss bugs like this:
>
> 	(message (format "File: %s" file))

I guess one of the reasons of such bugs is the misleading argument
name `string' of the function `message'.  This name doesn't suggest
that it is actually a format string used by the function `format'.
I propose to rename the argument name `string' to `format-string' in
docstrings of `message...' functions and in the Emacs Lisp Reference
Manual.

Index: src/editfns.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/editfns.c,v
retrieving revision 1.398
diff -c -r1.398 editfns.c
*** src/editfns.c	19 Sep 2005 08:13:14 -0000	1.398
--- src/editfns.c	20 Sep 2005 05:04:02 -0000
***************
*** 3124,3130 ****
  any existing message; this lets the minibuffer contents show.  See
  also `current-message'.
  
! usage: (message STRING &rest ARGS)  */)
       (nargs, args)
       int nargs;
       Lisp_Object *args;
--- 3124,3130 ----
  any existing message; this lets the minibuffer contents show.  See
  also `current-message'.
  
! usage: (message FORMAT-STRING &rest ARGS)  */)
       (nargs, args)
       int nargs;
       Lisp_Object *args;
***************
*** 3154,3160 ****
  If the first argument is nil or the empty string, clear any existing
  message; let the minibuffer contents show.
  
! usage: (message-box STRING &rest ARGS)  */)
       (nargs, args)
       int nargs;
       Lisp_Object *args;
--- 3154,3160 ----
  If the first argument is nil or the empty string, clear any existing
  message; let the minibuffer contents show.
  
! usage: (message-box FORMAT-STRING &rest ARGS)  */)
       (nargs, args)
       int nargs;
       Lisp_Object *args;
***************
*** 3216,3222 ****
  If the first argument is nil or the empty string, clear any existing
  message; let the minibuffer contents show.
  
! usage: (message-or-box STRING &rest ARGS)  */)
       (nargs, args)
       int nargs;
       Lisp_Object *args;
--- 3216,3222 ----
  If the first argument is nil or the empty string, clear any existing
  message; let the minibuffer contents show.
  
! usage: (message-or-box FORMAT-STRING &rest ARGS)  */)
       (nargs, args)
       int nargs;
       Lisp_Object *args;
***************
*** 3281,3288 ****
     : SBYTES (STRING))
  
  DEFUN ("format", Fformat, Sformat, 1, MANY, 0,
!        doc: /* Format a string out of a control-string and arguments.
! The first argument is a control string.
  The other arguments are substituted into it to make the result, a string.
  It may contain %-sequences meaning to substitute the next argument.
  %s means print a string argument.  Actually, prints any object, with `princ'.
--- 3281,3288 ----
     : SBYTES (STRING))
  
  DEFUN ("format", Fformat, Sformat, 1, MANY, 0,
!        doc: /* Format a string out of a format-string and arguments.
! The first argument is a format control string.
  The other arguments are substituted into it to make the result, a string.
  It may contain %-sequences meaning to substitute the next argument.
  %s means print a string argument.  Actually, prints any object, with `princ'.

Index: lispref/display.texi
===================================================================
RCS file: /cvsroot/emacs/emacs/lispref/display.texi,v
retrieving revision 1.187
diff -c -r1.187 display.texi
*** lispref/display.texi	18 Sep 2005 14:03:55 -0000	1.187
--- lispref/display.texi	20 Sep 2005 05:02:59 -0000
***************
*** 208,216 ****
    This section describes the functions for explicitly producing echo
  area messages.  Many other Emacs features display messages there, too.
  
! @defun message string &rest arguments
! This function displays a message in the echo area.  The
! argument @var{string} is similar to a C language @code{printf} control
  string.  See @code{format} in @ref{Formatting Strings}, for the details
  on the conversion specifications.  @code{message} returns the
  constructed string.
--- 208,216 ----
    This section describes the functions for explicitly producing echo
  area messages.  Many other Emacs features display messages there, too.
  
! @defun message format-string &rest arguments
! This function displays a message in the echo area.  The argument
! @var{format-string} is similar to a C language @code{printf} format
  string.  See @code{format} in @ref{Formatting Strings}, for the details
  on the conversion specifications.  @code{message} returns the
  constructed string.
***************
*** 218,231 ****
  In batch mode, @code{message} prints the message text on the standard
  error stream, followed by a newline.
  
! If @var{string}, or strings among the @var{arguments}, have @code{face}
! text properties, these affect the way the message is displayed.
  
  @c Emacs 19 feature
! If @var{string} is @code{nil}, @code{message} clears the echo area; if
! the echo area has been expanded automatically, this brings it back to
! its normal size.  If the minibuffer is active, this brings the
! minibuffer contents back onto the screen immediately.
  
  @example
  @group
--- 218,232 ----
  In batch mode, @code{message} prints the message text on the standard
  error stream, followed by a newline.
  
! If @var{format-string}, or strings among the @var{arguments}, have
! @code{face} text properties, these affect the way the message is displayed.
  
  @c Emacs 19 feature
! If @var{format-string} is @code{nil} or the empty string,
! @code{message} clears the echo area; if the echo area has been
! expanded automatically, this brings it back to its normal size.
! If the minibuffer is active, this brings the minibuffer contents back
! onto the screen immediately.
  
  @example
  @group
***************
*** 254,260 ****
  the previous echo area contents.
  @end defmac
  
! @defun message-or-box string &rest arguments
  This function displays a message like @code{message}, but may display it
  in a dialog box instead of the echo area.  If this function is called in
  a command that was invoked using the mouse---more precisely, if
--- 255,261 ----
  the previous echo area contents.
  @end defmac
  
! @defun message-or-box format-string &rest arguments
  This function displays a message like @code{message}, but may display it
  in a dialog box instead of the echo area.  If this function is called in
  a command that was invoked using the mouse---more precisely, if
***************
*** 268,274 ****
  @code{last-nonmenu-event} to a suitable value around the call.
  @end defun
  
! @defun message-box string &rest arguments
  This function displays a message like @code{message}, but uses a dialog
  box (or a pop-up menu) whenever that is possible.  If it is impossible
  to use a dialog box or pop-up menu, because the terminal does not
--- 269,275 ----
  @code{last-nonmenu-event} to a suitable value around the call.
  @end defun
  
! @defun message-box format-string &rest arguments
  This function displays a message like @code{message}, but uses a dialog
  box (or a pop-up menu) whenever that is possible.  If it is impossible
  to use a dialog box or pop-up menu, because the terminal does not

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: (message (format "File %s" file))

[Richard M. Stallman]

    I propose to rename the argument name `string' to `format-string' in
    docstrings of `message...' functions and in the Emacs Lisp Reference
    Manual.

Good idea.  Please do that.