Received: (at 72357) by debbugs.gnu.org; 14 Sep 2024 13:42:17 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Sat Sep 14 09:42:17 2024 Received: from localhost ([127.0.0.1]:45143 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1spT2W-0006xm-U8 for submit <at> debbugs.gnu.org; Sat, 14 Sep 2024 09:42:17 -0400 Received: from mail-ej1-f46.google.com ([209.85.218.46]:53688) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <stefankangas@HIDDEN>) id 1spT2V-0006xU-Bp for 72357 <at> debbugs.gnu.org; Sat, 14 Sep 2024 09:42:16 -0400 Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-a8a7903cb7dso128297666b.3 for <72357 <at> debbugs.gnu.org>; Sat, 14 Sep 2024 06:42:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726321259; x=1726926059; darn=debbugs.gnu.org; h=cc:to:subject:message-id:date:mime-version:references:in-reply-to :from:from:to:cc:subject:date:message-id:reply-to; bh=TJiXi2GpE5DDvCdsZOXUtCT5TYD29U/mkprFx4ri1mk=; b=bQ8Nj0lm/nV4HreTdFLsHQOnE1e164PXpZysTxWwRo0oLTWP4TcnEeGU/kZTzzmUky PNMLb2c489vr66FOQFRG4FETwCUtnraKHDnQZbE5omhu4hSHF1s1TxJHhGc2X2A8YN5U RAg6TeHGVV7vatK9MWkzz0Z8BXfN5AfkkbZ6G0WXX45aXf714wq5daoA/wmqqtAK8UU5 oKFZeAy/fuTEhp5m3wTPb7rbBWY+29WLgulvNHZ7VHFk2rFYnz8AHHdEECkvOIrUDo8h N3DsPU13aSkFkrewMYXLFRpedjCfT1RDv9/M56+Noc7PutCtXIbemVpL9i0RGytMN/iT yvjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726321259; x=1726926059; h=cc:to:subject:message-id:date:mime-version:references:in-reply-to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=TJiXi2GpE5DDvCdsZOXUtCT5TYD29U/mkprFx4ri1mk=; b=uO/AAhiP9v13kwOG/9ZiJTSOusJ049xiTOYliT4FQNfNa18tIIDX1XzHHlgAf4NH6E xRo72GXh+mqFx+C93CpZ2gXt+aFLUTpjtWrMYb7rx/EsK8WWV3LoAcSTrMKGdhXRG6s1 4RH2jdVrsIwyMROk8lGmAPBy5nuImgGpREPEulFloCnqFlYlM0ceemnfouLgWDgkGJmM TKk+jf1juITg5cHkMy8mdfUQUbJAIFOsJ/uy20d3fe9qXLrvk21GfDmmDHW5wrexFjfp 4aLInd27bEE2w3GulSM/e4PzS2jwK59Xhn88g/LjedDnWYleG+hNVpU61OkJa0OF+6Gy KmQg== X-Gm-Message-State: AOJu0YySCshsuTo5fLdNaPgXb1TOuoWdVNo1ZSdCqHCz3vfTmkrJGD45 6CWF0qZIFYUtUAMhQGfW/c9l5ZgN8SdR60kAHe/pggblE0BPobwMdijONv3WTyNrWOOs0pVugXW x7KFKqxEv5F6uHwgUCftwYtyrvIuqJ3p4mkw= X-Google-Smtp-Source: AGHT+IHNtQiHiPtf+3F57qem4zSIsQDdK1CvwJ8aCgxecYeL9klw+q8tbNEg3FfgwcSTdd/tSqFWnaLr8TqOzNU61o4= X-Received: by 2002:a05:6402:5108:b0:5c2:6311:c9d1 with SMTP id 4fb4d7f45d1cf-5c41e1b5325mr7619322a12.22.1726321258833; Sat, 14 Sep 2024 06:40:58 -0700 (PDT) Received: from 753933720722 named unknown by gmailapi.google.com with HTTPREST; Sat, 14 Sep 2024 06:40:58 -0700 From: Stefan Kangas <stefankangas@HIDDEN> In-Reply-To: <87a5hzn9qn.fsf@HIDDEN> References: <87a5hzn9qn.fsf@HIDDEN> MIME-Version: 1.0 Date: Sat, 14 Sep 2024 06:40:58 -0700 Message-ID: <CADwFkmnknpMFTo4PQyHB=BzHv094BA8=gMrEtXrME6BCH6iW5Q@HIDDEN> Subject: Re: Checkdoc fixes in transient.el To: Jonas Bernoulli <jonas@HIDDEN> Content-Type: text/plain; charset="UTF-8" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 72357 Cc: 72357 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org> X-Spam-Score: -1.0 (-) Jonas Bernoulli <jonas@HIDDEN> writes: > Looking at, for example, "C-h f transient-format-description", I feel > that it would not make sense if all the methods themselves began with a > summary line. Only the overall generic function *needs* a summary line. > In some case it may make sense to give individual methods their own > summary lines, but for very short, one paragraph method docstrings this > should not be a requirements. When a method is so simple that it can be > described using a single short paragraph (but not a single sentence, > which can fit on a single line), then that should be possible, without > being forced to mess up the justification of that paragraph. Sure, I understand your reasoning. I don't know if this needs pointing out, but do feel free to revert these changes. Meanwhile, Eli has proposed a rewording of the docstring that you might want to look at. > IMO checkdoc should be updated to not enforce the conventions, which > were designed for "top-level" functions (and variables) on methods > as well. Makes sense to me. BTW, with debbugs, it's better to put people in the "X-Debbugs-CC" header than "Cc". That way, I get the email forwarded to me by the bug tracker, and when I hit "wide reply", I send it to the right bug automatically (I had to edit in manually here).
bug-gnu-emacs@HIDDEN:bug#72357; Package emacs.
Full text available.
Received: (at 72357) by debbugs.gnu.org; 30 Jul 2024 11:42:30 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Jul 30 07:42:30 2024
Received: from localhost ([127.0.0.1]:47107 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1sYlFN-0002jH-G1
for submit <at> debbugs.gnu.org; Tue, 30 Jul 2024 07:42:30 -0400
Received: from eggs.gnu.org ([209.51.188.92]:48294)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <eliz@HIDDEN>) id 1sYlFK-0002j3-21
for 72357 <at> debbugs.gnu.org; Tue, 30 Jul 2024 07:42:27 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e])
by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
id 1sYlCu-0004h3-2C; Tue, 30 Jul 2024 07:39:56 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
mime-version; bh=nXlxp4HBjI0aC6Ot2fj5eEzwzEwbkpOpVzAfLz6VzFQ=; b=QqksXk/Qdwxj
799TlQyt0vEm2BI3wzwKaijHzc9KoiqpN0kaWeymiriawfC4DSqQF3OUWhS6qy7K5HPMl4JqLMaSI
6jfZbHFjF26n7ipJ/oI/00OHDdx26B4QqYgZKNfMMhtIKWC1PlcISlmkDzHzRZmVH2EvnkWWgSBxQ
4uzaAvLlfULMGXd2iflpt4orPp2SmiRHsCdIsxcv7b98kdwFv1QNtAOLVL7BMiKrl68kOiB4toPyk
IfEW6SMaXKjKSWq7UiXYI6Trj0xuFxiDzFpSPXzWB0QBsCDrNn9x+3AAmLhp3VK1ZD7xTGhZlO3Mq
yTXtDIY+xQQ3H6xCkxMPNQ==;
Date: Tue, 30 Jul 2024 14:39:53 +0300
Message-Id: <861q3byupi.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Jonas Bernoulli <jonas@HIDDEN>
In-Reply-To: <87a5hzn9qn.fsf@HIDDEN> (bug-gnu-emacs@HIDDEN)
Subject: Re: bug#72357: Checkdoc fixes in transient.el
References: <87a5hzn9qn.fsf@HIDDEN>
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 72357
Cc: stefankangas@HIDDEN, 72357 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)
> Date: Mon, 29 Jul 2024 23:56:00 +0200
> From: Jonas Bernoulli via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
>
> Hello Stefan,
>
> I don't think your commit e3bba63ecb9 is an improvement.
Thanks for your feedback.
However, I tend to disagree with you, see below.
> I generally agree that the first line of a docstring should be a short
> sentence, and that no other sentence should start on that line, even if
> there is plenty of space left. In fact I have contributed many similar
> fixes to package maintained by other people.
>
> However, I would argue that the usual reasoning for why one should do
> that, does not apply here. In this case, the first line won't appear
> anywhere by itself, without the rest of this docstring (such as in
> apropos output), because this is a method not a "regular" or generic
> function.
>
> Looking at, for example, "C-h f transient-format-description", I feel
> that it would not make sense if all the methods themselves began with a
> summary line. Only the overall generic function *needs* a summary line.
> In some case it may make sense to give individual methods their own
> summary lines, but for very short, one paragraph method docstrings this
> should not be a requirements. When a method is so simple that it can be
> described using a single short paragraph (but not a single sentence,
> which can fit on a single line), then that should be possible, without
> being forced to mess up the justification of that paragraph.
>
> IMO checkdoc should be updated to not enforce the conventions, which
> were designed for "top-level" functions (and variables) on methods
> as well.
>
> Also consider the case where a method can be described using a single
> sentence, but that sentence requires two lines. Forcing the author
> to prefix that short paragraph with a sorter sentence, which only
> serves to satisfies an ill-applied convention, feels wrong to me.
These are all general considerations. They do make sense, but IME
they should be discussed with specific cases in mind, because what
exactly is the best path in each case is a judgment call, which is
sometimes resolved due to very fine nuances.
If we now turn to the practical aspects of this, then first I'd like
to point out that the commit in question touched only 2 doc strings,
which were both very similar in their wording, and which IMO could use
some improvements. Just to put this on the table, here's one of the
two changes (the other one is almost identical):
(cl-defmethod transient-format-description ((obj transient-group))
- "Format the description by calling the next method. If the result
-doesn't use the `face' property at all, then apply the face
-`transient-heading' to the complete string."
+ "Format the description by calling the next method.
+If the result doesn't use the `face' property at all, then apply the
+face `transient-heading' to the complete string."
> In this particular case, separating the first sentence from the rest of
> the paragraph (but without completely rewording the paragraph) is a step
> backward.
IME, if done right, it is never a step backward.
> Each of these method docstrings consist of two sentences. The first
> sentence is very much not a summary of the second sentence. Each of
> these methods does two things and each thing is described using one
> sentence. The important thing, the one that makes the method useful, is
> described in the second sentence. The boring thing (call the next
> method) is described in the first sentence, because it is done first.
> Using the first sentence as the "summary" is wrong in these cases.
> Changing the order in which the two steps are described, would likely
> lead to awkward wording.
I agree that the current doc strings can be improved. But I don't
agree that the original doc strings were better. Their first sentence
basically says very little about what it does and leaves a lot unsaid
(what is "the next method"? what is "the description"?), and the next
sentence doesn't clarify that (instead, it adds some detail about what
the method does). The doc string of the corresponding cl-defgeneric
doesn't help, either.
So IMO these doc strings "need some work"(TM). In particular, if you
say that the important part of the description is in the second
sentence, then that sentence should be the first one. (The sentence
that is currently the first I'm not sure should be there at all.)
Also, I think the doc string should mention the types of arguments to
which the method is applicable (right now, it says absolutely nothing
about that). Preferably, this should be in the first line/sentence of
the doc string.
If you can describe in a few more words what one of these methods do,
I can try suggesting how to reword its doc string, and that will
hopefully illustrate what I mean above in practical terms. For now,
here's the first attempt based on just looking at the code:
(cl-defmethod transient-format-description ((obj transient-group))
"Format the description of `transient-group' object OBJ using faces.
If the description of OBJ already uses faces, return it unchanged.
Otherwise, apply the `transient-inapt-suffix' face to the entire
description stribng if the OBJ's inapt slot is non-nil, else
the `transient-heading' face."
bug-gnu-emacs@HIDDEN:bug#72357; Package emacs.
Full text available.
Received: (at submit) by debbugs.gnu.org; 30 Jul 2024 02:11:29 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon Jul 29 22:11:29 2024
Received: from localhost ([127.0.0.1]:46426 helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1sYcKn-0004sV-7O
for submit <at> debbugs.gnu.org; Mon, 29 Jul 2024 22:11:29 -0400
Received: from lists.gnu.org ([209.51.188.17]:53638)
by debbugs.gnu.org with esmtp (Exim 4.84_2)
(envelope-from <jonas@HIDDEN>) id 1sYcKk-0004sL-LO
for submit <at> debbugs.gnu.org; Mon, 29 Jul 2024 22:11:27 -0400
Received: from eggs.gnu.org ([2001:470:142:3::10])
by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <jonas@HIDDEN>) id 1sYcKW-0005kG-Sz
for bug-gnu-emacs@HIDDEN; Mon, 29 Jul 2024 22:11:12 -0400
Received: from mail.hostpark.net ([212.243.197.30])
by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
(Exim 4.90_1) (envelope-from <jonas@HIDDEN>) id 1sYcKU-0003kB-Nl
for bug-gnu-emacs@HIDDEN; Mon, 29 Jul 2024 22:11:12 -0400
Received: from localhost (localhost [127.0.0.1])
by mail.hostpark.net (Postfix) with ESMTP id C4D76164D4;
Mon, 29 Jul 2024 23:56:03 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=bernoul.li; h=
content-type:content-type:mime-version:message-id:date:date
:subject:subject:from:from; s=sel2011a; t=1722290163; bh=WO7I8SI
wvypjXrGIDoro4Z4KFL6weEvxlLzW16uGAJg=; b=MBWPjrpYOB6bUMe0F5kGn9l
oRAFaOeROHy661UCaIZSQ61m7PkL0aXAOMvhf2JDdOG7vn2IoXnOo/mAFRnPwqZx
XVgGG+YB98FdzX/gv0GtykUAp3xn1T4eiU91bevI9u+JIvKQeGNcIq+heUm2j7cp
xkWF9LjY/DYqvnq81rYg=
X-Virus-Scanned: by Hostpark/NetZone Mailprotection at hostpark.net
Received: from mail.hostpark.net ([127.0.0.1])
by localhost (mail0.hostpark.net [127.0.0.1]) (amavisd-new, port 10224)
with ESMTP id 43BnMUqfq4rp; Mon, 29 Jul 2024 23:56:03 +0200 (CEST)
Received: from customer (localhost [127.0.0.1])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange ECDHE (prime256v1) server-signature RSA-PSS (2048 bits)
server-digest SHA256) (No client certificate requested)
by mail.hostpark.net (Postfix) with ESMTPSA id DAA0E16463;
Mon, 29 Jul 2024 23:56:02 +0200 (CEST)
From: Jonas Bernoulli <jonas@HIDDEN>
To: Stefan Kangas <stefankangas@HIDDEN>, bug-gnu-emacs@HIDDEN
Subject: Checkdoc fixes in transient.el
Date: Mon, 29 Jul 2024 23:56:00 +0200
Message-ID: <87a5hzn9qn.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain
Received-SPF: pass client-ip=212.243.197.30; envelope-from=jonas@HIDDEN;
helo=mail.hostpark.net
X-Spam_score_int: -27
X-Spam_score: -2.8
X-Spam_bar: --
X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,
RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01,
SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: -1.4 (-)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -2.4 (--)
Hello Stefan,
I don't think your commit e3bba63ecb9 is an improvement.
I generally agree that the first line of a docstring should be a short
sentence, and that no other sentence should start on that line, even if
there is plenty of space left. In fact I have contributed many similar
fixes to package maintained by other people.
However, I would argue that the usual reasoning for why one should do
that, does not apply here. In this case, the first line won't appear
anywhere by itself, without the rest of this docstring (such as in
apropos output), because this is a method not a "regular" or generic
function.
Looking at, for example, "C-h f transient-format-description", I feel
that it would not make sense if all the methods themselves began with a
summary line. Only the overall generic function *needs* a summary line.
In some case it may make sense to give individual methods their own
summary lines, but for very short, one paragraph method docstrings this
should not be a requirements. When a method is so simple that it can be
described using a single short paragraph (but not a single sentence,
which can fit on a single line), then that should be possible, without
being forced to mess up the justification of that paragraph.
IMO checkdoc should be updated to not enforce the conventions, which
were designed for "top-level" functions (and variables) on methods
as well.
Also consider the case where a method can be described using a single
sentence, but that sentence requires two lines. Forcing the author
to prefix that short paragraph with a sorter sentence, which only
serves to satisfies an ill-applied convention, feels wrong to me.
---
In this particular case, separating the first sentence from the rest of
the paragraph (but without completely rewording the paragraph) is a step
backward.
Each of these method docstrings consist of two sentences. The first
sentence is very much not a summary of the second sentence. Each of
these methods does two things and each thing is described using one
sentence. The important thing, the one that makes the method useful, is
described in the second sentence. The boring thing (call the next
method) is described in the first sentence, because it is done first.
Using the first sentence as the "summary" is wrong in these cases.
Changing the order in which the two steps are described, would likely
lead to awkward wording.
Jonas
Jonas Bernoulli <jonas@HIDDEN>:bug-gnu-emacs@HIDDEN.
Full text available.bug-gnu-emacs@HIDDEN:bug#72357; Package emacs.
Full text available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997 nCipher Corporation Ltd,
1994-97 Ian Jackson.