From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#72357: Checkdoc fixes in transient.el Date: Tue, 30 Jul 2024 14:39:53 +0300 Message-ID: <861q3byupi.fsf@gnu.org> References: <87a5hzn9qn.fsf@bernoul.li> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3942"; mail-complaints-to="usenet@ciao.gmane.io" Cc: stefankangas@gmail.com, 72357@debbugs.gnu.org To: Jonas Bernoulli Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Jul 30 13:43:18 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1sYlG9-0000rp-NV for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 30 Jul 2024 13:43:17 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sYlFo-0006ho-VL; Tue, 30 Jul 2024 07:42:57 -0400 Original-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 ) id 1sYlFg-0006hF-J3 for bug-gnu-emacs@gnu.org; Tue, 30 Jul 2024 07:42:48 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sYlFg-0005OA-8o for bug-gnu-emacs@gnu.org; Tue, 30 Jul 2024 07:42:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=References:In-Reply-To:From:Date:To:Subject; bh=nXlxp4HBjI0aC6Ot2fj5eEzwzEwbkpOpVzAfLz6VzFQ=; b=FBbS7pToDdueroBxOQzH8neSVYv6oJmCm1HvGJVOrxIndsDHPWckeO4ODLnnqloJ04CSl6PnUP++nMxJlXbifWb+dvyd91YO++DJVyAI7r9EE3/1jGtVdrMbb9V4kRkSytlXHakndlyckinX27XeotN7ZsIvMlhUzMNhUFWbiJII9iM3KHYHXJ0cyjwsH5hplS1ZdCrZa3lKmhiLSOuKGDBvP1oJRnLh8Z4LnfsJQrGYpX6UrJITK7poDLJqw34ZDaKELvyRbEtAUwfr4xbddTPU5Al3seeqUnCxXOmVJq7KKy8wF8iNbT4Aadf1n/DomFIfTJmhKALNIcOB+4/IQw==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sYlFu-0002k6-2m for bug-gnu-emacs@gnu.org; Tue, 30 Jul 2024 07:43:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Jul 2024 11:43:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 72357 X-GNU-PR-Package: emacs Original-Received: via spool by 72357-submit@debbugs.gnu.org id=B72357.172233975010500 (code B ref 72357); Tue, 30 Jul 2024 11:43:02 +0000 Original-Received: (at 72357) by debbugs.gnu.org; 30 Jul 2024 11:42:30 +0000 Original-Received: from localhost ([127.0.0.1]:47107 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sYlFN-0002jH-G1 for submit@debbugs.gnu.org; Tue, 30 Jul 2024 07:42:30 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:48294) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sYlFK-0002j3-21 for 72357@debbugs.gnu.org; Tue, 30 Jul 2024 07:42:27 -0400 Original-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 ) 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==; In-Reply-To: <87a5hzn9qn.fsf@bernoul.li> (bug-gnu-emacs@gnu.org) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:289548 Archived-At: > 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" > > 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."