GNU bug report logs - #44858
[PATCH] Make byte-compiler warn about wide docstrings

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 44858 in the body.
You can then email your comments to 44858 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: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Make byte-compiler warn about wide docstrings
Date: Tue, 24 Nov 2020 17:36:34 -0800

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

Severity: wishlist

In etc/TODO we have:

** Make byte-compiler warn when a doc string is too wide

Please find attached a patch that implements this.  Notably, it omits
defuns, and ignores any lines containing `substitute-command-keys' style
substitutions.

I've been messing around with getting defuns to work, but I can't find a
way for it to work reliably.  The problem is that they are already macro
expanded when we start issuing warnings, so it's not trivial to reliably
separate defuns from other cases where lambda is used.  We also have
other macros like `define-derived-mode' that produces `defun's
themselves, and the line reporting gets messed up.  (The autogenerated
docstrings from these macros seem to generate too wide docstrings in
several cases.)

For `substitute-command-keys', it would be nice to get it to work, but I
don't think we can know the values of keymaps at compile-time.  Possibly
there is a good solution for this, but I couldn't find it.

As for the state of our own sources, we seem to have less than 100
docstrings that break our own conventions and generates a warning with
the patch.  It shouldn't take too much work to fix them.

(If you were to add defuns into the mix, we would get around 700
warnings for wide docstrings, several of which would be autogenerated.
We should probably look into that at some point...)

Thoughts?

[0001-Make-byte-compiler-warn-about-wide-docstrings.patch (text/x-diff, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 26 Nov 2020 11:49:55 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> ** Make byte-compiler warn when a doc string is too wide
>
> Please find attached a patch that implements this.

I like it.

> Notably, it omits defuns, and ignores any lines containing
> `substitute-command-keys' style substitutions.
>
> I've been messing around with getting defuns to work, but I can't find a
> way for it to work reliably.  The problem is that they are already macro
> expanded when we start issuing warnings, so it's not trivial to reliably
> separate defuns from other cases where lambda is used.

I'm probably misunderstanding you completely, but do we have to separate
between lambda and defuns?  lambdas can also have doc strings...

> (If you were to add defuns into the mix, we would get around 700
> warnings for wide docstrings, several of which would be autogenerated.
> We should probably look into that at some point...)

Yes, the autogenerated docstrings should be fixed, too -- mostly by
running them through `fill-paragraph'.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 26 Nov 2020 04:46:25 -0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I like it.

Thanks.

>> I've been messing around with getting defuns to work, but I can't find a
>> way for it to work reliably.  The problem is that they are already macro
>> expanded when we start issuing warnings, so it's not trivial to reliably
>> separate defuns from other cases where lambda is used.
>
> I'm probably misunderstanding you completely, but do we have to separate
> between lambda and defuns?  lambdas can also have doc strings...

Indeed they can.  The problem I had was how to differentiate between the
many different macros that generate lambdas.  I didn't record the exact
details, but it got pretty messy between `defun', `define-derived-mode',
`cl-defgeneric', etc. etc.  The problem I saw was basically warnings
about symbols only visible after macro expansion, and that warnings
would point to entirely the wrong line and column.  (If anyone wants to
take a look, it should be sufficient to uncomment the lambda part in my
patch and run "make bootstrap".)

I was looking at `byte-defop-compiler-1', but that seems to only be
usable for functions and special forms?  IIUC, when we compile all
macros have been expanded and all information about what macro was used
where is lost.  But it's possible that I'm misunderstanding things, as
I'm still wrapping my head around this code.

>> (If you were to add defuns into the mix, we would get around 700
>> warnings for wide docstrings, several of which would be autogenerated.
>> We should probably look into that at some point...)
>
> Yes, the autogenerated docstrings should be fixed, too -- mostly by
> running them through `fill-paragraph'.

I tried that in e.g. define-derived-mode, but fill.el is loaded after
derived.el.  So it seems like there is some fun to be had in figuring
out the dependencies there...

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 26 Nov 2020 13:53:31 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> The problem I saw was basically warnings
> about symbols only visible after macro expansion, and that warnings
> would point to entirely the wrong line and column.

Oh, OK, the problem was in the lines/column output, not with detecting
the doc strings themselves?

Yes, that sounds like a problem, but...  I think we can live with
inaccurate lines here.

>> Yes, the autogenerated docstrings should be fixed, too -- mostly by
>> running them through `fill-paragraph'.
>
> I tried that in e.g. define-derived-mode, but fill.el is loaded after
> derived.el.  So it seems like there is some fun to be had in figuring
> out the dependencies there...

Yeah, I guess fill.el is a pretty far down in the list of things that
loadup.el loads?

