functions which read by prompting should not add additional chars to prompt string

functions which read by prompting should not add additional chars to prompt string

[MON KEY]

Lisp functions which read user input and accept a PROMPT arg should
not append additional characters to the supplied PROMPT string
esp. when no additional arguments are supplied.

---
`read-regexp' lisp/replace.el

When read-regexp's optional arg DEFAULT-VALUE is not true, why are a
colon (char 58) and space (char 32) tacked onto end of PROMPT? e.g.:

 (if default-value
     (format "%s (default %s): " prompt
             (query-replace-descr default-value))
   (format "%s: " prompt))
              ^
This should be:

  (format "%s" prompt)

For a `read-*' style function, the behaviour of `read-regexp' is
annoying b/c its name suggest that it is the public interface for
reading regexps. esp.

Indeed, there are regexp readers in other packages which don't use the
`read-regexp' interface, instead rolling their own readers, e.g.:

 `dired-read-regexp' `ad-read-regexp' `grep-read-regexp'

As such, I would suggest that the `read-regexp' interface might be
adjusted to offer a more conformant behaviour without too much
trouble.

There are eight functions in ./lisp which directly call `read-regexp'.

Of the eight only four supply have provision for a DEFAULT-VALUE:

 `hi-lock-line-face-buffer'   `hi-lock-face-buffer',
 `hi-lock-face-phrase-buffer' `occur-read-primary-arg'

Each of the above rely on the value of (car regexp-history) for
DEFAULT-VALUE.  IOW, even where callers do supply DEFAULT-VALUE there
is no guarantee that the car of `regexp-history' isn't null.

A good fix might be to adjust the existing semantics of `read-regexp's
DEFAULT-VALUE to accept a boolean which when t defaults to whatever is
at the head `regexp-history' e.g.:

 (and (eq default-value t) (setq default-value (car regexp-history)))

Following enumerates the eight functions in ./lisp which rgrep shows
as direct callers of `read-regexp'. I've indicated their
PROPMT and supply annotation as to which provide a DEFAULT-VALUE.

-
lisp/replace.el
No defaulting behaviour:
`keep-lines-read-args'
 ;--> (read-regexp prompt)

With DEFAULT-VALUE supplied but regexp-history may may be nil:
`occur-read-primary-arg'
 ;--> (read-regexp "List lines matching regexp" (car regexp-history))

-
lisp/faces.el
No defaulting behaviour:
`list-faces-display'
 ;--> (read-regexp "List faces matching regexp")

-
lisp/hi-lock.el
Each supplies a DEFAULT-VAULUE, but regexp-history may may be nil:

`hi-lock-line-face-buffer'
 ;--> (read-regexp "Regexp to highlight line" (car regexp-history))

`hi-lock-face-buffer'
 ;--> (read-regexp "Regexp to highlight" (car regexp-history))

`hi-lock-face-phrase-buffer'
 ;--> (read-regexp "Phrase to highlight" (car regexp-history))

-
lisp/misearch.el
No defaulting behavior:
`multi-isearch-read-matching-buffers'
 ;--> (read-regexp "Search in buffers whose names match regexp")

`multi-isearch-read-matching-files'
 ;--> (read-regexp "Search in files whose names match wildcard")

---
Following two functions, `read-face-name' and `read-quoted-char' both
step on the supplied PROMPT and should be fixed:

(read-face-name   "face ")

(read-quoted-char "char ")

`read-quoted-char' behaves has doubly bad by tacking on a "-". Why?

---
`read-buffer' may step on the PROMPT string only when DEF is non-nil:

(read-buffer "buffer ")

However her buddy `read-buffer-to-switch' has non-standard prompt
string defaulting behavior similiar to that of `read-regexp'.

