unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
@ 2020-11-25  1:36 Stefan Kangas
  2020-11-26 10:49 ` Lars Ingebrigtsen
                   ` (3 more replies)
  0 siblings, 4 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-11-25  1:36 UTC (permalink / raw)
  To: 44858

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

Severity: wishlist

In etc/TODO we have:

** Make byte-compiler warn when a doc string is too wide

Please find attached a patch that implements this.  Notably, it omits
defuns, and ignores any lines containing `substitute-command-keys' style
substitutions.

I've been messing around with getting defuns to work, but I can't find a
way for it to work reliably.  The problem is that they are already macro
expanded when we start issuing warnings, so it's not trivial to reliably
separate defuns from other cases where lambda is used.  We also have
other macros like `define-derived-mode' that produces `defun's
themselves, and the line reporting gets messed up.  (The autogenerated
docstrings from these macros seem to generate too wide docstrings in
several cases.)

For `substitute-command-keys', it would be nice to get it to work, but I
don't think we can know the values of keymaps at compile-time.  Possibly
there is a good solution for this, but I couldn't find it.

As for the state of our own sources, we seem to have less than 100
docstrings that break our own conventions and generates a warning with
the patch.  It shouldn't take too much work to fix them.

(If you were to add defuns into the mix, we would get around 700
warnings for wide docstrings, several of which would be autogenerated.
We should probably look into that at some point...)

Thoughts?

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

From 03b343d0709625aa92fced65e56ad29ff6d84d0c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Wed, 25 Nov 2020 02:25:57 +0100
Subject: [PATCH] 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-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.

* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp--define-warning-file-test): New macro.
(bytecomp-warn/warn-long-docstring-autoload\.el)
(bytecomp-warn/warn-long-docstring-custom-declare-variable\.el)
(bytecomp-warn/warn-long-docstring-defalias\.el)
(bytecomp-warn/warn-long-docstring-defconst\.el)
(bytecomp-warn/warn-long-docstring-define-abbrev-table\.el)
(bytecomp-warn/warn-long-docstring-define-obsolete-function-alias\.el)
(bytecomp-warn/warn-long-docstring-define-obsolete-variable-alias\.el)
(bytecomp-warn/warn-long-docstring-defun\.el)
(bytecomp-warn/warn-long-docstring-defvar\.el)
(bytecomp-warn/warn-long-docstring-defvaralias\.el)
(bytecomp-warn/warn-long-docstring-ignore-fill-column\.el)
(bytecomp-warn/warn-long-docstring-ignore\.el): New tests.
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-abbrev-table.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-function-alias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-variable-alias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore-fill-column.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore.el:
New files.
---
 etc/NEWS                                      |  9 ++-
 etc/TODO                                      |  2 -
 lisp/emacs-lisp/bytecomp.el                   | 71 ++++++++++++++++++-
 .../warn-long-docstring-autoload.el           |  3 +
 ...-long-docstring-custom-declare-variable.el |  4 ++
 .../warn-long-docstring-defalias.el           |  3 +
 .../warn-long-docstring-defconst.el           |  3 +
 ...warn-long-docstring-define-abbrev-table.el |  3 +
 ...ocstring-define-obsolete-function-alias.el |  3 +
 ...ocstring-define-obsolete-variable-alias.el |  3 +
 .../warn-long-docstring-defun.el              |  3 +
 .../warn-long-docstring-defvar.el             |  3 +
 .../warn-long-docstring-defvaralias.el        |  3 +
 .../warn-long-docstring-ignore-fill-column.el |  7 ++
 .../warn-long-docstring-ignore.el             |  7 ++
 test/lisp/emacs-lisp/bytecomp-tests.el        | 69 ++++++++++++++++++
 16 files changed, 191 insertions(+), 5 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-abbrev-table.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-function-alias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-variable-alias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore-fill-column.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore.el

diff --git a/etc/NEWS b/etc/NEWS
index 95f801f60c..e768657841 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1978,13 +1978,20 @@ 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".
 
+*** 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'.  See 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 8e93e7fb10..38dfda424f 100644
--- a/etc/TODO
+++ b/etc/TODO
@@ -572,8 +572,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 a687d291e9..1eaac6a343 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -297,7 +297,8 @@ byte-compile-error-on-warn
 (defconst byte-compile-warning-types
   '(redefine callargs free-vars unresolved
 	     obsolete noruntime cl-functions 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 (no 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
@@ -1581,6 +1584,64 @@ byte-compile-arglist-warn
            (if (equal sig1 '(1 . 1)) "argument" "arguments")
            (byte-compile-arglist-signature-string sig2)))))))
 
+(defun byte-compile--wide-docstring-p (docstring col)
+  "Return t if string DOCSTRING is wider than COL.
+Ignore any `substitute-command-keys' substitutions."
+  (string-match
+   (format "^.\\{%s,\\}$" (int-to-string (1+ col)))
+   ;; Ignore any `substitute-command-keys' substitutions.
+   (replace-regexp-in-string
+    (rx "\\" (or "="
+                 (seq "[" (* (not "]")) "]")
+                 (seq "<" (* (not ">")) ">")
+                 (seq "{" (* (not "}")) "}")))
+    ""
+    docstring)))
+
+(defcustom byte-compile-docstring-max-column 80
+  "Length that a doc string can be before the byte-compiler reports a warning."
+  :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 is longer than the largest of `fill-column'
+and `byte-compile-docstring-max-column'."
+  ;; TODO: 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 ignore any line where there is a `subsititute-command-keys'
+  ;;    replacement.  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)
+
 (defvar byte-compile-cl-functions nil
   "List of functions defined in CL.")
 
@@ -2446,7 +2507,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)))
 