However, we could make a super-simple function for filling doc
strings -- doing something that's good enough for that particular task
shouldn't take more than a few lines.

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 26 Nov 2020 16:19:53 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Tue, 24 Nov 2020 17:36:34 -0800
> 
> For `substitute-command-keys', it would be nice to get it to work, but I
> don't think we can know the values of keymaps at compile-time.  Possibly
> there is a good solution for this, but I couldn't find it.

How about some simplified heuristics, like assume that the expansion
takes no more than N characters (where N could be something like 5)?
This should work in, like, 80% of cases, I think.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 27 Nov 2020 09:37:11 +0100

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

>> For `substitute-command-keys', it would be nice to get it to work, but I
>> don't think we can know the values of keymaps at compile-time.  Possibly
>> there is a good solution for this, but I couldn't find it.
>
> How about some simplified heuristics, like assume that the expansion
> takes no more than N characters (where N could be something like 5)?
> This should work in, like, 80% of cases, I think.

Yup.  And 15% is mostly when it expands to `M-x some-long-command'
because the keymap hasn't been loaded yet, I think?  Which we could
conceivably fix by loading the file when the used `C-h f's an autoloaded
function with one of these constructs?  Perhaps a bit hacky...

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 27 Nov 2020 12:15:03 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> For `substitute-command-keys', it would be nice to get it to work, but I
> >> don't think we can know the values of keymaps at compile-time.  Possibly
> >> there is a good solution for this, but I couldn't find it.
> >
> > How about some simplified heuristics, like assume that the expansion
> > takes no more than N characters (where N could be something like 5)?
> > This should work in, like, 80% of cases, I think.
>
> Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> because the keymap hasn't been loaded yet, I think?  Which we could
> conceivably fix by loading the file when the used `C-h f's an autoloaded
> function with one of these constructs?  Perhaps a bit hacky...

I would be wary of using a heuristic here, because I think false
positives are worse than false negatives in this case.  We
unfortunately don't have any way of silencing individual warnings, so
a user seeing a false positive is left with two suboptimal choices:
ignore the warning (a bad habit to train our users in) or change the
formatting of a docstring to stop it (potentially making it
subjectively worse in the process).

How about using the somewhat safer heuristic of treating substitutions
as one character wide?  Would that make sense?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 27 Nov 2020 14:44:11 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Fri, 27 Nov 2020 12:15:03 +0100
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 44858 <at> debbugs.gnu.org
> 
> > > How about some simplified heuristics, like assume that the expansion
> > > takes no more than N characters (where N could be something like 5)?
> > > This should work in, like, 80% of cases, I think.
> >
> > Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> > because the keymap hasn't been loaded yet, I think?  Which we could
> > conceivably fix by loading the file when the used `C-h f's an autoloaded
> > function with one of these constructs?  Perhaps a bit hacky...
> 
> I would be wary of using a heuristic here, because I think false
> positives are worse than false negatives in this case.

Can we have some numbers, please? how many false positives do you get
by assuming the expanded key sequence takes 5 characters? what about 3
or 4 or 7?

> We unfortunately don't have any way of silencing individual
> warnings, so a user seeing a false positive is left with two
> suboptimal choices: ignore the warning (a bad habit to train our
> users in) or change the formatting of a docstring to stop it
> (potentially making it subjectively worse in the process).

The assumption is that using such heuristic will cause false
negatives, not false positives.  Do you see any bad consequences to
false negatives?

> How about using the somewhat safer heuristic of treating substitutions
> as one character wide?  Would that make sense?

I'd say, at least 3, because there are very few non-trivial key
bindings bound to a single character.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: RE: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 27 Nov 2020 10:36:50 -0800 (PST)

> Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> because the keymap hasn't been loaded yet, I think?  Which we could
> conceivably fix by loading the file when the used `C-h f's an
> autoloaded function with one of these constructs?  Perhaps a bit hacky...

Please, no.  Users should be able to see the doc
without loading the library.  That's an important
feature, IMO, which has been present from Day One
(or whatever day autoloading was introduced).

I'm all for cosmetic improvements and not having
long lines, believe me.  But I think loading a
library just to (maybe!) change `M-x function-name'
to a key binding description is, I think, misguided.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: RE: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 27 Nov 2020 10:55:50 -0800 (PST)

> > Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> > because the keymap hasn't been loaded yet, I think?  Which we could
> > conceivably fix by loading the file when the used `C-h f's an
> > autoloaded function with one of these constructs?  Perhaps a bit
> hacky...
> 
> Please, no.  Users should be able to see the doc
> without loading the library.  That's an important
> feature, IMO, which has been present from Day One
> (or whatever day autoloading was introduced).
> 
> I'm all for cosmetic improvements and not having
> long lines, believe me.  But I think loading a
> library just to (maybe!) change `M-x function-name'
> to a key binding description is, I think, misguided.

Or maybe you meant load only for byte-compilation?

But even then, it's possible for byte-compiling to
be done during a user's session, and in cases where
there's no desire to load the library.

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

From: Tomas Nordin <tomasn <at> posteo.net>
To: Stefan Kangas <stefan <at> marxist.se>, 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 03 Dec 2020 21:18:48 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> Thoughts?

One thing that caught my eye

@@ -322,6 +323,8 @@ byte-compile-warnings
   make-local  calls to make-variable-buffer-local that may be incorrect.
   mapcar      mapcar called for effect.
   constants   let-binding of, or assignment to, constants/nonvariables.
+  docstrings  docstrings that are too wide (no longer than 80 characters,
+              or `fill-column', whichever is bigger)
   suspicious  constructs that usually don't do what the coder wanted.

