diff mbox series

[bug#59217] guix: lint: Improve message in linter warning.

Message ID 20221112150907.29793-1-jgart@dismail.de
State New
Headers show
Series [bug#59217] guix: lint: Improve message in linter warning. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success
cbaines/issue success View issue

Commit Message

jgart Nov. 12, 2022, 3:09 p.m. UTC
* guix/lint.scm (check-end-of-sentence-space): Improve message.
---
 guix/lint.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

jgart Nov. 12, 2022, 3:15 p.m. UTC | #1
On Sat, 12 Nov 2022 15:10:02 +0000 help-debbugs@gnu.org (GNU bug Tracking System) wrote:
> Thank you for filing a new bug report with debbugs.gnu.org.

Hi Arun,

Not sure if you remember that conversation we had about the linter
warning regarding emacs and having to use two spaces in description
field sentences.

Would you mind recapping the issue so that I can improve this message further.

What I wrote is currently vague:

Why do the sentences in the description field have to be separated by two spaces due to Emacs?

all best,

jgart
Ludovic Courtès Nov. 19, 2022, 6:37 p.m. UTC | #2
Hi,

jgart <jgart@dismail.de> skribis:

> * guix/lint.scm (check-end-of-sentence-space): Improve message.

[...]

> -by two spaces; possible infraction~p at ~{~a~^, ~}")
> +by two spaces due to issues wth Emacs; possible infraction~p at ~{~a~^, ~}")

Probably the two-space story needs an explanation, but what about adding
it to the manual, with a cross-ref to the relevant Emacs manual section?

Thanks,
Ludo’.
Arun Isaac Nov. 22, 2022, 8:58 p.m. UTC | #3
Hi jgart,

> Not sure if you remember that conversation we had about the linter
> warning regarding emacs and having to use two spaces in description
> field sentences.

Yes, I remember the conversation.

> Would you mind recapping the issue so that I can improve this message
> further.

My original point was that the linter should not simply complain about
the absence of double spaces but also explain why double spaces are
required. Double spaces are required because Emacs sentence commands
such as M-a and M-e only work when sentences are separated by double
spaces. To be honest, I would consider this an Emacs bug. But, it's
probably impossible to achieve consensus on such a bold claim. ;-)

So, at the very least, the Guix linter should explain why double spaces
are required. As a general rule for all linter messages, contributors
will be happier when the linter explains the rationale for various
messages. Most people don't like doing things without understanding why
they have to do them.

Regards,
Arun
Simon Tournier Nov. 23, 2022, 10:13 a.m. UTC | #4
Hi Arun,

On Tue, 22 Nov 2022 at 20:58, Arun Isaac <arunisaac@systemreboot.net> wrote:

> My original point was that the linter should not simply complain about
> the absence of double spaces but also explain why double spaces are
> required. Double spaces are required because Emacs sentence commands
> such as M-a and M-e only work when sentences are separated by double
> spaces. To be honest, I would consider this an Emacs bug. But, it's
> probably impossible to achieve consensus on such a bold claim. ;-)

Indeed, it is impossible to achieve consensus since it is probably not
an Emacs bug but an historical inheritance of typographical convention,
see [1,2].

1: <https://en.wikipedia.org/wiki/Sentence_spacing>
2: <https://en.wikipedia.org/wiki/History_of_sentence_spacing>


> So, at the very least, the Guix linter should explain why double spaces
> are required. As a general rule for all linter messages, contributors
> will be happier when the linter explains the rationale for various
> messages. Most people don't like doing things without understanding why
> they have to do them.

Well, maybe a paragraph in the manual under ’(guix) Synopses and
Descriptions’ is a better location for such explanations than the linter
message itself.  WDYT?


Cheers,
simon
Arun Isaac Nov. 23, 2022, 12:52 p.m. UTC | #5
> Well, maybe a paragraph in the manual under ’(guix) Synopses and
> Descriptions’ is a better location for such explanations than the linter
> message itself.  WDYT?

That sounds good. Detailed explanations do belong in the manual. But,
the linter CLI output should link to the relevant section of the web
manual. That would be better than sending people hunting in the
manual. Many good linters for other programming languages do do this.

Cheers!
Maxim Cournoyer March 21, 2023, 7:50 p.m. UTC | #6
Hi,

Arun Isaac <arunisaac@systemreboot.net> writes:

>> Well, maybe a paragraph in the manual under ’(guix) Synopses and
>> Descriptions’ is a better location for such explanations than the linter
>> message itself.  WDYT?
>
> That sounds good. Detailed explanations do belong in the manual. But,
> the linter CLI output should link to the relevant section of the web
> manual. That would be better than sending people hunting in the
> manual. Many good linters for other programming languages do do this.

