bug#24842: 24.5; `help-make-xrefs': False link-type determinations

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

`help-make-xrefs' tries to guess the type of a symbol that is enclosed
in `...', in help.

This determination needs to to be improved, or else programmers need
some way to inhibit it when it turns out to be inappropriate.

Example: This is part of a doc string I have:

`isearchp-movement-unit-alist'\t- units and their movement functions
`isearchp-on-demand-action-function'\t- on-demand action function
`isearchp-prompt-for-filter-name'\t- when to ask for filter name

Those symbols are all variables (in fact, user options), not functions.
but the word "function" at the end of the second line causes the quoted
symbol that follows it to be considered as a function name.  And then,
because it is not `fboundp', it gets no link.  It should instead get a
variable-name link to its definition.

1. One possible approach is to improve the behavior.  I have not tested
   this generally, but changing the `cond' clauses to include the test
   for type and for documentation as part of the `cond' test, instead of
   being part of the result clause after the test, fixes at least the
   immediate problem of the above example.

   For example, instead of:

   ((match-string 4)   ; `function' &c
    (and (fboundp sym) ; similarly
         (help-xref-button 8 'help-function sym)))

   use this:

   ((and (match-string 4) ; `function' &c
         (fboundp sym))   ; similarly
    (help-xref-button 8 'help-function sym))

   Similarly for the other clauses.

   Possibly this does not DTRT generally, since it lets subsequent
   `cond' clauses get tested if the code cannot determine that the
   symbol is a variable, function, etc.

   But that's the point of those clauses.  If they cannot find that some
   symbol is a variable, etc., then shouldn't the subsequent clauses be
   tried?  (The tests might need to be improved.  Dunno.)

   Again, I have not tested this, so I don't propose an exact code fix.
   I'm reporting the bug.

2. An alternative, if you don't find that #1 is reasonable, could be to
   explicitly provide for (and document) some way for a programmer to
   "escape" a given quoted symbol from a "determiner" such as "function"
   that immediately precedes it.

   The regexp used now looks for whitespace after the "determiner": any
   combination of space, tab, and newline chars.

   I can insert a NULL char, x00, after "function", and that "fixes"
   things, but that char then appears in the output, as "^@".  Another
   Unicode whitespace char could be used instead, etc.  But that still
   means another character that gets rendered, increasing the line
   length, etc.

   If #2 is chosen, the solution should hopefully be more robust and
   cleaner that such a hack.

I hope you will consider fixing this in some way.


In GNU Emacs 24.5.1 (i686-pc-mingw32)
 of 2015-04-11 on LEG570
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/usr --host=i686-pc-mingw32'

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[npostavs]

Drew Adams <drew.adams@oracle.com> writes:
>    I can insert a NULL char, x00, after "function", and that "fixes"
>    things, but that char then appears in the output, as "^@".  Another
>    Unicode whitespace char could be used instead, etc.  But that still
>    means another character that gets rendered, increasing the line
>    length, etc.

What about ZERO WIDTH SPACE?

`isearchp-movement-unit-alist'\t- units and their movement functions\u200B
`isearchp-on-demand-action-function'\t- on-demand action function\u200B
`isearchp-prompt-for-filter-name'\t- when to ask for filter name\u200B

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> What about ZERO WIDTH SPACE?
> 
> `isearchp-movement-unit-alist'\t- units and their movement
> functions\u200B
> `isearchp-on-demand-action-function'\t- on-demand action
> function\u200B
> `isearchp-prompt-for-filter-name'\t- when to ask for filter
> name\u200B

Yes, I thought there was such a character, and it could be
used.  That is a reasonable workaround, I guess, for a
programmer to override inappropriate behavior that can
occur now.

I should have separated #1 and #2 not as alternatives, but
as two approaches that should both, I think, be taken.

Do you think the code as written in help-mode.el is correct?
Do you not think that the cond clauses are just wrong?

Note that only a couple of the cond clauses have the weird
form of putting part of the test for type in the result,
and not in the test part.

The other clauses have, as one might expect, all of the
testing in the condition part, and only the `help-xref-button'
call in the result part.

(That doesn't prove that the same approach should be taken
for all of them.  But it does suggest a possible miscoding.)

I think it would be good to take a close look at this.

And also to document the workaround, letting people know
of a _recommended_ way to override the default behavior.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Eli Zaretskii]