Should 'no' maybe be removed? (if to tell something that should not be).

--
Tomas

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sun, 6 Dec 2020 05:09:45 -0600

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

>> > > How about some simplified heuristics, like assume that the expansion
>> > > takes no more than N characters (where N could be something like 5)?
>> > > This should work in, like, 80% of cases, I think.
>> >
>> > Yup.  And 15% is mostly when it expands to `M-x some-long-command'
>> > because the keymap hasn't been loaded yet, I think?  Which we could
>> > conceivably fix by loading the file when the used `C-h f's an autoloaded
>> > function with one of these constructs?  Perhaps a bit hacky...
>>
>> I would be wary of using a heuristic here, because I think false
>> positives are worse than false negatives in this case.
>
> Can we have some numbers, please? how many false positives do you get
> by assuming the expanded key sequence takes 5 characters? what about 3
> or 4 or 7?

I have run some tests.  The first column is the number of characters.
The second column is the number of "wide docstring" warnings with my
patch.  The third column is when I add in warnings for lambda
docstrings (commented out in the patch):

    | Characters | Warnings | Warnings (w/lambda) |
    |------------+----------+---------------------|
    |          0 |      109 |                 451 |
    |          1 |      110 |                 452 |
    |          2 |      110 |                 452 |
    |          3 |      110 |                 454 |
    |          5 |      110 |                 463 |
    |          6 |      111 |                 468 |
    |          7 |      111 |                 475 |

We don't seem to get much additional benefit from a heuristic with a
higher number of characters (i.e. not a lot more warnings).

I checked some of the warnings when using 7 characters (with lambda),
and all new warnings I checked were false positives:

   help.el:194:1: Warning: docstring wider than 80 characters
   isearch.el:1001:8: Warning: docstring wider than 80 characters
   simple.el:5616:8: Warning: docstring wider than 80 characters

>> We unfortunately don't have any way of silencing individual
>> warnings, so a user seeing a false positive is left with two
>> suboptimal choices: ignore the warning (a bad habit to train our
>> users in) or change the formatting of a docstring to stop it
>> (potentially making it subjectively worse in the process).
>
> The assumption is that using such heuristic will cause false
> negatives, not false positives.  Do you see any bad consequences to
> false negatives?

Not sure what you mean here; my assumption was that it would cause false
positives.  I see no bad consequences to false negatives, so I'm not
overly worried about them.  (False negatives are neither worse nor
better than the status quo.)

>> How about using the somewhat safer heuristic of treating substitutions
>> as one character wide?  Would that make sense?
>
> I'd say, at least 3, because there are very few non-trivial key
> bindings bound to a single character.

AFAIU, if N is the number of characters, N=1 would cause only false
negatives.  For any N>M, where M is the length of longest command name
in use, it would cause only false positives.  For any N where N>1 and
N<M, it would cause both false negatives and false positives.

Using 3 is not significantly better than 1, as the above numbers show.
But we do risk more false positives.  My preference is therefore still
the safer 1, as it will give no false positives.

We could start with 3 or 1 and adjust later as we learn more about how
this heuristic works in practice.  I don't have a very strong opinion,
as I think we will learn more in due time.

WDYT?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sun, 06 Dec 2020 13:19:31 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sun, 6 Dec 2020 05:09:45 -0600
> Cc: larsi <at> gnus.org, 44858 <at> debbugs.gnu.org
> 
> Using 3 is not significantly better than 1, as the above numbers show.
> But we do risk more false positives.  My preference is therefore still
> the safer 1, as it will give no false positives.
> 
> We could start with 3 or 1 and adjust later as we learn more about how
> this heuristic works in practice.  I don't have a very strong opinion,
> as I think we will learn more in due time.
> 
> WDYT?

Since 3 gives the same number of warnings, and since it covers 3 times
as many keys as 1, I prefer to use 3.

Thanks.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: RE: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sun, 6 Dec 2020 08:54:23 -0800 (PST)

