bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Matt Armstrong]

`try-completion' and `all-completions' support a completion COLLECTION
of a list of strings.  This is not documented in their docstrings.  This
is confusing because every other supported data structure is mentioned.

This support was added around 20 years ago.  See Stefan's commit
695deb1857 or
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=695deb1857dd95239df0139894e40719699a6cc8

I'm assuming this was just an oversight, since `test-completion'
mentions "list of strings" as does the description of `try-completion'
in the elisp manual.

bug#52202: [PATCH] try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Matt Armstrong]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

Attached is a docstring patch against the emacs-28 branch for this
issue.

Note/question: I wonder if the `all-completions' docstring should refer
to `try-completion' rather than copy much of the text.  The same
approach works well in the lisp ref manual, I think.


[-- Attachment #2: 0001-Improve-the-try-completion-and-all-completions-docst.patch --]
[-- Type: text/x-diff, Size: 8288 bytes --]

From 8bc1bee134f0626513efd00f4a283cf5a2d2a359 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Tue, 30 Nov 2021 10:54:17 -0800
Subject: [PATCH] Improve the `try-completion' and `all-completions' docstrings

* src/minibuf.c (Ftry_completion):
(Fall_completions): Use language from the Emacs lisp manual when
talking about the COLLECTION and PREDICATE arguments.  Most notably,
the way completion tables may be passed is now more clear.  The prior
docstring did not mention lists of strings/symbols at all.  Bug#52202
---
 src/minibuf.c | 124 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 77 insertions(+), 47 deletions(-)

