GNU bug report logs - #58131
[PATCH] docview: new customization options for imenu

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 58131 in the body.
You can then email your comments to 58131 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: Jose A Ortega Ruiz <jao <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 03:21:31 +0100

[Message part 1 (text/plain, inline)]

Tags: patch


A follow up to bug#58103 with a little fix for section title extraction
and a couple of easy to implement customizable options.  I've tried to
document them clearly enough in the docstrings and then just mention
their existence in the manual: is that a good practice or do we prefer
some duplication?

Cheers,
jao

In GNU Emacs 29.0.50 (build 20, x86_64-pc-linux-gnu, cairo version
 1.16.0) of 2022-09-27 built on rivendell
Repository revision: 7368cdd359325cb6ed83688178ae4b4eaf22f4d5
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure --prefix=/usr/local/stow/emacs29 --with-x-toolkit=no
 --with-imagemagick -C'

[0001-docview-new-customization-options-for-imenu.patch (text/patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
Too often we enjoy the comfort of opinion without the discomfort of
thought. -John F. Kennedy, 35th US president (1917-1963)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jose A Ortega Ruiz <jao <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 13:08:30 +0200

Jose A Ortega Ruiz <jao <at> gnu.org> writes:

> A follow up to bug#58103 with a little fix for section title extraction
> and a couple of easy to implement customizable options.  I've tried to
> document them clearly enough in the docstrings and then just mention
> their existence in the manual: is that a good practice or do we prefer
> some duplication?

It varies -- we don't want the manual to grow unbounded, so sometimes we
just mention variables if they're auxiliary things like this.  So I
think you have the right balance here, and I pushed it to Emacs 29.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jose A Ortega Ruiz <jao <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 15:34:01 +0300

> From: Jose A Ortega Ruiz <jao <at> gnu.org>
> Date: Wed, 28 Sep 2022 03:21:31 +0100
> 
> A follow up to bug#58103 with a little fix for section title extraction
> and a couple of easy to implement customizable options.  I've tried to
> document them clearly enough in the docstrings and then just mention
> their existence in the manual: is that a good practice or do we prefer
> some duplication?

It's a possibility.  But if you have nothing of essence to say about a
variable, why mention it in the manual at all?  We don't need to
mention every user option in the manual, only the important ones.  We
expect users who want to use a package to review its options (e.g., bu
"M-x customize-group") and decide which ones they want to change from
the default.

> +(defcustom doc-view-imenu-title-format "%t (%p)"
> +  "Format string for document section titles in imenu.
> +
> +The special markers '%t' and '%p' are replaced by the section
> +title and page number in this format string, which uses
> +`format-spec'.

Will users immediately understand what you mean by "document section
title" here?  If no, perhaps a sentence explaining what that is would
be beneficial.

> +(defcustom doc-view-imenu-flatten nil
> +  "Whether to generate a flat list of sections instead of a nested tree."

This doesn't mention imenu in the doc string; should it?

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

From: Jose A Ortega Ruiz <jao <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 15:05:50 +0100

On Wed, Sep 28 2022, Eli Zaretskii wrote:

>> From: Jose A Ortega Ruiz <jao <at> gnu.org>
>> Date: Wed, 28 Sep 2022 03:21:31 +0100
>> 
>> A follow up to bug#58103 with a little fix for section title extraction
>> and a couple of easy to implement customizable options.  I've tried to
>> document them clearly enough in the docstrings and then just mention
>> their existence in the manual: is that a good practice or do we prefer
>> some duplication?
>
> It's a possibility.  But if you have nothing of essence to say about a
> variable, why mention it in the manual at all?  We don't need to
> mention every user option in the manual, only the important ones.  We
> expect users who want to use a package to review its options (e.g., bu
> "M-x customize-group") and decide which ones they want to change from
> the default.

That makes sense.  In this case, there's just 3 variables related to the
functionality, and my impression is that they provide options that users
often will want... which might be taken yet as an argument for /not/
adding them to the manual, now that i think of it :)

>> +(defcustom doc-view-imenu-title-format "%t (%p)"
>> +  "Format string for document section titles in imenu.
>> +
>> +The special markers '%t' and '%p' are replaced by the section
>> +title and page number in this format string, which uses
>> +`format-spec'.
>
> Will users immediately understand what you mean by "document section
> title" here?  If no, perhaps a sentence explaining what that is would
> be beneficial.

I would say they will: in the context of a docview imenu for a PDF
document, there's little else it could reasonably be.  But i'm biased: do
you think otherwise?

>> +(defcustom doc-view-imenu-flatten nil
>> +  "Whether to generate a flat list of sections instead of a nested tree."
>
> This doesn't mention imenu in the doc string; should it?

Given that the name of the variable does, i think mentioning it would
just make that first sentence longer without adding too much
information.  But again, as the implementor of the functionality,
everything feels "obvious" to me.

Thanks,
jao
-- 
Lying to ourselves is more deeply ingrained than lying to
others. -Fyodor Dostoevsky, novelist (1821-1881)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jose A Ortega Ruiz <jao <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 17:25:19 +0300

> From: Jose A Ortega Ruiz <jao <at> gnu.org>
> Cc: 58131 <at> debbugs.gnu.org
> Date: Wed, 28 Sep 2022 15:05:50 +0100
> 
> >> +(defcustom doc-view-imenu-title-format "%t (%p)"
> >> +  "Format string for document section titles in imenu.
> >> +
> >> +The special markers '%t' and '%p' are replaced by the section
> >> +title and page number in this format string, which uses
> >> +`format-spec'.
> >
> > Will users immediately understand what you mean by "document section
> > title" here?  If no, perhaps a sentence explaining what that is would
> > be beneficial.
> 
> I would say they will: in the context of a docview imenu for a PDF
> document, there's little else it could reasonably be.  But i'm biased: do
> you think otherwise?

FWIW, I couldn't understand what that means.

How does being in the context of docview imenu for a PDF document help
understanding that here?  "Document section title" is general enough
terminology.  Using a "construct state" here doesn't help, either.

> >> +(defcustom doc-view-imenu-flatten nil
> >> +  "Whether to generate a flat list of sections instead of a nested tree."
> >
> > This doesn't mention imenu in the doc string; should it?
> 
> Given that the name of the variable does, i think mentioning it would
> just make that first sentence longer without adding too much
> information.  But again, as the implementor of the functionality,
> everything feels "obvious" to me.

My suggestion is

  Whether to flatten the list of sections in an imenu or show it nested.

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

From: Jose A Ortega Ruiz <jao <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 16:14:13 +0100

On Wed, Sep 28 2022, Eli Zaretskii wrote:

>> From: Jose A Ortega Ruiz <jao <at> gnu.org>
>> Cc: 58131 <at> debbugs.gnu.org
>> Date: Wed, 28 Sep 2022 15:05:50 +0100
>> 
>> >> +(defcustom doc-view-imenu-title-format "%t (%p)"
>> >> +  "Format string for document section titles in imenu.
>> >> +
>> >> +The special markers '%t' and '%p' are replaced by the section
>> >> +title and page number in this format string, which uses
>> >> +`format-spec'.
>> >
>> > Will users immediately understand what you mean by "document section
>> > title" here?  If no, perhaps a sentence explaining what that is would
>> > be beneficial.
>> 
>> I would say they will: in the context of a docview imenu for a PDF
>> document, there's little else it could reasonably be.  But i'm biased: do
>> you think otherwise?
>
> FWIW, I couldn't understand what that means.
>
> How does being in the context of docview imenu for a PDF document help
> understanding that here?  "Document section title" is general enough
> terminology.  Using a "construct state" here doesn't help, either.

To me, the context is that this is read by a user of the functionality,
wanting to fine-tune it (i rarely start exploring a functionality by
reading the docstring of one of its customizable variables).

Barring that: the Imenu section of the manual calls the items being
formatted here simply "definitions".  If we assume that the user knows
about imenu in general, one could have:

   "Format string for the imenu definitions extracted from documents."

Or, perhaps, trying to provide the missing context for not-yet-users:

   "Format string for the section titles extracted by imenu from docview documents."

Any better?

>> >> +(defcustom doc-view-imenu-flatten nil
>> >> +  "Whether to generate a flat list of sections instead of a nested tree."
>> >
>> > This doesn't mention imenu in the doc string; should it?
>> 
>> Given that the name of the variable does, i think mentioning it would
>> just make that first sentence longer without adding too much
>> information.  But again, as the implementor of the functionality,
>> everything feels "obvious" to me.
>
> My suggestion is
>
>   Whether to flatten the list of sections in an imenu or show it nested.

Sounds better to me too, yes.

Thanks,
jao
-- 
Nothing so soothes our vanity as a display of greater vanity in
others; it makes us vain, in fact, of our modesty.
  -Louis Kronenberger, writer (1904-1980)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jose A Ortega Ruiz <jao <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 18:49:58 +0300

> From: Jose A Ortega Ruiz <jao <at> gnu.org>
> Cc: 58131 <at> debbugs.gnu.org
> Date: Wed, 28 Sep 2022 16:14:13 +0100
> 
> >> I would say they will: in the context of a docview imenu for a PDF
> >> document, there's little else it could reasonably be.  But i'm biased: do
> >> you think otherwise?
> >
> > FWIW, I couldn't understand what that means.
> >
> > How does being in the context of docview imenu for a PDF document help
> > understanding that here?  "Document section title" is general enough
> > terminology.  Using a "construct state" here doesn't help, either.
> 
> To me, the context is that this is read by a user of the functionality,
> wanting to fine-tune it (i rarely start exploring a functionality by
> reading the docstring of one of its customizable variables).

Think about the various Help commands, such as 'apropos': they show
the doc strings or just their first sentence entirely out of any
context.  Users who invoke such commands should be capable to
understand at a glance whether the command/variable is something they
should examine further.  So having important keywords in there helps
immensely to make such triage steps much faster and more efficient.

> Barring that: the Imenu section of the manual calls the items being
> formatted here simply "definitions".  If we assume that the user knows
> about imenu in general, one could have:
> 
>    "Format string for the imenu definitions extracted from documents."
> 
> Or, perhaps, trying to provide the missing context for not-yet-users:
> 
>    "Format string for the section titles extracted by imenu from docview documents."
> 
> Any better?

Yes, thanks.  My suggestion is a very minor variation of the latter:

 Format spec for imenu's display of section titles from docview documents.

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

From: Jose A Ortega Ruiz <jao <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 16:55:56 +0100

On Wed, Sep 28 2022, Eli Zaretskii wrote:

[...]

> Think about the various Help commands, such as 'apropos': they show
> the doc strings or just their first sentence entirely out of any
> context.  Users who invoke such commands should be capable to
> understand at a glance whether the command/variable is something they
> should examine further.  So having important keywords in there helps
> immensely to make such triage steps much faster and more efficient.

Excellent point, thanks.

[...]

>> Any better?
>
> Yes, thanks.  My suggestion is a very minor variation of the latter:
>
>  Format spec for imenu's display of section titles from docview documents.

Looks good to me.  Would you like a patch with the two new strings or is
it quicker if you directly commit a change by yourself?


Cheers,
jao
-- 
All parts should go together without forcing. You must remember that
the parts you are reassembling were disassembled by you. Therefore, if
you can’t get them together again, there must be a reason. By all
means, do not use a hammer. —IBM Manual, 1925

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jose A Ortega Ruiz <jao <at> gnu.org>
Cc: 58131 <at> debbugs.gnu.org
Subject: Re: bug#58131: [PATCH] docview: new customization options for imenu
Date: Wed, 28 Sep 2022 19:01:46 +0300

> From: Jose A Ortega Ruiz <jao <at> gnu.org>
> Cc: 58131 <at> debbugs.gnu.org
> Date: Wed, 28 Sep 2022 16:55:56 +0100
> 
> Looks good to me.  Would you like a patch with the two new strings or is
> it quicker if you directly commit a change by yourself?

No need, I made these changes already.

Thanks for the feedback.

Previous Next

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