bug#34794: 26.1; doc of `read-buffer'

[Drew Adams <drew.adams@oracle.com>]

> > > > AFAICT neither the doc string nor the Elisp manual states what the
> > > > default value is if argument DEF is nil.  IOW, what is the default
> > > > buffer name if no explicit default is provided?  It seems (without
> > > > thorough testing) to be the value of `(buffer-name (current-buffer))'.
> > >
> > > No, it's an empty string, and I think the doc string already conveys
> > > that.
> >
> > I cannot tell from the doc string that the default value,
> > i.e., the value returned when DEF is nil, is the empty
> > string.
> 
>   Optional second arg DEF is value to return if user enters an empty line.
> 
> Doesn't this say that when DEF is omitted the function will return
> that empty line?

No, it doesn't.  If DEF is omitted it is nil.  Is
nil the "value to return if user enters an empty
line?"  I don't think so. 

`RET' with empty input can result in anything one
likes as default value.  And other input-reading
functions do so.  There is nothing here that says
that empty input results in the empty string being
returned.

> > > (Note that if read-buffer-function is non-nil, what happens
> > > then is entirely up to that function, which doesn't make it easy to
> > > say exactly how DEF is handled.)
> >
> > It's not hard to state what the default DEF behavior
> > is, and then later say that if `read-buffer-function'
> > is non-nil then the use of the other args is up to it,
> > i.e., not necessarily as described above.  This is
> > not unusual for a function that optionally accepts a
> > function arg as one possibility.
> 
> Please suggest such a text, because I definitely don't see an easy way
> of saying that, without triggering more bug reports like this one.

1. OK.  How about this?

Read the name of a buffer with completion and return it as a string.
If option `read-buffer-function' is non-nil then it is a function that
accepts all of the `read-buffer' arguments, in order, and returns a
buffer name.  That buffer name is returned by `read-buffer'.

Otherwise, prompt with first arg PROMPT, a string that should end with
a colon followed by a space char (`: ').  Other args control the read
behavior, as follows.

Optional arg DEFAULT determines the return value if user input is
empty (just `RET' with no minibuffer input):

* If a non-nil list then return its first element.
* If absent or nil then return the empty string, \"\".
* If anything else then return it (DEFAULT).

Non-nil optional arg REQUIRE-MATCH means allow only names of existing
buffers as input.  It is the same as for 'completing-read'.  The names
of all existing (live) buffers are completion candidates.

Non-nil optional arg PREDICATE should be a function that accepts a
buffer or buffer name as its first argument.  It filters the list of
candidate buffers, excluding any buffers for which it returns nil.

Non-nil option `read-buffer-completion-ignore-case' means that
buffer-name completion ignores case while reading the buffer name.

[If you prefer, the description of the use of
`read-buffer-function' could be moved to the end.
I think it is probably better where it is.  It
definitely should not be put in the middle of
descriptions of the arguments, IMO.]

2. I guessed wrt REQUIRE-MATCH, regarding which buffer
names are completion candidates.


3. I also had to guess wrt PREDICATE, as the current doc
does not say what is acceptable as an argument (buffer?
buffer name?), and it doesn't say which way the predicate
works as a filter: keeping things that satisfy it or
excluding them.  I guessed that it works with either a
buffer or its name, and I guessed that it keeps things
that satisfy it. 


4. There appears to be a fairly large bug in the
behavior, BTW.  The function is supposed to return a
buffer name, which is presumably a string.

But try this, hitting `RET' with empty minibuffer input:

 (read-buffer "b: " (selected-window) t)

That returns a window!  And this returns a number, not
a numeric string:

 (read-buffer "b: " 42 t)

It apparently can return anything at all.

This is in spite of the fact that the REQUIRE-MATCH
arg is `t', and according to the doc that should
mean that you cannot exit the minibuffer unless the
input corresponds to an existing buffer.

Is that the behavior we want?  That's the behavior
I documented, above.

Do you prefer a separate bug report for this bug, or
can you fix it based on this report?


5. Other doc-string bugs (fixed in my suggestion):

* Doesn't say that it reads with completion.  (You
  can guess that, when you read some of the argument
  descriptions - it mentions completion only in
  passing.)
* Doesn't say in what way REQUIRE-MATCH "determines
  whether non-existing buffer names are allowed".
  It refers to `completing-read', but that says
  nothing about existing buffers - that says only
  that WHATEVER the set of candidates, you cannot
  exit the minibuffer without matching one of them.
* Arguments are described out of order.
* Arg PREDICATE is described after the statement
  about `read-buffer-function'.