@@ -2474,6 +2536,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))
@@ -2497,6 +2560,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
@@ -2895,6 +2959,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))
@@ -4672,6 +4737,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))
@@ -4746,6 +4812,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-long-docstring-autoload.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
new file mode 100644
index 0000000000..96deb1bbb0
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(autoload 'foox "foo"
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el
new file mode 100644
index 0000000000..2a4700bfda
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-defalias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el
new file mode 100644
index 0000000000..a4235d22bd
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defalias 'foo #'ignore
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el
new file mode 100644
index 0000000000..946f01989a
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-define-abbrev-table.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-abbrev-table.el
new file mode 100644
index 0000000000..3da9ccd48c
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-define-obsolete-function-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-function-alias.el
new file mode 100644
index 0000000000..fea841b12e
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-define-obsolete-variable-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-variable-alias.el
new file mode 100644
index 0000000000..2d5f201cb6
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el
new file mode 100644
index 0000000000..94b0e80c97
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el
new file mode 100644
index 0000000000..4bf8c17c3f
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el
new file mode 100644
index 0000000000..52fdc17f5b
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-ignore-fill-column.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore-fill-column.el
new file mode 100644
index 0000000000..1ff554f370
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-ignore.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore.el
new file mode 100644
index 0000000000..c80ddd180d
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 680aa514a2..11db749539 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -26,6 +26,7 @@
 ;;; Commentary:
 
 (require 'ert)
+(require 'ert-x)
 (require 'cl-lib)
 (require 'subr-x)
 (require 'bytecomp)
@@ -536,6 +537,74 @@ 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-long-docstring/defconst ()
+  (bytecomp--with-warning-test "defconst.*foo.*wider than.*characters"
+    `(defconst foo t ,bytecomp-tests--docstring)))
+
+(ert-deftest bytecomp-warn-long-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-warn/%s" file)) ()
+     :expected-result ,(if reverse :failed :passed)
+     (with-current-buffer (get-buffer-create "*Compile-Log*")
+       (let ((inhibit-read-only t)) (erase-buffer))
+       (byte-compile-file ,(ert-resource-file file))
+       (ert-info ((buffer-string) :prefix "buffer: ")
+         (should (re-search-forward ,re-warning))))))
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-autoload.el"
+ "autoload.*foox.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-custom-declare-variable.el"
+ "custom-declare-variable.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defalias.el"
+ "defalias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defconst.el"
+ "defconst.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-define-abbrev-table.el"
+ "define-abbrev.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-define-obsolete-function-alias.el"
+ "defalias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-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-long-docstring-defun.el"
+ "wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defvar.el"
+ "defvar.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defvaralias.el"
+ "defvaralias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-ignore-fill-column.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-ignore.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
 (ert-deftest test-eager-load-macro-expansion ()
   (test-byte-comp-compile-and-load nil
     '(progn (defmacro abc (arg) 1) (defun def () (abc 2))))
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  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 14:19 ` Eli Zaretskii
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-26 10:49 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> ** Make byte-compiler warn when a doc string is too wide
>
> Please find attached a patch that implements this.

I like it.

> Notably, it omits defuns, and ignores any lines containing
> `substitute-command-keys' style substitutions.
>
> I've been messing around with getting defuns to work, but I can't find a
> way for it to work reliably.  The problem is that they are already macro
> expanded when we start issuing warnings, so it's not trivial to reliably
> separate defuns from other cases where lambda is used.

I'm probably misunderstanding you completely, but do we have to separate
between lambda and defuns?  lambdas can also have doc strings...

> (If you were to add defuns into the mix, we would get around 700
> warnings for wide docstrings, several of which would be autogenerated.
> We should probably look into that at some point...)

Yes, the autogenerated docstrings should be fixed, too -- mostly by
running them through `fill-paragraph'.

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-26 10:49 ` Lars Ingebrigtsen
@ 2020-11-26 12:46   ` Stefan Kangas
  2020-11-26 12:53     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2020-11-26 12:46 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I like it.

Thanks.

>> I've been messing around with getting defuns to work, but I can't find a
>> way for it to work reliably.  The problem is that they are already macro
>> expanded when we start issuing warnings, so it's not trivial to reliably
>> separate defuns from other cases where lambda is used.
>
> I'm probably misunderstanding you completely, but do we have to separate
> between lambda and defuns?  lambdas can also have doc strings...

Indeed they can.  The problem I had was how to differentiate between the
many different macros that generate lambdas.  I didn't record the exact
details, but it got pretty messy between `defun', `define-derived-mode',
`cl-defgeneric', etc. etc.  The problem I saw was basically warnings
about symbols only visible after macro expansion, and that warnings
would point to entirely the wrong line and column.  (If anyone wants to
take a look, it should be sufficient to uncomment the lambda part in my
patch and run "make bootstrap".)

I was looking at `byte-defop-compiler-1', but that seems to only be
usable for functions and special forms?  IIUC, when we compile all
macros have been expanded and all information about what macro was used
where is lost.  But it's possible that I'm misunderstanding things, as
I'm still wrapping my head around this code.

>> (If you were to add defuns into the mix, we would get around 700
>> warnings for wide docstrings, several of which would be autogenerated.
>> We should probably look into that at some point...)
>
> Yes, the autogenerated docstrings should be fixed, too -- mostly by
> running them through `fill-paragraph'.

I tried that in e.g. define-derived-mode, but fill.el is loaded after
derived.el.  So it seems like there is some fun to be had in figuring
out the dependencies there...





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-26 12:46   ` Stefan Kangas
@ 2020-11-26 12:53     ` Lars Ingebrigtsen
  2020-12-10 20:59       ` Stefan Kangas
  2020-12-30 12:07       ` Stefan Kangas
  0 siblings, 2 replies; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-26 12:53 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> The problem I saw was basically warnings
> about symbols only visible after macro expansion, and that warnings
> would point to entirely the wrong line and column.

Oh, OK, the problem was in the lines/column output, not with detecting
the doc strings themselves?

Yes, that sounds like a problem, but...  I think we can live with
inaccurate lines here.

>> Yes, the autogenerated docstrings should be fixed, too -- mostly by
>> running them through `fill-paragraph'.
>
> I tried that in e.g. define-derived-mode, but fill.el is loaded after
> derived.el.  So it seems like there is some fun to be had in figuring
> out the dependencies there...

Yeah, I guess fill.el is a pretty far down in the list of things that
loadup.el loads?

However, we could make a super-simple function for filling doc
strings -- doing something that's good enough for that particular task
shouldn't take more than a few lines.

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  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 14:19 ` Eli Zaretskii
  2020-11-27  8:37   ` Lars Ingebrigtsen
  2020-12-03 20:18 ` Tomas Nordin
  2021-09-24 17:25 ` Stefan Kangas
  3 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-11-26 14:19 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 24 Nov 2020 17:36:34 -0800
> 
> For `substitute-command-keys', it would be nice to get it to work, but I
> don't think we can know the values of keymaps at compile-time.  Possibly
> there is a good solution for this, but I couldn't find it.

How about some simplified heuristics, like assume that the expansion
takes no more than N characters (where N could be something like 5)?
This should work in, like, 80% of cases, I think.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-26 14:19 ` Eli Zaretskii
@ 2020-11-27  8:37   ` Lars Ingebrigtsen
  2020-11-27 11:15     ` Stefan Kangas
  2020-11-27 18:36     ` Drew Adams
  0 siblings, 2 replies; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-27  8:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, Stefan Kangas

Eli Zaretskii <eliz@gnu.org> writes:

>> For `substitute-command-keys', it would be nice to get it to work, but I
>> don't think we can know the values of keymaps at compile-time.  Possibly
>> there is a good solution for this, but I couldn't find it.
>
> How about some simplified heuristics, like assume that the expansion
> takes no more than N characters (where N could be something like 5)?
> This should work in, like, 80% of cases, I think.

Yup.  And 15% is mostly when it expands to `M-x some-long-command'
because the keymap hasn't been loaded yet, I think?  Which we could
conceivably fix by loading the file when the used `C-h f's an autoloaded
function with one of these constructs?  Perhaps a bit hacky...

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-27  8:37   ` Lars Ingebrigtsen
@ 2020-11-27 11:15     ` Stefan Kangas
  2020-11-27 12:44       ` Eli Zaretskii
  2020-11-27 18:36     ` Drew Adams
  1 sibling, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2020-11-27 11:15 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> For `substitute-command-keys', it would be nice to get it to work, but I
> >> don't think we can know the values of keymaps at compile-time.  Possibly
> >> there is a good solution for this, but I couldn't find it.
> >
> > How about some simplified heuristics, like assume that the expansion
> > takes no more than N characters (where N could be something like 5)?
> > This should work in, like, 80% of cases, I think.
>
> Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> because the keymap hasn't been loaded yet, I think?  Which we could
> conceivably fix by loading the file when the used `C-h f's an autoloaded
> function with one of these constructs?  Perhaps a bit hacky...

I would be wary of using a heuristic here, because I think false
positives are worse than false negatives in this case.  We
unfortunately don't have any way of silencing individual warnings, so
a user seeing a false positive is left with two suboptimal choices:
ignore the warning (a bad habit to train our users in) or change the
formatting of a docstring to stop it (potentially making it
subjectively worse in the process).

How about using the somewhat safer heuristic of treating substitutions
as one character wide?  Would that make sense?





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-27 11:15     ` Stefan Kangas
@ 2020-11-27 12:44       ` Eli Zaretskii
  2020-12-06 11:09         ` Stefan Kangas
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-11-27 12:44 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 27 Nov 2020 12:15:03 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 44858@debbugs.gnu.org
> 
> > > How about some simplified heuristics, like assume that the expansion
> > > takes no more than N characters (where N could be something like 5)?
> > > This should work in, like, 80% of cases, I think.
> >
> > Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> > because the keymap hasn't been loaded yet, I think?  Which we could
> > conceivably fix by loading the file when the used `C-h f's an autoloaded
> > function with one of these constructs?  Perhaps a bit hacky...
> 
> I would be wary of using a heuristic here, because I think false
> positives are worse than false negatives in this case.

Can we have some numbers, please? how many false positives do you get
by assuming the expanded key sequence takes 5 characters? what about 3
or 4 or 7?

> We unfortunately don't have any way of silencing individual
> warnings, so a user seeing a false positive is left with two
> suboptimal choices: ignore the warning (a bad habit to train our
> users in) or change the formatting of a docstring to stop it
> (potentially making it subjectively worse in the process).

The assumption is that using such heuristic will cause false
negatives, not false positives.  Do you see any bad consequences to
false negatives?

> How about using the somewhat safer heuristic of treating substitutions
> as one character wide?  Would that make sense?

I'd say, at least 3, because there are very few non-trivial key
bindings bound to a single character.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-27  8:37   ` Lars Ingebrigtsen
  2020-11-27 11:15     ` Stefan Kangas
@ 2020-11-27 18:36     ` Drew Adams
  2020-11-27 18:55       ` Drew Adams
  1 sibling, 1 reply; 38+ messages in thread
From: Drew Adams @ 2020-11-27 18:36 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: 44858, Stefan Kangas

> Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> because the keymap hasn't been loaded yet, I think?  Which we could
> conceivably fix by loading the file when the used `C-h f's an
> autoloaded function with one of these constructs?  Perhaps a bit hacky...

Please, no.  Users should be able to see the doc
without loading the library.  That's an important
feature, IMO, which has been present from Day One
(or whatever day autoloading was introduced).

I'm all for cosmetic improvements and not having
long lines, believe me.  But I think loading a
library just to (maybe!) change `M-x function-name'
to a key binding description is, I think, misguided.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-27 18:36     ` Drew Adams
@ 2020-11-27 18:55       ` Drew Adams
  0 siblings, 0 replies; 38+ messages in thread
From: Drew Adams @ 2020-11-27 18:55 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: 44858, Stefan Kangas

> > Yup.  And 15% is mostly when it expands to `M-x some-long-command'
> > because the keymap hasn't been loaded yet, I think?  Which we could
> > conceivably fix by loading the file when the used `C-h f's an
> > autoloaded function with one of these constructs?  Perhaps a bit
> hacky...
> 
> Please, no.  Users should be able to see the doc
> without loading the library.  That's an important
> feature, IMO, which has been present from Day One
> (or whatever day autoloading was introduced).
> 
> I'm all for cosmetic improvements and not having
> long lines, believe me.  But I think loading a
> library just to (maybe!) change `M-x function-name'
> to a key binding description is, I think, misguided.

Or maybe you meant load only for byte-compilation?

But even then, it's possible for byte-compiling to
be done during a user's session, and in cases where
there's no desire to load the library.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  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 14:19 ` Eli Zaretskii
@ 2020-12-03 20:18 ` Tomas Nordin
  2020-12-11 20:14   ` Stefan Kangas
  2021-09-24 17:25 ` Stefan Kangas
  3 siblings, 1 reply; 38+ messages in thread
From: Tomas Nordin @ 2020-12-03 20:18 UTC (permalink / raw)
  To: Stefan Kangas, 44858

Stefan Kangas <stefan@marxist.se> writes:

> Thoughts?

One thing that caught my eye

@@ -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 (no longer than 80 characters,
+              or `fill-column', whichever is bigger)
   suspicious  constructs that usually don't do what the coder wanted.

Should 'no' maybe be removed? (if to tell something that should not be).

--
Tomas





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  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
  0 siblings, 2 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-06 11:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, larsi

Eli Zaretskii <eliz@gnu.org> writes:

>> > > How about some simplified heuristics, like assume that the expansion
>> > > takes no more than N characters (where N could be something like 5)?
>> > > This should work in, like, 80% of cases, I think.
>> >
>> > Yup.  And 15% is mostly when it expands to `M-x some-long-command'
>> > because the keymap hasn't been loaded yet, I think?  Which we could
>> > conceivably fix by loading the file when the used `C-h f's an autoloaded
>> > function with one of these constructs?  Perhaps a bit hacky...
>>
>> I would be wary of using a heuristic here, because I think false
>> positives are worse than false negatives in this case.
>
> Can we have some numbers, please? how many false positives do you get
> by assuming the expanded key sequence takes 5 characters? what about 3
> or 4 or 7?

I have run some tests.  The first column is the number of characters.
The second column is the number of "wide docstring" warnings with my
patch.  The third column is when I add in warnings for lambda
docstrings (commented out in the patch):

    | Characters | Warnings | Warnings (w/lambda) |
    |------------+----------+---------------------|
    |          0 |      109 |                 451 |
    |          1 |      110 |                 452 |
    |          2 |      110 |                 452 |
    |          3 |      110 |                 454 |
    |          5 |      110 |                 463 |
    |          6 |      111 |                 468 |
    |          7 |      111 |                 475 |

We don't seem to get much additional benefit from a heuristic with a
higher number of characters (i.e. not a lot more warnings).

I checked some of the warnings when using 7 characters (with lambda),
and all new warnings I checked were false positives:

   help.el:194:1: Warning: docstring wider than 80 characters
   isearch.el:1001:8: Warning: docstring wider than 80 characters
   simple.el:5616:8: Warning: docstring wider than 80 characters

>> We unfortunately don't have any way of silencing individual
>> warnings, so a user seeing a false positive is left with two
>> suboptimal choices: ignore the warning (a bad habit to train our
>> users in) or change the formatting of a docstring to stop it
>> (potentially making it subjectively worse in the process).
>
> The assumption is that using such heuristic will cause false
> negatives, not false positives.  Do you see any bad consequences to
> false negatives?

Not sure what you mean here; my assumption was that it would cause false
positives.  I see no bad consequences to false negatives, so I'm not
overly worried about them.  (False negatives are neither worse nor
better than the status quo.)

>> How about using the somewhat safer heuristic of treating substitutions
>> as one character wide?  Would that make sense?
>
> I'd say, at least 3, because there are very few non-trivial key
> bindings bound to a single character.

AFAIU, if N is the number of characters, N=1 would cause only false
negatives.  For any N>M, where M is the length of longest command name
in use, it would cause only false positives.  For any N where N>1 and
N<M, it would cause both false negatives and false positives.

Using 3 is not significantly better than 1, as the above numbers show.
But we do risk more false positives.  My preference is therefore still
the safer 1, as it will give no false positives.

We could start with 3 or 1 and adjust later as we learn more about how
this heuristic works in practice.  I don't have a very strong opinion,
as I think we will learn more in due time.

WDYT?





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-06 11:09         ` Stefan Kangas
@ 2020-12-06 11:19           ` Eli Zaretskii
  2020-12-06 16:54           ` Drew Adams
  1 sibling, 0 replies; 38+ messages in thread
From: Eli Zaretskii @ 2020-12-06 11:19 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 6 Dec 2020 05:09:45 -0600
> Cc: larsi@gnus.org, 44858@debbugs.gnu.org
> 
> Using 3 is not significantly better than 1, as the above numbers show.
> But we do risk more false positives.  My preference is therefore still
> the safer 1, as it will give no false positives.
> 
> We could start with 3 or 1 and adjust later as we learn more about how
> this heuristic works in practice.  I don't have a very strong opinion,
> as I think we will learn more in due time.
> 
> WDYT?

Since 3 gives the same number of warnings, and since it covers 3 times
as many keys as 1, I prefer to use 3.

Thanks.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-06 11:09         ` Stefan Kangas
  2020-12-06 11:19           ` Eli Zaretskii
@ 2020-12-06 16:54           ` Drew Adams
  1 sibling, 0 replies; 38+ messages in thread
From: Drew Adams @ 2020-12-06 16:54 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii; +Cc: 44858, larsi

(FWIW, I'm not convinced this is very helpful.)

But if you do add this, please consider letting
the message provide the actual longest-line
length.

Someone concerned with fixing long doc-string
lines might be more concerned with fixing one
with, say, 179 chars than one with, say, 82.

Presumably the actual longest-line length is
available for the message.  Why not show it?

Or maybe not.  If your code finds only the
first line that's > 80 chars then it won't
help as much.  Doc strings are typically
fairly short.  Looking for the max line
length isn't very expensive.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-26 12:53     ` Lars Ingebrigtsen
@ 2020-12-10 20:59       ` Stefan Kangas
  2020-12-10 21:53         ` Stefan Kangas
                           ` (3 more replies)
  2020-12-30 12:07       ` Stefan Kangas
  1 sibling, 4 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-10 20:59 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> The problem I saw was basically warnings
>> about symbols only visible after macro expansion, and that warnings
>> would point to entirely the wrong line and column.
>
> Oh, OK, the problem was in the lines/column output, not with detecting
> the doc strings themselves?
>
> Yes, that sounds like a problem, but...  I think we can live with
> inaccurate lines here.

OK.  The next issue is actually fixing all those docstrings though...

>>> Yes, the autogenerated docstrings should be fixed, too -- mostly by
>>> running them through `fill-paragraph'.
>>
>> I tried that in e.g. define-derived-mode, but fill.el is loaded after
>> derived.el.  So it seems like there is some fun to be had in figuring
>> out the dependencies there...
>
> Yeah, I guess fill.el is a pretty far down in the list of things that
> loadup.el loads?
>
> However, we could make a super-simple function for filling doc
> strings -- doing something that's good enough for that particular task
> shouldn't take more than a few lines.

True.  Please find attached three patches:

1. The original patch adjusted with the heuristic proposed by Eli.

2. Avoid wide docstrings in define-minor-mode.

3. Fixing most wide docstring warnings.

With these three patches applied, around 22 wide docstring warnings
remain in our tree for defvar's.  I found it hard to fix them as I'm not
very familiar with the code in question, and I couldn't just simplify
the wording from the context.

I'm assuming we don't want to push this without fixing those warnings,
so I would really appreciate some help with the remaining couple of
warnings.

Should we perhaps try to get this applied before starting work on
defuns?  Or should I just push a branch for this work?

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

From b6e47cf7f9f2618a8509d7178125ce909914a75b 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/3] 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.

* 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-long-docstring-autoload\.el)
(bytecomp/warn-long-docstring-custom-declare-variable\.el)
(bytecomp/warn-long-docstring-defalias\.el)
(bytecomp/warn-long-docstring-defconst\.el)
(bytecomp/warn-long-docstring-define-abbrev-table\.el)
(bytecomp/warn-long-docstring-define-obsolete-function-alias\.el)
(bytecomp/warn-long-docstring-define-obsolete-variable-alias\.el)
(bytecomp/warn-long-docstring-defun\.el)
(bytecomp/warn-long-docstring-defvar\.el)
(bytecomp/warn-long-docstring-defvaralias\.el)
(bytecomp/warn-long-docstring-ignore-fill-column\.el)
(bytecomp/warn-long-docstring-ignore\.el): New tests.
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-abbrev-table.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-function-alias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-variable-alias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore-fill-column.el:
* test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore.el:
New files.
---
 etc/NEWS                                      |  9 ++-
 etc/TODO                                      |  2 -
 lisp/emacs-lisp/bytecomp.el                   | 76 ++++++++++++++++++-
 .../warn-long-docstring-autoload.el           |  3 +
 ...-long-docstring-custom-declare-variable.el |  4 +
 .../warn-long-docstring-defalias.el           |  3 +
 .../warn-long-docstring-defconst.el           |  3 +
 ...warn-long-docstring-define-abbrev-table.el |  3 +
 ...ocstring-define-obsolete-function-alias.el |  3 +
 ...ocstring-define-obsolete-variable-alias.el |  3 +
 .../warn-long-docstring-defun.el              |  3 +
 .../warn-long-docstring-defvar.el             |  3 +
 .../warn-long-docstring-defvaralias.el        |  3 +
 .../warn-long-docstring-ignore-fill-column.el |  7 ++
 .../warn-long-docstring-ignore.el             |  7 ++
 test/lisp/emacs-lisp/bytecomp-tests.el        | 59 ++++++++++++++
 16 files changed, 186 insertions(+), 5 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-abbrev-table.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-function-alias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-variable-alias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore-fill-column.el
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore.el

diff --git a/etc/NEWS b/etc/NEWS
index 83fe7a349e..4dbd15ee5b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2059,8 +2059,10 @@ 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
@@ -2070,6 +2072,11 @@ file can affect code in another.  For details, see the manual section
 ** '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'.  See 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 8e93e7fb10..38dfda424f 100644
--- a/etc/TODO
+++ b/etc/TODO
@@ -572,8 +572,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 0acd527697..c5f2c757dd 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -297,7 +297,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).
@@ -320,6 +321,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
@@ -1557,6 +1560,69 @@ 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'.")
+
+(defun byte-compile--wide-docstring-p (docstring col)
+  "Return t if string DOCSTRING is wider than COL.
+Ignore any `substitute-command-keys' substitutions."
+  (string-match
+   (format "^.\\{%s,\\}$" (int-to-string (1+ col)))
+   ;; Ignore these `substitute-command-keys' substitutions.
+   (replace-regexp-in-string
+    (rx "\\" (or "="
+                 (seq "<" (* (not ">")) ">")
+                 (seq "{" (* (not "}")) "}")))
+    ""
+    ;; Assume that these are of a given length.
+    (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
+  "Length that a doc string can be before the byte-compiler reports a warning."
+  :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 is 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))
@@ -2387,7 +2453,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)))
 
@@ -2415,6 +2482,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))
@@ -2438,6 +2506,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
@@ -2821,6 +2890,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))
@@ -4601,6 +4671,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))
@@ -4675,6 +4746,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-long-docstring-autoload.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
new file mode 100644
index 0000000000..96deb1bbb0
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(autoload 'foox "foo"
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-custom-declare-variable.el
new file mode 100644
index 0000000000..2a4700bfda
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-defalias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el
new file mode 100644
index 0000000000..a4235d22bd
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defalias.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defalias 'foo #'ignore
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defconst.el
new file mode 100644
index 0000000000..946f01989a
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-define-abbrev-table.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-abbrev-table.el
new file mode 100644
index 0000000000..3da9ccd48c
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-define-obsolete-function-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-function-alias.el
new file mode 100644
index 0000000000..fea841b12e
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-define-obsolete-variable-alias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-define-obsolete-variable-alias.el
new file mode 100644
index 0000000000..2d5f201cb6
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-defun.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el
new file mode 100644
index 0000000000..94b0e80c97
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defun.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el
new file mode 100644
index 0000000000..4bf8c17c3f
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvar.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defvar foo-bar nil
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-defvaralias.el
new file mode 100644
index 0000000000..52fdc17f5b
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-ignore-fill-column.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore-fill-column.el
new file mode 100644
index 0000000000..1ff554f370
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-long-docstring-ignore.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-ignore.el
new file mode 100644
index 0000000000..c80ddd180d
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-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-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 8fa4d278f1..caa9ab8f96 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,55 @@ "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-long-docstring-autoload.el"
+ "autoload.*foox.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-custom-declare-variable.el"
+ "custom-declare-variable.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defalias.el"
+ "defalias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defconst.el"
+ "defconst.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-define-abbrev-table.el"
+ "define-abbrev.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-define-obsolete-function-alias.el"
+ "defalias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-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-long-docstring-defun.el"
+ "wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defvar.el"
+ "defvar.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-defvaralias.el"
+ "defvaralias.*foo.*wider than.*characters")
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-ignore-fill-column.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
+(bytecomp--define-warning-file-test
+ "warn-long-docstring-ignore.el"
+ "defvar.*foo.*wider than.*characters" 'reverse)
+
 \f
 ;;;; Macro expansion.
 
-- 
2.29.2


[-- Attachment #3: 0002-Avoid-wide-doc-strings-in-define-minor-mode.patch --]
[-- Type: text/x-diff, Size: 1910 bytes --]

From e0a27cba79ad8e2713775c4216943138a2d70267 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Mon, 7 Dec 2020 09:49:13 +0100
Subject: [PATCH 2/3] Avoid wide doc strings in define-minor-mode

* lisp/emacs-lisp/easy-mmode.el (define-minor-mode): Avoid producing
long documentation strings.
---
 lisp/emacs-lisp/easy-mmode.el | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 261f2508af..60ea435e8a 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -115,6 +115,16 @@ easy-mmode--mode-docstring
                                   (concat filled "\\1")
                                   doc nil nil 1)))))
 
+(defun easy-mmode--fill-string (str)
+  "Fill string STR to `fill-column' to avoid generating wide docstrings."
+  (if (< (length 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 lst))))
+
 ;;;###autoload
 (defalias 'easy-mmode-define-minor-mode 'define-minor-mode)
 ;;;###autoload
@@ -278,8 +288,11 @@ 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)
+                        (easy-mmode--fill-string
+                         (format "Use the command `%s' to change this variable."
+                                 mode))))
              (make-variable-buffer-local ',mode)))
          (t
 	  (let ((base-doc-string
-- 
2.29.2


[-- Attachment #4: 0003-Adjust-some-docstrings-to-fit-80-columns.patch --]
[-- Type: text/x-diff, Size: 61276 bytes --]

From 3e74fcc66a623d290695ada2795d1f9a0102cbd1 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 10 Dec 2020 20:12:58 +0100
Subject: [PATCH 3/3] ; Adjust some docstrings to fit 80 columns

---
 lisp/calc/calc.el                  |  9 +++++----
 lisp/cedet/ede/generic.el          |  4 ++--
 lisp/cedet/semantic/analyze.el     |  6 ++++--
 lisp/cedet/semantic/bovine/c.el    |  3 ++-
 lisp/cedet/semantic/bovine/make.el |  4 +++-
 lisp/cedet/semantic/imenu.el       |  2 +-
 lisp/cedet/semantic/util-modes.el  |  3 ++-
 lisp/emacs-lisp/advice.el          |  5 +++--
 lisp/emacs-lisp/bytecomp.el        | 16 ++++++++++------
 lisp/emacs-lisp/cl-generic.el      |  4 ++--
 lisp/emacs-lisp/elint.el           |  2 +-
 lisp/emacs-lisp/find-func.el       |  2 +-
 lisp/emacs-lisp/seq.el             |  7 ++++---
 lisp/emulation/cua-base.el         | 10 +++++-----
 lisp/emulation/viper-init.el       |  5 +++--
 lisp/emulation/viper-keym.el       |  2 +-
 lisp/erc/erc-dcc.el                |  3 ++-
 lisp/expand.el                     |  2 +-
 lisp/font-lock.el                  |  4 ++--
 lisp/gnus/gnus-art.el              |  4 +++-
 lisp/gnus/gnus-group.el            |  2 +-
 lisp/gnus/gnus-score.el            |  2 +-
 lisp/gnus/gnus-sum.el              |  9 +++++----
 lisp/gnus/gnus-uu.el               | 12 ++++++------
 lisp/gnus/gnus.el                  |  7 ++++---
 lisp/gnus/message.el               |  6 ++++--
 lisp/gnus/nnmail.el                |  5 +++--
 lisp/gnus/nnmh.el                  |  2 +-
 lisp/gnus/nntp.el                  |  4 +++-
 lisp/ido.el                        |  2 +-
 lisp/international/mule-cmds.el    |  2 +-
 lisp/language/ethio-util.el        |  6 ++++--
 lisp/mail/feedmail.el              | 18 ++++++++++--------
 lisp/mh-e/mh-e.el                  |  2 +-
 lisp/net/imap.el                   |  2 +-
 lisp/net/rcirc.el                  |  2 +-
 lisp/net/tramp-sh.el               |  3 +--
 lisp/net/tramp.el                  |  4 ++--
 lisp/nxml/nxml-mode.el             |  6 ++++--
 lisp/obsolete/landmark.el          |  3 ++-
 lisp/obsolete/tls.el               |  6 ++++--
 lisp/org/ob-R.el                   |  2 +-
 lisp/org/org-agenda.el             |  3 ++-
 lisp/org/org-attach.el             |  2 +-
 lisp/org/org-indent.el             |  6 ++----
 lisp/org/org-protocol.el           |  6 ++++--
 lisp/org/org.el                    |  3 ++-
 lisp/progmodes/cc-vars.el          |  3 ++-
 lisp/progmodes/cperl-mode.el       |  5 ++++-
 lisp/progmodes/elisp-mode.el       |  3 ++-
 lisp/progmodes/flymake-proc.el     |  6 ++++--
 lisp/progmodes/hideif.el           |  4 ++--
 lisp/progmodes/ruby-mode.el        |  2 +-
 lisp/progmodes/verilog-mode.el     |  3 ++-
 lisp/ses.el                        |  3 ++-
 lisp/simple.el                     |  3 ++-
 lisp/tab-bar.el                    |  6 ++++--
 lisp/term.el                       |  4 ++--
 lisp/textmodes/reftex-vars.el      | 10 ++++++----
 lisp/textmodes/table.el            |  8 +++++---
 lisp/vc/ediff-diff.el              |  2 +-
 lisp/vc/ediff-init.el              |  6 +++---
 lisp/vc/ediff-merg.el              |  2 +-
 lisp/vc/ediff-mult.el              |  2 +-
 lisp/vc/ediff-wind.el              |  2 +-
 lisp/vc/smerge-mode.el             |  2 +-
 lisp/vc/vc.el                      |  5 +++--
 lisp/view.el                       |  3 ++-
 68 files changed, 180 insertions(+), 128 deletions(-)

diff --git a/lisp/calc/calc.el b/lisp/calc/calc.el
index bb02281111..5805ecdb7f 100644
--- a/lisp/calc/calc.el
+++ b/lisp/calc/calc.el
@@ -266,18 +266,18 @@ calc-embedded-announce-formula-alist
     (sgml-mode    . "<!-- Embed -->\n\\(<!-- .* -->\n\\)*")
     (xml-mode     . "<!-- Embed -->\n\\(<!-- .* -->\n\\)*")
     (texinfo-mode . "@c Embed\n\\(@c .*\n\\)*"))
-  "Alist of major modes with appropriate values for `calc-embedded-announce-formula'."
+  "Alist of major modes for `calc-embedded-announce-formula'."
   :type '(alist :key-type (symbol :tag "Major mode")
                 :value-type (regexp :tag "Regexp to announce formula")))
 
 (defcustom calc-embedded-open-formula
   "\\`\\|^\n\\|\\$\\$?\\|\\\\\\[\\|^\\\\begin[^{].*\n\\|^\\\\begin{.*[^x]}.*\n\\|^@.*\n\\|^\\.EQ.*\n\\|\\\\(\\|^%\n\\|^\\.\\\\\"\n"
-  "A regular expression for the opening delimiter of a formula used by calc-embedded."
+  "Regexp for opening delimiter of a formula used by `calc-embedded'."
   :type '(regexp))
 
 (defcustom calc-embedded-close-formula
   "\\'\\|\n$\\|\\$\\$?\\|\\\\]\\|^\\\\end[^{].*\n\\|^\\\\end{.*[^x]}.*\n\\|^@.*\n\\|^\\.EN.*\n\\|\\\\)\\|\n%\n\\|^\\.\\\\\"\n"
-  "A regular expression for the closing delimiter of a formula used by calc-embedded."
+  "Regexp for the closing delimiter of a formula used by calc-embedded."
   :type '(regexp))
 
 (defcustom calc-embedded-open-close-formula-alist
@@ -721,7 +721,8 @@ calc-symbolic-mode
 (defcalcmodevar calc-matrix-mode nil
   "If `matrix', variables are assumed to be matrix-valued.
 If a number, variables are assumed to be NxN matrices.
-If `sqmatrix', variables are assumed to be square matrices of an unspecified size.
+If `sqmatrix', variables are assumed to be square matrices of an
+  unspecified size.
 If `scalar', variables are assumed to be scalar-valued.
 If nil, symbolic math routines make no assumptions about variables.")
 
diff --git a/lisp/cedet/ede/generic.el b/lisp/cedet/ede/generic.el
index b9805f6fac..0f202ddfc8 100644
--- a/lisp/cedet/ede/generic.el
+++ b/lisp/cedet/ede/generic.el
@@ -258,8 +258,8 @@ ede-generic-new-autoloader
 INTERNAL-NAME is obsolete and ignored.
 EXTERNAL-NAME is a human readable name to describe the project; it
 must be unique among all autoloaded projects.
-PROJECTFILE is a file name that identifies a project of this type to EDE, such as
-a Makefile, or SConstruct file.
+PROJECTFILE is a file name that identifies a project of this type to EDE, such
+as a Makefile, or SConstruct file.
 CLASS is the EIEIO class that is used to track this project.  It should subclass
 `ede-generic-project'."
   (ede-add-project-autoload
diff --git a/lisp/cedet/semantic/analyze.el b/lisp/cedet/semantic/analyze.el
index cafdc3bee1..f2d2279c00 100644
--- a/lisp/cedet/semantic/analyze.el
+++ b/lisp/cedet/semantic/analyze.el
@@ -235,7 +235,8 @@ semantic-analyze-find-tag-sequence
 which doesn't need to be dereferenced.
 Optional argument TYPERETURN is a symbol in which the types of all found
 will be stored.  If nil, that data is thrown away.
-Optional argument THROWSYM specifies a symbol the throw on non-recoverable error.
+Optional argument THROWSYM specifies a symbol the throw on non-recoverable
+error.
 Remaining arguments FLAGS are additional flags to apply when searching.")
 
 (defun semantic-analyze-find-tag-sequence-default
@@ -246,7 +247,8 @@ semantic-analyze-find-tag-sequence-default
 SCOPE are extra tags which are in scope.
 TYPERETURN is a symbol in which to place a list of tag classes that
 are found in SEQUENCE.
-Optional argument THROWSYM specifies a symbol the throw on non-recoverable error.
+Optional argument THROWSYM specifies a symbol the throw on non-recoverable
+error.
 Remaining arguments FLAGS are additional flags to apply when searching.
 This function knows of flags:
   `mustbeclassvariable'"
diff --git a/lisp/cedet/semantic/bovine/c.el b/lisp/cedet/semantic/bovine/c.el
index 3649d1c2f1..7f0c16136c 100644
--- a/lisp/cedet/semantic/bovine/c.el
+++ b/lisp/cedet/semantic/bovine/c.el
@@ -368,7 +368,8 @@ semantic-c-convert-spp-value-to-hideif-value
 
 (defun semantic-c-evaluate-symbol-for-hideif (spp-symbol)
   "Lookup the symbol SPP-SYMBOL (a string) to something hideif can use.
-Pulls out the symbol list, and call `semantic-c-convert-spp-value-to-hideif-value'."
+Pull out the symbol list, and call
+`semantic-c-convert-spp-value-to-hideif-value'."
   (interactive "sSymbol name: ")
   (when (symbolp spp-symbol) (setq spp-symbol (symbol-name spp-symbol)))
 
diff --git a/lisp/cedet/semantic/bovine/make.el b/lisp/cedet/semantic/bovine/make.el
index 07c55b46e2..1f15669140 100644
--- a/lisp/cedet/semantic/bovine/make.el
+++ b/lisp/cedet/semantic/bovine/make.el
@@ -50,7 +50,9 @@ semantic-lex-make-backslash-no-newline
    nil)
 
 (define-lex-regex-analyzer semantic-lex-make-command
-  "A command in a Makefile consists of a line starting with TAB, and ending at the newline."
+  "Regexp for a command in a Makefile.
+It consists of a line starting with TAB, and ending at the
+newline."
   "^\\(\t\\)"
   (let ((start (match-end 0)))
     (while (progn (end-of-line)
diff --git a/lisp/cedet/semantic/imenu.el b/lisp/cedet/semantic/imenu.el
index 25f7fdb842..ebb8ac0cd8 100644
--- a/lisp/cedet/semantic/imenu.el
+++ b/lisp/cedet/semantic/imenu.el
@@ -99,7 +99,7 @@ semantic-imenu-bucketize-type-members
 
 (defcustom semantic-imenu-sort-bucket-function nil
   "Function to use when sorting tags in the buckets of functions.
-See `semantic-bucketize' and the FILTER argument for more details on this function."
+See `semantic-bucketize' and the FILTER argument for more details."
   :group 'semantic-imenu
   :type '(radio (const :tag "No Sorting" nil)
 		(const semantic-sort-tags-by-name-increasing)
diff --git a/lisp/cedet/semantic/util-modes.el b/lisp/cedet/semantic/util-modes.el
index 776c6b1894..8bfee432c3 100644
--- a/lisp/cedet/semantic/util-modes.el
+++ b/lisp/cedet/semantic/util-modes.el
@@ -837,7 +837,8 @@ semantic-highlight-func-mode-map
   "Keymap for highlight-func minor mode.")
 
 (defvar semantic-highlight-func-popup-menu nil
-  "Menu used if the user clicks on the header line used by `semantic-highlight-func-mode'.")
+  "Menu used if the user clicks on the header line.
+Used by `semantic-highlight-func-mode'.")
 
 (easy-menu-define
   semantic-highlight-func-popup-menu
diff --git a/lisp/emacs-lisp/advice.el b/lisp/emacs-lisp/advice.el
index c8a6676b66..caa436ce23 100644
--- a/lisp/emacs-lisp/advice.el
+++ b/lisp/emacs-lisp/advice.el
@@ -2405,8 +2405,9 @@ ad-map-arglists
 Excess source arguments will be neglected, missing source arguments will be
 supplied as nil.  Returns a `funcall' or `apply' form with the second element
 being `function' which has to be replaced by an actual function argument.
-Example: (ad-map-arglists \\='(a &rest args) \\='(w x y z)) will return
-         (funcall ad--addoit-function a (car args) (car (cdr args)) (nth 2 args))."
+Example:
+   (ad-map-arglists \\='(a &rest args) \\='(w x y z)) will return
+   (funcall ad--addoit-function a (car args) (car (cdr args)) (nth 2 args))."
   (let* ((parsed-source-arglist (ad-parse-arglist source-arglist))
 	 (source-reqopt-args (append (nth 0 parsed-source-arglist)
 				     (nth 1 parsed-source-arglist)))
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index c5f2c757dd..ebb18f9e5c 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -708,7 +708,8 @@ 127
 
 ;; These store their argument in the next two bytes
 (byte-defop 129  1 byte-constant2
-   "for reference to a constant with vector index >= byte-constant-limit")
+   "for reference to a constant with vector
+index >= byte-constant-limit")
 (byte-defop 130  0 byte-goto "for unconditional jump")
 (byte-defop 131 -1 byte-goto-if-nil "to pop value and jump if it's nil")
 (byte-defop 132 -1 byte-goto-if-not-nil "to pop value and jump if it's not nil")
@@ -728,11 +729,14 @@ 138
 (byte-defop 139  0 byte-save-window-excursion-OBSOLETE
   "to make a binding to record entire window configuration")
 (byte-defop 140  0 byte-save-restriction
-  "to make a binding to record the current buffer clipping restrictions")
+  "to make a binding to record the current buffer clipping
+restrictions")
 (byte-defop 141 -1 byte-catch-OBSOLETE   ; Not generated since Emacs 25.
-  "for catch.  Takes, on stack, the tag and an expression for the body")
+  "for catch.  Takes, on stack, the tag and an expression for
+the body")
 (byte-defop 142 -1 byte-unwind-protect
-  "for unwind-protect.  Takes, on stack, an expression for the unwind-action")
+  "for unwind-protect.  Takes, on stack, an expression for
+the unwind-action")
 
 ;; For condition-case.  Takes, on stack, the variable to bind,
 ;; an expression for the body, and a list of clauses.
@@ -792,8 +796,8 @@ 182
 (defconst byte-discardN-preserve-tos byte-discardN)
 
 (byte-defop 183 -2 byte-switch
- "to take a hash table and a value from the stack, and jump to the address
-the value maps to, if any.")
+ "to take a hash table and a value from the stack, and jump to
+the address the value maps to, if any.")
 
 ;; unused: 182-191
 
diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el
index b37b05b9a3..9ddf9e7333 100644
--- a/lisp/emacs-lisp/cl-generic.el
+++ b/lisp/emacs-lisp/cl-generic.el
@@ -811,8 +811,8 @@ cl-generic-combine-methods
 GENERIC is the generic function (mostly used for its name).
 METHODS is the list of the selected methods.
 The METHODS list is sorted from most specific first to most generic last.
-The function can use `cl-generic-call-method' to create functions that call those
-methods.")
+The function can use `cl-generic-call-method' to create functions that call
+those methods.")
 
 (unless (ignore-errors (cl-generic-generalizers t))
   ;; Temporary definition to let the next defmethod succeed.
diff --git a/lisp/emacs-lisp/elint.el b/lisp/emacs-lisp/elint.el
index ef97c8279d..b143d3ff39 100644
--- a/lisp/emacs-lisp/elint.el
+++ b/lisp/emacs-lisp/elint.el
@@ -521,7 +521,7 @@ elint-top-form
   "The currently linted top form, or nil.")
 
 (defvar elint-top-form-logged nil
-  "The value t if the currently linted top form has been mentioned in the log buffer.")
+  "Non-nil t if the currently linted top form has been mentioned in the log buffer.")
 
 (defun elint-top-form (form)
   "Lint a top FORM."
diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el
index ee94e1fbff..074e7db295 100644
--- a/lisp/emacs-lisp/find-func.el
+++ b/lisp/emacs-lisp/find-func.el
@@ -103,7 +103,7 @@ find-face-regexp
 
 (defcustom find-feature-regexp
   (concat ";;; Code:")
-  "The regexp used by `xref-find-definitions' when searching for a feature definition.
+  "Regexp used by `xref-find-definitions' when searching for a feature definition.
 Note it may contain up to one `%s' at the place where `format'
 should insert the feature name."
   ;; We search for ";;; Code" rather than (feature '%s) because the
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4656277ea1..d91a33c140 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -317,7 +317,7 @@ seq-into
 
 ;;;###autoload
 (cl-defgeneric seq-filter (pred sequence)
-  "Return a list of all the elements for which (PRED element) is non-nil in SEQUENCE."
+  "Return a list of all elements for which (PRED element) is non-nil in SEQUENCE."
   (let ((exclude (make-symbol "exclude")))
     (delq exclude (seq-map (lambda (elt)
                              (if (funcall pred elt)
@@ -411,7 +411,8 @@ seq-contains-p
       nil))
 
 (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn)
-  "Return non-nil if SEQUENCE1 and SEQUENCE2 contain the same elements, regardless of order.
+  "Return non-nil if SEQUENCE1 and SEQUENCE2 contain the same elements.
+This does not depend on the order of the elements.
 Equality is defined by TESTFN if non-nil or by `equal' if nil."
   (and (seq-every-p (lambda (item1) (seq-contains-p sequence2 item1 testfn)) sequence1)
        (seq-every-p (lambda (item2) (seq-contains-p sequence1 item2 testfn)) sequence2)))
@@ -444,7 +445,7 @@ seq-mapcat
          (seq-map function sequence)))
 
 (cl-defgeneric seq-partition (sequence n)
-  "Return a list of the elements of SEQUENCE grouped into sub-sequences of length N.
+  "Return list of elements of SEQUENCE grouped into sub-sequences of length N.
 The last sequence may contain less than N elements.  If N is a
 negative integer or 0, nil is returned."
   (unless (< n 1)
diff --git a/lisp/emulation/cua-base.el b/lisp/emulation/cua-base.el
index 926305e607..55578d0622 100644
--- a/lisp/emulation/cua-base.el
+++ b/lisp/emulation/cua-base.el
@@ -375,11 +375,11 @@ cua-check-pending-input
 
 (defcustom cua-paste-pop-rotate-temporarily nil
   "If non-nil, \\[cua-paste-pop] only rotates the kill-ring temporarily.
-This means that both \\[yank] and the first \\[yank-pop] in a sequence always insert
-the most recently killed text.  Each immediately following \\[cua-paste-pop] replaces
-the previous text with the next older element on the `kill-ring'.
-With prefix arg, \\[universal-argument] \\[yank-pop] inserts the same text as the most
-recent \\[yank-pop] (or \\[yank]) command."
+This means that both \\[yank] and the first \\[yank-pop] in a sequence always
+insert the most recently killed text.  Each immediately following \\[cua-paste-pop]
+replaces the previous text with the next older element on the `kill-ring'.
+With prefix arg, \\[universal-argument] \\[yank-pop] inserts the same text as the
+most recent \\[yank-pop] (or \\[yank]) command."
   :type 'boolean
   :group 'cua)
 
diff --git a/lisp/emulation/viper-init.el b/lisp/emulation/viper-init.el
index 6c4afe519f..c2aae9b87f 100644
--- a/lisp/emulation/viper-init.el
+++ b/lisp/emulation/viper-init.el
@@ -475,7 +475,8 @@ viper-temp-command-ring
 
 ;; Fast keyseq and ESC keyseq timeouts
 (defcustom viper-fast-keyseq-timeout 200
-  "Key sequence separated by no more than this many milliseconds is viewed as a Vi-style macro, if such a macro is defined.
+  "Max milliseconds for a key sequence to be regarded as a Vi-style macro.
+Only regard key sequence as a macro if it is defined.
 Setting this too high may slow down your typing.  Setting this value too low
 will make it hard to use Vi-style timeout macros."
   :type 'integer
@@ -705,7 +706,7 @@ viper-search-wrap-around
 
 (viper-deflocalvar viper-related-files-and-buffers-ring nil "")
 (defcustom viper-related-files-and-buffers-ring nil
-  "List of file and buffer names that are considered to be related to the current buffer.
+  "List of file and buffer names to consider related to the current buffer.
 Related buffers can be cycled through via :R and :P commands."
   :type 'boolean
   :group 'viper-misc)
diff --git a/lisp/emulation/viper-keym.el b/lisp/emulation/viper-keym.el
index d76cf71b31..6a0fc2e984 100644
--- a/lisp/emulation/viper-keym.el
+++ b/lisp/emulation/viper-keym.el
@@ -69,7 +69,7 @@ viper-no-multiple-ESC
   :group 'viper)
 
 (defcustom viper-want-ctl-h-help nil
-  "If non-nil, C-h gets bound to help-command; otherwise, C-h gets the usual Vi bindings."
+  "If non-nil, bind C-h to help-command; otherwise, C-h gets the usual Vi bindings."
   :type 'boolean
   :group 'viper)
 
diff --git a/lisp/erc/erc-dcc.el b/lisp/erc/erc-dcc.el
index 1bce986a80..6e44001259 100644
--- a/lisp/erc/erc-dcc.el
+++ b/lisp/erc/erc-dcc.el
@@ -83,7 +83,8 @@ erc-dcc-connection-types
 
 (defvar erc-dcc-list nil
   "List of DCC connections. Looks like:
-  ((:nick \"nick!user@host\" :type GET :peer proc :parent proc :size size :file file)
+  ((:nick \"nick!user@host\" :type GET :peer proc :parent proc
+    :size size :file file)
    (:nick \"nick!user@host\" :type CHAT :peer proc :parent proc)
    (:nick \"nick\" :type SEND :peer server-proc :parent parent-proc :file
    file :sent <marker> :confirmed <marker>))
diff --git a/lisp/expand.el b/lisp/expand.el
index 77e4fc2657..c4e1d22790 100644
--- a/lisp/expand.el
+++ b/lisp/expand.el
@@ -290,7 +290,7 @@ expand-add-abbrevs
 (defvar expand-list nil "Temporary variable used by the Expand package.")
 
 (defvar expand-pos nil
-  "If non-nil, stores a vector containing markers to positions defined by the last expansion.")
+  "If non-nil, store a vector with position markers defined by the last expansion.")
 (make-variable-buffer-local 'expand-pos)
 
 (defvar expand-index 0
diff --git a/lisp/font-lock.el b/lisp/font-lock.el
index 0e771e8e0a..a2cf71f946 100644
--- a/lisp/font-lock.el
+++ b/lisp/font-lock.el
@@ -2280,8 +2280,8 @@ cpp-font-lock-keywords-source-directives
 ;;			 "ifndef" "import" "include" "line" "pragma" "undef" "warning")))
 ;;
 (defconst cpp-font-lock-keywords-source-depth 0
-  "An integer representing regular expression depth of `cpp-font-lock-keywords-source-directives'.
-Used in `cpp-font-lock-keywords'.")
+  "Regular expression depth of `cpp-font-lock-keywords-source-directives'.
+This should be an integer.  Used in `cpp-font-lock-keywords'.")
 
 (defconst cpp-font-lock-keywords
   (let* ((directives cpp-font-lock-keywords-source-directives)
diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index 5b50bcbbe1..79d4d9087f 100644
--- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -289,7 +289,9 @@ gnus-article-x-face-too-ugly
 (defcustom gnus-article-banner-alist nil
   "Banner alist for stripping.
 For example,
-     ((egroups . \"^[ \\t\\n]*-------------------+\\\\( \\\\(e\\\\|Yahoo! \\\\)Groups Sponsor -+\\\\)?....\\n\\\\(.+\\n\\\\)+\"))"
+     ((egroups . (concat \"^[ \\t\\n]*-------------------+\\\\\"
+                         \"( \\\\(e\\\\|Yahoo! \\\\)Groups Sponsor -+\\\\)?\"
+                         \"....\\n\\\\(.+\\n\\\\)+\")))"
   :version "21.1"
   :type '(repeat (cons symbol regexp))
   :group 'gnus-article-washing)
diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index 24534a1b66..9bb3ec765f 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -60,7 +60,7 @@ gnus-no-groups-message
   :type 'string)
 
 (defcustom gnus-keep-same-level nil
-  "Non-nil means that the next newsgroup after the current will be on the same level.
+  "Non-nil means that the newsgroup after this one will be on the same level.
 When you type, for instance, `n' after reading the last article in the
 current newsgroup, you will go to the next newsgroup.  If this variable
 is nil, the next newsgroup will be the next from the group
diff --git a/lisp/gnus/gnus-score.el b/lisp/gnus/gnus-score.el
index 94f2cc310f..33c5803f5a 100644
--- a/lisp/gnus/gnus-score.el
+++ b/lisp/gnus/gnus-score.el
@@ -248,7 +248,7 @@ gnus-default-adaptive-score-alist
 				     (integer :tag "Score"))))))
 
 (defcustom gnus-adaptive-word-length-limit nil
-  "Words of a length lesser than this limit will be ignored when doing adaptive scoring."
+  "Words shorter than this limit will be ignored when doing adaptive scoring."
   :version "22.1"
   :group 'gnus-score-adapt
   :type '(radio (const :format "Unlimited " nil)
diff --git a/lisp/gnus/gnus-sum.el b/lisp/gnus/gnus-sum.el
index 9432eefcb4..3b11a1a8a4 100644
--- a/lisp/gnus/gnus-sum.el
+++ b/lisp/gnus/gnus-sum.el
@@ -744,7 +744,8 @@ gnus-auto-expirable-marks
   :type '(repeat character))
 
 (defcustom gnus-inhibit-user-auto-expire t
-  "If non-nil, user marking commands will not mark an article as expirable, even if the group has auto-expire turned on."
+  "If non-nil, user marking commands will not mark an article as expirable.
+This is true even if the group has auto-expire turned on."
   :version "21.1"
   :group 'gnus-summary
   :type 'boolean)
@@ -1399,7 +1400,7 @@ gnus-newsgroup-process-stack
 (defvar gnus-thread-indent-array nil)
 (defvar gnus-thread-indent-array-level gnus-thread-indent-level)
 (defvar gnus-sort-gathered-threads-function #'gnus-thread-sort-by-number
-  "Function called to sort the articles within a thread after it has been gathered together.")
+  "Function to sort articles within a thread after it has been gathered together.")
 
 (defvar gnus-summary-save-parts-type-history nil)
 (defvar gnus-summary-save-parts-last-directory mm-default-directory)
@@ -1525,7 +1526,7 @@ gnus-last-shell-command
   "Default shell command on article.")
 
 (defvar gnus-newsgroup-agentized nil
-  "Locally bound in each summary buffer to indicate whether the server has been agentized.")
+  "Locally bound in each summary buffer to indicate if server has been agentized.")
 (defvar gnus-newsgroup-begin nil)
 (defvar gnus-newsgroup-end nil)
 (defvar gnus-newsgroup-last-rmail nil)
@@ -1555,7 +1556,7 @@ gnus-newsgroup-reads
 (defvar gnus-newsgroup-expunged-tally nil)
 
 (defvar gnus-newsgroup-marked nil
-  "Sorted list of ticked articles in the current newsgroup (a subset of unread art).")
+  "Sorted list of ticked articles in current newsgroup (a subset of unread art).")
 
 (defvar gnus-newsgroup-spam-marked nil
   "List of ranges of articles that have been marked as spam.")
diff --git a/lisp/gnus/gnus-uu.el b/lisp/gnus/gnus-uu.el
index 70aeac00d7..5980051ee4 100644
--- a/lisp/gnus/gnus-uu.el
+++ b/lisp/gnus/gnus-uu.el
@@ -162,7 +162,7 @@ gnus-uu-ignore-files-by-name
 		 (regexp :format "%v")))
 
 (defcustom gnus-uu-ignore-files-by-type nil
-  "A regular expression saying what files that shouldn't be viewed, based on MIME file type.
+  "Regexp matching files that shouldn't be viewed, based on MIME file type.
 If, for instance, you want gnus-uu to ignore all audio files and all mpegs,
 you could say something like
 
@@ -224,7 +224,7 @@ gnus-uu-tmp-dir
   :type 'directory)
 
 (defcustom gnus-uu-do-not-unpack-archives nil
-  "Non-nil means that gnus-uu won't peek inside archives looking for files to display.
+  "If non-nil, gnus-uu won't peek inside archives looking for files to display.
 Default is nil."
   :group 'gnus-extract-archive
   :type 'boolean)
@@ -265,19 +265,19 @@ gnus-uu-view-with-metamail
   :type 'boolean)
 
 (defcustom gnus-uu-unmark-articles-not-decoded nil
-  "Non-nil means that gnus-uu will mark articles that were unsuccessfully decoded as unread.
+  "If non-nil, gnus-uu will mark unsuccessfully decoded articles as unread.
 Default is nil."
   :group 'gnus-extract
   :type 'boolean)
 
 (defcustom gnus-uu-correct-stripped-uucode nil
-  "Non-nil means that gnus-uu will *try* to fix uuencoded files that have had trailing spaces deleted.
+  "If non-nil, *try* to fix uuencoded files that have had trailing spaces deleted.
 Default is nil."
   :group 'gnus-extract
   :type 'boolean)
 
 (defcustom gnus-uu-save-in-digest nil
-  "Non-nil means that gnus-uu, when asked to save without decoding, will save in digests.
+  "If non-nil, gnus-uu, when asked to save without decoding, will save in digests.
 If this variable is nil, gnus-uu will just save everything in a
 file without any embellishments.  The digesting almost conforms to RFC1153 -
 no easy way to specify any meaningful volume and issue numbers were found,
@@ -1858,7 +1858,7 @@ gnus-uu-post-encode-method
 		(function :tag "Other")))
 
 (defcustom gnus-uu-post-include-before-composing nil
-  "Non-nil means that gnus-uu will ask for a file to encode before you compose the article.
+  "If non-nil, gnus-uu asks for a file to encode before you compose the article.
 If this variable is t, you can either include an encoded file with
 \\[gnus-uu-post-insert-binary-in-article] or have one included for you when you post the article."
   :group 'gnus-extract-post
diff --git a/lisp/gnus/gnus.el b/lisp/gnus/gnus.el
index abe7b1ae76..04adaaba39 100644
--- a/lisp/gnus/gnus.el
+++ b/lisp/gnus/gnus.el
@@ -1195,7 +1195,7 @@ gnus-large-newsgroup
 		 integer))
 
 (defcustom gnus-use-long-file-name (not (memq system-type '(usg-unix-v)))
-  "Non-nil means that the default name of a file to save articles in is the group name.
+  "Non-nil means that the default file name to save articles in is the group name.
 If it's nil, the directory form of the group name is used instead.
 
 If this variable is a list, and the list contains the element
@@ -1545,7 +1545,7 @@ 'gnus-email-address
    ("\\(^\\|:\\)soc.culture.vietnamese\\>" vietnamese-viqr)
    ("\\(^\\|:\\)\\(comp\\|rec\\|alt\\|sci\\|soc\\|news\\|gnu\\|bofh\\)\\>" iso-8859-1))
  :variable-document
- "Alist of regexps (to match group names) and default charsets to be used when reading."
+ "Alist of regexps (to match group names) and default charsets to use when reading."
  :variable-group gnus-charset
  :variable-type '(repeat (list (regexp :tag "Group")
 			       (symbol :tag "Charset")))
@@ -1618,7 +1618,8 @@ 'gnus-email-address
 ;; group parameters for spam processing added by Ted Zlatanov <tzz@lifelogs.com>
 (defcustom gnus-install-group-spam-parameters t
   "Disable the group parameters for spam detection.
-Enable if `G c' in XEmacs is giving you trouble, and make sure to submit a bug report."
+Enable if `G c' in XEmacs is giving you trouble, and make sure to
+submit a bug report."
   :version "22.1"
   :type 'boolean
   :group 'gnus-start)
diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index b6c1c0b071..86800f28cc 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -1172,7 +1172,8 @@ message-cite-style
 message-cite-style-* variables.  This variable is intended for
 use in `gnus-posting-styles', such as:
 
-  ((posting-from-work-p) (eval (setq-local message-cite-style message-cite-style-outlook)))"
+  ((posting-from-work-p) (eval (setq-local message-cite-style
+                                           message-cite-style-outlook)))"
   :version "24.1"
   :group 'message-insertion
   :type '(choice (const :tag "Do not override variables" :value nil)
@@ -1199,7 +1200,8 @@ message-cite-style-thunderbird
     (message-yank-cited-prefix  ">")
     (message-yank-empty-prefix  ">")
     (message-citation-line-format "On %D %R %p, %N wrote:"))
-  "Message citation style used by Mozilla Thunderbird.  Use with `message-cite-style'.")
+  "Message citation style used by Mozilla Thunderbird.
+Use with `message-cite-style'.")
 
 (defconst message-cite-style-gmail
   '((message-cite-function  'message-cite-original)
diff --git a/lisp/gnus/nnmail.el b/lisp/gnus/nnmail.el
index 57801d6f9e..6ee29a25cd 100644
--- a/lisp/gnus/nnmail.el
+++ b/lisp/gnus/nnmail.el
@@ -115,7 +115,7 @@ nnmail-crosspost
   :type 'boolean)
 
 (defcustom nnmail-split-fancy-with-parent-ignore-groups nil
-  "Regexp that matches group names to be ignored when applying `nnmail-split-fancy-with-parent'.
+  "Regexp matching group names ignored by `nnmail-split-fancy-with-parent'.
 This can also be a list of regexps."
   :version "22.1"
   :group 'nnmail-split
@@ -124,7 +124,8 @@ nnmail-split-fancy-with-parent-ignore-groups
 		 (repeat :value (".*") regexp)))
 
 (defcustom nnmail-cache-ignore-groups nil
-  "Regexp that matches group names to be ignored when inserting message ids into the cache (`nnmail-cache-insert').
+  "Regexp matching group ignored when inserting message ids into the cache.
+This is used by `nnmail-cache-insert'.
 This can also be a list of regexps."
   :version "22.1"
   :group 'nnmail-split
diff --git a/lisp/gnus/nnmh.el b/lisp/gnus/nnmh.el
index 581a408009..5584dad45f 100644
--- a/lisp/gnus/nnmh.el
+++ b/lisp/gnus/nnmh.el
@@ -46,7 +46,7 @@ nnmh-prepare-save-mail-hook
   "Hook run narrowed to an article before saving.")
 
 (defvoo nnmh-be-safe nil
-  "If non-nil, nnmh will check all articles to make sure whether they are new or not.
+  "If non-nil, nnmh will check all articles to make sure if they are new or not.
 Go through the .nnmh-articles file and compare with the actual
 articles in this folder.  The articles that are \"new\" will be marked
 as unread by Gnus.")
diff --git a/lisp/gnus/nntp.el b/lisp/gnus/nntp.el
index a5c8244792..7d73610267 100644
--- a/lisp/gnus/nntp.el
+++ b/lisp/gnus/nntp.el
@@ -1751,7 +1751,9 @@ nntp-wait-for-string
 ;; ==========================================================================
 
 (defvoo nntp-open-telnet-envuser nil
-  "If non-nil, telnet session (client and server both) will support the ENVIRON option and not prompt for login name.")
+  "If non-nil, telnet session supports the ENVIRON option.
+Don't prompt for login name.  This applies to both client and
+server.")
 
 (defvoo nntp-telnet-shell-prompt "bash\\|[$>] *\r?$"
   "Regular expression to match the shell prompt on the remote machine.")
diff --git a/lisp/ido.el b/lisp/ido.el
index 5758d3fdea..89c70786d2 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -842,7 +842,7 @@ ido-rewrite-file-prompt-functions
   max-width - the max width of the resulting dirname; nil means no limit
   prompt    - the basic prompt (e.g. \"Find File: \")
   literal   - the string shown if doing \"literal\" find; set to nil to omit
-  vc-off    - the string shown if version control is inhibited; set to nil to omit
+  vc-off    - the string shown if version control is inhibited; use nil to omit
   prefix    - either nil or a fixed prefix for the dirname
 
 The following variables are available, but should not be changed:
diff --git a/lisp/international/mule-cmds.el b/lisp/international/mule-cmds.el
index d59f2c0ebf..99c014e310 100644
--- a/lisp/international/mule-cmds.el
+++ b/lisp/international/mule-cmds.el
@@ -1356,7 +1356,7 @@ default-transient-input-method
   :version "28.1")
 
 (defvar current-transient-input-method nil
-  "The current input method temporarily enabled by `activate-transient-input-method'.
+  "Input method temporarily enabled by `activate-transient-input-method'.
 If nil, that means no transient input method is active now.")
 (make-variable-buffer-local 'current-transient-input-method)
 (put 'current-transient-input-method 'permanent-local t)
diff --git a/lisp/language/ethio-util.el b/lisp/language/ethio-util.el
index 55e59ab516..f0df8be7d5 100644
--- a/lisp/language/ethio-util.el
+++ b/lisp/language/ethio-util.el
@@ -113,12 +113,14 @@ ethio-use-colon-for-colon
 variable.")
 
 (defvar ethio-use-three-dot-question nil
-  "Non-nil means associate ASCII question mark with Ethiopic old style question mark (three vertically stacked dots).
+  "If non-nil, associate ASCII question mark with Ethiopic old style question mark.
+The old style question mark is three vertically stacked dots.
+
 If nil, associate ASCII question mark with Ethiopic stylized question
 mark.  All SERA <--> FIDEL converters refer this variable.")
 
 (defvar ethio-quote-vowel-always nil
-  "Non-nil means always put an apostrophe before an isolated vowel (except at word initial) in FIDEL --> SERA conversion.
+  "If non-nil, always put an apostrophe before an isolated vowel (except at word initial) in FIDEL --> SERA conversion.
 If nil, put an apostrophe only between a 6th-form consonant and an
 isolated vowel.")
 
diff --git a/lisp/mail/feedmail.el b/lisp/mail/feedmail.el
index 6effe13986..cc8ab0148b 100644
--- a/lisp/mail/feedmail.el
+++ b/lisp/mail/feedmail.el
@@ -807,7 +807,8 @@ feedmail-fiddle-plex-user-list
 
 feedmail will use this list of fiddle-plexes to manipulate user-specified
 message header fields.  It does this after it has completed all normal
-message header field manipulation and before calling `feedmail-last-chance-hook'.
+message header field manipulation and before calling
+`feedmail-last-chance-hook'.
 
 For an explanation of fiddle-plexes, see the documentation for the
 variable `feedmail-fiddle-plex-blurb'.  In contrast to some other fiddle-plex
@@ -889,13 +890,14 @@ feedmail-spray-address-fiddle-plex-list
 stripped envelope email address (no comments or angle brackets).  The
 function should return an embellished form of the address.
 
-The recipe for sending form letters is:  (1) create a message with all
-addressees on Bcc: headers; (2) tell feedmail to remove Bcc: headers
-before sending the message; (3) create a function which will embellish
-stripped addresses, if desired; (4) define `feedmail-spray-address-fiddle-plex-list'
-appropriately; (5) send the message with `feedmail-enable-spray' set
-non-nil; (6) stand back and watch co-workers wonder at how efficient
-you are at accomplishing inherently inefficient things."
+The recipe for sending form letters is: (1) create a message with
+all addressees on Bcc: headers; (2) tell feedmail to remove Bcc:
+headers before sending the message; (3) create a function which
+will embellish stripped addresses, if desired; (4) define
+`feedmail-spray-address-fiddle-plex-list' appropriately; (5) send
+the message with `feedmail-enable-spray' set non-nil; (6) stand
+back and watch co-workers wonder at how efficient you are at
+accomplishing inherently inefficient things."
   :group 'feedmail-spray
   :type 'sexp ; too complex to be described accurately
   )
diff --git a/lisp/mh-e/mh-e.el b/lisp/mh-e/mh-e.el
index 3ac5c8f7ae..e5f69a5ae8 100644
--- a/lisp/mh-e/mh-e.el
+++ b/lisp/mh-e/mh-e.el
@@ -3182,7 +3182,7 @@ mh-alias-reloaded-hook
   :package-version '(MH-E . "8.0"))
 
 (defcustom-mh mh-annotate-msg-hook nil
-  "Hook run whenever a message is sent and after the scan lines and message are annotated.
+  "Hook run when a message is sent and after annotating the scan lines and message.
 Hook functions can access the current folder name with
 `mh-current-folder' and obtain the message numbers of the
 annotated messages with `mh-annotate-list'."
diff --git a/lisp/net/imap.el b/lisp/net/imap.el
index 0394f0efea..27c2d869f6 100644
--- a/lisp/net/imap.el
+++ b/lisp/net/imap.el
@@ -190,7 +190,7 @@ imap-shell-program
   :type '(repeat string))
 
 (defcustom imap-process-connection-type nil
-  "Value for `process-connection-type' to use for Kerberos4, GSSAPI, shell, and SSL.
+  "Value for `process-connection-type' to use for Kerberos4, GSSAPI, shell and SSL.
 The `process-connection-type' variable controls the type of device
 used to communicate with subprocesses.  Values are nil to use a
 pipe, or t or `pty' to use a pty.  The value has no effect if the
diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index c4b68f1be4..6a32fa9255 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -1507,7 +1507,7 @@ rcirc-last-sender
 (make-variable-buffer-local 'rcirc-last-sender)
 
 (defcustom rcirc-omit-threshold 100
-  "Number of lines since last activity from a nick before `rcirc-omit-responses' are omitted."
+  "Lines since last activity from a nick before `rcirc-omit-responses' are omitted."
   :type 'integer)
 
 (defcustom rcirc-log-process-buffers nil
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 98537a100f..28c23fa6f7 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -58,8 +58,7 @@ tramp-inline-compress-start-size
 
 ;;;###tramp-autoload
 (defcustom tramp-copy-size-limit 10240
-  "The maximum file size where inline copying is preferred over an \
-out-of-the-band copy.
+  "Maximum file size where inline copying is preferred to an out-of-the-band copy.
 If it is nil, out-of-the-band copy will be used without a check."
   :group 'tramp
   :type '(choice (const nil) integer))
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 6750a7ff4c..984bfcda0c 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -1022,8 +1022,8 @@ tramp-build-file-name-structure
    5 6 7 8 1))
 
 (defvar tramp-file-name-structure nil ;Initialized when defining `tramp-syntax'!
-  "List of six elements (REGEXP METHOD USER HOST FILE HOP), detailing \
-the Tramp file name structure.
+  "List detailing the Tramp file name structure.
+This is a list of six elements (REGEXP METHOD USER HOST FILE HOP).
 
 The first element REGEXP is a regular expression matching a Tramp file
 name.  The regex should contain parentheses around the method name,
diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index 5bb904e691..080b8c0c6e 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -107,8 +107,10 @@ nxml-prefer-utf-16-little-to-big-endian-flag
 
 (defcustom nxml-default-buffer-file-coding-system nil
   "Default value for `buffer-file-coding-system' for a buffer for a new file.
-A value of nil means use the default value of `buffer-file-coding-system' as normal.
-A buffer's `buffer-file-coding-system' affects what \\[nxml-insert-xml-declaration] inserts."
+A value of nil means use the default value of
+`buffer-file-coding-system' as normal.
+A buffer's `buffer-file-coding-system' affects what
+\\[nxml-insert-xml-declaration] inserts."
   :group 'nxml
   :type 'coding-system)
 
diff --git a/lisp/obsolete/landmark.el b/lisp/obsolete/landmark.el
index df3c5d6cc9..39e0f50e73 100644
--- a/lisp/obsolete/landmark.el
+++ b/lisp/obsolete/landmark.el
@@ -1278,7 +1278,8 @@ landmark-no-payoff
   :group 'landmark)
 (defcustom landmark-max-stall-time 2
   "The maximum number of cycles that the robot can remain stuck in a place.
-After this limit is reached, landmark-random-move is called to push him out of it."
+After this limit is reached, landmark-random-move is called to
+push him out of it."
   :type 'integer
   :group 'landmark)
 
diff --git a/lisp/obsolete/tls.el b/lisp/obsolete/tls.el
index d1b215cbfb..a9d7b84373 100644
--- a/lisp/obsolete/tls.el
+++ b/lisp/obsolete/tls.el
@@ -130,8 +130,10 @@ tls-checktrust
 consider trustworthy, e.g.:
 
 \(setq tls-program
-      \\='(\"gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt -p %p %h\"
-	\"gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt -p %p %h --protocols ssl3\"))"
+      \\='(\"gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt \\
+-p %p %h\"
+        \"gnutls-cli --x509cafile /etc/ssl/certs/ca-certificates.crt \\
+-p %p %h --protocols ssl3\"))"
   :type '(choice (const :tag "Always" t)
 		 (const :tag "Never" nil)
 		 (const :tag "Ask" ask))
diff --git a/lisp/org/ob-R.el b/lisp/org/ob-R.el
index b52c7591ad..0493ebc7db 100644
--- a/lisp/org/ob-R.el
+++ b/lisp/org/ob-R.el
@@ -360,7 +360,7 @@ org-babel-R-write-object-command
             )
     }
 }(object=%s,transfer.file=\"%s\")"
-  "A template for an R command to evaluate a block of code and write the result to a file.
+  "Template for an R command to evaluate a block of code and write result to file.
 
 Has four %s escapes to be filled in:
 1. Row names, \"TRUE\" or \"FALSE\"
diff --git a/lisp/org/org-agenda.el b/lisp/org/org-agenda.el
index 82bf1b23f9..822276d3d9 100644
--- a/lisp/org/org-agenda.el
+++ b/lisp/org/org-agenda.el
@@ -7495,7 +7495,8 @@ org-agenda-filter-by-category
   "Filter lines in the agenda buffer that have a specific category.
 The category is that of the current line.
 With a `\\[universal-argument]' prefix argument, exclude the lines of that category.
-When there is already a category filter in place, this command removes the filter."
+When there is already a category filter in place, this command removes the
+filter."
   (interactive "P")
   (if (and org-agenda-filtered-by-category
 	   org-agenda-category-filter)
diff --git a/lisp/org/org-attach.el b/lisp/org/org-attach.el
index 1ed305c9ff..8b0c05e063 100644
--- a/lisp/org/org-attach.el
+++ b/lisp/org/org-attach.el
@@ -181,7 +181,7 @@ org-attach-id-to-path-function-list
   :type '(repeat (function :tag "Function with ID as input")))
 
 (defvar org-attach-after-change-hook nil
-  "Hook to be called when files have been added or removed to the attachment folder.")
+  "Hook called when files have been added or removed to the attachment folder.")
 
 (defvar org-attach-open-hook nil
   "Hook that is invoked by `org-attach-open'.
diff --git a/lisp/org/org-indent.el b/lisp/org/org-indent.el
index 5171919465..a6a9fe145c 100644
--- a/lisp/org/org-indent.el
+++ b/lisp/org/org-indent.el
@@ -87,15 +87,13 @@ org-indent-boundary-char
   :type 'character)
 
 (defcustom org-indent-mode-turns-off-org-adapt-indentation t
-  "Non-nil means setting the variable `org-indent-mode' will \
-turn off indentation adaptation.
+  "Non-nil means setting `org-indent-mode' will turn off indentation adaptation.
 For details see the variable `org-adapt-indentation'."
   :group 'org-indent
   :type 'boolean)
 
 (defcustom org-indent-mode-turns-on-hiding-stars t
-  "Non-nil means setting the variable `org-indent-mode' will \
-turn on `org-hide-leading-stars'."
+  "Non-nil means setting `org-indent-mode' will turn on `org-hide-leading-stars'."
   :group 'org-indent
   :type 'boolean)
 
diff --git a/lisp/org/org-protocol.el b/lisp/org/org-protocol.el
index 55a534d0dc..06f50061a4 100644
--- a/lisp/org/org-protocol.el
+++ b/lisp/org/org-protocol.el
@@ -181,7 +181,8 @@ org-protocol-project-alist
   :working-directory - the local working directory.  This is, what base-url will
                        be replaced with.
   :redirects         - A list of cons cells, each of which maps a regular
-                       expression to match to a path relative to :working-directory.
+                       expression to match to a path relative to
+                       :working-directory.
 
 Example:
 
@@ -202,7 +203,8 @@ org-protocol-project-alist
           :working-directory \"~/site/content/post/\"
           :online-suffix \".html\"
           :working-suffix \".md\"
-          :rewrites ((\"\\(https://site.com/[0-9]+/[0-9]+/[0-9]+/\\)\" . \".md\")))))
+          :rewrites ((\"\\(https://site.com/[0-9]+/[0-9]+/[0-9]+/\\)\"
+                      . \".md\")))))
 
 
    The last line tells `org-protocol-open-source' to open
diff --git a/lisp/org/org.el b/lisp/org/org.el
index 5dc9fa9d8d..f597da6885 100644
--- a/lisp/org/org.el
+++ b/lisp/org/org.el
@@ -4177,7 +4177,8 @@ org-stamp-time-of-day-regexp
   "Regular expression to match a timestamp time or time range.
 After a match, the following groups carry important information:
 0  the full match
-1  date plus weekday, for back referencing to make sure both times are on the same day
+1  date plus weekday, for back referencing to
+     make sure both times are on the same day
 2  the first time, range or not
 4  the second time, if it is a range.")
 
diff --git a/lisp/progmodes/cc-vars.el b/lisp/progmodes/cc-vars.el
index 9e6f9527ca..8772ed0632 100644
--- a/lisp/progmodes/cc-vars.el
+++ b/lisp/progmodes/cc-vars.el
@@ -575,7 +575,8 @@ c-doc-comment-style
 
  javadoc -- Javadoc style for \"/** ... */\" comments (default in Java mode).
  autodoc -- Pike autodoc style for \"//! ...\" comments (default in Pike mode).
- gtkdoc  -- GtkDoc style for \"/** ... **/\" comments (default in C and C++ modes).
+ gtkdoc  -- GtkDoc style for \"/** ... **/\" comments
+						   (default in C and C++ modes).
  doxygen -- Doxygen style.
 
 The value may also be a list of doc comment styles, in which case all
diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index ed9b234d69..f67ec6b6cd 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -232,7 +232,10 @@ cperl-indent-wrt-brace
   :group 'cperl-indentation-details)
 
 (defcustom cperl-indent-subs-specially t
-  "Non-nil means indent subs that are inside other blocks (hash values, for example) relative to the beginning of the \"sub\" keyword, rather than relative to the statement that contains the declaration."
+  "If non-nil, indent subs inside other blocks relative to \"sub\" keyword.
+If nil, indent them relative to the statement that contains the
+declaration.
+This applies to hash values, for example."
   :type 'boolean
   :group 'cperl-indentation-details)
 
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index fa360a8f3f..90b0263fe1 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -682,7 +682,8 @@ elisp--xref-make-xref
 	     (xref-make-elisp-location symbol type file)))
 
 (defvar elisp-xref-find-def-functions nil
-  "List of functions to be run from `elisp--xref-find-definitions' to add additional xrefs.
+  "List of functions to be run from `elisp--xref-find-definitions'.
+These are used to add additional xrefs.
 Called with one arg; the symbol whose definition is desired.
 Each function should return a list of xrefs, or nil; the first
 non-nil result supersedes the xrefs produced by
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 152dc725c7..744c110f6b 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -120,8 +120,10 @@ flymake-proc-allowed-file-name-masks
   REGEXP INIT [CLEANUP [NAME]]
 REGEXP is a regular expression that matches a file name.
 INIT is the init function to use.
-CLEANUP is the cleanup function to use, default `flymake-proc-simple-cleanup'.
-NAME is the file name function to use, default `flymake-proc-get-real-file-name'."
+CLEANUP is the cleanup function to use, default
+  `flymake-proc-simple-cleanup'.
+NAME is the file name function to use, default
+  `flymake-proc-get-real-file-name'."
   :group 'flymake
   :type '(alist :key-type (regexp :tag "File regexp")
                 :value-type
diff --git a/lisp/progmodes/hideif.el b/lisp/progmodes/hideif.el
index 7cbc9708fc..0ca2ff53b2 100644
--- a/lisp/progmodes/hideif.el
+++ b/lisp/progmodes/hideif.el
@@ -153,8 +153,8 @@ hide-ifdef-expand-reinclusion-protection
 undefined, and so nothing is hidden.  The next time we visit it, everything will
 be hidden.
 
-This behavior is generally undesirable.  If this option is non-nil, the outermost
-#if is always visible."
+This behavior is generally undesirable.  If this option is non-nil, the
+outermost #if is always visible."
   :type 'boolean
   :version "25.1")
 
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index fbc6e424eb..8cb0350dc0 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -75,7 +75,7 @@ ruby-block-mid-keywords
 
 (defconst ruby-block-mid-re
   (regexp-opt ruby-block-mid-keywords)
-  "Regexp to match where the indentation gets shallower in middle of block statements.")
+  "Regexp for where the indentation gets shallower in middle of block statements.")
 
 (defconst ruby-block-op-keywords
   '("and" "or" "not")
diff --git a/lisp/progmodes/verilog-mode.el b/lisp/progmodes/verilog-mode.el
index b1abefe534..f6e95b9cb6 100644
--- a/lisp/progmodes/verilog-mode.el
+++ b/lisp/progmodes/verilog-mode.el
@@ -10112,7 +10112,8 @@ verilog-module-filenames
 ;; A modi is:  [module-name-string file-name begin-point]
 
 (defvar verilog-cache-enabled t
-  "Non-nil enables caching of signals, etc.  Set to nil for debugging to make things SLOW!")
+  "Non-nil enables caching of signals, etc.
+Set to nil for debugging to make things SLOW!")
 
 (defvar verilog-modi-cache-list nil
   "Cache of ((Module Function) Buf-Tick Buf-Modtime Func-Returns)...
diff --git a/lisp/ses.el b/lisp/ses.el
index bfafc132bf..2a28ad3add 100644
--- a/lisp/ses.el
+++ b/lisp/ses.el
@@ -430,7 +430,8 @@ ses-get-cell
   local-printer-list)
 
 (defmacro ses-cell-symbol (row &optional col)
-  "From a CELL or a pair (ROW,COL), get the symbol that names the local-variable holding its value.  (0,0) => A1."
+  "Return symbol of the local-variable holding value of CELL or pair (ROW,COL).
+For example, (0,0) => A1."
   (declare (debug t))
   `(ses-cell--symbol ,(if col `(ses-get-cell ,row ,col) row)))
 (put 'ses-cell-symbol 'safe-function t)
diff --git a/lisp/simple.el b/lisp/simple.el
index 6059c23a14..1d4ac01638 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2195,7 +2195,8 @@ minibuffer-history-case-insensitive-variables
   "Minibuffer history variables for which matching should ignore case.
 If a history variable is a member of this list, then the
 \\[previous-matching-history-element] and \\[next-matching-history-element]\
- commands ignore case when searching it, regardless of `case-fold-search'."
+ commands ignore case when searching it,
+regardless of `case-fold-search'."
   :type '(repeat variable)
   :group 'minibuffer)
 
diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 1327bde908..cd152fb092 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -841,8 +841,10 @@ tab-bar-close-last-tab-choice
   "Defines what to do when the last tab is closed.
 If nil, do nothing and show a message, like closing the last window or frame.
 If `delete-frame', delete the containing frame, as a web browser would do.
-If `tab-bar-mode-disable', disable tab-bar-mode so that tabs no longer show in the frame.
-If the value is a function, call that function with the tab to be closed as an argument."
+If `tab-bar-mode-disable', disable tab-bar-mode so that tabs no longer show in
+the frame.
+If the value is a function, call that function with the tab to be closed as an
+ argument."
   :type '(choice (const    :tag "Do nothing and show message" nil)
                  (const    :tag "Close the containing frame" delete-frame)
                  (const    :tag "Disable tab-bar-mode" tab-bar-mode-disable)
diff --git a/lisp/term.el b/lisp/term.el
index 2e69af0735..d73a9b0d01 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -365,8 +365,8 @@ term-scroll-start
 (defvar-local term-scroll-end nil
   "Bottom-most line (inclusive) of the scrolling region.
 `term-scroll-end' must be in the range [0,term-height).  In addition, its
-value has to be greater than `term-scroll-start', i.e. one line scroll regions are
-not allowed.")
+value has to be greater than `term-scroll-start', i.e. one line scroll regions
+are not allowed.")
 (defvar term-pager-count nil
   "Number of lines before we need to page; if nil, paging is disabled.")
 (defvar term-saved-cursor nil)
diff --git a/lisp/textmodes/reftex-vars.el b/lisp/textmodes/reftex-vars.el
index c9fd19d232..b4b7db579c 100644
--- a/lisp/textmodes/reftex-vars.el
+++ b/lisp/textmodes/reftex-vars.el
@@ -741,8 +741,8 @@ reftex-special-environment-functions
 boundary for backwards searches which should be observed.
 
 Here is an example.  The LaTeX package linguex.sty defines list macros
-`\\ex.', `\\a.', etc for lists which are terminated by `\\z.' or an empty
-line.
+`\\ex.', `\\a.', etc for lists which are terminated by `\\z.' or an
+empty line.
 
     \\ex.  \\label{ex:12} Some text in an exotic language ...
           \\a. \\label{ex:13} more stuff
@@ -766,10 +766,12 @@ reftex-special-environment-functions
        (save-excursion
          ;; Search for any of the linguex item macros at the beginning of a line
          (if (re-search-backward
-              \"^[ \\t]*\\\\(\\\\\\\\\\\\(ex\\\\|a\\\\|b\\\\|c\\\\|d\\\\|e\\\\|f\\\\)g?\\\\.\\\\)\" bound t)
+              (concat \"^[ \\t]*\\\\(\\\\\\\\\\\\(ex\\\\|a\\\\|b\"
+                      \"\\\\|c\\\\|d\\\\|e\\\\|f\\\\)g?\\\\.\\\\)\")
+              bound t)
              (progn
                (setq p1 (match-beginning 1))
-               ;; Make sure no empty line or \\z. is between us and the item macro
+               ;; Make sure no empty line or \\z. is between us and item macro
                (if (re-search-forward \"\\n[ \\t]*\\n\\\\|\\\\\\\\z\\\\.\" pos t)
                    ;; Return nil because list was already closed
                    nil
diff --git a/lisp/textmodes/table.el b/lisp/textmodes/table.el
index e42615e515..3582f15731 100644
--- a/lisp/textmodes/table.el
+++ b/lisp/textmodes/table.el
@@ -853,10 +853,12 @@ table-heighten-timer
   "Timer id for deferred cell update.")
 (defvar table-inhibit-update nil
   "Non-nil inhibits implicit cell and cache updates.
-It inhibits `table-with-cache-buffer' to update data in both direction, cell to cache and cache to cell.")
+It inhibits `table-with-cache-buffer' to update data in both
+direction, cell to cache and cache to cell.")
 (defvar table-inhibit-auto-fill-paragraph nil
   "Non-nil inhibits auto fill paragraph when `table-with-cache-buffer' exits.
-This is always set to nil at the entry to `table-with-cache-buffer' before executing body forms.")
+This is always set to nil at the entry to
+`table-with-cache-buffer' before executing body forms.")
 (defvar table-mode-indicator nil
   "For mode line indicator")
 ;; This is not a real minor-mode but placed in the minor-mode-alist
@@ -957,7 +959,7 @@ table-command-remap-alist
     (describe-bindings . *table--cell-describe-bindings)
     (dabbrev-expand . *table--cell-dabbrev-expand)
     (dabbrev-completion . *table--cell-dabbrev-completion))
-  "List of cons cells consisting of (ORIGINAL-COMMAND . TABLE-VERSION-OF-THE-COMMAND).")
+  "List of the form (ORIGINAL-COMMAND . TABLE-VERSION-OF-THE-COMMAND).")
 
 (defvar table-command-list
   ;; Construct the real contents of the `table-command-list'.
diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index ccf5a7807f..adb6ce8053 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -149,7 +149,7 @@ ediff-auto-refine
 Use `setq-default' if setting it in .emacs")
 
 (ediff-defvar-local ediff-ignore-similar-regions nil
-  "If t, skip over difference regions that differ only in the white space and line breaks.
+  "If t, skip difference regions that differ only in white space and line breaks.
 This variable can be set either in .emacs or toggled interactively.
 Use `setq-default' if setting it in .emacs")
 
diff --git a/lisp/vc/ediff-init.el b/lisp/vc/ediff-init.el
index 04926af16e..b5c2c45583 100644
--- a/lisp/vc/ediff-init.el
+++ b/lisp/vc/ediff-init.el
@@ -432,7 +432,7 @@ ediff-after-setup-control-frame-hook
   :type 'hook
   :group 'ediff-hook)
 (defcustom ediff-startup-hook nil
-  "Hooks to run in the control buffer after Ediff has been set up and is ready for the job."
+  "Hooks to run in the control buffer after Ediff has been set up and is ready."
   :type 'hook
   :group 'ediff-hook)
 (defcustom ediff-select-hook nil
@@ -480,7 +480,7 @@ ediff-quit-hook
   :type 'hook
   :group 'ediff-hook)
 (defcustom ediff-cleanup-hook nil
-  "Hooks to run on exiting Ediff but before killing the control and variant buffers."
+  "Hooks to run on exiting Ediff, before killing the control and variant buffers."
   :type 'hook
   :group 'ediff-hook)
 
@@ -1268,7 +1268,7 @@ ediff-temp-file-mode
 ;; Metacharacters that have to be protected from the shell when executing
 ;; a diff/diff3 command.
 (defcustom ediff-metachars "[ \t\n!\"#$&'()*;<=>?[\\^`{|~]"
-  "Regexp that matches characters that must be quoted with `\\' in shell command line.
+  "Regexp matching characters that must be quoted with `\\' in shell command line.
 This default should work without changes."
   :type 'regexp
   :group 'ediff)
diff --git a/lisp/vc/ediff-merg.el b/lisp/vc/ediff-merg.el
index 22656761d9..1c1d5219c7 100644
--- a/lisp/vc/ediff-merg.el
+++ b/lisp/vc/ediff-merg.el
@@ -70,7 +70,7 @@ ediff-combination-pattern
   :group 'ediff-merge)
 
 (defcustom ediff-show-clashes-only nil
-  "If t, show only those diff regions where both buffers disagree with the ancestor.
+  "If t, show only diff regions where both buffers disagree with the ancestor.
 This means that regions that have status prefer-A or prefer-B will be
 skipped over.  A value of nil means show all regions."
   :type 'boolean
diff --git a/lisp/vc/ediff-mult.el b/lisp/vc/ediff-mult.el
index b48377815a..3d79630f3b 100644
--- a/lisp/vc/ediff-mult.el
+++ b/lisp/vc/ediff-mult.el
@@ -181,7 +181,7 @@ ediff-recurse-to-subdirectories
 (defvar ediff-filtering-regexp-history nil "")
 
 (defcustom ediff-default-filtering-regexp nil
-  "The default regular expression used as a filename filter in multifile comparisons.
+  "Default regular expression used as a filename filter in multifile comparisons.
 Should be a sexp.  For instance (car ediff-filtering-regexp-history) or nil."
   :type 'sexp                           ; yuck - why not just a regexp?
   :risky t)
diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
index c68dc71884..3d90ccb1cb 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -182,7 +182,7 @@ ediff-mouse-pixel-position
 
 ;; not used for now
 (defvar ediff-mouse-pixel-threshold 30
-  "If the user moves mouse more than this many pixels, Ediff won't warp mouse into control window.")
+  "If mouse moved more than this many pixels, don't warp mouse into control window.")
 
 (defcustom ediff-grab-mouse t
   "If t, Ediff will always grab the mouse and put it in the control frame.
diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 13f875b192..5c41761a04 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -925,7 +925,7 @@ smerge-refine-forward-function
   This only matters if `smerge-refine-weight-hack' is nil.")
 
 (defvar smerge-refine-ignore-whitespace t
-  "If non-nil, indicate that `smerge-refine' should try to ignore change in whitespace.")
+  "If non-nil, `smerge-refine' should try to ignore change in whitespace.")
 
 (defvar smerge-refine-weight-hack t
   "If non-nil, pass to diff as many lines as there are chars in the region.
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index b3b0583966..a73774d090 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2385,8 +2385,9 @@ vc-retrieve-tag
 ;; for the root directory.
 (defvar vc-log-short-style '(directory)
   "Whether or not to show a short log.
-If it contains `directory' then if the fileset contains a directory show a short log.
-If it contains `file' then show short logs for files.
+If it contains `directory', show a short log if the fileset
+contains a directory.
+If it contains `file', show short logs for files.
 Not all VC backends support short logs!")
 
 (defvar log-view-vc-fileset)
diff --git a/lisp/view.el b/lisp/view.el
index 6f576f8c04..c1b788a739 100644
--- a/lisp/view.el
+++ b/lisp/view.el
@@ -141,7 +141,8 @@ view-return-to-alist
 (put 'view-return-to-alist 'permanent-local t)
 
 (defvar view-exit-action nil
-  "If non-nil, a function with one argument (a buffer) called when finished viewing.
+  "If non-nil, a function called when finished viewing.
+The function should take one argument (a buffer).
 Commands like \\[view-file] and \\[view-file-other-window] may
 set this to bury or kill the viewed buffer.
 Observe that the buffer viewed might not appear in any window at
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-10 20:59       ` Stefan Kangas
@ 2020-12-10 21:53         ` Stefan Kangas
  2020-12-11  8:16           ` Eli Zaretskii
  2020-12-11  7:33         ` Eli Zaretskii
                           ` (2 subsequent siblings)
  3 siblings, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2020-12-10 21:53 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

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

Stefan Kangas <stefan@marxist.se> writes:

> 2. Avoid wide docstrings in define-minor-mode.

And here's a patch also for define-derived-mode.

It seems like we still need to fix cl-defun, cl-defstruct,
define-globalized-minor-mode and perhaps a few others.
So I'll work on that next.

[-- Attachment #2: 0004-Fill-docstrings-in-define-derived-mode.patch --]
[-- Type: text/x-diff, Size: 4063 bytes --]

From 50f2a3cce5de966599952fdd1e6ea05913a950b1 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Thu, 10 Dec 2020 22:36:18 +0100
Subject: [PATCH] Fill docstrings in define-derived-mode

* lisp/subr.el (internal--fill-string): New function.
* lisp/emacs-lisp/derived.el (derived-mode-make-docstring): Fill
docstrings.
---
 lisp/emacs-lisp/derived.el | 42 +++++++++++++++++++++-----------------
 lisp/subr.el               | 13 ++++++++++++
 2 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/lisp/emacs-lisp/derived.el b/lisp/emacs-lisp/derived.el
index 6a11f1c394..00a6c12b10 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--fill-string
+                  (format "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--fill-string
+                     (concat
+                      (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/subr.el b/lisp/subr.el
index c28807f694..4d10b70db7 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -5927,4 +5927,17 @@ run-hook-query-error-with-timeout
      ;; Continue running.
      nil)))
 
+(defun internal--fill-string (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 (< (length 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 lst)))))
+
 ;;; subr.el ends here
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-10 20:59       ` Stefan Kangas
  2020-12-10 21:53         ` Stefan Kangas
@ 2020-12-11  7:33         ` Eli Zaretskii
  2020-12-11 20:36           ` Stefan Kangas
  2020-12-11  7:53         ` Eli Zaretskii
  2020-12-11 15:13         ` Lars Ingebrigtsen
  3 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-12-11  7:33 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 10 Dec 2020 14:59:16 -0600
> Cc: 44858@debbugs.gnu.org
> 
> With these three patches applied, around 22 wide docstring warnings
> remain in our tree for defvar's.  I found it hard to fix them as I'm not
> very familiar with the code in question, and I couldn't just simplify
> the wording from the context.
> 
> I'm assuming we don't want to push this without fixing those warnings,
> so I would really appreciate some help with the remaining couple of
> warnings.

Please show those problematic doc strings, and I will try to fix them.

Thanks.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-10 20:59       ` Stefan Kangas
  2020-12-10 21:53         ` Stefan Kangas
  2020-12-11  7:33         ` Eli Zaretskii
@ 2020-12-11  7:53         ` Eli Zaretskii
  2020-12-19 23:55           ` Stefan Kangas
  2020-12-11 15:13         ` Lars Ingebrigtsen
  3 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-12-11  7:53 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 10 Dec 2020 14:59:16 -0600
> Cc: 44858@debbugs.gnu.org
> 
> +** byte compiler

Please capitalize "Byte".

> +*** 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'.  See the new user
> +option 'byte-compile-docstring-max-column'.

The last sentence will be more useful if you make it more informative,
e.g.

  This is controlled by the new user option
  'byte-compile-docstring-max-column'.

> +(defvar byte-compile--wide-docstring-substitution-len 3
> +  "Substitution width used in `byte-compile--wide-docstring-p'.")

The doc string should be more detailed.  Let's at least explain what
hides behind "substitution", and say that the value is a heuristic.

> +(defcustom byte-compile-docstring-max-column 80
> +  "Length that a doc string can be before the byte-compiler reports a warning."

I suggest "width", not "length", since you both use the former
elsewhere, and because "column" goes better with "width".

Also, "can be before" is not the clearest way of saying this.  I
suggest

    Recommended maximum width of doc string lines.
  The byte-compiler will emit a warning for doc-string lines
  wider than this.  If `fill-column' has a larger value, it will
  override this variable.

> +(defun byte-compile-docstring-length-warn (form)
> +  "Warn if documentation string of FORM is too wide.
> +It is too wide if it is longer than the largest of `fill-column'
> +and `byte-compile-docstring-max-column'."

"It is too wide if it is longer" is inaccurate, I think, because we
actually test each _line_ of the doc string against the limit, while
the above seems to imply the overall length of the doc string (which
could be multi-line) is tested.  So:

  It is too wide if some of its lines is wider than the largest of...

> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-long-docstring-autoload.el
> @@ -0,0 +1,3 @@
> +;;; -*- lexical-binding: t -*-
> +(autoload 'foox "foo"
> +  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")

I see that none of the tests use multi-line doc strings, is that
right?  If so, I think it would be good to have at least a couple, one
where the first line is too wide, and another where some line other
than the first is too wide.  Also, what about testing the fact that
fill-column overrides the value of the new option?

Thanks.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-10 21:53         ` Stefan Kangas
@ 2020-12-11  8:16           ` Eli Zaretskii
  2020-12-11 20:03             ` Stefan Kangas
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-12-11  8:16 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 10 Dec 2020 15:53:13 -0600
> Cc: 44858@debbugs.gnu.org
> 
> +(defun internal--fill-string (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 (< (length str) fill-column)  <<<<<<<<<<<<<<<<<<<<<<<<<

Can we use string-width here instead of length?  If this will work
during bootstrap, it is preferable, I think.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-10 20:59       ` Stefan Kangas
                           ` (2 preceding siblings ...)
  2020-12-11  7:53         ` Eli Zaretskii
@ 2020-12-11 15:13         ` Lars Ingebrigtsen
  3 siblings, 0 replies; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-11 15:13 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> With these three patches applied, around 22 wide docstring warnings
> remain in our tree for defvar's.  I found it hard to fix them as I'm not
> very familiar with the code in question, and I couldn't just simplify
> the wording from the context.
>
> I'm assuming we don't want to push this without fixing those warnings,
> so I would really appreciate some help with the remaining couple of
> warnings.

I think it's fine to push even if it generates warnings -- then people
can help with stamping out those warnings.  (My guess it that it won't
take long.)

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-11  8:16           ` Eli Zaretskii
@ 2020-12-11 20:03             ` Stefan Kangas
  0 siblings, 0 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-11 20:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, larsi

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Thu, 10 Dec 2020 15:53:13 -0600
>> Cc: 44858@debbugs.gnu.org
>>
>> +(defun internal--fill-string (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 (< (length str) fill-column)  <<<<<<<<<<<<<<<<<<<<<<<<<
>
> Can we use string-width here instead of length?  If this will work
> during bootstrap, it is preferable, I think.

Yes, it seems to work fine.  Now changed.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-03 20:18 ` Tomas Nordin
@ 2020-12-11 20:14   ` Stefan Kangas
  0 siblings, 0 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-11 20:14 UTC (permalink / raw)
  To: Tomas Nordin, 44858

Tomas Nordin <tomasn@posteo.net> writes:

> +  docstrings  docstrings that are too wide (no longer than 80 characters,
> +              or `fill-column', whichever is bigger)
>    suspicious  constructs that usually don't do what the coder wanted.
>
> Should 'no' maybe be removed? (if to tell something that should not be).

Indeed, thanks for spotting that.  Now changed.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-11  7:33         ` Eli Zaretskii
@ 2020-12-11 20:36           ` Stefan Kangas
  2020-12-19 11:22             ` Eli Zaretskii
  0 siblings, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2020-12-11 20:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, larsi

Eli Zaretskii <eliz@gnu.org> writes:

>> I'm assuming we don't want to push this without fixing those warnings,
>> so I would really appreciate some help with the remaining couple of
>> warnings.
>
> Please show those problematic doc strings, and I will try to fix them.

Thanks, I appreciate the help.  Please see the below list that should be
easy to just put in a compilation-mode buffer.


gnus/deuglify.el:252:1: Warning: custom-declare-variable
    `gnus-outlook-deuglify-unwrap-stop-chars' docstring wider than 80
    characters
gnus/deuglify.el:259:1: Warning: custom-declare-variable
    `gnus-outlook-deuglify-no-wrap-chars' docstring wider than 80 characters
gnus/deuglify.el:265:1: Warning: custom-declare-variable
    `gnus-outlook-deuglify-attrib-cut-regexp' docstring wider than 80
    characters
gnus/gnus.el:1529:1: Warning: custom-declare-variable
    `gnus-group-charset-alist' docstring wider than 80 characters
gnus/nnvirtual.el:63:1: Warning: defvar `nnvirtual-mapping-table' docstring
    wider than 80 characters
gnus/nnvirtual.el:66:1: Warning: defvar `nnvirtual-mapping-offsets' docstring
    wider than 80 characters
gnus/nnvirtual.el:72:1: Warning: defvar `nnvirtual-mapping-reads' docstring
    wider than 80 characters
gnus/nnvirtual.el:75:1: Warning: defvar `nnvirtual-mapping-marks' docstring
    wider than 80 characters
gnus/nnvirtual.el:78:1: Warning: defvar `nnvirtual-info-installed' docstring
    wider than 80 characters
language/ethio-util.el:122:1: Warning: defvar `ethio-quote-vowel-always'
    docstring wider than 80 characters
language/ethio-util.el:127:1: Warning: defvar `ethio-W-sixth-always' docstring
    wider than 80 characters
mail/feedmail.el:624:1: Warning: custom-declare-variable
    `feedmail-sendmail-f-doesnt-sell-me-out' docstring wider than 80
    characters
mh-e/mh-e.el:2827:1: Warning: custom-declare-variable
    `mh-invisible-header-fields' docstring wider than 80 characters
mh-e/mh-e.el:2850:1: Warning: custom-declare-variable
    `mh-invisible-header-fields-default' docstring wider than 80 characters
net/dbus.el:79:1: Warning: defconst `dbus-interface-peer' docstring wider than
    80 characters
net/dbus.el:91:1: Warning: defconst `dbus-interface-introspectable' docstring
    wider than 80 characters
net/dbus.el:102:1: Warning: defconst `dbus-interface-properties' docstring
    wider than 80 characters
net/dbus.el:128:1: Warning: defconst `dbus-interface-objectmanager' docstring
    wider than 80 characters
net/dbus.el:148:1: Warning: defconst `dbus-interface-monitoring' docstring
    wider than 80 characters
org/org-ctags.el:164:1: Warning: custom-declare-variable
    `org-ctags-open-link-functions' docstring wider than 80 characters
org/org-protocol.el:166:1: Warning: custom-declare-variable
    `org-protocol-project-alist' docstring wider than 80 characters
vc/ediff-init.el:556:1: Warning: custom-declare-variable
    `ediff-before-flag-bol' docstring wider than 80 characters
vc/ediff-init.el:562:1: Warning: custom-declare-variable
    `ediff-after-flag-eol' docstring wider than 80 characters
vc/ediff-init.el:568:1: Warning: custom-declare-variable
    `ediff-before-flag-mol' docstring wider than 80 characters
progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
    docstring wider than 80 characters





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-11 20:36           ` Stefan Kangas
@ 2020-12-19 11:22             ` Eli Zaretskii
  2020-12-19 16:50               ` Stefan Kangas
  0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-12-19 11:22 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 11 Dec 2020 14:36:33 -0600
> Cc: larsi@gnus.org, 44858@debbugs.gnu.org
> 
> > Please show those problematic doc strings, and I will try to fix them.
> 
> Thanks, I appreciate the help.  Please see the below list that should be
> easy to just put in a compilation-mode buffer.
> 
> 
> gnus/deuglify.el:252:1: Warning: custom-declare-variable
>     `gnus-outlook-deuglify-unwrap-stop-chars' docstring wider than 80
>     characters
> gnus/deuglify.el:259:1: Warning: custom-declare-variable
>     `gnus-outlook-deuglify-no-wrap-chars' docstring wider than 80 characters
> gnus/deuglify.el:265:1: Warning: custom-declare-variable
>     `gnus-outlook-deuglify-attrib-cut-regexp' docstring wider than 80
>     characters
> gnus/gnus.el:1529:1: Warning: custom-declare-variable
>     `gnus-group-charset-alist' docstring wider than 80 characters
> gnus/nnvirtual.el:63:1: Warning: defvar `nnvirtual-mapping-table' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:66:1: Warning: defvar `nnvirtual-mapping-offsets' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:72:1: Warning: defvar `nnvirtual-mapping-reads' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:75:1: Warning: defvar `nnvirtual-mapping-marks' docstring
>     wider than 80 characters
> gnus/nnvirtual.el:78:1: Warning: defvar `nnvirtual-info-installed' docstring
>     wider than 80 characters
> language/ethio-util.el:122:1: Warning: defvar `ethio-quote-vowel-always'
>     docstring wider than 80 characters
> language/ethio-util.el:127:1: Warning: defvar `ethio-W-sixth-always' docstring
>     wider than 80 characters
> mail/feedmail.el:624:1: Warning: custom-declare-variable
>     `feedmail-sendmail-f-doesnt-sell-me-out' docstring wider than 80
>     characters

I fixed those.

> mh-e/mh-e.el:2827:1: Warning: custom-declare-variable
>     `mh-invisible-header-fields' docstring wider than 80 characters
> mh-e/mh-e.el:2850:1: Warning: custom-declare-variable
>     `mh-invisible-header-fields-default' docstring wider than 80 characters
> net/dbus.el:79:1: Warning: defconst `dbus-interface-peer' docstring wider than
>     80 characters
> net/dbus.el:91:1: Warning: defconst `dbus-interface-introspectable' docstring
>     wider than 80 characters
> net/dbus.el:102:1: Warning: defconst `dbus-interface-properties' docstring
>     wider than 80 characters
> net/dbus.el:128:1: Warning: defconst `dbus-interface-objectmanager' docstring
>     wider than 80 characters
> net/dbus.el:148:1: Warning: defconst `dbus-interface-monitoring' docstring
>     wider than 80 characters

I see nothing wrong with these, so either I'm missing something or
there's a bug in the code which reports these problems.

> org/org-ctags.el:164:1: Warning: custom-declare-variable
>     `org-ctags-open-link-functions' docstring wider than 80 characters

Fixed.

> org/org-protocol.el:166:1: Warning: custom-declare-variable
>     `org-protocol-project-alist' docstring wider than 80 characters

I don't see a problem here.

> vc/ediff-init.el:556:1: Warning: custom-declare-variable
>     `ediff-before-flag-bol' docstring wider than 80 characters
> vc/ediff-init.el:562:1: Warning: custom-declare-variable
>     `ediff-after-flag-eol' docstring wider than 80 characters
> vc/ediff-init.el:568:1: Warning: custom-declare-variable
>     `ediff-before-flag-mol' docstring wider than 80 characters

Fixed.

> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
>     docstring wider than 80 characters

I don't see how I can do anything about this one.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-19 11:22             ` Eli Zaretskii
@ 2020-12-19 16:50               ` Stefan Kangas
  2020-12-19 17:14                 ` Eli Zaretskii
  2020-12-19 17:18                 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-19 16:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, larsi

Eli Zaretskii <eliz@gnu.org> writes:

> I see nothing wrong with these, so either I'm missing something or
> there's a bug in the code which reports these problems.

Thanks for the fixes.  See my comments below:

>> mh-e/mh-e.el:2827:1: Warning: custom-declare-variable
>>     `mh-invisible-header-fields' docstring wider than 80 characters
>> mh-e/mh-e.el:2850:1: Warning: custom-declare-variable
>>     `mh-invisible-header-fields-default' docstring wider than 80 characters

AFAICT, the offending lines are the ones containing URLs:

line 2839:

`https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').

line 2683:

`https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').

I see two issues here:

a) The URL does not easily break over two lines.  I'm not sure what we
   should do.  Add a special case to ignore lines with URLs?

b) The warning doesn't point you to the actually offending line, but to
   the definition containing the docstring.  I'm not sure if this is
   easily fixable or not, or if it's important enough to be worth
   spending time on.

>> net/dbus.el:79:1: Warning: defconst `dbus-interface-peer' docstring wider than
>>     80 characters
>> net/dbus.el:91:1: Warning: defconst `dbus-interface-introspectable' docstring
>>     wider than 80 characters
>> net/dbus.el:102:1: Warning: defconst `dbus-interface-properties' docstring
>>     wider than 80 characters
>> net/dbus.el:128:1: Warning: defconst `dbus-interface-objectmanager' docstring
>>     wider than 80 characters
>> net/dbus.el:148:1: Warning: defconst `dbus-interface-monitoring' docstring
>>     wider than 80 characters

Again we have URLs, like on line 81:

See URL `https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-peer'."

>> org/org-protocol.el:166:1: Warning: custom-declare-variable
>>     `org-protocol-project-alist' docstring wider than 80 characters
>
> I don't see a problem here.

I fixed that one.

>> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
>>     docstring wider than 80 characters
>
> I don't see how I can do anything about this one.

I'm also stomped.  Perhaps we could ask Alan for help with this one.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-19 16:50               ` Stefan Kangas
@ 2020-12-19 17:14                 ` Eli Zaretskii
  2020-12-29  1:27                   ` Basil L. Contovounesios
  2020-12-19 17:18                 ` Lars Ingebrigtsen
  1 sibling, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2020-12-19 17:14 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858, larsi

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 19 Dec 2020 10:50:33 -0600
> Cc: larsi@gnus.org, 44858@debbugs.gnu.org
> 
> AFAICT, the offending lines are the ones containing URLs:
> 
> line 2839:
> 
> `https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').
> 
> line 2683:
> 
> `https://sourceforge.net/tracker/index.php?func=detail&aid=1916032&group_id=13357&atid=113357').
> 
> I see two issues here:
> 
> a) The URL does not easily break over two lines.  I'm not sure what we
>    should do.  Add a special case to ignore lines with URLs?

If you can detect and ignore long lines that consist entirely of URL,
yes, that would be good.  Otherwise, simply ignore these warnings.

> >> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
> >>     docstring wider than 80 characters
> >
> > I don't see how I can do anything about this one.
> 
> I'm also stomped.  Perhaps we could ask Alan for help with this one.

If he can, sure.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-19 16:50               ` Stefan Kangas
  2020-12-19 17:14                 ` Eli Zaretskii
@ 2020-12-19 17:18                 ` Lars Ingebrigtsen
  2020-12-19 23:48                   ` Stefan Kangas
  1 sibling, 1 reply; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-19 17:18 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> a) The URL does not easily break over two lines.  I'm not sure what we
>    should do.  Add a special case to ignore lines with URLs?

Yes, I think that would be good.

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-19 17:18                 ` Lars Ingebrigtsen
@ 2020-12-19 23:48                   ` Stefan Kangas
  0 siblings, 0 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-19 23:48 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> a) The URL does not easily break over two lines.  I'm not sure what we
>>    should do.  Add a special case to ignore lines with URLs?
>
> Yes, I think that would be good.

OK, done.  See the updated patches in the email following this one.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-11  7:53         ` Eli Zaretskii
@ 2020-12-19 23:55           ` Stefan Kangas
  2020-12-20 17:53             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2020-12-19 23:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, larsi

[-- 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


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-19 23:55           ` Stefan Kangas
@ 2020-12-20 17:53             ` Lars Ingebrigtsen
  2020-12-28  5:18               ` Stefan Kangas
  0 siblings, 1 reply; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-20 17:53 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> 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.

Sure; looks good to me.

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-20 17:53             ` Lars Ingebrigtsen
@ 2020-12-28  5:18               ` Stefan Kangas
  0 siblings, 0 replies; 38+ messages in thread
From: Stefan Kangas @ 2020-12-28  5:18 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> 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.
>
> Sure; looks good to me.

No further comments within a week, so I've pushed this first part to
master.  See commit 0ebea8ffbf and 6b8bb47ac0.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-19 17:14                 ` Eli Zaretskii
@ 2020-12-29  1:27                   ` Basil L. Contovounesios
  2020-12-29  2:16                     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 38+ messages in thread
From: Basil L. Contovounesios @ 2020-12-29  1:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44858, larsi, Stefan Kangas

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Sat, 19 Dec 2020 10:50:33 -0600
>> Cc: larsi@gnus.org, 44858@debbugs.gnu.org
>> 
>> >> progmodes/cc-engine.el:183:1: Warning: defvar `c-vsemi-status-unknown-p-fn'
>> >>     docstring wider than 80 characters
>> >
>> > I don't see how I can do anything about this one.
>> 
>> I'm also stomped.  Perhaps we could ask Alan for help with this one.
>
> If he can, sure.

I hope the following fix is okay.

Reword a long docstring in cc-langs.el
3334dd9041 2020-12-29 01:23:14 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3334dd904157e7b3787f5d32f30b3c31585d047c

Thanks,

-- 
Basil





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-29  1:27                   ` Basil L. Contovounesios
@ 2020-12-29  2:16                     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-29  2:16 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 44858, Alan Mackenzie, Stefan Kangas

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

>>> I'm also stomped.  Perhaps we could ask Alan for help with this one.
>>
>> If he can, sure.
>
> I hope the following fix is okay.
>
> Reword a long docstring in cc-langs.el

Makes sense to me at least, but I see that Alan wasn't added to the CCs?
So I've done that now.

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-26 12:53     ` Lars Ingebrigtsen
  2020-12-10 20:59       ` Stefan Kangas
@ 2020-12-30 12:07       ` Stefan Kangas
  2020-12-31  4:42         ` Lars Ingebrigtsen
  1 sibling, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2020-12-30 12:07 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> The problem I saw was basically warnings
>> about symbols only visible after macro expansion, and that warnings
>> would point to entirely the wrong line and column.
>
> Oh, OK, the problem was in the lines/column output, not with detecting
> the doc strings themselves?
>
> Yes, that sounds like a problem, but...  I think we can live with
> inaccurate lines here.

So I tried enabling warnings also for lambda's and here is a fairly
exhaustive list of things that leads to problems:

1. cl-defun

In auth-source-netrc-search:
auth-source.el:1224:11: Warning: docstring wider than 80 characters

Here, the offending line is this autogenerated line:

(fn &rest SPEC &key BACKEND REQUIRE CREATE TYPE MAX HOST USER PORT
&allow-other-keys)

But this line has to be like this if `C-h f' is to show anything but
(&rest spec) for the function parameters.  So should we just add some
special case where we ignore the last line if it matches "^([^)])$"?

2. cl-defstruct

In epg-context--make:
epg.el:197:30: Warning: docstring wider than 80 characters

Here I can't figure out why the docstring is too long.  Using
`macrostep-expand' doesn't reveal why.  Does anyone have any ideas?

3. defclass

These are autogenerated docstrings that I don't trivially see how we can
just wrap as it's the first line that is too long:

In jsonrpc-request:
jsonrpc.el:349:15: Warning: docstring wider than 80 characters

In toplevel form:
cedet/ede/proj-comp.el:71:26: Warning: docstring wider than 80 characters

I suppose we need to rethink these, somehow.

4. semantic

These macros results in wide docstrings in some cases:

- define-overloadable-function
- define-semantic-decoration-style
- define-mode-local-override

5. define-derived-mode

Finally, we have some remaining cases where define-derived-mode leads to
wide docstrings, e.g.:

In newsticker-treeview-list-mode:
net/newst-treeview.el:2031:52: Warning: docstring wider than 80 characters

These should be easy to fix, and I already have an idea for how.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-12-30 12:07       ` Stefan Kangas
@ 2020-12-31  4:42         ` Lars Ingebrigtsen
  0 siblings, 0 replies; 38+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-31  4:42 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> But this line has to be like this if `C-h f' is to show anything but
> (&rest spec) for the function parameters.  So should we just add some
> special case where we ignore the last line if it matches "^([^)])$"?

Sounds reasonable.

>
> 2. cl-defstruct
>
> In epg-context--make:
> epg.el:197:30: Warning: docstring wider than 80 characters
>
> Here I can't figure out why the docstring is too long.  Using
> `macrostep-expand' doesn't reveal why.  Does anyone have any ideas?

Hm.  The doc string (as displayed by the help functions) is:

epg-context--make is a compiled Lisp function in ‘epg.el’.

(epg-context--make PROTOCOL &optional ARMOR TEXTMODE INCLUDE-CERTS
CIPHER-ALGORITHM DIGEST-ALGORITHM COMPRESS-ALGORITHM)

  This function does not change global state, including the match data.

Constructor for objects of type ‘epg-context’.

Has that been folded at some point after your function has looked at it?

> 3. defclass
>
> These are autogenerated docstrings that I don't trivially see how we can
> just wrap as it's the first line that is too long:
>
> In jsonrpc-request:
> jsonrpc.el:349:15: Warning: docstring wider than 80 characters
>
> In toplevel form:
> cedet/ede/proj-comp.el:71:26: Warning: docstring wider than 80 characters
>
> I suppose we need to rethink these, somehow.

These bits can be filled:

Specialized Methods:

‘jsonrpc--next-request-id’ ((this jsonrpc-connection))
Retrieve the slot ‘-next-request-id’ from an object of class ‘jsonrpc-connection’.

These bits:

Instance Allocated Slots:

	Name	Type	Default	Doc
	————	————	———————	———
	name	t	unbound	A name for the connection
	-request-dispatcher	t	#'ignore	Dispatcher for remotely invoked requests.
	-notification-dispatcher	t	#'ignore	Dispatc


Have to be reformatted -- just moving the Doc to the next line should be
sufficient for most things.

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2020-11-25  1:36 bug#44858: [PATCH] Make byte-compiler warn about wide docstrings Stefan Kangas
                   ` (2 preceding siblings ...)
  2020-12-03 20:18 ` Tomas Nordin
@ 2021-09-24 17:25 ` Stefan Kangas
  2021-09-25  1:07   ` Lars Ingebrigtsen
  3 siblings, 1 reply; 38+ messages in thread
From: Stefan Kangas @ 2021-09-24 17:25 UTC (permalink / raw)
  To: 44858; +Cc: Lars Ingebrigtsen

Stefan Kangas <stefan@marxist.se> writes:

> I've been messing around with getting defuns to work, but I can't find a
> way for it to work reliably.

In this bug report, we have discussed several issues relating to warning
about too wide docstrings also for defun/lambda.  I believe I've solved
all remaining issues; I can now run "make bootstrap" with no warnings.

Please see the branch 'scratch/bug-44858'.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2021-09-24 17:25 ` Stefan Kangas
@ 2021-09-25  1:07   ` Lars Ingebrigtsen
  2021-09-26 11:38     ` Stefan Kangas
  0 siblings, 1 reply; 38+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-25  1:07 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44858

Stefan Kangas <stefan@marxist.se> writes:

> In this bug report, we have discussed several issues relating to warning
> about too wide docstrings also for defun/lambda.  I believe I've solved
> all remaining issues; I can now run "make bootstrap" with no warnings.

Cool!

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





^ permalink raw reply	[flat|nested] 38+ messages in thread

* bug#44858: [PATCH] Make byte-compiler warn about wide docstrings
  2021-09-25  1:07   ` Lars Ingebrigtsen
@ 2021-09-26 11:38     ` Stefan Kangas
  0 siblings, 0 replies; 38+ messages in thread
From: Stefan Kangas @ 2021-09-26 11:38 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 44858

tags 44858 fixed
close 44858 28.1
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> In this bug report, we have discussed several issues relating to warning
>> about too wide docstrings also for defun/lambda.  I believe I've solved
>> all remaining issues; I can now run "make bootstrap" with no warnings.
>
> Cool!

Now pushed to master (commits c78e16962e..c51b1c02db).

I'm consequently closing this bug report.





^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2021-09-26 11:38 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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).