bytecomp: doc string wider than 80 spurious warnings are back

bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

I had reported this a few months ago and it was fixed; sadly they are
back; hitting  this when using hydra --- same as before.

The warnings are spurious since they are not fixable in the code where
the warnings point.

-- 

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Stefan Kangas]

"T.V Raman" <raman@google.com> writes:

> I had reported this a few months ago and it was fixed; sadly they are
> back; hitting  this when using hydra --- same as before.
>
> The warnings are spurious since they are not fixable in the code where
> the warnings point.

Thanks, but please give more details.  In which code are you seeing
these warnings?  Could you provide some sample code?

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

Here are some examples:

Warning:
emacspeak-muggles.el:327:2: Warning: docstring wider than 80 characters
https://github.com/tvraman/emacspeak/blob/master/lisp/emacspeak-muggles.el#L327

Hope this helps.

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Stefan Kangas]

"T.V Raman" <raman@google.com> writes:

> Here are some examples:
>
> Warning:
> emacspeak-muggles.el:327:2: Warning: docstring wider than 80 characters
> https://github.com/tvraman/emacspeak/blob/master/lisp/emacspeak-muggles.el#L327
>
> Hope this helps.

Thanks, it does.

It seems like this is caused by the `defhydra' macro, which creates
defuns with overly long docstrings.  Thus, I don't think we can fix it
on our end, and this should be reported to the hydra maintainers
instead.  I've Cc:ed the hydra maintainer Oleh Krehel.

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

Hope  it gets fixed upstream in Hydra. But stepping back a level:

1. Byte Compiler warnings are really useful when developing in Emacs
   Lisp.
   2. But they lose their value if the byte compiler gets noisy -- in
      this case I think this warning qualifies as noise because:

      A. The developer who is hit with this warning can do nothing
         about it
         B. It obscures more useful warnings

            And by the way when this was fixed a few  months ago, it
            ws fixed in the Emacs tree.
            

Stefan Kangas writes:
 > "T.V Raman" <raman@google.com> writes:
 > 
 > > Here are some examples:
 > >
 > > Warning:
 > > emacspeak-muggles.el:327:2: Warning: docstring wider than 80 characters
 > > https://github.com/tvraman/emacspeak/blob/master/lisp/emacspeak-muggles.el#L327
 > >
 > > Hope this helps.
 > 
 > Thanks, it does.
 > 
 > It seems like this is caused by the `defhydra' macro, which creates
 > defuns with overly long docstrings.  Thus, I don't think we can fix it
 > on our end, and this should be reported to the hydra maintainers
 > instead.  I've Cc:ed the hydra maintainer Oleh Krehel.

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Emanuel Berg]

T.V Raman wrote:

> Hope  it gets fixed upstream in Hydra. But stepping back a level:
>
> 1. Byte Compiler warnings are really useful when developing in Emacs
>    Lisp.
>    2. But they lose their value if the byte compiler gets noisy -- in
>       this case I think this warning qualifies as noise because:
>
>       A. The developer who is hit with this warning can do nothing
>          about it
>          B. It obscures more useful warnings
>
>             And by the way when this was fixed a few  months ago, it
>             ws fixed in the Emacs tree.

Maybe, but the only way to approach it is still "does this
warning make sense or not?" If it does, it should, regardless
of everything else, be left to say what it says.

Especially with native-compile there are tons of warnings,
instalation, vanilla Emacs, ELPA, you name it, but again, the
problem isn't the warnings but the people who write to code
who don't fix them, really.

Maybe native-compile in particular is too new and people will,
when they start to use it more, be more aware of those
warnings and more motivated to fix them.

As for this docstring warning not to have lines over 80 chars
... isn't that a good warning, that makes sense to have?

There is also this you can run, for packages

  (checkdoc-current-buffer t)

And this - related - to improve the Elisp quality:

  https://hyperscope.link/3/7/7/3/0/Your-hjälpsam-Package-Header-Assistant-37730.html

Hm, that's a lot of stuff to help us improve, maybe someone
can integrate it into a linter or whatever the technical term
is, a static code analyzer?

In *Packages* for ELPA and MELPA, M-x how-many RET lint RET is
88, but that is for all languages represented there, e.g.

  closure-lint-mode 20101118.2124 available melpa minor mode for the Closure Linter

for Closure and so on.

-- 
underground experts united
https://dataswamp.org/~incal

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

For docstring, since Emacs is the one that is going to display the
docstring, I think code in the Help system should take care of
formatting the  docstring to match the size of the window, not rely on
the developer to  write the docstring to match  a hard-coded width. The
easiest thing for the help buffer to do might be to just turn on
visual-line-mode, so yes, I think this warning is ill-adviced and noisy

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Emanuel Berg]

T.V Raman wrote:

> For docstring, since Emacs is the one that is going to
> display the docstring, I think code in the Help system
> should take care of formatting the docstring to match the
> size of the window, not rely on the developer to write the
> docstring to match a hard-coded width. The easiest thing for
> the help buffer to do might be to just turn on
> visual-line-mode, so yes, I think this warning is
> ill-adviced and noisy

But maybe that will break lines where not intended/indented?

