Silence checkdoc for symbols designating major and minor modes

Silence checkdoc for symbols designating major and minor modes

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 730 bytes --]


Hi,

an issue I have been having when documenting code is that checkdoc wants
me to disambiguate symbols referring to major or minor modes. That
usually means adding "function", "command" "variable", "option" or
"symbol" but in my experience these usually do not fit. In fact, most of
the time it is obvious what is meant when referring to a major/minor
mode.

        ... Foo when `bar-mode' is active ...

The patch I added below silences the disambiguation request when a
symbol ends in "-mode". Alternatively, one could also add a "minor mode"
and "major mode" to the list of accepted keywords, but I think that
would sound to repetitive:

        ... Foo when minor mode `bar-mode' is active ...

Opinions?

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ignore-ambiguous-symbols-that-end-in-mode.patch --]
[-- Type: text/x-diff, Size: 891 bytes --]

From afbfcec6f0da120c99ff1ff3e0355544c768cb8c Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Tue, 27 Jul 2021 23:43:13 +0200
Subject: [PATCH] Ignore ambiguous symbols that end in "-mode"

* checkdoc.el (checkdoc-this-string-valid-engine): Check if bound and
  fbound symbol ends in "-mode"
---
 lisp/emacs-lisp/checkdoc.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 00cc7777e1..9831a5aa06 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1559,6 +1559,7 @@ checkdoc-this-string-valid-engine
 	     (setq mb (match-beginning 1)
 		   me (match-end 1))
 	     (if (and sym (boundp sym) (fboundp sym)
+                      (not (string-match-p "-mode\\'" (symbol-name sym)))
 		      (save-excursion
 			(goto-char mb)
 			(forward-word-strictly -1)
-- 
2.30.2

Re: Silence checkdoc for symbols designating major and minor modes

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Date: Tue, 27 Jul 2021 21:50:43 +0000
> 
> an issue I have been having when documenting code is that checkdoc wants
> me to disambiguate symbols referring to major or minor modes. That
> usually means adding "function", "command" "variable", "option" or
> "symbol" but in my experience these usually do not fit. In fact, most of
> the time it is obvious what is meant when referring to a major/minor
> mode.
> 
>         ... Foo when `bar-mode' is active ...
> 
> The patch I added below silences the disambiguation request when a
> symbol ends in "-mode". Alternatively, one could also add a "minor mode"
> and "major mode" to the list of accepted keywords, but I think that
> would sound to repetitive:
> 
>         ... Foo when minor mode `bar-mode' is active ...
> 
> Opinions?

First, the code, if installed, should have a comment explaining why
these strings are being exempted.

More generally, could you please show an example of a doc string and
the warning(s) it triggers?  I'm not sure I understand the problem
well enough to make up my mind.  (Do all of our existing modes suffer
from this problem?)

Thanks.

Re: Silence checkdoc for symbols designating major and minor modes

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Tue, 27 Jul 2021 21:50:43 +0000
>> 
>> an issue I have been having when documenting code is that checkdoc wants
>> me to disambiguate symbols referring to major or minor modes. That
>> usually means adding "function", "command" "variable", "option" or
>> "symbol" but in my experience these usually do not fit. In fact, most of
>> the time it is obvious what is meant when referring to a major/minor
>> mode.

First of all, I have to correct myself: This only applies to minor
modes, because they are both bound and fbound.

>> 
>>         ... Foo when `bar-mode' is active ...
>> 
>> The patch I added below silences the disambiguation request when a
>> symbol ends in "-mode". Alternatively, one could also add a "minor mode"
>> and "major mode" to the list of accepted keywords, but I think that
>> would sound to repetitive:
>> 
>>         ... Foo when minor mode `bar-mode' is active ...
>> 
>> Opinions?
>
> First, the code, if installed, should have a comment explaining why
> these strings are being exempted.

Of course, this was just a first suggestion.

> More generally, could you please show an example of a doc string and
> the warning(s) it triggers?  I'm not sure I understand the problem
> well enough to make up my mind.  (Do all of our existing modes suffer
> from this problem?)

I noticed this when updating the rcirc docstrings. For example, I still
have the issue here:

        (defcustom rcirc-omit-responses
          '("JOIN" "PART" "QUIT" "NICK")
          "Responses which will be hidden when `rcirc-omit-mode' is enabled."
          :type '(repeat string))

triggering this response:

        Disambiguate rcirc-omit-mode by preceding w/ function,command,variable,option or symbol.

Browsing through the repository, I also discovered the same issue in
turn-on-eldoc-mode's docstring. There are other examples in third-party
code I remember, but couldn't find right now.

> Thanks.

-- 
	Philip Kaludercic

Re: Silence checkdoc for symbols designating major and minor modes

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> I noticed this when updating the rcirc docstrings. For example, I still
> have the issue here:
>
>         (defcustom rcirc-omit-responses
>           '("JOIN" "PART" "QUIT" "NICK")
>           "Responses which will be hidden when `rcirc-omit-mode' is enabled."
>           :type '(repeat string))
>
> triggering this response:
>
>         Disambiguate rcirc-omit-mode by preceding w/ function,command,variable,option or symbol.

Ah, I see.  Yes, I think your patch makes perfect sense here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Silence checkdoc for symbols designating major and minor modes

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: emacs-devel@gnu.org
> Date: Wed, 28 Jul 2021 14:46:57 +0000
> 
>         (defcustom rcirc-omit-responses
>           '("JOIN" "PART" "QUIT" "NICK")
>           "Responses which will be hidden when `rcirc-omit-mode' is enabled."
>           :type '(repeat string))
> 
> triggering this response:
> 
>         Disambiguate rcirc-omit-mode by preceding w/ function,command,variable,option or symbol.

FWIW, I think this warning should be removed wholesale.  It is too
harsh to require such finesses in doc strings.

But if we want to keep it, I think we should include in the regexp
also the "enabled" and "disabled" part that follows it.  Just
exempting "-mode" sounds too general for a heuristic.

Re: Silence checkdoc for symbols designating major and minor modes

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: emacs-devel@gnu.org
>> Date: Wed, 28 Jul 2021 14:46:57 +0000
>> 
>>         (defcustom rcirc-omit-responses
>>           '("JOIN" "PART" "QUIT" "NICK")
>>           "Responses which will be hidden when `rcirc-omit-mode' is enabled."
>>           :type '(repeat string))
>> 
>> triggering this response:
>> 
>>         Disambiguate rcirc-omit-mode by preceding w/ function,command,variable,option or symbol.
>
> FWIW, I think this warning should be removed wholesale.  It is too
> harsh to require such finesses in doc strings.

I am inclined to agree, as in my experience, I have only ever seen
false-positives being highlighted by this rule.

> But if we want to keep it, I think we should include in the regexp
> also the "enabled" and "disabled" part that follows it.  Just
> exempting "-mode" sounds too general for a heuristic.

-- 
	Philip Kaludercic

Re: Silence checkdoc for symbols designating major and minor modes

[Stefan Monnier]

Philip Kaludercic [2021-07-27 21:50:43] wrote:
> an issue I have been having when documenting code is that checkdoc wants
> me to disambiguate symbols referring to major or minor modes. That
> usually means adding "function", "command" "variable", "option" or
> "symbol" but in my experience these usually do not fit.

I think it's a problem in checkdoc, which wants to impose a style which
is not usually followed in Emacs (and which `describe-symbol` makes
even less important).


        Stefan