Though, unlike `read-regexp' which is implemented in lisp,
`read-buffer-to-switch' is a lisp wrapper around the C primitive
`read-buffer' defined in src/minibuf.c `read-buffer' will acknowledge
`read-buffer-function' when non-nil and allows callers to override its
own non standard prompt string defaulting.

(read-buffer-to-switch "switch to buffer ")

---
For comparsion, following is a list of "reading" functions which do
not not step on a supplied PROMPT string by tacking on additional
characters:

(read-minibuffer            "lispy thing ")
(read-from-minibuffer       "string ")
(read-string                "string ")
(read-no-blanks-input       "no spaces please ")
(read-number                "number ")
(read-passwd                "password ")
(read-event                 "event ")
(read-key-sequence          "key sequence ")
(read-key-sequence-vector   "key sequence ")
(read-variable              "variable ")
(read-command               "command ")
(read-file-name             "file ")
(read-directory-name        "directory ")
(read-color                 "color ")
(read-envvar-name           "envvar ")
(read-shell-command         "shell-command ")
(read-char                  "char ")
(read-char-by-name          "char-name ")
(read-char-exclusive        "char ")
(read-charset               "charset ")
(read-file-modes            "modes ")
(read-multilingual-string   "string ")
(read-non-nil-coding-system "coding system ")
(read-input-method-name     "input method ")
(read-language-name         'documentation "lang ")
(read-coding-system         "coding sytem ")

--
/s_P\

RE: functions which read by prompting should not add additional chars to prompt string

[Drew Adams]

> Lisp functions which read user input and accept a PROMPT arg should
> not append additional characters to the supplied PROMPT string
> esp. when no additional arguments are supplied.

Not sure we need an explicit rule to handle the problems you point to.  But I
also don't have any special use case for such a function tacking on extra chars,
and I agree about the principle as well as the cases that need to be fixed.

It would be one thing to express such a rule wrt Emacs's own sources (makes
sense); it would be another thing to proclaim it wider (e.g. as a convention,
say, in the doc).

I would say let's just take care of the concrete problems you point to, and then
adhere to the same approach generally (in Emacs sources), without promulgating
another rule to users.

>  (if default-value
>      (format "%s (default %s): " prompt
>              (query-replace-descr default-value))
>    (format "%s: " prompt))
> This should be:
>   (format "%s" prompt)

1+.

IMO there is no need to add the default value, whatever it might be, to the
prompt string.  As always, it is available via `M-n'.  There's nothing special
about `read-regexp' (or the other functions mentioned below) in this regard.

However, this function is weird wrt the default value.  DEFAULT-VALUE is not
included in the list of defaults passed to `read-from-minibuffer', so it is not
even accessible using `M-n'.  It is accessible _only_ by hitting RET with empty
input.  Why the weird design here?

If there is no good reason for it then let's get rid of this confusing
exception.  Just put DEFAULT-VALUE at the head of the defaults list, and let it
go at that.

[Another point about `read-regexp': It is a completely general function, and as
such should be moved out of `replace.el'.  There is already enough stuff in that
file that is not specific to replacement (e.g. `occur').  Presumably it is there
only out of laziness: someone added it for use in `replace.el' code, and it was
never moved to a better home.]

> Each of the above rely on the value of (car regexp-history) for
> DEFAULT-VALUE.  IOW, even where callers do supply DEFAULT-VALUE there
> is no guarantee that the car of `regexp-history' isn't null.

Null (car regexp-history) is not a problem, that I can see.

> A good fix might be to adjust the existing semantics of `read-regexp's
> DEFAULT-VALUE to accept a boolean which when t defaults to whatever is
> at the head `regexp-history' e.g.:
>  (and (eq default-value t) (setq default-value (car regexp-history)))

Not a good idea, IMO.  No gain, and some loss of clarity (more doc lookup, for
little info).

`read-regexp' puts a bunch of defaults ahead of the history, for `M-n'.  If a
caller wants to privilege (car regexp-history), then it's better for everyone if
that is explicit in the call.  We gain nothing by identifying an apparently
common case ((car regexp-history)) and adding a layer of source
indirection/obfuscation by having `t' stand for it.

> Following two functions, `read-face-name' and `read-quoted-char' both
> step on the supplied PROMPT and should be fixed:
> (read-face-name   "face ")
> (read-quoted-char "char ")

1+.  Let the caller supply the prompt, including any `: '.  Always.  Unless
there is some special countervailing reason.

And let users get the default values with `M-n' - forget about adding defaults
to prompts (that's not done consistently anyway).

> `read-buffer' may step on the PROMPT string only when DEF is non-nil:
> (read-buffer "buffer ")

1+.  Oddly, here the C source apparently tries to compensate for the presence or
absence of a space and a colon in the supplied PROMPT.  These all produce the
same prompt:

(read-buffer "foo:" "toto")
(read-buffer "foo: " "toto")
(read-buffer "foo "  "toto")
(read-buffer "foo"   "toto")

The code should not be so "clever".  Again, let the caller be responsible for
the prompt.  Likewise for `read-buffer-to-switch' (which just calls
`read-buffer').

In sum, I agree with you: 1+.

All such attempts at "clever prompting" or easing the caller's burden (just some
guesses at the motivations) are misguided.  Keep it simple.

Re: functions which read by prompting should not add additional chars to prompt string

[Stefan Monnier]

> IMO there is no need to add the default value, whatever it might be,
> to the prompt string.  As always, it is available via `M-n'.

That's the whole point of this calling convention for read-regexp: the
choice of whether to place the default in the prompt is centralized in
read-regexp, so it can easily be changed, instead of being duplicated in
each and every call to read-regexp (as is currently the case for pretty
much all other read-<foo> functions) which hard-codes the behavior and
renders it impossible to let users choose another behavior.

I.e. read-regexp's behavior is the right one, the other functions would
need to be adjusted to follow the same principle (tho doing it in
backward-compatible way is difficult).


        Stefan