GNU bug report logs - #31962
27.0; imenu.el constructs - names and doc

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31962 in the body.
You can then email your comments to 31962 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0; imenu.el constructs - names and doc
Date: Sun, 24 Jun 2018 18:51:23 -0700 (PDT)

1. `imenu--generic-function' should not have a double-hyphen in its
name.  It is the main function in the library, and it is explicitly
referenced by multiple doc strings in the file, including that of a user
option (`imenu-generic-skip-comments-and-strings') and that of the
principal configuration variable (`imenu-generic-expression'), which is
defined per mode.  The function should be renamed to
`imenu-generic-function', and the old name should be deprecated.

2. The doc string of `imenu--generic-function' says this, with zero
description of INDEX-NAME INDEX-POSITION FUNCTION and ARGUMENTS:

  The return value is an alist of the form
   (INDEX-NAME . INDEX-POSITION)
  or
   (INDEX-NAME INDEX-POSITION FUNCTION ARGUMENTS...)
  The return value may also consist of nested index alists like:
   (INDEX-NAME . INDEX-ALIST)
  depending on PATTERNS.

Each of those pieces of the return value should be specified, not just
named.

3. Similarly, other functions and variables should be renamed to use a
single, not a double, hyphen, starting with `imenu--sort-by-name' and
`imenu--index-alist'.  I don't argue now that NO functions wnd variables
in the library should have names declaring that they are internal, but a
better judgment call is in order.

They too are referenced from other doc strings.  For example, the doc
string of `imenu-sort-function' says to look at `imenu--sort-by-name'
for an example of a sort function.  And the doc string of autoloaded
variable `imenu-create-index-function' refers to `imenu--index-alist'
for the format of the buffer index alist.

We are documenting the structure and behavior of Imenu by referring to
"internal" variables and functions.  That makes no sense - or rather, it
points out how silly was declaring these to be "internal".



In GNU Emacs 27.0.50 (build 3, x86_64-w64-mingw32)
 of 2018-03-21
Repository revision: e70d0c9e66d7a8609450b2889869d16aeb0363b5
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install -C 'CFLAGS=-O2 -static -g3''

Message #8 received at 31962 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 31962 <at> debbugs.gnu.org
Subject: Re: bug#31962: 27.0; imenu.el constructs - names and doc
Date: Sat, 30 Jun 2018 12:28:24 +0300

> Date: Sun, 24 Jun 2018 18:51:23 -0700 (PDT)
> From: Drew Adams <drew.adams <at> oracle.com>
> 
> 1. `imenu--generic-function' should not have a double-hyphen in its
> name.  It is the main function in the library, and it is explicitly
> referenced by multiple doc strings in the file

As long as this function is not autoloaded, we have no unequivocal
proof that it's the main function in the library.  A function that is
the workhorse of a library can legitimately be an internal function.

I agree that this case is somewhat borderline, but I don't have enough
time and energy to make the changes in the current system, because the
problems it creates sound very minor to me.

I made small fixes in 2 doc strings, but that's as far as I was
prepared to go in this case.

> including that of a user option
> (`imenu-generic-skip-comments-and-strings') and that of the
> principal configuration variable (`imenu-generic-expression'), which
> is defined per mode.

The doc string of imenu-generic-skip-comments-and-strings was one of
those I changed.

> 2. The doc string of `imenu--generic-function' says this, with zero
> description of INDEX-NAME INDEX-POSITION FUNCTION and ARGUMENTS:
> 
>   The return value is an alist of the form
>    (INDEX-NAME . INDEX-POSITION)
>   or
>    (INDEX-NAME INDEX-POSITION FUNCTION ARGUMENTS...)
>   The return value may also consist of nested index alists like:
>    (INDEX-NAME . INDEX-ALIST)
>   depending on PATTERNS.
> 
> Each of those pieces of the return value should be specified, not just
> named.

Not as long as this is an internal function.

> 3. Similarly, other functions and variables should be renamed to use a
> single, not a double, hyphen, starting with `imenu--sort-by-name' and
> `imenu--index-alist'.  I don't argue now that NO functions wnd variables
> in the library should have names declaring that they are internal, but a
> better judgment call is in order.

Likewise.

> We are documenting the structure and behavior of Imenu by referring to
> "internal" variables and functions.  That makes no sense - or rather, it
> points out how silly was declaring these to be "internal".

Please don't use "silly" to describe work of others, as doing so makes
the discussion much less pleasant.

Message #13 received at 31962-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 31962-done <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#31962: 27.0; imenu.el constructs - names and doc
Date: Sat, 19 Oct 2019 03:09:14 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sun, 24 Jun 2018 18:51:23 -0700 (PDT)
>> From: Drew Adams <drew.adams <at> oracle.com>
>>
>> 1. `imenu--generic-function' should not have a double-hyphen in its
>> name.  It is the main function in the library, and it is explicitly
>> referenced by multiple doc strings in the file
>
> As long as this function is not autoloaded, we have no unequivocal
> proof that it's the main function in the library.  A function that is
> the workhorse of a library can legitimately be an internal function.
>
> I agree that this case is somewhat borderline, but I don't have enough
> time and energy to make the changes in the current system, because the
> problems it creates sound very minor to me.
>
> I made small fixes in 2 doc strings, but that's as far as I was
> prepared to go in this case.
>
>> including that of a user option
>> (`imenu-generic-skip-comments-and-strings') and that of the
>> principal configuration variable (`imenu-generic-expression'), which
>> is defined per mode.
>
> The doc string of imenu-generic-skip-comments-and-strings was one of
> those I changed.
>
>> 2. The doc string of `imenu--generic-function' says this, with zero
>> description of INDEX-NAME INDEX-POSITION FUNCTION and ARGUMENTS:
>>
>>   The return value is an alist of the form
>>    (INDEX-NAME . INDEX-POSITION)
>>   or
>>    (INDEX-NAME INDEX-POSITION FUNCTION ARGUMENTS...)
>>   The return value may also consist of nested index alists like:
>>    (INDEX-NAME . INDEX-ALIST)
>>   depending on PATTERNS.
>>
>> Each of those pieces of the return value should be specified, not just
>> named.
>
> Not as long as this is an internal function.
>
>> 3. Similarly, other functions and variables should be renamed to use a
>> single, not a double, hyphen, starting with `imenu--sort-by-name' and
>> `imenu--index-alist'.  I don't argue now that NO functions wnd variables
>> in the library should have names declaring that they are internal, but a
>> better judgment call is in order.
>
> Likewise.

It looks like Eli made the necessary changes above, and I see nothing
more to do here.  I'm therefore closing this bug report.  Please reopen
if that's incorrect.

Best regards,
Stefan Kangas

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.