severity 12016 minor
severity 24842 minor
merge 9445 12016 24842
thanks

Duplicate of 12016 and 9445.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Noam Postavsky]

On Tue, Nov 1, 2016 at 11:20 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> severity 12016 minor
> severity 24842 minor
> merge 9445 12016 24842
> thanks
>
> Duplicate of 12016 and 9445.

I don't think #12016 (ff-find-other-file is not considering
cc-other-file-alist for .cxx files) is related, typo?

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Eli Zaretskii]

unmerge 12016
severity 12016 normal
severity 12106 minor
merge 12106 9445
thanks

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Tue, 1 Nov 2016 11:34:36 -0400
> Cc: Drew Adams <drew.adams@oracle.com>, 24842@debbugs.gnu.org
> 
> > Duplicate of 12016 and 9445.
> 
> I don't think #12016 (ff-find-other-file is not considering
> cc-other-file-alist for .cxx files) is related, typo?

Oops.  It's 12106, now fixed.

Thanks.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> severity 12016 minor
> severity 24842 minor
> merge 9445 12016 24842
> thanks
> 
> Duplicate of 12016 and 9445.

Not really a duplicate, IMO.

One of the things #24842 asks for is _documentation_ for users writing doc
strings:

1. Tell them about the behavior - preceding quoted symbols with "function" etc.
2. Tell them how to override the behavior when inappropriate, effectively
   "escaping" an occurrence of a word such as "function" that would otherwise
   cause the symbol that follows it to be handled incorrectly.

Do you need a separate bug report for just this part (doc)?

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Eli Zaretskii]

> Date: Tue, 1 Nov 2016 09:15:25 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: npostavs@users.sourceforge.net, 24842@debbugs.gnu.org
> 
> > Duplicate of 12016 and 9445.
> 
> Not really a duplicate, IMO.
> 
> One of the things #24842 asks for is _documentation_ for users writing doc
> strings:
> 
> 1. Tell them about the behavior - preceding quoted symbols with "function" etc.
> 2. Tell them how to override the behavior when inappropriate, effectively
>    "escaping" an occurrence of a word such as "function" that would otherwise
>    cause the symbol that follows it to be handled incorrectly.
> 
> Do you need a separate bug report for just this part (doc)?

No need for a separate report, because I still think these 3 reports
basically talk about the same issue.

I also don't think these issues will be ever fixed unless someone
motivated takes upon themselves to clean up this area, which IMO is
too full of semi-kludges and heuristics that only works 90% of the
time.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> unmerge 12016
> severity 12016 normal
> severity 12106 minor
> merge 12106 9445
> thanks
> 
> > > Duplicate of 12016 and 9445.
> >
> > I don't think #12016 (ff-find-other-file is not considering
> > cc-other-file-alist for .cxx files) is related, typo?
> 
> Oops.  It's 12106, now fixed.

12106 is not really a duplicate either, IMO.  It's about preventing
a link that would be inappropriate.  24842 is about ensuring a link -
preventing a non-link that would be inappropriate.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Eli Zaretskii]

> Date: Tue, 1 Nov 2016 09:29:08 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 24842@debbugs.gnu.org
> 
> > unmerge 12016
> > severity 12016 normal
> > severity 12106 minor
> > merge 12106 9445
> > thanks
> > 
> > > > Duplicate of 12016 and 9445.
> > >
> > > I don't think #12016 (ff-find-other-file is not considering
> > > cc-other-file-alist for .cxx files) is related, typo?
> > 
> > Oops.  It's 12106, now fixed.
> 
> 12106 is not really a duplicate either, IMO.  It's about preventing
> a link that would be inappropriate.  24842 is about ensuring a link -
> preventing a non-link that would be inappropriate.