Instead of a bug in Emacs, I'd say it's rather a convention, rooted in
the fact that without the two-spaces trick, how would you reliably
locate the end of a sentence?  A period followed by a capital letter
might happen in the middle of a sentence, which makes it ambiguous.
Note that two-spaces also exists as a typographical convention, called
"double sentence spacing".  Here's some interesting quote from the
Wikipedia page [0]:

   The text-editing environment in Emacs uses a double space following a
   period to identify the end of sentences unambiguously; the
   double-space convention prevents confusion with periods within
   sentences that signify abbreviations. How Emacs recognizes the end of
   a sentence is controlled by the settings sentence-end-double-space
   and sentence-end.[71]

   The Unix typesetter program Troff uses two spaces to mark the end of
   a sentence.[72] This allows the typesetter to distinguish sentence
   endings from abbreviations and to typeset them differently. Early
   versions of Troff,[72] which only typeset in fixed-width fonts, would
   automatically add a second space between sentences, which were
   detected based on the combination of terminal punctuation and a line
   feed.

Personally, I'd prefer not having explanations directly in the output of
Guix lint; it should be terse, as it's involved often and repeatedly.

[0]  https://en.wikipedia.org/wiki/Sentence_spacing
Arun Isaac March 22, 2023, 9:22 p.m. UTC | #7
Hi Maxim,

> Personally, I'd prefer not having explanations directly in the output of
> Guix lint; it should be terse, as it's involved often and repeatedly.

I see your point. But, perhaps there should at least be a
link. Something like

/home/arun/guix/gnu/packages/web.scm:8030:17: tissue@0.1.0: sentences in
description should be followed by two spaces; possible infraction at 313
(see
https://guix.gnu.org/manual/en/html_node/guix-lint-errors.html#two-spaces
for details)

Or a lint error code that can be explained in detail in the manual:

/home/arun/guix/gnu/packages/web.scm:8030:17: tissue@0.1.0: sentences in
description should be followed by two spaces; possible infraction at 313
(#12345)

Or a --verbose flag that can enable longer explanations.

New users should not be baffled by error messages and should have some
way of learning more without having to ask on the mailing list or do
extensive research.

Cheers!
jgart March 22, 2023, 9:37 p.m. UTC | #8
> New users should not be baffled by error messages and should have some
> way of learning more without having to ask on the mailing list or do
> extensive research.

I agree with this. This is esoteric historical knowledge that could benefit from a bit of help by hinting.
Maxim Cournoyer March 23, 2023, 2:52 a.m. UTC | #9
Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> writes:

> Hi Maxim,
>
>> Personally, I'd prefer not having explanations directly in the output of
>> Guix lint; it should be terse, as it's involved often and repeatedly.
>
> I see your point. But, perhaps there should at least be a
> link. Something like
>
> /home/arun/guix/gnu/packages/web.scm:8030:17: tissue@0.1.0: sentences in
> description should be followed by two spaces; possible infraction at 313
> (see
> https://guix.gnu.org/manual/en/html_node/guix-lint-errors.html#two-spaces
> for details)
>
> Or a lint error code that can be explained in detail in the manual:
>
> /home/arun/guix/gnu/packages/web.scm:8030:17: tissue@0.1.0: sentences in
> description should be followed by two spaces; possible infraction at 313
> (#12345)

I think I like the error code more; the link is too verbose to my taste,
and I prefer to stay outside of the browser as much as I can ;-).

It seems error codes that can be documented somewhere else are common
for linters (e.g., pylint).  It then also give some way to disable the
lint checks by annotating the source with e.g.

--8<---------------cut here---------------start------------->8---
;disable #12345
--8<---------------cut here---------------end--------------->8---

I'm not sure we'd want to do that, but that'd be an option for the
future if we had some documented codes for the checks.

> Or a --verbose flag that can enable longer explanations.

That's another interesting approach.  On the bad side, you'd probably
often run 'guix lint', then realize you needed --verbose, and have to
run it again (and it's slow).

> New users should not be baffled by error messages and should have some
> way of learning more without having to ask on the mailing list or do
> extensive research.

I agree there's merit to improving the situation!
diff mbox series

Patch

diff --git a/guix/lint.scm b/guix/lint.scm
index 8e3976171f..37a9539e2c 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -14,6 +14,7 @@ 
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos@telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2022 jgart <jgart@dismail.de>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -409,7 +410,7 @@  (define (check-end-of-sentence-space description)
           (list
            (make-warning package
                          (G_ "sentences in description should be followed ~
-by two spaces; possible infraction~p at ~{~a~^, ~}")
+by two spaces due to issues wth Emacs; possible infraction~p at ~{~a~^, ~}")
                          (list (length infractions)
                                infractions)
                          #:field 'description)))))