unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Kangas <stefan@marxist.se>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 44858@debbugs.gnu.org, larsi@gnus.org
Subject: bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
Date: Sat, 19 Dec 2020 15:55:20 -0800	[thread overview]
Message-ID: <CADwFkmk98GU5WKen2gQgJLthXiG93kqSy=1vwZVC+KqVhsOFYA@mail.gmail.com> (raw)
In-Reply-To: <83lfe43hjc.fsf@gnu.org>

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

Thanks for reviewing.  I've fixed all comments and added the tests you
suggested.  Please find attached two updated patches.

This still doesn't include warning for lambda docstrings: I think it
could make sense to get these two patches merged to see if there is any
feedback first.

[-- Attachment #2: 0001-Make-byte-compiler-warn-about-wide-docstrings.patch --]
[-- Type: text/x-diff, Size: 25282 bytes --]

From 085438b1e9ac6504bb84ab86bbe0b92694684a42 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Sun, 6 Dec 2020 12:44:19 +0100
Subject: [PATCH 1/2] Make byte-compiler warn about wide docstrings

* lisp/emacs-lisp/bytecomp.el (byte-compile--wide-docstring-p):
(byte-compile-docstring-length-warn): New defuns.
(byte-compile-docstring-max-column): New defcustom.
(byte-compile--wide-docstring-substitution-len): New variable.
(byte-compile-warning-types, byte-compile-warnings): New value
'docstrings'.
(byte-compile-file-form-autoload, byte-compile-file-form-defvar):
(byte-compile-file-form-defvar-function, byte-compile-lambda):
(byte-compile-defvar, byte-compile-file-form-defalias): Warn about too
wide docstrings.  (Bug#44858)

* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-warn-wide-docstring/defconst)
(bytecomp-warn-wide-docstring/defvar): New tests.
(bytecomp--define-warning-file-test): New macro.
(bytecomp/warn-wide-docstring-autoload\.el)
(bytecomp/warn-wide-docstring-custom-declare-variable\.el)
(bytecomp/warn-wide-docstring-defalias\.el)
(bytecomp/warn-wide-docstring-defconst\.el)
(bytecomp/warn-wide-docstring-define-abbrev-table\.el)
(bytecomp/warn-wide-docstring-define-obsolete-function-alias\.el)
(bytecomp/warn-wide-docstring-define-obsolete-variable-alias\.el)
(bytecomp/warn-wide-docstring-defun\.el)
(bytecomp/warn-wide-docstring-defvar\.el)
(bytecomp/warn-wide-docstring-defvaralias\.el)
(bytecomp/warn-wide-docstring-ignore-fill-column\.el)
(bytecomp/warn-wide-docstring-ignore-override\.el)
(bytecomp/warn-wide-docstring-ignore\.el)
(bytecomp/warn-wide-docstring-multiline-first\.el)
(bytecomp/warn-wide-docstring-multiline\.el): New tests.
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el:
New files.
---
 etc/NEWS                                      | 11 ++-
 etc/TODO                                      |  2 -
 lisp/emacs-lisp/bytecomp.el                   | 88 ++++++++++++++++++-
 .../warn-wide-docstring-autoload.el           |  3 +
 ...-wide-docstring-custom-declare-variable.el |  4 +
 .../warn-wide-docstring-defalias.el           |  3 +
 .../warn-wide-docstring-defconst.el           |  3 +
 ...warn-wide-docstring-define-abbrev-table.el |  3 +
 ...ocstring-define-obsolete-function-alias.el |  3 +
 ...ocstring-define-obsolete-variable-alias.el |  3 +
 .../warn-wide-docstring-defun.el              |  3 +
 .../warn-wide-docstring-defvar.el             |  6 ++
 .../warn-wide-docstring-defvaralias.el        |  3 +
 .../warn-wide-docstring-ignore-fill-column.el |  7 ++
 .../warn-wide-docstring-ignore-override.el    |  8 ++
 .../warn-wide-docstring-ignore.el             |  7 ++
 .../warn-wide-docstring-multiline-first.el    |  5 ++
 .../warn-wide-docstring-multiline.el          |  6 ++
 test/lisp/emacs-lisp/bytecomp-tests.el        | 71 +++++++++++++++
 19 files changed, 233 insertions(+), 6 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el

diff --git a/etc/NEWS b/etc/NEWS
index 4a8e70e6a6..1596976238 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2139,17 +2139,24 @@ menu handling.
 +++
 ** 'inhibit-nul-byte-detection' is renamed to 'inhibit-null-byte-detection'.
 
+** Byte compiler
+
 +++
-** New byte-compiler check for missing dynamic variable declarations.
+*** New byte-compiler check for missing dynamic variable declarations.
 It is meant as an (experimental) aid for converting Emacs Lisp code
 to lexical binding, where dynamic (special) variables bound in one
 file can affect code in another.  For details, see the manual section
 "(Elisp) Converting to Lexical Binding".
 
 +++
-** 'byte-recompile-directory' can now compile symlinked ".el" files.
+*** 'byte-recompile-directory' can now compile symlinked ".el" files.
 This is achieved by giving a non-nil FOLLOW-SYMLINKS parameter.
 
+*** The byte-compiler now warns about too wide documentation strings.
+By default, it will warn if a documentation string is wider than the
+largest of 80 characters or 'fill-column'.  This is controlled by the
+new user option 'byte-compile-docstring-max-column'.
+
 ---
 ** 'unload-feature' now also tries to undo additions to buffer-local hooks.
 
diff --git a/etc/TODO b/etc/TODO
index 08f851076c..45be76b786 100644
--- a/etc/TODO
+++ b/etc/TODO
@@ -608,8 +608,6 @@ Do this for some or all errors associated with using subprocesses.
 ** Maybe reinterpret 'parse-error' as a category of errors
 Put some other errors under it.
 
-** Make byte-compiler warn when a doc string is too wide
-
 ** Make byte-optimization warnings issue accurate line numbers
 
 ** Record the sxhash of the default value for customized variables
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 7e1a3304cc..f14ad93d2e 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -299,7 +299,8 @@ byte-compile-error-on-warn
 (defconst byte-compile-warning-types
   '(redefine callargs free-vars unresolved
              obsolete noruntime interactive-only
-	     make-local mapcar constants suspicious lexical lexical-dynamic)
+             make-local mapcar constants suspicious lexical lexical-dynamic
+             docstrings)
   "The list of warning types used when `byte-compile-warnings' is t.")
 (defcustom byte-compile-warnings t
   "List of warnings that the byte-compiler should issue (t for all).
@@ -322,6 +323,8 @@ byte-compile-warnings
   make-local  calls to make-variable-buffer-local that may be incorrect.
   mapcar      mapcar called for effect.
   constants   let-binding of, or assignment to, constants/nonvariables.
+  docstrings  docstrings that are too wide (longer than 80 characters,
+              or `fill-column', whichever is bigger)
   suspicious  constructs that usually don't do what the coder wanted.
 
 If the list begins with `not', then the remaining elements specify warnings to
@@ -1563,6 +1566,81 @@ byte-compile-arglist-warn
            (if (equal sig1 '(1 . 1)) "argument" "arguments")
            (byte-compile-arglist-signature-string sig2)))))))
 