The underlying issue is the same.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> 1. One possible approach is to improve the behavior.  I have not tested
>    this generally, but changing the `cond' clauses to include the test
>    for type and for documentation as part of the `cond' test, instead of
>    being part of the result clause after the test, fixes at least the
>    immediate problem of the above example.
> 
>    For example, instead of:
> 
>    ((match-string 4)   ; `function' &c
>     (and (fboundp sym) ; similarly
>          (help-xref-button 8 'help-function sym)))
> 
>    use this:
> 
>    ((and (match-string 4) ; `function' &c
>          (fboundp sym))   ; similarly
>     (help-xref-button 8 'help-function sym))
> 
>    Similarly for the other clauses.
> 
>    Possibly this does not DTRT generally, since it lets subsequent
>    `cond' clauses get tested if the code cannot determine that the
>    symbol is a variable, function, etc.
> 
>    But that's the point of those clauses.  If they cannot find that some
>    symbol is a variable, etc., then shouldn't the subsequent clauses be
>    tried?  (The tests might need to be improved.  Dunno.)
> 
>    Again, I have not tested this, so I don't propose an exact code fix.
>    I'm reporting the bug.

I've been using this for a while, and it seems to be an improvement.
I have not noticed any bad cases.  Can you please consider making
this change?  I think the logic is better.  Or can you point to a
case where it is not an improvement?

Thx.

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> I've been using this for a while, and it seems to be an improvement.
> I have not noticed any bad cases.  Can you please consider making
> this change?  I think the logic is better.  Or can you point to a
> case where it is not an improvement?
> 
> Thx.

ping

bug#24842: bug#9445: 24.0.50; `help-make-xrefs' has bad logic

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> `help-make-xrefs' tries to guess the type of a symbol that is enclosed
> in `...', in help.
>
> This determination needs to to be improved, or else programmers need
> some way to inhibit it when it turns out to be inappropriate.
>
> Example: This is part of a doc string I have:
>
> `isearchp-movement-unit-alist'\t- units and their movement functions
> `isearchp-on-demand-action-function'\t- on-demand action function
> `isearchp-prompt-for-filter-name'\t- when to ask for filter name
>
> Those symbols are all variables (in fact, user options), not functions.
> but the word "function" at the end of the second line causes the quoted
> symbol that follows it to be considered as a function name.  And then,
> because it is not `fboundp', it gets no link.  It should instead get a
> variable-name link to its definition.

There wasn't any recipe for reproduction here, so I came up with this:

(defun foo1 ())
(defvar foo2 ())

(defvar foo nil
  "
`foo1' something.
`foo1'\t- units and their movement functions
`foo2' something.
`foo2'\t- units and their movement functions
")

When evaling this and then `C-h v foo RET' in Emacs 28, all these are
recognised as symbols in the *Help* buffer, so this seems to work for me
in this test case, at least.

Are you still seeing this issue in recent Emacs versions?

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

bug#24842: [External] : Re: bug#9445: 24.0.50; `help-make-xrefs' has bad logic

[Drew Adams]

> Are you still seeing this issue in recent Emacs versions?

Thanks for testing.  This was apparently fixed in release 24.
The bug was filed in a snapshot between 23 and 24.  (The bug
still exists in 23.)

So this can be closed.  Thx.

(I can't speak to the merges with other bugs, however.
Maybe they should not all be closed.)

bug#24842: [External] : Re: bug#9445: 24.0.50; `help-make-xrefs' has bad logic

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

>> Are you still seeing this issue in recent Emacs versions?
>
> Thanks for testing.  This was apparently fixed in release 24.
> The bug was filed in a snapshot between 23 and 24.  (The bug
> still exists in 23.)
>
> So this can be closed.  Thx.

Thanks; closing this, then.

> (I can't speak to the merges with other bugs, however.
> Maybe they should not all be closed.)

Yeah, they aren't about the same thing as this one, so I've now
de-merged them.

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

bug#9445: bug#24842: bug#9445: 24.0.50; `help-make-xrefs' has bad logic

[Stephen Berman]

On Tue, 07 Sep 2021 20:00:20 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Drew Adams <drew.adams@oracle.com> writes:
>
>> `help-make-xrefs' tries to guess the type of a symbol that is enclosed
>> in `...', in help.
>>
>> This determination needs to to be improved, or else programmers need
>> some way to inhibit it when it turns out to be inappropriate.
>>
>> Example: This is part of a doc string I have:
>>
>> `isearchp-movement-unit-alist'\t- units and their movement functions
>> `isearchp-on-demand-action-function'\t- on-demand action function
>> `isearchp-prompt-for-filter-name'\t- when to ask for filter name
>>
>> Those symbols are all variables (in fact, user options), not functions.
>> but the word "function" at the end of the second line causes the quoted
>> symbol that follows it to be considered as a function name.  And then,
>> because it is not `fboundp', it gets no link.  It should instead get a
>> variable-name link to its definition.
>
> There wasn't any recipe for reproduction here, so I came up with this:
>
> (defun foo1 ())
> (defvar foo2 ())
>
> (defvar foo nil
>   "
> `foo1' something.
> `foo1'\t- units and their movement functions
> `foo2' something.
> `foo2'\t- units and their movement functions
> ")
>
> When evaling this and then `C-h v foo RET' in Emacs 28, all these are
> recognised as symbols in the *Help* buffer, so this seems to work for me
> in this test case, at least.
>
> Are you still seeing this issue in recent Emacs versions?

On Tue, 07 Sep 2021 21:28:22 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Drew Adams <drew.adams@oracle.com> writes:
>
>>> Are you still seeing this issue in recent Emacs versions?
>>
>> Thanks for testing.  This was apparently fixed in release 24.
>> The bug was filed in a snapshot between 23 and 24.  (The bug
>> still exists in 23.)
>>
>> So this can be closed.  Thx.
>
> Thanks; closing this, then.

But the recipe Lars gave doesn't quite match the description Drew gave,
IIUC: if in the recipe you change the first occurrence of `functions' to
`function', then eval and do `C-h v foo', then the first occurrence of
`foo2' in the doc string is not turned into a link.  Isn't that what the
problem was, and still is?

Steve Berman

bug#9445: bug#24842: bug#9445: 24.0.50; `help-make-xrefs' has bad logic

[Lars Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

> But the recipe Lars gave doesn't quite match the description Drew gave,
> IIUC: if in the recipe you change the first occurrence of `functions' to
> `function', then eval and do `C-h v foo', then the first occurrence of
> `foo2' in the doc string is not turned into a link.  Isn't that what the
> problem was, and still is?

Yup.  So here's the recipe to reproduce the problem:

(defun foo1 ())
(defvar foo2 ())

(defvar foo nil
  "
`foo1'\t- on-demand action function
`foo2'")

So I'm reopening the bug report.

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

bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yup.  So here's the recipe to reproduce the problem:
>
> (defun foo1 ())
> (defvar foo2 ())
>
> (defvar foo nil
>   "
> `foo1'\t- on-demand action function
> `foo2'")
>
> So I'm reopening the bug report.

But looking at this thing again, the DWIM code trying to parse doc
strings and adding links just can't get this case right.

It's, reasonably enough, interpreting "function `foo2'" as a function
(even if it's on two lines), and then seeing that it isn't, so it
doesn't add a link.  I think the answer here "don't do that, then" --
i.e., you have to write this in a different way to help the DWIM logic.

So I'm re-closing this as a "wontfix".

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

bug#24842: [External] : Re: bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> > Yup.  So here's the recipe to reproduce the problem:
> >
> > (defun foo1 ())
> > (defvar foo2 ())
> >
> > (defvar foo nil
> >   "
> > `foo1'\t- on-demand action function
> > `foo2'")
> >
> > So I'm reopening the bug report.
> 
> But looking at this thing again, the DWIM code trying to parse doc
> strings and adding links just can't get this case right.
> 
> It's, reasonably enough, interpreting "function `foo2'" as a function
> (even if it's on two lines), and then seeing that it isn't, so it
> doesn't add a link.  I think the answer here "don't do that, then" --
> i.e., you have to write this in a different way to help the DWIM logic.
> 
> So I'm re-closing this as a "wontfix".

That's just saying that you haven't found an
easy solution.  It's not saying that there's
no bug or that it can't be fixed.

I think there's more in this bug thread than
your latest recipe for reproducing what you
think the problem is.  But I won't insist on
that perception.

The priority shouldn't be to close bugs.  It
should be to report bugs, so they can perhaps
be fixed by someone who has the interest and
energy.

bug#24842: [External] : Re: bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> Date: Tue, 15 Feb 2022 16:58:01 +0000
> Cc: "9445@debbugs.gnu.org" <9445@debbugs.gnu.org>,
>  "24842@debbugs.gnu.org" <24842@debbugs.gnu.org>
> 
> The priority shouldn't be to close bugs.  It
> should be to report bugs, so they can perhaps
> be fixed by someone who has the interest and
> energy.

It makes no sense to keep bugs open for years without anyone picking
up the gauntlet.  After so many years it is quite reasonable to assume
that no one is interested enough to work on this.

bug#24842: [External] : Re: bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Drew Adams]

> > The priority shouldn't be to close bugs.  It
> > should be to report bugs, so they can perhaps
> > be fixed by someone who has the interest and
> > energy.
> 
> It makes no sense to keep bugs open for years without anyone picking
> up the gauntlet.  After so many years it is quite reasonable to assume
> that no one is interested enough to work on this.

And the reason is?  No reason given.

Just what is the cost of leaving confirmed bugs
open?  Just what is the benefit of closing them
(apart perhaps from showing fewer in a report)?

You might have some good reasons.  So far, none
have been presented.

"It makes no sense" and "it is quite reasonable",
themselves make little sense with nothing
presented to back them up.

bug#24842: [External] : Re: bug#24842: 24.5; `help-make-xrefs': False link-type determinations

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "larsi@gnus.org" <larsi@gnus.org>,
>         "stephen.berman@gmx.net"
> 	<stephen.berman@gmx.net>,
>         "9445@debbugs.gnu.org" <9445@debbugs.gnu.org>,
>         "24842@debbugs.gnu.org" <24842@debbugs.gnu.org>
> Date: Tue, 15 Feb 2022 19:22:00 +0000
> Accept-Language: en-US
> 
> > > The priority shouldn't be to close bugs.  It
> > > should be to report bugs, so they can perhaps
> > > be fixed by someone who has the interest and
> > > energy.
> > 
> > It makes no sense to keep bugs open for years without anyone picking
> > up the gauntlet.  After so many years it is quite reasonable to assume
> > that no one is interested enough to work on this.
> 
> And the reason is?  No reason given.

I just gave it, above.

> Just what is the cost of leaving confirmed bugs
> open?

It's a cost to the project maintenance to have open bugs that no one
works on.

> Just what is the benefit of closing them
> (apart perhaps from showing fewer in a report)?

The benefit is to have a manageable number of bugs that we really want
to fix.

> "It makes no sense" and "it is quite reasonable",
> themselves make little sense

Perhaps for you.  But this is not your call and not your
responsibility.

bug#24842: [External] : Re: bug#24842: bug#9445: 24.0.50; `help-make-xrefs' has bad logic

[Drew Adams]

> > (defun foo1 ())
> > (defvar foo2 ())
> >
> > (defvar foo nil
> >   "
> > `foo1' something.
> > `foo1'\t- units and their movement functions
> > `foo2' something.
> > `foo2'\t- units and their movement functions
> > ")
> >
> But the recipe Lars gave doesn't quite match the description Drew gave,
> IIUC: if in the recipe you change the first occurrence of `functions'
> to
> `function', then eval and do `C-h v foo', then the first occurrence of
> `foo2' in the doc string is not turned into a link.  Isn't that what
> the
> problem was, and still is?

Hm.  You're right, Stephen.  But not with just
that change, I think.  And it looks like the
problem is maybe for defun doc strings but not
for defvar doc strings?  (But see below.)

(defun foo1 ())
(defvar foo2 ())
(defvar foo nil
  "
`foo2' something.
`foo2'\t- units and their movement function
`foo1' something.
`foo1'\t- units and their movement function
")

(defun bar ()
  "
`foo2' something.
`foo2'\t- units and their movement function
`foo' something.
`foo'\t- units and their movement function
" 42)

`C-h v foo' shows links everywhere, for both
var foo2 and function foo1.

But `C-h f bar' does not show a link for the
first occurrence of `foo'.
___

What's odd, and the reason I thought this was
fixed, is that I checked  (using `emacs -Q')
the doc string in my code that I was reporting
about.  That doc string is for a function, not
a variable, but it (correctly) has links for
all of the variables mentioned in it.

The recipe for the original report is this:

1. Download library isearch+.el from here:

https://www.emacswiki.org/emacs/download/isearch%2b.el

2. Load that source code.

3. `C-h f isearch-forward'

4. This variable (user option) in the doc was
   not linked, but should have been:
   `isearchp-prompt-for-filter-name'.

But now that option has a link.  (With the above
recipe all have links except those with `[*]',
which the doc tells you are defined in another
library - so no links is correct without that
library loaded.)