* bug#66264: Disable warning about wide doc strings by default
@ 2023-09-29 12:59 Mattias Engdegård
2023-09-29 15:43 ` Eli Zaretskii
0 siblings, 1 reply; 3+ messages in thread
From: Mattias Engdegård @ 2023-09-29 12:59 UTC (permalink / raw)
To: 66264; +Cc: Stefan Kangas, T.V Raman
[-- Attachment #1: Type: text/plain, Size: 785 bytes --]
This is a proposal to disable the warning about doc string line length by default.
While it is a useful warning, it does not actually indicate that anything may be wrong in the code, nor that the doc string would risk being misinterpreted by the user. It is rather a stylistic complaint, such as the ones produced by checkdoc.
There have been complaints about the warning being difficult to avoid in code where the doc string is generated by macros, sometimes outside the control of the programmer. See https://lists.gnu.org/archive/html/emacs-devel/2023-09/msg01326.html for one such recent discussion.
For this reason I'm proposing that it be disabled by default but remain enabled in builds of Emacs itself, like the warning about curved single quotes.
Patch attached.
[-- Attachment #2: docstrings-wide.diff --]
[-- Type: application/octet-stream, Size: 11663 bytes --]
diff --git a/etc/NEWS b/etc/NEWS
index 97ebc9a5de4..7e29a987c76 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1220,6 +1220,10 @@ name 'ignored-return-value'.
The warning will only be issued for calls to functions declared
'important-return-value' or 'side-effect-free' (but not 'error-free').
+---
+*** The warning about too-wide doc strings is now disabled by default.
+It can be enabled by setting 'byte-compile-warning-types' to 'all'.
+
+++
** New function declaration and property 'important-return-value'.
The declaration '(important-return-value t)' sets the
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 387d7ef4de1..2fcadc19a5a 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -295,7 +295,7 @@ byte-compile-warning-types
'(redefine callargs free-vars unresolved
obsolete noruntime interactive-only
make-local mapcar constants suspicious lexical lexical-dynamic
- docstrings docstrings-non-ascii-quotes not-unused
+ docstrings docstrings-wide docstrings-non-ascii-quotes not-unused
empty-body)
"The list of warning types used when `byte-compile-warnings' is t.")
(defcustom byte-compile-warnings t
@@ -322,12 +322,15 @@ byte-compile-warnings
is likely to be a mistake
not-unused warning about using variables with symbol names starting with _.
constants let-binding of, or assignment to, constants/nonvariables.
- docstrings docstrings that are too wide (longer than
- `byte-compile-docstring-max-column' or
- `fill-column' characters, whichever is bigger) or
- have other stylistic issues.
- docstrings-non-ascii-quotes docstrings that have non-ASCII quotes.
- This depends on the `docstrings' warning type.
+ docstrings various docstring stylistic issues, such as incorrect use
+ of single quotes
+ docstrings-wide
+ docstrings that are too wide, containing lines longer than both
+ `byte-compile-docstring-max-column' and `fill-column' characters.
+ Only enabled when `docstrings' also is.
+ docstrings-non-ascii-quotes
+ docstrings that have non-ASCII quotes.
+ Only enabled when `docstrings' also is.
suspicious constructs that usually don't do what the coder wanted.
empty-body body argument to a special form or macro is empty.
mutate-constant
@@ -345,7 +348,7 @@ byte-compile-warnings
byte-compile-warning-types))))
(defconst byte-compile--emacs-build-warning-types
- '(docstrings-non-ascii-quotes)
+ '(docstrings-non-ascii-quotes docstrings-wide)
"List of warning types that are only enabled during Emacs builds.
This is typically either warning types that are being phased in
(but shouldn't be enabled for packages yet), or that are only relevant
@@ -1753,7 +1756,8 @@ byte-compile-docstring-style-warn
(setq name (cadr name)))
(setq name (if name (format " `%s' " name) ""))
(when (and kind docs (stringp docs))
- (when (byte-compile--wide-docstring-p docs col)
+ (when (and (byte-compile-warning-enabled-p 'docstrings-wide)
+ (byte-compile--wide-docstring-p docs col))
(byte-compile-warn-x
name
"%s%sdocstring wider than %s characters"
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index e644417c3d4..45cf8b577ba 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -925,20 +925,21 @@ bytecomp-tests--warnings
;; Should not warn that mt--test2 is not known to be defined.
(should-not (re-search-forward "my--test2" nil t))))
-(defun bytecomp--with-warning-test (re-warning form)
+(defun bytecomp--with-warning-test (re-warning form &optional warnings)
(declare (indent 1))
(with-current-buffer (get-buffer-create "*Compile-Log*")
(let ((inhibit-read-only t)) (erase-buffer))
(ert-info ((prin1-to-string form) :prefix "form: ")
- (let ((text-quoting-style 'grave))
+ (let ((text-quoting-style 'grave)
+ (byte-compile-warnings (or warnings t)))
(bytecomp-tests--with-fresh-warnings
(byte-compile form)))
(ert-info ((prin1-to-string (buffer-string)) :prefix "buffer: ")
(should (re-search-forward
(string-replace " " "[ \n]+" re-warning)))))))
-(defun bytecomp--without-warning-test (form)
- (bytecomp--with-warning-test "\\`\\'" form))
+(defun bytecomp--without-warning-test (form &optional warnings)
+ (bytecomp--with-warning-test "\\`\\'" form warnings))
(ert-deftest bytecomp-warn--ignore ()
(bytecomp--with-warning-test "unused"
@@ -966,39 +967,49 @@ bytecomp-tests--docstring
(ert-deftest bytecomp-warn-wide-docstring/defconst ()
(bytecomp--with-warning-test "defconst.*foo.*wider than.*characters"
- `(defconst foo t ,bytecomp-tests--docstring)))
+ `(defconst foo t ,bytecomp-tests--docstring)
+ 'all))
(ert-deftest bytecomp-warn-wide-docstring/defvar ()
(bytecomp--with-warning-test "defvar.*foo.*wider than.*characters"
- `(defvar foo t ,bytecomp-tests--docstring)))
+ `(defvar foo t ,bytecomp-tests--docstring)
+ 'all))
(ert-deftest bytecomp-warn-wide-docstring/cl-defsubst ()
(bytecomp--without-warning-test
`(cl-defsubst short-name ()
- "Do something."))
+ "Do something."
+ 'all))
(bytecomp--without-warning-test
`(cl-defsubst long-name-with-less-80-characters-but-still-quite-a-bit ()
- "Do something."))
+ "Do something.")
+ 'all)
(bytecomp--with-warning-test "wider than.*characters"
`(cl-defsubst long-name-with-more-than-80-characters-yes-this-is-a-very-long-name-but-why-not!! ()
- "Do something.")))
+ "Do something.")
+ 'all))
(ert-deftest bytecomp-warn-wide-docstring/cl-defstruct ()
(bytecomp--without-warning-test
`(cl-defstruct short-name
- field))
+ field)
+ 'all)
(bytecomp--without-warning-test
`(cl-defstruct short-name
- long-name-with-less-80-characters-but-still-quite-a-bit))
+ long-name-with-less-80-characters-but-still-quite-a-bit)
+ 'all)
(bytecomp--without-warning-test
`(cl-defstruct long-name-with-less-80-characters-but-still-quite-a-bit
- field))
+ field)
+ 'all)
(bytecomp--with-warning-test "wider than.*characters"
`(cl-defstruct short-name
- long-name-with-more-than-80-characters-yes-this-is-a-very-long-name-but-why-not!!))
+ long-name-with-more-than-80-characters-yes-this-is-a-very-long-name-but-why-not!!)
+ 'all)
(bytecomp--with-warning-test "wider than.*characters"
`(cl-defstruct long-name-with-more-than-80-characters-yes-this-is-a-very-long-name-but-why-not!!
- field)))
+ field)
+ 'all))
(ert-deftest bytecomp-warn-quoted-condition ()
(bytecomp--with-warning-test
@@ -1060,11 +1071,13 @@ bytecomp-warn-dodgy-args-memq
(bytecomp--with-warning-test (msg2 "integer") (form2 #x10000000000))
(bytecomp--with-warning-test (msg2 "float") (form2 1.0))))))
-(defmacro bytecomp--define-warning-file-test (file re-warning &optional reverse)
+(defmacro bytecomp--define-warning-file-test (file re-warning
+ &optional reverse warnings)
`(ert-deftest ,(intern (format "bytecomp/%s" file)) ()
(with-current-buffer (get-buffer-create "*Compile-Log*")
(let ((inhibit-read-only t)) (erase-buffer))
- (byte-compile-file ,(ert-resource-file file))
+ (let ((byte-compile-warnings ,(or warnings t)))
+ (byte-compile-file ,(ert-resource-file file)))
(ert-info ((buffer-string) :prefix "buffer: ")
(,(if reverse 'should-not 'should)
(re-search-forward ,re-warning nil t))))))
@@ -1170,71 +1183,71 @@ "warn-variable-setq-odd.el"
(bytecomp--define-warning-file-test
"warn-wide-docstring-autoload.el"
- "autoload .foox. docstring wider than .* characters")
+ "autoload .foox. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-custom-declare-variable.el"
- "custom-declare-variable .foo. docstring wider than .* characters")
+ "custom-declare-variable .foo. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-defalias.el"
- "defalias .foo. docstring wider than .* characters")
+ "defalias .foo. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-defconst.el"
- "defconst .foo-bar. docstring wider than .* characters")
+ "defconst .foo-bar. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-define-abbrev-table.el"
- "define-abbrev-table .foo. docstring wider than .* characters")
+ "define-abbrev-table .foo. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-define-obsolete-function-alias.el"
- "defalias .foo. docstring wider than .* characters")
+ "defalias .foo. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-define-obsolete-variable-alias.el"
- "defvaralias .foo. docstring wider than .* characters")
+ "defvaralias .foo. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-defun.el"
- "Warning: docstring wider than .* characters")
+ "Warning: docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-defvar.el"
- "defvar .foo-bar. docstring wider than .* characters")
+ "defvar .foo-bar. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-defvaralias.el"
- "defvaralias .foo-bar. docstring wider than .* characters")
+ "defvaralias .foo-bar. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-ignore-fill-column.el"
- "defvar .foo-bar. docstring wider than .* characters" 'reverse)
+ "defvar .foo-bar. docstring wider than .* characters" 'reverse 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-ignore-function-signature.el"
- "defvar .foo-bar. docstring wider than .* characters" 'reverse)
+ "defvar .foo-bar. docstring wider than .* characters" 'reverse 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-ignore-override.el"
- "defvar .foo-bar. docstring wider than .* characters" 'reverse)
+ "defvar .foo-bar. docstring wider than .* characters" 'reverse 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-ignore-substitutions.el"
- "defvar .foo-bar. docstring wider than .* characters" 'reverse)
+ "defvar .foo-bar. docstring wider than .* characters" 'reverse 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-ignore.el"
- "defvar .foo-bar. docstring wider than .* characters" 'reverse)
+ "defvar .foo-bar. docstring wider than .* characters" 'reverse 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-multiline-first.el"
- "defvar .foo-bar. docstring wider than .* characters")
+ "defvar .foo-bar. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"warn-wide-docstring-multiline.el"
- "defvar .foo-bar. docstring wider than .* characters")
+ "defvar .foo-bar. docstring wider than .* characters" nil 'all)
(bytecomp--define-warning-file-test
"nowarn-inline-after-defvar.el"
^ permalink raw reply related [flat|nested] 3+ messages in thread
* bug#66264: Disable warning about wide doc strings by default
2023-09-29 12:59 bug#66264: Disable warning about wide doc strings by default Mattias Engdegård
@ 2023-09-29 15:43 ` Eli Zaretskii
2023-09-30 8:17 ` Richard Stallman
0 siblings, 1 reply; 3+ messages in thread
From: Eli Zaretskii @ 2023-09-29 15:43 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: raman, stefankangas, 66264
> Cc: Stefan Kangas <stefankangas@gmail.com>, "T.V Raman" <raman@google.com>
> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 29 Sep 2023 14:59:09 +0200
>
> This is a proposal to disable the warning about doc string line length by default.
Thanks, but I'm against this change. It has been, and continues to
be, tremendously useful in making sure our doc strings are in a good
shape. I don't see how leaving this on only in "Emacs builds" is TRT,
since this means ELPA packages will be exempt from the same scrutiny,
and it also means that byte-compiling a file manually (i.e. not via
lisp/Makefile) will not produce this warning. I think the small
annoyance of some people about overly-long doc strings produced by
macros is not a sufficient reason to turn this off by default, and
that those who are annoyed can turn it off in their configurations.
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#66264: Disable warning about wide doc strings by default
2023-09-29 15:43 ` Eli Zaretskii
@ 2023-09-30 8:17 ` Richard Stallman
0 siblings, 0 replies; 3+ messages in thread
From: Richard Stallman @ 2023-09-30 8:17 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: mattias.engdegard, 66264, stefankangas, raman
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> > This is a proposal to disable the warning about doc string line length by default.
> Thanks, but I'm against this change. It has been, and continues to
> be, tremendously useful in making sure our doc strings are in a good
> shape.
I agree. The point of a warning is to remind programmers to write
things the way that will give good results, and the width of a doc
string is part of what determines whether it gives good results
when users look at it.
--
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-30 8:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-29 12:59 bug#66264: Disable warning about wide doc strings by default Mattias Engdegård
2023-09-29 15:43 ` Eli Zaretskii
2023-09-30 8:17 ` Richard Stallman
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).