diff --git a/src/minibuf.c b/src/minibuf.c
index 6c0cd358c5..f1ee7b2453 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1569,34 +1569,49 @@ match_regexps (Lisp_Object string, Lisp_Object regexps,
 DEFUN ("try-completion", Ftry_completion, Stry_completion, 2, 3, 0,
        doc: /* Return common substring of all completions of STRING in COLLECTION.
 Test each possible completion specified by COLLECTION
-to see if it begins with STRING.  The possible completions may be
-strings or symbols.  Symbols are converted to strings before testing,
-see `symbol-name'.
-All that match STRING are compared together; the longest initial sequence
-common to all these matches is the return value.
-If there is no match at all, the return value is nil.
-For a unique match which is exact, the return value is t.
-
-If COLLECTION is an alist, the keys (cars of elements) are the
-possible completions.  If an element is not a cons cell, then the
-element itself is the possible completion.
-If COLLECTION is a hash-table, all the keys that are strings or symbols
-are the possible completions.
-If COLLECTION is an obarray, the names of all symbols in the obarray
-are the possible completions.
+to see if it begins with STRING.
 
-COLLECTION can also be a function to do the completion itself.
-It receives three arguments: the values STRING, PREDICATE and nil.
+All completions that match STRING are compared together; the longest
+initial sequence common to all these matches is the return value.  If
+there is no match at all, the return value is nil.  For a unique match
+which is exact, the return value is t.
+
+COLLECTION is called the "completion table."  Its value must be a
+list, a hash table, an obarray, or a completion function.
+
+If COLLECTION is a list, the possible completions are specified by the
+elements of the list, each of which should be either a string, a
+symbol, or a cons cell whose CAR is either a string or a symbol.  If
+the list contains elements of any other type, those are ignored.
+
+If COLLECTION is a hash-table, all the keys that are strings or
+symbols are the possible completions.
+
+If COLLECTION is an obarray, all the symbols in the obarray are the
+possible completions.
+
+The possible completions from COLLECTION that are symbols are
+converted to strings before testing, see `symbol-name'.
+
+COLLECTION can also be a function to do the completion itself.  It
+receives three arguments: the values STRING, PREDICATE and nil (the
+third argument is so that the same function can be used in
+‘all-completions’ and do the appropriate thing in either case).
 Whatever it returns becomes the value of `try-completion'.
 
-If optional third argument PREDICATE is non-nil,
-it is used to test each possible match.
-The match is a candidate only if PREDICATE returns non-nil.
-The argument given to PREDICATE is the alist element
-or the symbol from the obarray.  If COLLECTION is a hash-table,
-predicate is called with two arguments: the key and the value.
-Additionally to this predicate, `completion-regexp-list'
-is used to further constrain the set of candidates.  */)
+If optional third argument PREDICATE is non-nil, it is used to test
+each possible match.  The match is a candidate only if PREDICATE
+returns non-nil.  The argument given to PREDICATE is an element from
+COLLECTION.  If COLLECTION is a list it is a list element (either a
+symbol, string, or cons cell whose CAR is a symbol or string).  If
+COLLECTION is an obarray, a symbol from it.  If COLLECTION is a
+hash-table, predicate is called with two arguments: a key and the
+value.
+
+In addition, to be acceptable, a completion must also match all the
+regular expressions in ‘completion-regexp-list’.  (Unless COLLECTION
+is a function, in which case that function has to handle
+‘completion-regexp-list’ itself.)  */)
   (Lisp_Object string, Lisp_Object collection, Lisp_Object predicate)
 {
 
@@ -1808,31 +1823,46 @@ DEFUN ("try-completion", Ftry_completion, Stry_completion, 2, 3, 0,
 DEFUN ("all-completions", Fall_completions, Sall_completions, 2, 4, 0,
        doc: /* Search for partial matches to STRING in COLLECTION.
 Test each of the possible completions specified by COLLECTION
-to see if it begins with STRING.  The possible completions may be
-strings or symbols.  Symbols are converted to strings before testing,
-see `symbol-name'.
-The value is a list of all the possible completions that match STRING.
-
-If COLLECTION is an alist, the keys (cars of elements) are the
-possible completions.  If an element is not a cons cell, then the
-element itself is the possible completion.
-If COLLECTION is a hash-table, all the keys that are strings or symbols
-are the possible completions.
-If COLLECTION is an obarray, the names of all symbols in the obarray
-are the possible completions.
+to see if it begins with STRING.
+The value is a list of strings for all the possible completions that
+match STRING.
 
-COLLECTION can also be a function to do the completion itself.
-It receives three arguments: the values STRING, PREDICATE and t.
+COLLECTION is called the "completion table."  Its value must be a
+list, a hash table, an obarray, or a completion function.
+
+If COLLECTION is a list, the permissible completions are specified by
+the elements of the list, each of which should be either a string, a
+symbol, or a cons cell whose CAR is either a string or a symbol.  If
+the list contains elements of any other type, those are ignored.
+
+If COLLECTION is a hash-table, all the keys that are strings or
+symbols are the possible completions.
+
+If COLLECTION is an obarray, all the symbols in the obarray are the
+possible completions.
+
+The possible completions from COLLECTION that are symbols are
+converted to strings before testing, see `symbol-name'.
+
+COLLECTION can also be a function to do the completion itself.  It
+receives three arguments: the values STRING, PREDICATE and t (the
+third argument is so that the same function can be used in
+‘try-completion’ and do the appropriate thing in either case).
 Whatever it returns becomes the value of `all-completions'.
 
-If optional third argument PREDICATE is non-nil,
-it is used to test each possible match.
-The match is a candidate only if PREDICATE returns non-nil.
-The argument given to PREDICATE is the alist element
-or the symbol from the obarray.  If COLLECTION is a hash-table,
-predicate is called with two arguments: the key and the value.
-Additionally to this predicate, `completion-regexp-list'
-is used to further constrain the set of candidates.
+If optional third argument PREDICATE is non-nil, it is used to test
+each possible match.  The match is a candidate only if PREDICATE
+returns non-nil.  The argument given to PREDICATE is an element from
+COLLECTION.  If COLLECTION is a list it is a list element (either a
+symbol, string, or cons cell whose CAR is a symbol or string).  If
+COLLECTION is an obarray, a symbol from it.  If COLLECTION is a
+hash-table, predicate is called with two arguments: a key and the
+value.
+
+In addition, to be acceptable, a completion must also match all the
+regular expressions in ‘completion-regexp-list’.  (Unless COLLECTION
+is a function, in which case that function has to handle
+‘completion-regexp-list’ itself.)
 
 An obsolete optional fourth argument HIDE-SPACES is still accepted for
 backward compatibility.  If non-nil, strings in COLLECTION that start
-- 
2.33.0

bug#52202: [PATCH] try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Eli Zaretskii]

> From: Matt Armstrong <matt@rfc20.org>
> Date: Tue, 30 Nov 2021 11:18:05 -0800
> 
> Attached is a docstring patch against the emacs-28 branch for this
> issue.

Thanks, but ... all these changes just to say that a list of strings
can also be a COLLECTION?  This looks like a complete rewrite, and
comparing it with the existing doc string is a lot of work.  Is the
current doc string really that awful and requires such a complete
rewrite?  Why can't we get away with a smaller, simpler change (that
would be easier to review and approve)?

> Note/question: I wonder if the `all-completions' docstring should refer
> to `try-completion' rather than copy much of the text.  The same
> approach works well in the lisp ref manual, I think.

ELisp manual describes both one next to the other, so it can get away
with such a technique.  Doc strings don't have that luxury; referring
the user to another complex doc string generally raises the bar and
makes the documentation harder to use, because one needs to constantly
go back to the original function to figure out how the arguments
described in the other function are used in the original one.  It's
annoying.

bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Matt Armstrong]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Matt Armstrong <matt@rfc20.org>
>> Date: Tue, 30 Nov 2021 11:18:05 -0800
>> 
>> Attached is a docstring patch against the emacs-28 branch for this
>> issue.
>
> Thanks, but ... all these changes just to say that a list of strings
> can also be a COLLECTION?  This looks like a complete rewrite, and
> comparing it with the existing doc string is a lot of work.  Is the
> current doc string really that awful and requires such a complete
> rewrite?  Why can't we get away with a smaller, simpler change (that
> would be easier to review and approve)?

Thanks Eli, I can sympathize with respect to the review difficulty, but
I couldn't find a simple edit that did a good job.  It is more than just
"list of strings", but I did not realize when I filed the bug.  This
became clear only after I started reading code and trying to document
the current behavior.

Then I realized that the lisp reference manual is in good shape.  This
patch copies text almost verbatim from it.  So, the diff is both large
but hopefully not hard to verify in review.

Another issue is that these function provide extension points that pass
COLLECTION or elements from it directly to caller-supplied functions.
To write those functions correctly the programmer has to know all the
details of how the default C implementation works if the callbacks are
to work in a robust way.  In other words, this docstring is for
completion extension implementers as much as it is for callers.  I
figure an exhaustively pedantic approach is justifiable here.

One problem is the semantic complexity of how these functions interpret
COLLECTION.  They defy a simple and concise explanation.

E.g. COLLECTION can be...

 ...a list of strings
 ...a list of symbols
 ...an alist whose keys are strings
 ...an alist whose keys are symbols
 ...a list whose elements are a mixture of any of the above four
    possibilities.

And we have hash tables whose keys are strings, symbols, or a mix...

Also, any PREDICATE is passed the list elements (or hash key/values)
directly, without any symbol->string normalization.

Little of this is clear from the current docstrings, but the lisp ref
does a good job with it.


>> Note/question: I wonder if the `all-completions' docstring should refer
>> to `try-completion' rather than copy much of the text.  The same
>> approach works well in the lisp ref manual, I think.
>
> ELisp manual describes both one next to the other, so it can get away
> with such a technique.  Doc strings don't have that luxury; referring
> the user to another complex doc string generally raises the bar and
> makes the documentation harder to use, because one needs to constantly
> go back to the original function to figure out how the arguments
> described in the other function are used in the original one.  It's
> annoying.

That makes sense, thanks.

bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Eli Zaretskii]

> From: Matt Armstrong <matt@rfc20.org>
> Cc: 52202@debbugs.gnu.org
> Date: Tue, 30 Nov 2021 12:39:45 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Matt Armstrong <matt@rfc20.org>
> >> Date: Tue, 30 Nov 2021 11:18:05 -0800
> >> 
> >> Attached is a docstring patch against the emacs-28 branch for this
> >> issue.
> >
> > Thanks, but ... all these changes just to say that a list of strings
> > can also be a COLLECTION?  This looks like a complete rewrite, and
> > comparing it with the existing doc string is a lot of work.  Is the
> > current doc string really that awful and requires such a complete
> > rewrite?  Why can't we get away with a smaller, simpler change (that
> > would be easier to review and approve)?
> 
> Thanks Eli, I can sympathize with respect to the review difficulty, but
> I couldn't find a simple edit that did a good job.  It is more than just
> "list of strings", but I did not realize when I filed the bug.  This
> became clear only after I started reading code and trying to document
> the current behavior.

Can you list the aspects which the current doc string doesn't
describe, or describes inaccurately or incorrectly?

Thanks.

bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Matt Armstrong]

Eli Zaretskii <eliz@gnu.org> writes:

>> Thanks Eli, I can sympathize with respect to the review difficulty, but
>> I couldn't find a simple edit that did a good job.  It is more than just
>> "list of strings", but I did not realize when I filed the bug.  This
>> became clear only after I started reading code and trying to document
>> the current behavior.
>
> Can you list the aspects which the current doc string doesn't
> describe, or describes inaccurately or incorrectly?

Good question; maybe I went overboard.  Quotes below are from the
current docstring:

> If COLLECTION is an alist, the keys (cars of elements) are the
> possible completions.  If an element is not a cons cell, then the
> element itself is the possible completion.

I find this confusing because "If COLLECTION is an alist" strongly leads
the reader away from the idea that "COLLECTION may also be a list of
symbols/strings."  So perhaps replace this paragraph with slightly
edited text from the manual:

     If COLLECTION is a list (or alist), the permissible completions are
     specified by the elements of the list, each of which should be
     either a string, or a cons cell whose CAR is either a string or a
     symbol.  If the list contains elements of any other type, those are
     ignored.

> The argument given to PREDICATE is the alist element or the symbol
> from the obarray.

I found this confusing too, for the same reason.

So how about:

    The argument given to PREDICATE is either a string, symbol or a cons
    cell (the CAR of which is a string or symbol) from the list (or
    alist), or a symbol (_not_ a symbol name) from the obarray.  If
    COLLECTION is a hash table, PREDICATE is called with two arguments,
    the string or symbol key and the associated value.

These two blurbs are slight edits from the elisp manual.  I think this
new wording would be good in both the doc string and the manual.

If these smaller edits seem good I'm happy to reformulate the patch (I
can update the manual as well).

bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Lars Ingebrigtsen]

Matt Armstrong <matt@rfc20.org> writes:

> Then I realized that the lisp reference manual is in good shape.  This
> patch copies text almost verbatim from it.  So, the diff is both large
> but hopefully not hard to verify in review.

In these cases, we usually just say "See (elisp)Section" in the doc
strings, and leave the description of the complex data structures to the
manual.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52202: 29.0.50; try-completion, all-completions support for a list of strings COLLECTION is not documented in their docstrings

[Matt Armstrong]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Matt Armstrong <matt@rfc20.org> writes:
>
>> Then I realized that the lisp reference manual is in good shape.  This
>> patch copies text almost verbatim from it.  So, the diff is both large
>> but hopefully not hard to verify in review.
>
> In these cases, we usually just say "See (elisp)Section" in the doc
> strings, and leave the description of the complex data structures to
> the manual.

I can see it both ways.

Eli suggested that a complete docstring would be more handy (up thread),
and I now think this might be achievable with simpler edits, so absent a
different consensus I'll go that direction for now.