bug#44858: [PATCH] Make byte-compiler warn about wide docstrings

[Stefan Kangas <stefan@marxist.se>]

Eli Zaretskii <eliz@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?