Also 80 columns is some safe standard for virtual
terminals, right?

Try 'echo $COLUMNS', what do you get? I get 80 in the
Linux VTs, and 90 in xterm.

-- 
underground experts united
https://dataswamp.org/~incal

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Stefan Kangas]

"T.V Raman" <raman@google.com> writes:

> Hope  it gets fixed upstream in Hydra. But stepping back a level:
>
> 1. Byte Compiler warnings are really useful when developing in Emacs
>    Lisp.
>    2. But they lose their value if the byte compiler gets noisy

No disagreement there.

>                                                                 -- in
>       this case I think this warning qualifies as noise because:
>
>       A. The developer who is hit with this warning can do nothing
>          about it
>          B. It obscures more useful warnings
>

A typical case looks like this:

  (defmacro foo (name)
    `(defun bar ()
       ,(format "foo %s." name)))

If someone passes in a string NAME longer than 80 characters, that will
generate a warning.  It is the responsibility of whoever writes a macro
to ensure it doesn't generate long docstrings by properly wrapping it.
The same is true for any byte-compiler warning.

In core we use `internal--format-docstring-line' for this, which means
that fixed code for the above would look like this:

    (defmacro foo (name)
      `(defun bar ()
         ,(internal--format-docstring-line
           (format "foo %s." name))))

I don't think there's anything we can do about macros in third-party
packages, unfortunately.  Perhaps `internal--format-docstring-line' is
useful enough not to be marked internal, though?  I'm not sure.

>             And by the way when this was fixed a few  months ago, it
>             ws fixed in the Emacs tree.

But that warning was due to a macro in our tree, right?

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Stefan Kangas]

"T.V Raman" <raman@google.com> writes:

> For docstring, since Emacs is the one that is going to display the
> docstring, I think code in the Help system should take care of
> formatting the  docstring to match the size of the window, not rely on
> the developer to  write the docstring to match  a hard-coded width. The
> easiest thing for the help buffer to do might be to just turn on
> visual-line-mode, so yes, I think this warning is ill-adviced and noisy

Note that you can set `byte-compile-docstring-max-column' to a large
number if you don't like this warning, or just want to tweak it.

Automating more aspects of docstring formatting is a good idea, but it
is hard to do correctly.  Our docstrings just vary too much in style and
format.  I've thought about it, and I don't see how to solve this
problem generally without introducing more powerful markup, or perhaps
even replacing the one we have now with something better.

So, as always, we're stuck with what we have now until someone works on
something better.

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

Agreed on everything except the last: Yes, we  cant do anything about
third party macros, but we  == Emacs is the one displaying the
docstring, so we can surely break ourselves free from the docstring
not  being wider than 80 chars; and for instance, in your example, the
function name passed is  a 128-char hyphenated string, then how to
handle that is still better handled in the display algorithm, 

Stefan Kangas writes:
 > "T.V Raman" <raman@google.com> writes:
 > 
 > > Hope  it gets fixed upstream in Hydra. But stepping back a level:
 > >
 > > 1. Byte Compiler warnings are really useful when developing in Emacs
 > >    Lisp.
 > >    2. But they lose their value if the byte compiler gets noisy
 > 
 > No disagreement there.
 > 
 > >                                                                 -- in
 > >       this case I think this warning qualifies as noise because:
 > >
 > >       A. The developer who is hit with this warning can do nothing
 > >          about it
 > >          B. It obscures more useful warnings
 > >
 > 
 > A typical case looks like this:
 > 
 >   (defmacro foo (name)
 >     `(defun bar ()
 >        ,(format "foo %s." name)))
 > 
 > If someone passes in a string NAME longer than 80 characters, that will
 > generate a warning.  It is the responsibility of whoever writes a macro
 > to ensure it doesn't generate long docstrings by properly wrapping it.
 > The same is true for any byte-compiler warning.
 > 
 > In core we use `internal--format-docstring-line' for this, which means
 > that fixed code for the above would look like this:
 > 
 >     (defmacro foo (name)
 >       `(defun bar ()
 >          ,(internal--format-docstring-line
 >            (format "foo %s." name))))
 > 
 > I don't think there's anything we can do about macros in third-party
 > packages, unfortunately.  Perhaps `internal--format-docstring-line' is
 > useful enough not to be marked internal, though?  I'm not sure.
 > 
 > >             And by the way when this was fixed a few  months ago, it
 > >             ws fixed in the Emacs tree.
 > 
 > But that warning was due to a macro in our tree, right?

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

Incidentally adding a form of the form (setq byte-compiler-warnings
'(not docstrings))
failed to suppress that warning. I dont like turning off warnings
everywhere -- even an annoying warning in general.

We can agree to disagree, but I think this is an example of the
good-enough being chased away by a desire for perfection.
-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Michael Heerdegen]

"T.V Raman" <raman@google.com> writes:

> Incidentally adding a form of the form (setq byte-compiler-warnings
> '(not docstrings))
> failed to suppress that warning.

Note: The name of the variable is `byte-compile-warnings', not
`byte-compiler-warnings' (i.e., without character "r" after "compile").

If the authors of hydra don't care they should probably just add a file
local binding for that variable.  Or use `with-suppressed-warnings' in
the respective macro definitions, that should also work I think.