(FWIW, I'm not convinced this is very helpful.)

But if you do add this, please consider letting
the message provide the actual longest-line
length.

Someone concerned with fixing long doc-string
lines might be more concerned with fixing one
with, say, 179 chars than one with, say, 82.

Presumably the actual longest-line length is
available for the message.  Why not show it?

Or maybe not.  If your code finds only the
first line that's > 80 chars then it won't
help as much.  Doc strings are typically
fairly short.  Looking for the max line
length isn't very expensive.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 10 Dec 2020 14:59:16 -0600

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

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> The problem I saw was basically warnings
>> about symbols only visible after macro expansion, and that warnings
>> would point to entirely the wrong line and column.
>
> Oh, OK, the problem was in the lines/column output, not with detecting
> the doc strings themselves?
>
> Yes, that sounds like a problem, but...  I think we can live with
> inaccurate lines here.

OK.  The next issue is actually fixing all those docstrings though...

>>> Yes, the autogenerated docstrings should be fixed, too -- mostly by
>>> running them through `fill-paragraph'.
>>
>> I tried that in e.g. define-derived-mode, but fill.el is loaded after
>> derived.el.  So it seems like there is some fun to be had in figuring
>> out the dependencies there...
>
> Yeah, I guess fill.el is a pretty far down in the list of things that
> loadup.el loads?
>
> However, we could make a super-simple function for filling doc
> strings -- doing something that's good enough for that particular task
> shouldn't take more than a few lines.

True.  Please find attached three patches:

1. The original patch adjusted with the heuristic proposed by Eli.

2. Avoid wide docstrings in define-minor-mode.

3. Fixing most wide docstring warnings.

With these three patches applied, around 22 wide docstring warnings
remain in our tree for defvar's.  I found it hard to fix them as I'm not
very familiar with the code in question, and I couldn't just simplify
the wording from the context.

I'm assuming we don't want to push this without fixing those warnings,
so I would really appreciate some help with the remaining couple of
warnings.

Should we perhaps try to get this applied before starting work on
defuns?  Or should I just push a branch for this work?

[0001-Make-byte-compiler-warn-about-wide-docstrings.patch (text/x-diff, attachment)]

[0002-Avoid-wide-doc-strings-in-define-minor-mode.patch (text/x-diff, attachment)]

[0003-Adjust-some-docstrings-to-fit-80-columns.patch (text/x-diff, attachment)]

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 10 Dec 2020 15:53:13 -0600

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

Stefan Kangas <stefan <at> marxist.se> writes:

> 2. Avoid wide docstrings in define-minor-mode.

And here's a patch also for define-derived-mode.

It seems like we still need to fix cl-defun, cl-defstruct,
define-globalized-minor-mode and perhaps a few others.
So I'll work on that next.

[0004-Fill-docstrings-in-define-derived-mode.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 09:33:47 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Thu, 10 Dec 2020 14:59:16 -0600
> Cc: 44858 <at> debbugs.gnu.org
> 
> With these three patches applied, around 22 wide docstring warnings
> remain in our tree for defvar's.  I found it hard to fix them as I'm not
> very familiar with the code in question, and I couldn't just simplify
> the wording from the context.
> 
> I'm assuming we don't want to push this without fixing those warnings,
> so I would really appreciate some help with the remaining couple of
> warnings.

Please show those problematic doc strings, and I will try to fix them.

Thanks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 09:53:27 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Thu, 10 Dec 2020 14:59:16 -0600
> Cc: 44858 <at> debbugs.gnu.org
> 
> +** byte compiler

Please capitalize "Byte".

> +*** The byte-compiler now warns about too wide documentation strings.
> +By default, it will warn if a documentation string is wider than the
> +largest of 80 characters or 'fill-column'.  See the new user
> +option 'byte-compile-docstring-max-column'.

The last sentence will be more useful if you make it more informative,
e.g.

  This is controlled by the new user option
  'byte-compile-docstring-max-column'.

> +(defvar byte-compile--wide-docstring-substitution-len 3
> +  "Substitution width used in `byte-compile--wide-docstring-p'.")

The doc string should be more detailed.  Let's at least explain what
hides behind "substitution", and say that the value is a heuristic.

> +(defcustom byte-compile-docstring-max-column 80
> +  "Length that a doc string can be before the byte-compiler reports a warning."

I suggest "width", not "length", since you both use the former
elsewhere, and because "column" goes better with "width".

Also, "can be before" is not the clearest way of saying this.  I
suggest

    Recommended maximum width of doc string lines.
  The byte-compiler will emit a warning for doc-string lines
  wider than this.  If `fill-column' has a larger value, it will
  override this variable.

> +(defun byte-compile-docstring-length-warn (form)
> +  "Warn if documentation string of FORM is too wide.
> +It is too wide if it is longer than the largest of `fill-column'
> +and `byte-compile-docstring-max-column'."

"It is too wide if it is longer" is inaccurate, I think, because we
actually test each _line_ of the doc string against the limit, while
the above seems to imply the overall length of the doc string (which
could be multi-line) is tested.  So:

  It is too wide if some of its lines is wider than the largest of...

> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
> @@ -0,0 +1,3 @@
> +;;; -*- lexical-binding: t -*-
> +(autoload 'foox "foo"
> +  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")

I see that none of the tests use multi-line doc strings, is that
right?  If so, I think it would be good to have at least a couple, one
where the first line is too wide, and another where some line other
than the first is too wide.  Also, what about testing the fact that
fill-column overrides the value of the new option?

Thanks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 10:16:15 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Thu, 10 Dec 2020 15:53:13 -0600
> Cc: 44858 <at> debbugs.gnu.org
> 
> +(defun internal--fill-string (str)
> +  "Fill string STR to `fill-column'.
> +This is intended for very simple filling while bootstrapping
> +Emacs itself, and does not support all the customization options
> +of fill.el (for example `fill-region')."
> +  (if (< (length str) fill-column)  <<<<<<<<<<<<<<<<<<<<<<<<<

Can we use string-width here instead of length?  If this will work
during bootstrap, it is preferable, I think.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 16:13:03 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> With these three patches applied, around 22 wide docstring warnings
> remain in our tree for defvar's.  I found it hard to fix them as I'm not
> very familiar with the code in question, and I couldn't just simplify
> the wording from the context.
>
> I'm assuming we don't want to push this without fixing those warnings,
> so I would really appreciate some help with the remaining couple of
> warnings.

I think it's fine to push even if it generates warnings -- then people
can help with stamping out those warnings.  (My guess it that it won't
take long.)

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 14:03:08 -0600

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

>> From: Stefan Kangas <stefan <at> marxist.se>
>> Date: Thu, 10 Dec 2020 15:53:13 -0600
>> Cc: 44858 <at> debbugs.gnu.org
>>
>> +(defun internal--fill-string (str)
>> +  "Fill string STR to `fill-column'.
>> +This is intended for very simple filling while bootstrapping
>> +Emacs itself, and does not support all the customization options
>> +of fill.el (for example `fill-region')."
>> +  (if (< (length str) fill-column)  <<<<<<<<<<<<<<<<<<<<<<<<<
>
> Can we use string-width here instead of length?  If this will work
> during bootstrap, it is preferable, I think.

Yes, it seems to work fine.  Now changed.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Tomas Nordin <tomasn <at> posteo.net>, 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 14:14:29 -0600

Tomas Nordin <tomasn <at> posteo.net> writes:

> +  docstrings  docstrings that are too wide (no longer than 80 characters,
> +              or `fill-column', whichever is bigger)
>    suspicious  constructs that usually don't do what the coder wanted.
>
> Should 'no' maybe be removed? (if to tell something that should not be).

Indeed, thanks for spotting that.  Now changed.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 11 Dec 2020 14:36:33 -0600

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

>> I'm assuming we don't want to push this without fixing those warnings,
>> so I would really appreciate some help with the remaining couple of
>> warnings.
>
> Please show those problematic doc strings, and I will try to fix them.

Thanks, I appreciate the help.  Please see the below list that should be
easy to just put in a compilation-mode buffer.


gnus/deuglify.el:252:1: Warning: custom-declare-variable
    `gnus-outlook-deuglify-unwrap-stop-chars' docstring wider than 80
    characters
gnus/deuglify.el:259:1: Warning: custom-declare-variable
    `gnus-outlook-deuglify-no-wrap-chars' docstring wider than 80 characters
gnus/deuglify.el:265:1: Warning: custom-declare-variable
    `gnus-outlook-deuglify-attrib-cut-regexp' docstring wider than 80
    characters
gnus/gnus.el:1529:1: Warning: custom-declare-variable
    `gnus-group-charset-alist' docstring wider than 80 characters
gnus/nnvirtual.el:63:1: Warning: defvar `nnvirtual-mapping-table' docstring
    wider than 80 characters
gnus/nnvirtual.el:66:1: Warning: defvar `nnvirtual-mapping-offsets' docstring
    wider than 80 characters
gnus/nnvirtual.el:72:1: Warning: defvar `nnvirtual-mapping-reads' docstring
    wider than 80 characters
gnus/nnvirtual.el:75:1: Warning: defvar `nnvirtual-mapping-marks' docstring
    wider than 80 characters
gnus/nnvirtual.el:78:1: Warning: defvar `nnvirtual-info-installed' docstring
    wider than 80 characters
language/ethio-util.el:122:1: Warning: defvar `ethio-quote-vowel-always'
    docstring wider than 80 characters
language/ethio-util.el:127:1: Warning: defvar `ethio-W-sixth-always' docstring
    wider than 80 characters
mail/feedmail.el:624:1: Warning: custom-declare-variable
    `feedmail-sendmail-f-doesnt-sell-me-out' docstring wider than 80
    characters
mh-e/mh-e.el:2827:1: Warning: custom-declare-variable
    `mh-invisible-header-fields' docstring wider than 80 characters
mh-e/mh-e.el:2850:1: Warning: custom-declare-variable
    `mh-invisible-header-fields-default' docstring wider than 80 characters
net/dbus.el:79:1: Warning: defconst `dbus-interface-peer' docstring wider than
    80 characters
net/dbus.el:91:1: Warning: defconst `dbus-interface-introspectable' docstring
    wider than 80 characters
net/dbus.el:102:1: Warning: defconst `dbus-interface-properties' docstring
    wider than 80 characters
net/dbus.el:128:1: Warning: defconst `dbus-interface-objectmanager' docstring
    wider than 80 characters
net/dbus.el:148:1: Warning: defconst `dbus-interface-monitoring' docstring
    wider than 80 characters
org/org-ctags.el:164:1: Warning: custom-declare-variable
    `org-ctags-open-link-functions' docstring wider than 80 characters
org/org-protocol.el:166:1: Warning: custom-declare-variable
    `org-protocol-project-alist' docstring wider than 80 characters
vc/ediff-init.el:556:1: Warning: custom-declare-variable
    `ediff-before-flag-bol' docstring wider than 80 characters
vc/ediff-init.el:562:1: Warning: custom-declare-variable
    `ediff-after-flag-eol' docstring wider than 80 characters
vc/ediff-init.el:568:1: Warning: custom-declare-variable
    `ediff-before-flag-mol' docstring wider than 80 characters
progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
    docstring wider than 80 characters

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 13:22:50 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Fri, 11 Dec 2020 14:36:33 -0600
> Cc: larsi <at> gnus.org, 44858 <at> debbugs.gnu.org
> 
> > Please show those problematic doc strings, and I will try to fix them.
> 
> Thanks, I appreciate the help.  Please see the below list that should be
> easy to just put in a compilation-mode buffer.
> 
> 
> gnus/deuglify.el:252:1: Warning: custom-declare-variable
>     `gnus-outlook-deuglify-unwrap-stop-chars' docstring wider than 80
>     characters
> gnus/deuglify.el:259:1: Warning: custom-declare-variable
>     `gnus-outlook-deuglify-no-wrap-chars' docstring wider than 80 characters
> gnus/deuglify.el:265:1: Warning: custom-declare-variable
>     `gnus-outlook-deuglify-attrib-cut-regexp' docstring wider than 80
>     characters
> gnus/gnus.el:1529:1: Warning: custom-declare-variable
>     `gnus-group-charset-alist' docstring wider than 80 characters
> gnus/nnvirtual.el:63:1: Warning: defvar `nnvirtual-mapping-table' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:66:1: Warning: defvar `nnvirtual-mapping-offsets' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:72:1: Warning: defvar `nnvirtual-mapping-reads' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:75:1: Warning: defvar `nnvirtual-mapping-marks' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:78:1: Warning: defvar `nnvirtual-info-installed' docstring
>     wider than 80 characters
> language/ethio-util.el:122:1: Warning: defvar `ethio-quote-vowel-always'
>     docstring wider than 80 characters
> language/ethio-util.el:127:1: Warning: defvar `ethio-W-sixth-always' docstring
>     wider than 80 characters
> mail/feedmail.el:624:1: Warning: custom-declare-variable
>     `feedmail-sendmail-f-doesnt-sell-me-out' docstring wider than 80
>     characters

I fixed those.

> mh-e/mh-e.el:2827:1: Warning: custom-declare-variable
>     `mh-invisible-header-fields' docstring wider than 80 characters
> mh-e/mh-e.el:2850:1: Warning: custom-declare-variable
>     `mh-invisible-header-fields-default' docstring wider than 80 characters
> net/dbus.el:79:1: Warning: defconst `dbus-interface-peer' docstring wider than
>     80 characters
> net/dbus.el:91:1: Warning: defconst `dbus-interface-introspectable' docstring
>     wider than 80 characters
> net/dbus.el:102:1: Warning: defconst `dbus-interface-properties' docstring
>     wider than 80 characters
> net/dbus.el:128:1: Warning: defconst `dbus-interface-objectmanager' docstring
>     wider than 80 characters
> net/dbus.el:148:1: Warning: defconst `dbus-interface-monitoring' docstring
>     wider than 80 characters

I see nothing wrong with these, so either I'm missing something or
there's a bug in the code which reports these problems.

> org/org-ctags.el:164:1: Warning: custom-declare-variable
>     `org-ctags-open-link-functions' docstring wider than 80 characters

Fixed.

> org/org-protocol.el:166:1: Warning: custom-declare-variable
>     `org-protocol-project-alist' docstring wider than 80 characters

I don't see a problem here.

> vc/ediff-init.el:556:1: Warning: custom-declare-variable
>     `ediff-before-flag-bol' docstring wider than 80 characters
> vc/ediff-init.el:562:1: Warning: custom-declare-variable
>     `ediff-after-flag-eol' docstring wider than 80 characters
> vc/ediff-init.el:568:1: Warning: custom-declare-variable
>     `ediff-before-flag-mol' docstring wider than 80 characters

Fixed.

> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
>     docstring wider than 80 characters

I don't see how I can do anything about this one.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 10:50:33 -0600

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

> I see nothing wrong with these, so either I'm missing something or
> there's a bug in the code which reports these problems.

Thanks for the fixes.  See my comments below:

>> mh-e/mh-e.el:2827:1: Warning: custom-declare-variable
>>     `mh-invisible-header-fields' docstring wider than 80 characters
>> mh-e/mh-e.el:2850:1: Warning: custom-declare-variable
>>     `mh-invisible-header-fields-default' docstring wider than 80 characters

AFAICT, the offending lines are the ones containing URLs:

line 2839:

`https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').

line 2683:

`https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').

I see two issues here:

a) The URL does not easily break over two lines.  I'm not sure what we
   should do.  Add a special case to ignore lines with URLs?

b) The warning doesn't point you to the actually offending line, but to
   the definition containing the docstring.  I'm not sure if this is
   easily fixable or not, or if it's important enough to be worth
   spending time on.

>> net/dbus.el:79:1: Warning: defconst `dbus-interface-peer' docstring wider than
>>     80 characters
>> net/dbus.el:91:1: Warning: defconst `dbus-interface-introspectable' docstring
>>     wider than 80 characters
>> net/dbus.el:102:1: Warning: defconst `dbus-interface-properties' docstring
>>     wider than 80 characters
>> net/dbus.el:128:1: Warning: defconst `dbus-interface-objectmanager' docstring
>>     wider than 80 characters
>> net/dbus.el:148:1: Warning: defconst `dbus-interface-monitoring' docstring
>>     wider than 80 characters

Again we have URLs, like on line 81:

See URL `https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-peer'."

>> org/org-protocol.el:166:1: Warning: custom-declare-variable
>>     `org-protocol-project-alist' docstring wider than 80 characters
>
> I don't see a problem here.

I fixed that one.

>> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
>>     docstring wider than 80 characters
>
> I don't see how I can do anything about this one.

I'm also stomped.  Perhaps we could ask Alan for help with this one.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 19:14:20 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sat, 19 Dec 2020 10:50:33 -0600
> Cc: larsi <at> gnus.org, 44858 <at> debbugs.gnu.org
> 
> AFAICT, the offending lines are the ones containing URLs:
> 
> line 2839:
> 
> `https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').
> 
> line 2683:
> 
> `https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').
> 
> I see two issues here:
> 
> a) The URL does not easily break over two lines.  I'm not sure what we
>    should do.  Add a special case to ignore lines with URLs?

If you can detect and ignore long lines that consist entirely of URL,
yes, that would be good.  Otherwise, simply ignore these warnings.

> >> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
> >>     docstring wider than 80 characters
> >
> > I don't see how I can do anything about this one.
> 
> I'm also stomped.  Perhaps we could ask Alan for help with this one.

If he can, sure.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 18:18:13 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> a) The URL does not easily break over two lines.  I'm not sure what we
>    should do.  Add a special case to ignore lines with URLs?

Yes, I think that would be good.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 15:48:22 -0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> a) The URL does not easily break over two lines.  I'm not sure what we
>>    should do.  Add a special case to ignore lines with URLs?
>
> Yes, I think that would be good.

OK, done.  See the updated patches in the email following this one.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 15:55:20 -0800

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

Thanks for reviewing.  I've fixed all comments and added the tests you
suggested.  Please find attached two updated patches.

This still doesn't include warning for lambda docstrings: I think it
could make sense to get these two patches merged to see if there is any
feedback first.

[0001-Make-byte-compiler-warn-about-wide-docstrings.patch (text/x-diff, attachment)]

[0002-Fill-some-auto-generated-docstrings.patch (text/x-diff, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sun, 20 Dec 2020 18:53:57 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> Thanks for reviewing.  I've fixed all comments and added the tests you
> suggested.  Please find attached two updated patches.
>
> This still doesn't include warning for lambda docstrings: I think it
> could make sense to get these two patches merged to see if there is any
> feedback first.

Sure; looks good to me.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Mon, 28 Dec 2020 06:18:23 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> Thanks for reviewing.  I've fixed all comments and added the tests you
>> suggested.  Please find attached two updated patches.
>>
>> This still doesn't include warning for lambda docstrings: I think it
>> could make sense to get these two patches merged to see if there is any
>> feedback first.
>
> Sure; looks good to me.

No further comments within a week, so I've pushed this first part to
master.  See commit 0ebea8ffbf and 6b8bb47ac0.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 44858 <at> debbugs.gnu.org, larsi <at> gnus.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Tue, 29 Dec 2020 01:27:37 +0000

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

>> From: Stefan Kangas <stefan <at> marxist.se>
>> Date: Sat, 19 Dec 2020 10:50:33 -0600
>> Cc: larsi <at> gnus.org, 44858 <at> debbugs.gnu.org
>> 
>> >> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
>> >>     docstring wider than 80 characters
>> >
>> > I don't see how I can do anything about this one.
>> 
>> I'm also stomped.  Perhaps we could ask Alan for help with this one.
>
> If he can, sure.

I hope the following fix is okay.

Reword a long docstring in cc-langs.el
3334dd9041 2020-12-29 01:23:14 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3334dd904157e7b3787f5d32f30b3c31585d047c

Thanks,

-- 
Basil

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 44858 <at> debbugs.gnu.org, Alan Mackenzie <acm <at> muc.de>,
 Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Tue, 29 Dec 2020 03:16:52 +0100

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

>>> I'm also stomped.  Perhaps we could ask Alan for help with this one.
>>
>> If he can, sure.
>
> I hope the following fix is okay.
>
> Reword a long docstring in cc-langs.el

Makes sense to me at least, but I see that Alan wasn't added to the CCs?
So I've done that now.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Wed, 30 Dec 2020 04:07:14 -0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> The problem I saw was basically warnings
>> about symbols only visible after macro expansion, and that warnings
>> would point to entirely the wrong line and column.
>
> Oh, OK, the problem was in the lines/column output, not with detecting
> the doc strings themselves?
>
> Yes, that sounds like a problem, but...  I think we can live with
> inaccurate lines here.

So I tried enabling warnings also for lambda's and here is a fairly
exhaustive list of things that leads to problems:

1. cl-defun

In auth-source-netrc-search:
auth-source.el:1224:11: Warning: docstring wider than 80 characters

Here, the offending line is this autogenerated line:

(fn &rest SPEC &key BACKEND REQUIRE CREATE TYPE MAX HOST USER PORT
&allow-other-keys)

But this line has to be like this if `C-h f' is to show anything but
(&rest spec) for the function parameters.  So should we just add some
special case where we ignore the last line if it matches "^([^)])$"?

2. cl-defstruct

In epg-context--make:
epg.el:197:30: Warning: docstring wider than 80 characters

Here I can't figure out why the docstring is too long.  Using
`macrostep-expand' doesn't reveal why.  Does anyone have any ideas?

3. defclass

These are autogenerated docstrings that I don't trivially see how we can
just wrap as it's the first line that is too long:

In jsonrpc-request:
jsonrpc.el:349:15: Warning: docstring wider than 80 characters

In toplevel form:
cedet/ede/proj-comp.el:71:26: Warning: docstring wider than 80 characters

I suppose we need to rethink these, somehow.

4. semantic

These macros results in wide docstrings in some cases:

- define-overloadable-function
- define-semantic-decoration-style
- define-mode-local-override

5. define-derived-mode

Finally, we have some remaining cases where define-derived-mode leads to
wide docstrings, e.g.:

In newsticker-treeview-list-mode:
net/newst-treeview.el:2031:52: Warning: docstring wider than 80 characters

These should be easy to fix, and I already have an idea for how.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Thu, 31 Dec 2020 05:42:16 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

> But this line has to be like this if `C-h f' is to show anything but
> (&rest spec) for the function parameters.  So should we just add some
> special case where we ignore the last line if it matches "^([^)])$"?

Sounds reasonable.

>
> 2. cl-defstruct
>
> In epg-context--make:
> epg.el:197:30: Warning: docstring wider than 80 characters
>
> Here I can't figure out why the docstring is too long.  Using
> `macrostep-expand' doesn't reveal why.  Does anyone have any ideas?

Hm.  The doc string (as displayed by the help functions) is:

epg-context--make is a compiled Lisp function in ‘epg.el’.

(epg-context--make PROTOCOL &optional ARMOR TEXTMODE INCLUDE-CERTS
CIPHER-ALGORITHM DIGEST-ALGORITHM COMPRESS-ALGORITHM)

  This function does not change global state, including the match data.

Constructor for objects of type ‘epg-context’.

Has that been folded at some point after your function has looked at it?

> 3. defclass
>
> These are autogenerated docstrings that I don't trivially see how we can
> just wrap as it's the first line that is too long:
>
> In jsonrpc-request:
> jsonrpc.el:349:15: Warning: docstring wider than 80 characters
>
> In toplevel form:
> cedet/ede/proj-comp.el:71:26: Warning: docstring wider than 80 characters
>
> I suppose we need to rethink these, somehow.

These bits can be filled:

Specialized Methods:

‘jsonrpc--next-request-id’ ((this jsonrpc-connection))
Retrieve the slot ‘-next-request-id’ from an object of class ‘jsonrpc-connection’.

These bits:

Instance Allocated Slots:

	Name	Type	Default	Doc
	————	————	———————	———
	name	t	unbound	A name for the connection
	-request-dispatcher	t	#'ignore	Dispatcher for remotely invoked requests.
	-notification-dispatcher	t	#'ignore	Dispatc


Have to be reformatted -- just moving the Doc to the next line should be
sufficient for most things.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: 44858 <at> debbugs.gnu.org
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: [PATCH] Make byte-compiler warn about wide docstrings
Date: Fri, 24 Sep 2021 10:25:29 -0700

Stefan Kangas <stefan <at> marxist.se> writes:

> I've been messing around with getting defuns to work, but I can't find a
> way for it to work reliably.

In this bug report, we have discussed several issues relating to warning
about too wide docstrings also for defun/lambda.  I believe I've solved
all remaining issues; I can now run "make bootstrap" with no warnings.

Please see the branch 'scratch/bug-44858'.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 25 Sep 2021 03:07:07 +0200

Stefan Kangas <stefan <at> marxist.se> writes:

> In this bug report, we have discussed several issues relating to warning
> about too wide docstrings also for defun/lambda.  I believe I've solved
> all remaining issues; I can now run "make bootstrap" with no warnings.

Cool!

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 44858 <at> debbugs.gnu.org
Subject: Re: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sun, 26 Sep 2021 04:38:47 -0700

tags 44858 fixed
close 44858 28.1
thanks

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> In this bug report, we have discussed several issues relating to warning
>> about too wide docstrings also for defun/lambda.  I believe I've solved
>> all remaining issues; I can now run "make bootstrap" with no warnings.
>
> Cool!

Now pushed to master (commits c78e16962e..c51b1c02db).

I'm consequently closing this bug report.

Previous Next

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