+(defvar byte-compile--wide-docstring-substitution-len 3
+  "Substitution width used in `byte-compile--wide-docstring-p'.
+This is a heuristic for guessing the width of a documentation
+string: `byte-compile--wide-docstring-p' assumes that any
+`substitute-command-keys' command substitutions are this long.")
+
+(defun byte-compile--wide-docstring-p (docstring col)
+  "Return t if string DOCSTRING is wider than COL.
+Ignore all `substitute-command-keys' substitutions, except for
+the `\\\\=[command]' ones that are assumed to be of length
+`byte-compile--wide-docstring-substitution-len'.  Also ignore
+URLs."
+  (string-match
+   (format "^.\\{%s,\\}$" (int-to-string (1+ col)))
+   (replace-regexp-in-string
+    (rx (or
+         ;; Ignore some URLs.
+         (seq "http" (? "s") "://" (* anychar))
+         ;; Ignore these `substitute-command-keys' substitutions.
+         (seq "\\" (or "="
+                       (seq "<" (* (not ">")) ">")
+                       (seq "{" (* (not "}")) "}")))))
+    ""
+    ;; Heuristic: assume these substitutions are of some length N.
+    (replace-regexp-in-string
+     (rx "\\" (or (seq "[" (* (not "]")) "]")))
+     (make-string byte-compile--wide-docstring-substitution-len ?x)
+     docstring))))
+
+(defcustom byte-compile-docstring-max-column 80
+  "Recommended maximum width of doc string lines.
+The byte-compiler will emit a warning for documentation strings
+containing lines wider than this.  If `fill-column' has a larger
+value, it will override this variable."
+  :group 'bytecomp
+  :type 'integer
+  :safe #'integerp
+  :version "28.1")
+
+(defun byte-compile-docstring-length-warn (form)
+  "Warn if documentation string of FORM is too wide.
+It is too wide if it has any lines longer than the largest of
+`fill-column' and `byte-compile-docstring-max-column'."
+  ;; This has some limitations that it would be nice to fix:
+  ;; 1. We don't try to handle defuns.  It is somewhat tricky to get
+  ;;    it right since `defun' is a macro.  Also, some macros
+  ;;    themselves produce defuns (e.g. `define-derived-mode').
+  ;; 2. We assume that any `subsititute-command-keys' command replacement has a
+  ;;    given length.  We can't reliably do these replacements, since the value
+  ;;    of the keymaps in general can't be known at compile time.
+  (when (byte-compile-warning-enabled-p 'docstrings)
+    (let ((col (max byte-compile-docstring-max-column fill-column))
+          kind name docs)
+      (pcase (car form)
+        ((or 'autoload 'custom-declare-variable 'defalias
+             'defconst 'define-abbrev-table
+             'defvar 'defvaralias)
+         (setq kind (nth 0 form))
+         (setq name (nth 1 form))
+         (setq docs (nth 3 form)))
+        ;; Here is how one could add lambda's here:
+        ;; ('lambda
+        ;;   (setq kind "")   ; can't be "function", unfortunately
+        ;;   (setq docs (and (stringp (nth 2 form))
+        ;;                   (nth 2 form))))
+        )
+      (when (and (consp name) (eq (car name) 'quote))
+        (setq name (cadr name)))
+      (setq name (if name (format " `%s'" name) ""))
+      (when (and kind docs (stringp docs)
+                 (byte-compile--wide-docstring-p docs col))
+        (byte-compile-warn "%s%s docstring wider than %s characters"
+                           kind name col))))
+  form)
+
 (defun byte-compile-print-syms (str1 strn syms)
   (when syms
     (byte-compile-set-symbol-position (car syms) t))
@@ -2410,7 +2488,8 @@ byte-compile-file-form-autoload
              (delq (assq funsym byte-compile-unresolved-functions)
                    byte-compile-unresolved-functions)))))
   (if (stringp (nth 3 form))
-      form
+      (prog1 form
+        (byte-compile-docstring-length-warn form))
     ;; No doc string, so we can compile this as a normal form.
     (byte-compile-keep-pending form 'byte-compile-normal-call)))
 
@@ -2438,6 +2517,7 @@ byte-compile-file-form-defvar
   (if (and (null (cddr form))		;No `value' provided.
            (eq (car form) 'defvar))     ;Just a declaration.
       nil
+    (byte-compile-docstring-length-warn form)
     (cond ((consp (nth 2 form))
            (setq form (copy-sequence form))
            (setcar (cdr (cdr form))
@@ -2461,6 +2541,7 @@ byte-compile-file-form-defvar-function
        (if (byte-compile-warning-enabled-p 'suspicious)
            (byte-compile-warn
             "Alias for `%S' should be declared before its referent" newname)))))
+  (byte-compile-docstring-length-warn form)
   (byte-compile-keep-pending form))
 
 (put 'custom-declare-variable 'byte-hunk-handler
@@ -2844,6 +2925,7 @@ byte-compile-lambda
     (unless (eq 'lambda (car-safe fun))
       (error "Not a lambda list: %S" fun))
     (byte-compile-set-symbol-position 'lambda))
+  (byte-compile-docstring-length-warn fun)
   (byte-compile-check-lambda-list (nth 1 fun))
   (let* ((arglist (nth 1 fun))
          (arglistvars (byte-compile-arglist-vars arglist))
@@ -4624,6 +4706,7 @@ byte-compile-defvar
              (byte-compile-warning-enabled-p 'lexical (nth 1 form)))
     (byte-compile-warn "global/dynamic var `%s' lacks a prefix"
                        (nth 1 form)))
+  (byte-compile-docstring-length-warn form)
   (let ((fun (nth 0 form))
 	(var (nth 1 form))
 	(value (nth 2 form))
@@ -4698,6 +4781,7 @@ byte-compile-file-form-defalias
       ;; - `arg' is the expression to which it is defined.
       ;; - `rest' is the rest of the arguments.
       (`(,_ ',name ,arg . ,rest)
+       (byte-compile-docstring-length-warn form)
        (pcase-let*
            ;; `macro' is non-nil if it defines a macro.
            ;; `fun' is the function part of `arg' (defaults to `arg').
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el
new file mode 100644
index 0000000000..96deb1bbb0
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-autoload.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(autoload 'foox "foo"
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el
new file mode 100644
index 0000000000..2a4700bfda
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-custom-declare-variable.el
@@ -0,0 +1,4 @@
+;;; -*- lexical-binding: t -*-
+(custom-declare-variable
+ 'foo t
+ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el
new file mode 100644
index 0000000000..a4235d22bd
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defalias.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defalias 'foo #'ignore
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el
new file mode 100644
index 0000000000..946f01989a
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defconst.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defconst foo-bar nil
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el
new file mode 100644
index 0000000000..3da9ccd48c
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-abbrev-table.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(define-abbrev-table 'foo ()
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el
new file mode 100644
index 0000000000..fea841b12e
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-function-alias.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(define-obsolete-function-alias 'foo #'ignore "99.1"
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el
new file mode 100644
index 0000000000..2d5f201cb6
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-define-obsolete-variable-alias.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(define-obsolete-variable-alias 'foo 'ignore "99.1"
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el
new file mode 100644
index 0000000000..94b0e80c97
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defun.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el
new file mode 100644
index 0000000000..99aacd09cb
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvar.el
@@ -0,0 +1,6 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "multiline
+foo
+xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+bar")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el
new file mode 100644
index 0000000000..52fdc17f5b
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-defvaralias.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defvaralias 'foo-bar #'ignore
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el
new file mode 100644
index 0000000000..1ff554f370
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-fill-column.el
@@ -0,0 +1,7 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
+
+;; Local Variables:
+;; fill-column: 100
+;; End:
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el
new file mode 100644
index 0000000000..0bcf7b1d63
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore-override.el
@@ -0,0 +1,8 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "123456789012345")
+
+;; Local Variables:
+;; byte-compile-docstring-max-column: 10
+;; fill-column: 20
+;; End:
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el
new file mode 100644
index 0000000000..c80ddd180d
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-ignore.el
@@ -0,0 +1,7 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
+
+;; Local Variables:
+;; byte-compile-docstring-max-column: 100
+;; End:
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el
new file mode 100644
index 0000000000..2563dbbb3b
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline-first.el
@@ -0,0 +1,5 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+This is a multiline docstring where the first line is long.
+foobar")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el
new file mode 100644
index 0000000000..9ae7bc9b9f
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-wide-docstring-multiline.el
@@ -0,0 +1,6 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "This is a multiline docstring.
+But it's not the first line that is long.
+xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+foobar")
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 4a6e28f7c7..47aab563f6 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -540,6 +540,16 @@ bytecomp-warn-variable-lacks-prefix
   (bytecomp--with-warning-test "foo.*lacks a prefix"
     '(defvar foo nil)))
 
+(defvar bytecomp-tests--docstring (make-string 100 ?x))
+
+(ert-deftest bytecomp-warn-wide-docstring/defconst ()
+  (bytecomp--with-warning-test "defconst.*foo.*wider than.*characters"
+    `(defconst foo t ,bytecomp-tests--docstring)))
+
+(ert-deftest bytecomp-warn-wide-docstring/defvar ()
+  (bytecomp--with-warning-test "defvar.*foo.*wider than.*characters"
+    `(defvar foo t ,bytecomp-tests--docstring)))
+
 (defmacro bytecomp--define-warning-file-test (file re-warning &optional reverse)
   `(ert-deftest ,(intern (format "bytecomp/%s" file)) ()
      :expected-result ,(if reverse :failed :passed)
@@ -639,6 +649,67 @@ "warn-variable-set-constant.el"
 (bytecomp--define-warning-file-test "warn-variable-set-nonvariable.el"
                             "variable reference to nonvariable")
 
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-autoload.el"
+ "autoload.*foox.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-custom-declare-variable.el"
+ "custom-declare-variable.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-defalias.el"
+ "defalias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-defconst.el"
+ "defconst.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-define-abbrev-table.el"
+ "define-abbrev.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-define-obsolete-function-alias.el"
+ "defalias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-define-obsolete-variable-alias.el"
+ "defvaralias.*foo.*wider than.*characters")
+
+;; TODO: We don't yet issue warnings for defuns.
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-defun.el"
+ "wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-defvar.el"
+ "defvar.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-defvaralias.el"
+ "defvaralias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-ignore-fill-column.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-ignore-override.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-ignore.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-multiline-first.el"
+ "defvar.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-wide-docstring-multiline.el"
+ "defvar.*foo.*wider than.*characters")
+
 \f
 ;;;; Macro expansion.
 
-- 
2.29.2


[-- Attachment #3: 0002-Fill-some-auto-generated-docstrings.patch --]
[-- Type: text/x-diff, Size: 7068 bytes --]

From 97beb0b1f93481e9eba6e3f70d079b2b3d14ade8 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 10 Dec 2020 22:36:18 +0100
Subject: [PATCH 2/2] Fill some auto-generated docstrings

* lisp/emacs-lisp/easy-mmode.el (define-minor-mode)
(define-globalized-minor-mode): Fill auto-generated documentation
strings.  (Bug#44858)
* lisp/subr.el (internal--fill-string-single-line)
(internal--format-docstring-line): New functions.
---
 lisp/emacs-lisp/derived.el    | 42 +++++++++++++++++++----------------
 lisp/emacs-lisp/easy-mmode.el | 39 ++++++++++++++++----------------
 lisp/subr.el                  | 18 +++++++++++++++
 3 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/lisp/emacs-lisp/derived.el b/lisp/emacs-lisp/derived.el
index 6a11f1c394..dee507269b 100644
--- a/lisp/emacs-lisp/derived.el
+++ b/lisp/emacs-lisp/derived.el
@@ -306,11 +306,13 @@ derived-mode-make-docstring
       ;; Use a default docstring.
       (setq docstring
 	    (if (null parent)
-		;; FIXME filling.
-		(format "Major-mode.\nUses keymap `%s'%s%s." map
-			(if abbrev (format "%s abbrev table `%s'"
-					   (if syntax "," " and") abbrev) "")
-			(if syntax (format " and syntax-table `%s'" syntax) ""))
+                (concat
+                 "Major-mode.\n"
+                 (internal--format-docstring-line
+                  "Uses keymap `%s'%s%s." map
+                  (if abbrev (format "%s abbrev table `%s'"
+                                     (if syntax "," " and") abbrev) "")
+                  (if syntax (format " and syntax-table `%s'" syntax) "")))
 	      (format "Major mode derived from `%s' by `define-derived-mode'.
 It inherits all of the parent's attributes, but has its own keymap%s:
 
@@ -336,20 +338,22 @@ derived-mode-make-docstring
     (unless (string-match (regexp-quote (symbol-name hook)) docstring)
       ;; Make sure the docstring mentions the mode's hook.
       (setq docstring
-	    (concat docstring
-		    (if (null parent)
-			"\n\nThis mode "
-		      (concat
-		       "\n\nIn addition to any hooks its parent mode "
-		       (if (string-match (format "[`‘]%s['’]"
-                                                 (regexp-quote
-						  (symbol-name parent)))
-					 docstring)
-                           nil
-			 (format "`%s' " parent))
-		       "might have run,\nthis mode "))
-		    (format "runs the hook `%s'" hook)
-		    ", as the final or penultimate step\nduring initialization.")))
+            (concat docstring "\n\n"
+                    (internal--format-docstring-line
+                     "%s%s%s"
+                     (if (null parent)
+                         "This mode "
+                       (concat
+                        "In addition to any hooks its parent mode "
+                        (if (string-match (format "[`‘]%s['’]"
+                                                  (regexp-quote
+                                                   (symbol-name parent)))
+                                          docstring)
+                            nil
+                          (format "`%s' " parent))
+                        "might have run, this mode "))
+                     (format "runs the hook `%s'" hook)
+                     ", as the final or penultimate step during initialization."))))
 
     (unless (string-match "\\\\[{[]" docstring)
       ;; And don't forget to put the mode's keymap.
diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 261f2508af..1344c3391b 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -278,8 +278,10 @@ define-minor-mode
          ((not globalp)
           `(progn
              :autoload-end
-             (defvar ,mode ,init-value ,(format "Non-nil if %s is enabled.
-Use the command `%s' to change this variable." pretty-name mode))
+             (defvar ,mode ,init-value
+               ,(concat (format "Non-nil if %s is enabled.\n" pretty-name)
+                        (internal--format-docstring-line
+                         "Use the command `%s' to change this variable." mode)))
              (make-variable-buffer-local ',mode)))
          (t
 	  (let ((base-doc-string
@@ -455,24 +457,23 @@ define-globalized-minor-mode
          (make-variable-buffer-local ',MODE-major-mode))
        ;; The actual global minor-mode
        (define-minor-mode ,global-mode
-	 ;; Very short lines to avoid too long lines in the generated
-	 ;; doc string.
-	 ,(format "Toggle %s in all buffers.
-With prefix ARG, enable %s if ARG is positive;
-otherwise, disable it.  If called from Lisp, enable the mode if
-ARG is omitted or nil.
-
-%s is enabled in all buffers where
-`%s' would do it.
-
-See `%s' for more information on
-%s.%s"
-		  pretty-name pretty-global-name
-		  pretty-name turn-on mode pretty-name
+         ,(concat (format "Toggle %s in all buffers.\n" pretty-name)
+                  (internal--format-docstring-line
+                   "With prefix ARG, enable %s if ARG is positive; otherwise, \
+disable it.  If called from Lisp, enable the mode if ARG is omitted or nil.\n\n"
+                   pretty-global-name)
+                  (internal--format-docstring-line
+                   "%s is enabled in all buffers where `%s' would do it.\n\n"
+                   pretty-name turn-on)
+                  (internal--format-docstring-line
+                   "See `%s' for more information on %s."
+                   mode pretty-name)
                   (if predicate
-                      (format "\n\n`%s' is used to control which modes
-this minor mode is used in."
-                              MODE-predicate)
+                      (concat
+                       "\n\n"
+                       (internal--format-docstring-line
+                        "`%s' is used to control which modes this minor mode is used in."
+                        MODE-predicate))
                     ""))
          :global t ,@group ,@(nreverse extra-keywords)
 
diff --git a/lisp/subr.el b/lisp/subr.el
index 1b2d778454..0a46221a7f 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -5952,4 +5952,22 @@ run-hook-query-error-with-timeout
      ;; Continue running.
      nil)))
 
+(defun internal--fill-string-single-line (str)
+  "Fill string STR to `fill-column'.
+This is intended for very simple filling while bootstrapping
+Emacs itself, and does not support all the customization options
+of fill.el (for example `fill-region')."
+  (if (< (string-width str) fill-column)
+      str
+    (let ((fst (substring str 0 fill-column))
+          (lst (substring str fill-column)))
+      (if (string-match ".*\\( \\(.+\\)\\)$" fst)
+          (setq fst (replace-match "\n\\2" nil nil fst 1)))
+      (concat fst (internal--fill-string-single-line lst)))))
+
+(defun internal--format-docstring-line (string &rest objects)
+  "Format a documentation string out of STRING and OBJECTS.
+This is intended for internal use only."
+  (internal--fill-string-single-line (apply #'format string objects)))
+
 ;;; subr.el ends here
-- 
2.29.2


  reply	other threads:[~2020-12-19 23:55 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-25  1:36 bug#44858: [PATCH] Make byte-compiler warn about wide docstrings Stefan Kangas
2020-11-26 10:49 ` Lars Ingebrigtsen
2020-11-26 12:46   ` Stefan Kangas
2020-11-26 12:53     ` Lars Ingebrigtsen
2020-12-10 20:59       ` Stefan Kangas
2020-12-10 21:53         ` Stefan Kangas
2020-12-11  8:16           ` Eli Zaretskii
2020-12-11 20:03             ` Stefan Kangas
2020-12-11  7:33         ` Eli Zaretskii
2020-12-11 20:36           ` Stefan Kangas
2020-12-19 11:22             ` Eli Zaretskii
2020-12-19 16:50               ` Stefan Kangas
2020-12-19 17:14                 ` Eli Zaretskii
2020-12-29  1:27                   ` Basil L. Contovounesios
2020-12-29  2:16                     ` Lars Ingebrigtsen
2020-12-19 17:18                 ` Lars Ingebrigtsen
2020-12-19 23:48                   ` Stefan Kangas
2020-12-11  7:53         ` Eli Zaretskii
2020-12-19 23:55           ` Stefan Kangas [this message]
2020-12-20 17:53             ` Lars Ingebrigtsen
2020-12-28  5:18               ` Stefan Kangas
2020-12-11 15:13         ` Lars Ingebrigtsen
2020-12-30 12:07       ` Stefan Kangas
2020-12-31  4:42         ` Lars Ingebrigtsen
2020-11-26 14:19 ` Eli Zaretskii
2020-11-27  8:37   ` Lars Ingebrigtsen
2020-11-27 11:15     ` Stefan Kangas
2020-11-27 12:44       ` Eli Zaretskii
2020-12-06 11:09         ` Stefan Kangas
2020-12-06 11:19           ` Eli Zaretskii
2020-12-06 16:54           ` Drew Adams
2020-11-27 18:36     ` Drew Adams
2020-11-27 18:55       ` Drew Adams
2020-12-03 20:18 ` Tomas Nordin
2020-12-11 20:14   ` Stefan Kangas
2021-09-24 17:25 ` Stefan Kangas
2021-09-25  1:07   ` Lars Ingebrigtsen
2021-09-26 11:38     ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CADwFkmk98GU5WKen2gQgJLthXiG93kqSy=1vwZVC+KqVhsOFYA@mail.gmail.com' \
    --to=stefan@marxist.se \
    --cc=44858@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).