bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

* lisp/progmodes/etags.el (tags-complete-tags-table-file): Add docstring.
---
  lisp/progmodes/etags.el | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index a03516100087..398a315e3725 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1843,13 +1843,17 @@ tags-query-replace
     delimited)
    (fileloop-continue))
  
-(defun tags-complete-tags-table-file (string predicate what) ; Doc string?
+(defun tags-complete-tags-table-file (string predicate what)
+  "Complete file names in the current tags table.
+The meaning of the arguments are the same as the function form of
+COLLECTION as explained in Info node `(elisp) Programmed
+Completion'."
    (save-excursion
      ;; If we need to ask for the tag table, allow that.
      (let ((enable-recursive-minibuffers t))
        (visit-tags-table-buffer))
      (if (eq what t)
-       (all-completions string (tags-table-files) predicate)
+        (all-completions string (tags-table-files) predicate)
        (try-completion string (tags-table-files) predicate))))
  
  ;;;###autoload
-- 
2.20.1

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Eli Zaretskii]

> From: Hong Xu <hong@topbug.net>
> Date: Sat, 28 Sep 2019 00:59:21 -0700
> 
> * lisp/progmodes/etags.el (tags-complete-tags-table-file): Add docstring.

Thanks.  A comment below.

> -(defun tags-complete-tags-table-file (string predicate what) ; Doc string?
> +(defun tags-complete-tags-table-file (string predicate what)
> +  "Complete file names in the current tags table.
> +The meaning of the arguments are the same as the function form of
> +COLLECTION as explained in Info node `(elisp) Programmed
> +Completion'."

The first line of the doc string should mention the arguments.  And we
should explain STRING and WHAT there, because those aren't passed to
all-completions and try-completion.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

On 9/28/19 1:11 AM, Eli Zaretskii wrote:
>> From: Hong Xu <hong@topbug.net>
>> Date: Sat, 28 Sep 2019 00:59:21 -0700
>>
>> * lisp/progmodes/etags.el (tags-complete-tags-table-file): Add docstring.
> 
> Thanks.  A comment below.
> 
>> -(defun tags-complete-tags-table-file (string predicate what) ; Doc string?
>> +(defun tags-complete-tags-table-file (string predicate what)
>> +  "Complete file names in the current tags table.
>> +The meaning of the arguments are the same as the function form of
>> +COLLECTION as explained in Info node `(elisp) Programmed
>> +Completion'."
> 
> The first line of the doc string should mention the arguments.  And we
> should explain STRING and WHAT there, because those aren't passed to
> all-completions and try-completion.
> 

Please see my update below. I'm writing in this concise form because `(elisp) Programmed Completion' has a lot of explanation and I don't think we should repeat them here.

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index a03516100087..2770baa7067c 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1843,13 +1843,18 @@ tags-query-replace
     delimited)
    (fileloop-continue))
  
-(defun tags-complete-tags-table-file (string predicate what) ; Doc string?
+(defun tags-complete-tags-table-file (string predicate what)
+  "Complete STRING from file names in the current tags table.
+The meaning of the arguments are the same as the function form of
+COLLECTION as explained in Info node `(elisp) Programmed
+Completion', except that if WHAT does not equal t, it is treated
+as if it were nil."
    (save-excursion
      ;; If we need to ask for the tag table, allow that.
      (let ((enable-recursive-minibuffers t))
        (visit-tags-table-buffer))
      (if (eq what t)
-       (all-completions string (tags-table-files) predicate)
+        (all-completions string (tags-table-files) predicate)
        (try-completion string (tags-table-files) predicate))))
  
  ;;;###autoload
-- 
2.20.1

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Lars Ingebrigtsen]

Hong Xu <hong@topbug.net> writes:

> Please see my update below. I'm writing in this concise form because `(elisp) Programmed Completion' has a lot of explanation and I don't think we should repeat them here.

Looks good to me.  I've now applied it to Emacs 27.

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

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  37538@debbugs.gnu.org
> Date: Mon, 07 Oct 2019 06:13:54 +0200
> 
> Hong Xu <hong@topbug.net> writes:
> 
> > Please see my update below. I'm writing in this concise form because `(elisp) Programmed Completion' has a lot of explanation and I don't think we should repeat them here.
> 
> Looks good to me.  I've now applied it to Emacs 27.

The doc string talks about WHAT and refers to the ELisp manual, but
that node in the manual doesn't mention anything called WHAT.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

On 10/7/19 9:18 AM, Eli Zaretskii wrote:
>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  37538@debbugs.gnu.org
>> Date: Mon, 07 Oct 2019 06:13:54 +0200
>>
>> Hong Xu <hong@topbug.net> writes:
>>
>>> Please see my update below. I'm writing in this concise form because `(elisp) Programmed Completion' has a lot of explanation and I don't think we should repeat them here.
>>
>> Looks good to me.  I've now applied it to Emacs 27.
> 
> The doc string talks about WHAT and refers to the ELisp manual, but
> that node in the manual doesn't mention anything called WHAT.
> 

Would the following docstring work for you (will be reformatted)? Because the function accepts the same arguments as the one documented in `(elisp) Programmed Completion', I thought it should be understandable that WHAT refers to the third argument.

The meaning of the arguments are the same as the function form of
COLLECTION as explained in Info node `(elisp) Programmed
Completion', except that if the third parameter (WHAT) does not equal t, it is treated
as if it were nil.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Lars Ingebrigtsen]

Hong Xu <hong@topbug.net> writes:

> Would the following docstring work for you (will be reformatted)?
> Because the function accepts the same arguments as the one documented
> in `(elisp) Programmed Completion', I thought it should be
> understandable that WHAT refers to the third argument.
>
> The meaning of the arguments are the same as the function form of
> COLLECTION as explained in Info node `(elisp) Programmed
> Completion', except that if the third parameter (WHAT) does not equal
> t, it is treated
> as if it were nil.

The problem is that `(elisp) Programmed Completion' (at least in Emacs
27) doesn't mention a WHAT parameter at all, so it's unclear what this
refers to.

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

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

On 10/8/19 9:21 AM, Lars Ingebrigtsen wrote:
> 
> The problem is that `(elisp) Programmed Completion' (at least in Emacs
> 27) doesn't mention a WHAT parameter at all, so it's unclear what this
> refers to.
> 


How about the following change?

diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index a9d6e83cf854..73af67ab083f 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -1769,7 +1769,8 @@ Programmed Completion
  completion functions, which will then let your function do all
  the work.
  
-  The completion function should accept three arguments:
+  The completion function should accept three arguments in the
+following order:
  
  @itemize @bullet
  @item
diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 6784894ba87e..d58e2cb2a187 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1853,8 +1853,8 @@ tags-complete-tags-table-file
    "Complete STRING from file names in the current tags table.
  The meaning of the arguments are the same as the function form of
  COLLECTION as explained in Info node `(elisp) Programmed
-Completion', except that if WHAT does not equal t, it is treated
-as if it were nil."
+Completion', except that if the third argument does not equal t,
+it is treated as if it were nil."
    (save-excursion
      ;; If we need to ask for the tag table, allow that.
      (let ((enable-recursive-minibuffers t))

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, 37538@debbugs.gnu.org
> From: Hong Xu <hong@topbug.net>
> Date: Tue, 8 Oct 2019 23:39:08 -0700
> 
> On 10/8/19 9:21 AM, Lars Ingebrigtsen wrote:
> > 
> > The problem is that `(elisp) Programmed Completion' (at least in Emacs
> > 27) doesn't mention a WHAT parameter at all, so it's unclear what this
> > refers to.
> 
> How about the following change?

I don't see how it solves the problem, sorry.  It attempts to assign
some significance to the ordinal number of the argument WHAT, in the
hope that the reader will be able to connect the dots.  But a good
documentation should not leave the dots unconnected, it should spell
them out, ideally in one place.

I actually don't understand why we would like to send the reader to
the manual, instead of describing the effects of WHAT in the doc
string.  Can we just say it right there?

Thanks.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

On 10/9/19 1:09 AM, Eli Zaretskii wrote:
>> Cc: Eli Zaretskii <eliz@gnu.org>, 37538@debbugs.gnu.org
>> From: Hong Xu <hong@topbug.net>
>> Date: Tue, 8 Oct 2019 23:39:08 -0700
>>
>> On 10/8/19 9:21 AM, Lars Ingebrigtsen wrote:
>>>
>>> The problem is that `(elisp) Programmed Completion' (at least in Emacs
>>> 27) doesn't mention a WHAT parameter at all, so it's unclear what this
>>> refers to.
>>
>> How about the following change?
> 
> I don't see how it solves the problem, sorry.  It attempts to assign
> some significance to the ordinal number of the argument WHAT, in the
> hope that the reader will be able to connect the dots.  But a good
> documentation should not leave the dots unconnected, it should spell
> them out, ideally in one place.
> 
> I actually don't understand why we would like to send the reader to
> the manual, instead of describing the effects of WHAT in the doc
> string.  Can we just say it right there?
> 

The description was quite long and I don't think it is justifiable to copy so much text over here to the docstring, plus there are additional reference in the referred manual section.

IMO the docstring here really tells the reader that this is a standard completion function -- I doubt anyone would consult this docstring to call `tags-complete-tags-table-file', but most likely want to understand that this is a completion function so that they can better understand the codebase. To read the referred portion of the manual, for the purpose of understanding the codebase, is a somewhat inevitable task by itself.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Eli Zaretskii]

> Cc: larsi@gnus.org, 37538@debbugs.gnu.org
> From: Hong Xu <hong@topbug.net>
> Date: Wed, 9 Oct 2019 15:58:46 -0700
> 
> > I actually don't understand why we would like to send the reader to
> > the manual, instead of describing the effects of WHAT in the doc
> > string.  Can we just say it right there?
> > 
> 
> The description was quite long and I don't think it is justifiable to copy so much text over here to the docstring, plus there are additional reference in the referred manual section.

If you show me what long description you had in mind, I could try
saying that more concisely as appropriate for a doc string.

> IMO the docstring here really tells the reader that this is a standard completion function -- I doubt anyone would consult this docstring to call `tags-complete-tags-table-file', but most likely want to understand that this is a completion function so that they can better understand the codebase. To read the referred portion of the manual, for the purpose of understanding the codebase, is a somewhat inevitable task by itself.

There are different use cases for reading the doc string.  you seem to
be thinking of only one: the first-time reading by someone who has
never used this function or any of the other completion APIs.

But there's also another, no less important use case: a reading by
someone who already knows most of this stuff, but just doesn't
remember which argument is which, and in what order to specify them.
For these users, a short description of each argument is all that's
needed.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

On 10/10/19 12:49 AM, Eli Zaretskii wrote:
>> Cc: larsi@gnus.org, 37538@debbugs.gnu.org
>> From: Hong Xu <hong@topbug.net>
>> Date: Wed, 9 Oct 2019 15:58:46 -0700
>>
>>> I actually don't understand why we would like to send the reader to
>>> the manual, instead of describing the effects of WHAT in the doc
>>> string.  Can we just say it right there?
>>>
>>
>> The description was quite long and I don't think it is justifiable to copy so much text over here to the docstring, plus there are additional reference in the referred manual section.
> 
> If you show me what long description you had in mind, I could try
> saying that more concisely as appropriate for a doc string.
> 

This snippet in Programmed Completion section:

    • A flag specifying the type of completion operation to perform; see
      *note Basic Completion::, for the details of those operations.
      This flag may be one of the following values.

      ‘nil’
           This specifies a ‘try-completion’ operation.  The function
           should return ‘nil’ if there are no matches; it should return
           ‘t’ if the specified string is a unique and exact match; and
           it should return the longest common prefix substring of all
           matches otherwise.

      ‘t’
           This specifies an ‘all-completions’ operation.  The function
           should return a list of all possible completions of the
           specified string.

      [other kinds of values]

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Eli Zaretskii]

> Cc: larsi@gnus.org, 37538@debbugs.gnu.org
> From: Hong Xu <hong@topbug.net>
> Date: Fri, 11 Oct 2019 17:48:16 -0700
> 
> >> The description was quite long and I don't think it is justifiable to copy so much text over here to the docstring, plus there are additional reference in the referred manual section.
> > 
> > If you show me what long description you had in mind, I could try
> > saying that more concisely as appropriate for a doc string.
> > 
> 
> This snippet in Programmed Completion section:
> 
>     • A flag specifying the type of completion operation to perform; see
>       *note Basic Completion::, for the details of those operations.
>       This flag may be one of the following values.
> 
>       ‘nil’
>            This specifies a ‘try-completion’ operation.  The function
>            should return ‘nil’ if there are no matches; it should return
>            ‘t’ if the specified string is a unique and exact match; and
>            it should return the longest common prefix substring of all
>            matches otherwise.
> 
>       ‘t’
>            This specifies an ‘all-completions’ operation.  The function
>            should return a list of all possible completions of the
>            specified string.

How about the below:

  (defun tags-complete-tags-table-file (string predicate what)
    "Complete STRING from file names in the current tags table.
  PREDICATE, if non-nil is a function to filter possible matches:
  if it returns nil, the match is ignored.  If PREDICATE is nil,
  every possible match is acceptable.
  WHAT is a flag specifying the type of completion: t means `all-completions'
  operation, any other value means `try-completions' operation.

  This function serves as COLLECTION argument to `completing-read',
  see the Info node `(elisp) Programmed Completion' for more detailed
  description of the arguments."

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Hong Xu]

Eli Zaretskii <eliz@gnu.org> writes:
> How about the below:
>
>   (defun tags-complete-tags-table-file (string predicate what)
>     "Complete STRING from file names in the current tags table.
>   PREDICATE, if non-nil is a function to filter possible matches:
>   if it returns nil, the match is ignored.  If PREDICATE is nil,
>   every possible match is acceptable.
>   WHAT is a flag specifying the type of completion: t means `all-completions'
>   operation, any other value means `try-completions' operation.
>
>   This function serves as COLLECTION argument to `completing-read',
>   see the Info node `(elisp) Programmed Completion' for more detailed
>   description of the arguments."

Looks good to me. Thanks.

bug#37538: [PATCH] Add docstring for `tags-complete-tags-table-file'.

[Eli Zaretskii]

> From: Hong Xu <hong@topbug.net>
> Cc: larsi@gnus.org,  37538@debbugs.gnu.org
> Date: Sat, 12 Oct 2019 11:46:25 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > How about the below:
> >
> >   (defun tags-complete-tags-table-file (string predicate what)
> >     "Complete STRING from file names in the current tags table.
> >   PREDICATE, if non-nil is a function to filter possible matches:
> >   if it returns nil, the match is ignored.  If PREDICATE is nil,
> >   every possible match is acceptable.
> >   WHAT is a flag specifying the type of completion: t means `all-completions'
> >   operation, any other value means `try-completions' operation.
> >
> >   This function serves as COLLECTION argument to `completing-read',
> >   see the Info node `(elisp) Programmed Completion' for more detailed
> >   description of the arguments."
> 
> Looks good to me. Thanks.

Thanks, I installed it.