bug#14254: 24.3; read-number fails to recognize faulty numbers (string-to-number to blame)

[Stefan Monnier]

> I should have mentioned that original code contained the line
>           visargs[i] = args[i];
> but I omitted it in the change since it has no effect
> because this code at the end of `Fcall_interactively'

>       for (i = 1; i < nargs; i++)
>         {
>           if (varies[i] > 0)
>             visargs[i] = Fcons (intern (callint_argfuns[varies[i]]), Qnil);
>           else
>             visargs[i] = quotify_arg (args[i]);
>         }

Here's what's going on:
visargs is used in two different ways, just in order to avoid allocating
a third array:
- inside the loop, visargs keeps a representation of the arguments,
  which is used when the prompt of an argument contains a % (in which
  case it's passed to `format').  This is a rarely used feature which
  can be seen in:

   % grep '(interactive ".*%' lisp/**/*.el
   lisp/abbrev.el:  (interactive "sDefine global abbrev: \nsExpansion for %s: ")
   lisp/abbrev.el:  (interactive "sDefine mode abbrev: \nsExpansion for %s: ")
   lisp/dired-x.el:  (interactive "FRelSymLink: \nFRelSymLink %s: \np")
   lisp/mail/mailabbrev.el:  (interactive "sDefine mail alias: \nsDefine %s as 
mail alias for: ")
   lisp/mail/mailalias.el:  (interactive "sDefine mail alias: \nsDefine %s as 
mail alias for: ")
   lisp/net/ange-ftp.el:  (interactive "fCopy file: \nFCopy %s to file: \np")
   lisp/net/ange-ftp.el:  (interactive "fRename file: \nFRename %s to file: 
\np")
   lisp/subr.el:  (interactive "KSet key globally: \nCSet key %s to command: ")
   lisp/subr.el:  (interactive "KSet key locally: \nCSet key %s locally to 
command: ")
   %

  This is the reason why it's called "visargs" because it contains
  a representation of the argument which should be appropriate for
  display (e.g. it keeps a key-description instead of a key).

  FWIW, I think this is a misfeature.  Better force people to use a Lisp
  form for the interactive spec when they need access to previous args
  while building subsequent args.

- after the loop, this is not needed any more, but the same array is
  reused to build the arguments to pass to `Flist' to make the entry to
  add in command-history.

> I don't understand why `Fcall_interactively' contains many lines of
> such useless code as
>         visargs[i] = last_minibuf_string;
> If the intention was to collect strings in `visargs' and use them later
> then old code for numbers (currently still useless) was wrong,
> it should convert numbers to strings with something like
>           visargs[i] = Fnumber_to_string(args[i]);

I don't understand all those "visargs[i] = last_minibuf_string;" either,
because visargs doesn't need to contain only strings, since format's %s
will handle non-strings.


        Stefan