GNU bug report logs - #65790
29.1; "docstring wider than 80 characters" when there is no docstring

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 65790 in the body.
You can then email your comments to 65790 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: Damien Cassou <damien <at> cassou.me>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.1; "docstring wider than 80 characters" when there is no docstring
Date: Wed, 06 Sep 2023 21:08:29 +0200

I have this piece of code:

(cl-defstruct (json-process-client-application
               (:constructor json-process-client--application-create)
               (:conc-name json-process-client--application-))
  message-callbacks)

In GNU Emacs 29.1, "Warning: docstring wider than 80 characters" even
though I wrote no docstring here. I understand the problem lies in the
generated code and I agree the lisp names in my code are long. But, I
don't think the byte compiler should annoy me about docstrings I didn't
write.

What do you think?

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Damien Cassou <damien <at> cassou.me>
Cc: 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1;
 "docstring wider than 80 characters" when there is no docstring
Date: Thu, 07 Sep 2023 08:04:28 +0300

> From: Damien Cassou <damien <at> cassou.me>
> Date: Wed, 06 Sep 2023 21:08:29 +0200
> 
> I have this piece of code:
> 
> (cl-defstruct (json-process-client-application
>                (:constructor json-process-client--application-create)
>                (:conc-name json-process-client--application-))
>   message-callbacks)
> 
> In GNU Emacs 29.1, "Warning: docstring wider than 80 characters" even
> though I wrote no docstring here. I understand the problem lies in the
> generated code and I agree the lisp names in my code are long. But, I
> don't think the byte compiler should annoy me about docstrings I didn't
> write.
> 
> What do you think?

How else will we/you know there's a potential problem there?

If you think you can do nothing about this problem, just ignore the
warning.  It's just a warning.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Damien Cassou <damien <at> cassou.me>, 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when there
 is no docstring
Date: Thu, 7 Sep 2023 01:52:00 -0700

Damien Cassou <damien <at> cassou.me> writes:

> I have this piece of code:
>
> (cl-defstruct (json-process-client-application
>                (:constructor json-process-client--application-create)
>                (:conc-name json-process-client--application-))
>   message-callbacks)
>
> In GNU Emacs 29.1, "Warning: docstring wider than 80 characters" even
> though I wrote no docstring here. I understand the problem lies in the
> generated code and I agree the lisp names in my code are long. But, I
> don't think the byte compiler should annoy me about docstrings I didn't
> write.
>
> What do you think?

Yes, it would be nice not to get that warning.  I fixed many such
problems when I first added the warning about long docstrings.

Patches welcome.

PS. As a work-around, you can set `byte-compile-docstring-max-column'.

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

From: Damien Cassou <damien <at> cassou.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when
 there is no docstring
Date: Thu, 07 Sep 2023 11:30:10 +0200

Hi Eli,

Eli Zaretskii <eliz <at> gnu.org> writes:
> How else will we/you know there's a potential problem there?


my opinion is that if more than 80 characters in a docstring is a
problem then the macro should make sure not to generate one.


> If you think you can do nothing about this problem, just ignore the
> warning.  It's just a warning.


I hate warnings: either a problem is important and I want the build to
fail or there is no problem and I don't want to see anything. If I let
warnings exist then the ones I want to ignore will slowly be too
numerous for me to notice those I would like to fix.

This is why in my tool chain (makel) I'm always setting
`byte-compile-error-on-warn' to t and I do similar things in non
elisp-projects.


This is just my opinion.

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

From: Damien Cassou <damien <at> cassou.me>
To: Stefan Kangas <stefankangas <at> gmail.com>, 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when
 there is no docstring
Date: Thu, 07 Sep 2023 11:33:01 +0200

Stefan Kangas <stefankangas <at> gmail.com> writes:
> Yes, it would be nice not to get that warning.  I fixed many such
> problems when I first added the warning about long docstrings.
>
> Patches welcome.


The only patch I can think of would change the macro so that the
docstring references each name on a line of its own. This way, we would
only get a warning if the name itself is more than 80 characters. Would
that be a reasonable patch for you?


> PS. As a work-around, you can set `byte-compile-docstring-max-column'.

Thank you for this.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Damien Cassou <damien <at> cassou.me>, 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when there
 is no docstring
Date: Thu, 7 Sep 2023 02:47:09 -0700

Damien Cassou <damien <at> cassou.me> writes:

> The only patch I can think of would change the macro so that the
> docstring references each name on a line of its own. This way, we would
> only get a warning if the name itself is more than 80 characters. Would
> that be a reasonable patch for you?

That's basically the solution, yes, but we probably want to wrap the
lines only when it's necessary.  See `internal--fill-string-single-line'
and where it is used for inspiration.

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

From: Damien Cassou <damien <at> cassou.me>
To: Stefan Kangas <stefankangas <at> gmail.com>, 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when
 there is no docstring
Date: Sun, 10 Sep 2023 08:47:42 +0200

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

Stefan Kangas <stefankangas <at> gmail.com> writes:
> That's basically the solution, yes, but we probably want to wrap the
> lines only when it's necessary.  See `internal--fill-string-single-line'
> and where it is used for inspiration.

What do you think about attached patch? The patch replaces one usage of
`format' with `internal--format-docstring-line' and also slightly change
2 docstring texts.