Michael.

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

P.S. Mike, thanks for pointing out the typo in the variable name that I
had that was causing the warnings to be not suppressed, fixed now and
suppressed -- so I can move to more productive things:-)

Talking of opinionated warnings, the warning "foo might not be defined
at runtime" was extremely useful when making sure that cl calls were
namespaced correctly; but now on trunk there is another "opinionated"
change where that warning is being applied to all packages being loaded
and that is also noisy and likely unnecessary though not at the same
level as docstrings.

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Eli Zaretskii]

> From: "T.V Raman" <raman@google.com>
> Date: Wed, 27 Sep 2023 12:56:01 -0700
> 
> For docstring, since Emacs is the one that is going to display the
> docstring, I think code in the Help system should take care of
> formatting the  docstring to match the size of the window, not rely on
> the developer to  write the docstring to match  a hard-coded width.

This cannot be done reliably, since doc strings are not really free
text.  They have sections that must not be merged, the first line must
be distinct, there are subtleties with quoting and buttons, etc.

> The easiest thing for the help buffer to do might be to just turn on
> visual-line-mode

I don't want to do that for the above reasons, sorry.

> so yes, I think this warning is ill-adviced and noisy

We disagree.  This warning helped clean up many packages, both in and
outside of Emacs, so it is definitely useful.  If the warning is
caused by some macro package, it should be reported to the developers
of that macro package, and fixed by them.  But disregarding these
issues is a bad policy in the long run.

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> We disagree.  This warning helped clean up many packages, both in and
> outside of Emacs, so it is definitely useful.  If the warning is
> caused by some macro package, it should be reported to the developers
> of that macro package, and fixed by them.  But disregarding these
> issues is a bad policy in the long run.

I don't feel very strongly about this, and can see myself going either
way:

- On the one hand, the benefits of this warning are clear.

- On the other, one could argue that this is a tad too opinionated a
  default for the byte-compiler.

I personally wouldn't object to a patch that disabled these warnings by
default, as long as we kept them enabled in our tree.  But that's me.

BTW, I'm curious why people find these particular warnings so intrusive,
and not the checkdoc defaults, which I personally find both more
opinionated and more noisy (e.g. when `checkdoc--argument-missing-flag'
is t, entire docstrings turns red in `flymake-mode').

Re: bytecomp: doc string wider than 80 spurious warnings are back

[T.V Raman]

Specifically re the checkdoc vs bytecomp warning:

Checkdoc is something you run yourself specifically to check, it's
like Lint, so you run it when you want it.

Bytecomp is something far more central to elisp development.


Stefan Kangas writes:
 > Eli Zaretskii <eliz@gnu.org> writes:
 > 
 > > We disagree.  This warning helped clean up many packages, both in and
 > > outside of Emacs, so it is definitely useful.  If the warning is
 > > caused by some macro package, it should be reported to the developers
 > > of that macro package, and fixed by them.  But disregarding these
 > > issues is a bad policy in the long run.
 > 
 > I don't feel very strongly about this, and can see myself going either
 > way:
 > 
 > - On the one hand, the benefits of this warning are clear.
 > 
 > - On the other, one could argue that this is a tad too opinionated a
 >   default for the byte-compiler.
 > 
 > I personally wouldn't object to a patch that disabled these warnings by
 > default, as long as we kept them enabled in our tree.  But that's me.
 > 
 > BTW, I'm curious why people find these particular warnings so intrusive,
 > and not the checkdoc defaults, which I personally find both more
 > opinionated and more noisy (e.g. when `checkdoc--argument-missing-flag'
 > is t, entire docstrings turns red in `flymake-mode').

-- 

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Emanuel Berg]

T.V Raman wrote:

> Checkdoc is something you run yourself specifically to
> check, it's like Lint, so you run it when you want it.
>
> Bytecomp is something far more central to elisp development.

Perhaps, but I don't see any huge difference or reason the
byte compiler can say what checkdoc says for files that
are packages?

-- 
underground experts united
https://dataswamp.org/~incal

Re: bytecomp: doc string wider than 80 spurious warnings are back

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 29 Sep 2023 07:16:37 -0700
> Cc: emacs-devel@gnu.org
> 
> I don't feel very strongly about this, and can see myself going either
> way:
> 
> - On the one hand, the benefits of this warning are clear.
> 
> - On the other, one could argue that this is a tad too opinionated a
>   default for the byte-compiler.

We added quite a few of warnings on master, which to me says we are
actively trying to discover and warn about more problems.  Why would
we handle this particular warning differently?

> I personally wouldn't object to a patch that disabled these warnings by
> default, as long as we kept them enabled in our tree.  But that's me.

I tried to explain in another message that this has non-trivial
downsides.

> BTW, I'm curious why people find these particular warnings so intrusive,
> and not the checkdoc defaults, which I personally find both more
> opinionated and more noisy (e.g. when `checkdoc--argument-missing-flag'
> is t, entire docstrings turns red in `flymake-mode').

Agreed.  And we do ask packages to pass checkdoc before they are
accepted.