-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[0001-Shorten-docstrings-generated-in-cl-macs.el.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Damien Cassou <damien <at> cassou.me>, 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when there
 is no docstring
Date: Sun, 10 Sep 2023 02:55:59 -0700

Damien Cassou <damien <at> cassou.me> writes:

> What do you think about attached patch? The patch replaces one usage of
> `format' with `internal--format-docstring-line' and also slightly change
> 2 docstring texts.

Thanks, some comments below.

I think this also could use some tests, see bytecomp-tests.el:964 and
onwards.

> From 093c76caa8ac551868565d0e690b9979593cf94d Mon Sep 17 00:00:00 2001
> From: Damien Cassou <damien <at> cassou.me>
> Date: Sun, 10 Sep 2023 08:40:52 +0200
> Subject: [PATCH] Shorten docstrings generated in cl-macs.el
>
> * lisp/emacs-lisp/cl-macs.el (cl-defsubst): Reduce likelihood of
> "docstring wider than 80 characters" errors in generated code.

Please remember to include the bug number when it is known.

> ---
>  lisp/emacs-lisp/cl-macs.el | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
> index c8e2610c8b0..0f142b87e07 100644
> --- a/lisp/emacs-lisp/cl-macs.el
> +++ b/lisp/emacs-lisp/cl-macs.el
> @@ -2931,7 +2931,15 @@ cl-defsubst
>               ,(if (memq '&key args)
>                    `(&whole cl-whole &cl-quote ,@args)
>                  (cons '&cl-quote args))
> -             ,(format "compiler-macro for inlining `%s'." name)
> +             ;; NB.  This will produce incorrect results
> +             ;; in some cases, as our coding conventions
> +             ;; says that the first line must be a full
> +             ;; sentence.  However, if we don't word wrap
> +             ;; we will have byte-compiler warnings about
> +             ;; overly long docstrings.  So we can't have
> +             ;; a perfect result here, and choose to avoid
> +             ;; the byte-compiler warnings.
> +             ,(internal--format-docstring-line "compiler-macro for `%s'." name)

Why drop the word "inlining"?

(This comment might need reflowing.)

>               (cl--defsubst-expand
>                ',argns '(cl-block ,name ,@(cdr (macroexp-parse-body body)))
>                nil
> @@ -3190,7 +3198,10 @@ cl-defstruct
>                           ;; a perfect result here, and choose to avoid
>                           ;; the byte-compiler warnings.
>                           (internal--format-docstring-line
> -                          "Access slot \"%s\" of `%s' struct CL-X." slot name)
> +                          "Access slot \"%s\" of CL-X." slot)
> +                         "\n"
> +                         (internal--format-docstring-line
> +                          "Struct CL-X is a `%s'." name)

Could we keep the old format when possible, and use the new one only
when needed?

>                           (if doc (concat "\n" doc) ""))
>                         (declare (side-effect-free t))
>                         ,access-body)
> --
> 2.41.0

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

From: Damien Cassou <damien <at> cassou.me>
To: Stefan Kangas <stefankangas <at> gmail.com>, 65790 <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when
 there is no docstring
Date: Tue, 12 Sep 2023 21:22:26 +0200

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

Thank you for your feedback. 3 patch files are attached to this
email. My answers below:

Stefan Kangas <stefankangas <at> gmail.com> writes:
> Damien Cassou <damien <at> cassou.me> writes:
> I think this also could use some tests, see bytecomp-tests.el:964 and
> onwards.


I've added tests to bytecomp-tests.el as suggested. (why not
cl-macs-tests.el instead?)


>> * lisp/emacs-lisp/cl-macs.el (cl-defsubst): Reduce likelihood of
>> "docstring wider than 80 characters" errors in generated code.
>
> Please remember to include the bug number when it is known.


done

>> -             ,(format "compiler-macro for inlining `%s'." name)
>> […]
>> +             ,(internal--format-docstring-line "compiler-macro for `%s'." name)
>
> Why drop the word "inlining"?


I felt this word was not really important because (1) this is generated
code and (2) the docstring starts with "compiler-macro". Removing it
reduces the likelihood of the error message. I added an explanation in
the commit message. If the word is important I can add it back.


> (This comment might need reflowing.)


done


> Could we keep the old format when possible, and use the new one only
> when needed?


I tried to do that, is that what you want? I feel the added complexity
isn't worth it so I'm fine changing it back to the simpler version if
you change your mind.


-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[0001-bytecomp-tests.el-Add-new-helper-function.patch (text/x-patch, attachment)]

[0002-Shorten-docstrings-generated-by-cl-defsubst-bug-6579.patch (text/x-patch, attachment)]

[0003-Shorten-docstrings-generated-by-cl-defstruct-bug-657.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Damien Cassou <damien <at> cassou.me>, 65790-done <at> debbugs.gnu.org
Subject: Re: bug#65790: 29.1; "docstring wider than 80 characters" when there
 is no docstring
Date: Wed, 13 Sep 2023 07:38:31 -0700

Version: 30.1

Damien Cassou <damien <at> cassou.me> writes:

> I've added tests to bytecomp-tests.el as suggested. (why not
> cl-macs-tests.el instead?)

Good question.  Perhaps we should move all of those tests to more
relevant locations.  OTOH, the macro for this is in bytecomp-tests.el
currently, so I guess we'd have to move that.  Maybe it would fit in
ert-x.el.  It seems outside the scope of this bug report though.

>> Could we keep the old format when possible, and use the new one only
>> when needed?
>
> I tried to do that, is that what you want? I feel the added complexity
> isn't worth it so I'm fine changing it back to the simpler version if
> you change your mind.

The code is less nice obviously, but I think not changing the
autogenerated docstrings in the typical cases is also important.
Therefore, I think the complexity is ultimately worth it.  Perhaps
someone should do a round over all of this to see if it can be cleaned
up a bit, though...

I've now installed your patches on master.  Feel free to post followups
if you want to fix any of the above, or if you spot anything else that
could be improved in relation to this.

Thanks again for your efforts.

Previous Next

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