Subject: bug#38583: closed (Re: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 16 Jan 2020 18:45:02 +0000

Your bug report

#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
38583: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=38583
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
From: Eli Zaretskii <[email-masked]>
To: Damien Cassou <[email-masked]>
Cc: [email-masked], [email-masked], [email-masked], [email-masked], [email-masked], [email-masked]
Date: Thu, 16 Jan 2020 20:43:55 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> From: Damien Cassou <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked],
>  [email-masked], [email-masked], [email-masked]
> Date: Mon, 13 Jan 2020 06:19:56 +0100
> 
> >From 46df39d2f7ddfe79dd55d8342ea20a2dc17ac31c Mon Sep 17 00:00:00 2001
> From: Damien Cassou <[email-masked]>
> Date: Fri, 27 Dec 2019 15:35:52 +0100
> Subject: [PATCH] Add unattended spell-checking to checkdoc

Thanks, I've now pushed this to the master branch.

Sorry for the delays and thanks for persevering.

Subject: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
From: Damien Cassou <[email-masked]>
To: [email-masked]
Cc: Alex Branham <[email-masked]>, Paul Eggert <[email-masked]>, Ken Stevens <[email-masked]>, Lars Ingebrigtsen <[email-masked]>, "Eric M. Ludlam" <[email-masked]>, Eli Zaretskii <[email-masked]>
Date: Thu, 12 Dec 2019 22:26:35 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

This series of patches makes checkdoc capable of spell-checking even
when the user isn't using it interactively. When TAKE-NOTES is non-nil,
checkdoc will run spell-checking (with ispell) and report spelling
mistakes.

Damien Cassou (5):
  Add function `ispell-correct-p`
  Fix indentation of `checkdoc-ispell-docstring-engine`
  Cleanup of `checkdoc-ispell-docstring-engine`
  Properly initialize ispell in checkdoc
  Add unattended spell-checking to checkdoc

 lisp/emacs-lisp/checkdoc.el | 114 ++++++++++++++++++++----------------
 lisp/textmodes/ispell.el    |  46 +++++++++++----
 2 files changed, 98 insertions(+), 62 deletions(-)

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill
>From aca6b5bc1571018efa3f05e972882738ddb3a271 Mon Sep 17 00:00:00 2001
From: Damien Cassou <[email-masked]>
Date: Sat, 7 Dec 2019 16:01:13 +0100
Subject: [PATCH 1/5] Add function `ispell-correct-p`

* lisp/textmodes/ispell.el (ispell-word): Extract part of it to
`ispell--run-on-word`.
(ispell--run-on-word): New function, extracted from `ispell-word`.
(ispell-correct-p): New function. Use `ispell--run-on-word`.
---
 lisp/textmodes/ispell.el | 46 +++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index dd1eeb4530..7ec445211d 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -1951,18 +1951,7 @@ ispell-word
       (or quietly
 	  (message "Checking spelling of %s..."
 		   (funcall ispell-format-word-function word)))
-      (ispell-send-string "%\n")	; put in verbose mode
-      (ispell-send-string (concat "^" word "\n"))
-      ;; wait until ispell has processed word
-      (while (progn
-	       (ispell-accept-output)
-	       (not (string= "" (car ispell-filter)))))
-      ;;(ispell-send-string "!\n") ;back to terse mode.
-      (setq ispell-filter (cdr ispell-filter)) ; remove extra \n
-      (if (and ispell-filter (listp ispell-filter))
-	  (if (> (length ispell-filter) 1)
-	      (error "Ispell and its process have different character maps")
-	    (setq poss (ispell-parse-output (car ispell-filter)))))
+      (setq poss (ispell--run-on-word))
       (cond ((eq poss t)
 	     (or quietly
 		 (message "%s is correct"
@@ -2024,6 +2013,39 @@ ispell-word
       (goto-char cursor-location)	; return to original location
       replace))))
 
+(defun ispell--run-on-word (word)
+  "Run ispell on WORD."
+  (ispell-send-string "%\n")	; put in verbose mode
+  (ispell-send-string (concat "^" word "\n"))
+  ;; wait until ispell has processed word
+  (while (progn
+           (ispell-accept-output)
+           (not (string= "" (car ispell-filter)))))
+  (setq ispell-filter (cdr ispell-filter))
+  (when (and ispell-filter (listp ispell-filter))
+    (if (> (length ispell-filter) 1)
+        (error "Ispell and its processs have different character maps: %s" ispell-filter)
+      (ispell-parse-output (car ispell-filter)))))
+
+(defun ispell-correct-p (&optional following)
+  "Return t if the word at point is correct. Nil otherwise.
+
+If optional argument FOLLOWING is non-nil then the following
+word (rather than preceding) is checked when the cursor is not
+over a word."
+  (save-excursion
+    ; reset ispell-filter so it only contains the result of
+    ; spell-checking the current-word:
+    (setq ispell-filter nil)
+    (let* ((word-and-boundaries (ispell-get-word following))
+           (poss (ispell--run-on-word (car word-and-boundaries))))
+      (unless poss
+        (error "Error checking word %s using %s with %s dictionary"
+               (funcall ispell-format-word-function word)
+               (file-name-nondirectory ispell-program-name)
+               (or ispell-current-dictionary "default")))
+      (or (eq poss t)
+          (stringp poss)))))
 
 (defun ispell-get-word (following &optional extra-otherchars)
   "Return the word for spell-checking according to ispell syntax.
-- 
2.23.0

>From 2e78244d3da3796dbf960ce5acbea82ba476bd12 Mon Sep 17 00:00:00 2001
From: Damien Cassou <[email-masked]>
Date: Sat, 7 Dec 2019 17:37:46 +0100
Subject: [PATCH 2/5] Fix indentation of `checkdoc-ispell-docstring-engine`

* lisp/emacs-lisp/checkdoc.el (checkdoc-ispell-docstring-engine):
Replace tabs with white spaces.
---
 lisp/emacs-lisp/checkdoc.el | 72 ++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 6c40bdf632..b4c20778bf 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -2111,47 +2111,47 @@ checkdoc-ispell-docstring-engine
 Since Ispell isn't Lisp-smart, we must pre-process the doc string
 before using the Ispell engine on it."
   (if (or (not checkdoc-spellcheck-documentation-flag)
-	  ;; If the user wants no questions or fixing, then we must
-	  ;; disable spell checking as not useful.
-	  (not checkdoc-autofix-flag)
-	  (eq checkdoc-autofix-flag 'never))
+          ;; If the user wants no questions or fixing, then we must
+          ;; disable spell checking as not useful.
+          (not checkdoc-autofix-flag)
+          (eq checkdoc-autofix-flag 'never))
       nil
     (checkdoc-ispell-init)
     (save-excursion
       (skip-chars-forward "^a-zA-Z")
       (let ((word nil) (sym nil) (case-fold-search nil) (err nil))
-	(while (and (not err) (< (point) end))
-	  (if (save-excursion (forward-char -1) (looking-at "[('`]"))
-	      ;; Skip lists describing meta-syntax, or bound variables
-	      (forward-sexp 1)
-	    (setq word (buffer-substring-no-properties
-			(point) (progn
-				  (skip-chars-forward "a-zA-Z-")
-				  (point)))
-		  sym (intern-soft word))
-	    (if (and sym (or (boundp sym) (fboundp sym)))
-		;; This is probably repetitive in most cases, but not always.
-		nil
-	      ;; Find out how we spell-check this word.
-	      (if (or
-		   ;; All caps w/ option th, or s tacked on the end
-		   ;; for pluralization or number.
-		   (string-match "^[A-Z][A-Z]+\\(s\\|th\\)?$" word)
-		   (looking-at "}") ; a keymap expression
-		   )
-		  nil
-		(save-excursion
-		  (if (not (eq checkdoc-autofix-flag 'never))
-		      (let ((lk last-input-event))
-			(ispell-word nil t)
-			(if (not (equal last-input-event lk))
-			    (progn
-			      (sit-for 0)
-			      (message "Continuing..."))))
-		    ;; Nothing here.
-		    )))))
-	  (skip-chars-forward "^a-zA-Z"))
-	err))))
+        (while (and (not err) (< (point) end))
+          (if (save-excursion (forward-char -1) (looking-at "[('`]"))
+              ;; Skip lists describing meta-syntax, or bound variables
+              (forward-sexp 1)
+            (setq word (buffer-substring-no-properties
+                        (point) (progn
+                                  (skip-chars-forward "a-zA-Z-")
+                                  (point)))
+                  sym (intern-soft word))
+            (if (and sym (or (boundp sym) (fboundp sym)))
+                ;; This is probably repetitive in most cases, but not always.
+                nil
+              ;; Find out how we spell-check this word.
+              (if (or
+                   ;; All caps w/ option th, or s tacked on the end
+                   ;; for pluralization or number.
+                   (string-match "^[A-Z][A-Z]+\\(s\\|th\\)?$" word)
+                   (looking-at "}") ; a keymap expression
+                   )
+                  nil
+                (save-excursion
+                  (if (not (eq checkdoc-autofix-flag 'never))
+                      (let ((lk last-input-event))
+                        (ispell-word nil t)
+                        (if (not (equal last-input-event lk))
+                            (progn
+                              (sit-for 0)
+                              (message "Continuing..."))))
+                    ;; Nothing here.
+                    )))))
+          (skip-chars-forward "^a-zA-Z"))
+        err))))
 
 ;;; Rogue space checking engine
 ;;
-- 
2.23.0

>From b3044d5068fa17d1626a2345ffaadf81e6b4e86b Mon Sep 17 00:00:00 2001
From: Damien Cassou <[email-masked]>
Date: Sat, 7 Dec 2019 17:40:35 +0100
Subject: [PATCH 3/5] Cleanup of `checkdoc-ispell-docstring-engine`

* lisp/emacs-lisp/checkdoc.el (checkdoc-ispell-docstring-engine):
Cleanup.  Replace a few (if cond nil body) with (unless cond
body). Replace (let ((var nil)))
with (let (var)). Replace (if (not (eq checkdoc-autofix-flag 'never))
body) with just body because `checkdoc-autofix-flag` is checked at the
beginning of the function.
---
 lisp/emacs-lisp/checkdoc.el | 45 ++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index b4c20778bf..a28bf414bd 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -2110,16 +2110,15 @@ checkdoc-ispell-docstring-engine
   "Run the Ispell tools on the doc string between point and END.
 Since Ispell isn't Lisp-smart, we must pre-process the doc string
 before using the Ispell engine on it."
-  (if (or (not checkdoc-spellcheck-documentation-flag)
-          ;; If the user wants no questions or fixing, then we must
-          ;; disable spell checking as not useful.
-          (not checkdoc-autofix-flag)
-          (eq checkdoc-autofix-flag 'never))
-      nil
+  (when (and checkdoc-spellcheck-documentation-flag
+             ;; If the user wants no questions or fixing, then we must
+             ;; disable spell checking as not useful.
+             checkdoc-autofix-flag
+             (not (eq checkdoc-autofix-flag 'never)))
     (checkdoc-ispell-init)
     (save-excursion
       (skip-chars-forward "^a-zA-Z")
-      (let ((word nil) (sym nil) (case-fold-search nil) (err nil))
+      (let (word sym case-fold-search err)
         (while (and (not err) (< (point) end))
           (if (save-excursion (forward-char -1) (looking-at "[('`]"))
               ;; Skip lists describing meta-syntax, or bound variables
@@ -2129,27 +2128,21 @@ checkdoc-ispell-docstring-engine
                                   (skip-chars-forward "a-zA-Z-")
                                   (point)))
                   sym (intern-soft word))
-            (if (and sym (or (boundp sym) (fboundp sym)))
-                ;; This is probably repetitive in most cases, but not always.
-                nil
+            (unless (and sym (or (boundp sym) (fboundp sym)))
               ;; Find out how we spell-check this word.
-              (if (or
-                   ;; All caps w/ option th, or s tacked on the end
-                   ;; for pluralization or number.
-                   (string-match "^[A-Z][A-Z]+\\(s\\|th\\)?$" word)
-                   (looking-at "}") ; a keymap expression
-                   )
-                  nil
+              (unless (or
+                       ;; All caps w/ option th, or s tacked on the end
+                       ;; for pluralization or number.
+                       (string-match "^[A-Z][A-Z]+\\(s\\|th\\)?$" word)
+                       (looking-at "}") ; a keymap expression
+                       )
                 (save-excursion
-                  (if (not (eq checkdoc-autofix-flag 'never))
-                      (let ((lk last-input-event))
-                        (ispell-word nil t)
-                        (if (not (equal last-input-event lk))
-                            (progn
-                              (sit-for 0)
-                              (message "Continuing..."))))
-                    ;; Nothing here.
-                    )))))
+                  (let ((lk last-input-event))
+                    (ispell-word nil t)
+                    (if (not (equal last-input-event lk))
+                        (progn
+                          (sit-for 0)
+                          (message "Continuing..."))))))))
           (skip-chars-forward "^a-zA-Z"))
         err))))
 
-- 
2.23.0

>From 58eff3a38959479691ede0d42b841d6696d64a29 Mon Sep 17 00:00:00 2001
From: Damien Cassou <[email-masked]>
Date: Thu, 12 Dec 2019 21:38:08 +0100
Subject: [PATCH 4/5] Properly initialize ispell in checkdoc

* lisp/emacs-lisp/checkdoc.el (checkdoc-ispell-init): Call
`ispell-set-spellchecker-params` and
`ispell-accept-buffer-local-defs`. These calls are required to
properly use ispell. The problem went unnoticed until now because
checkdoc was only using ispell through the high-level command
`ispell-word` which takes care of all the initialization for the user.
---
 lisp/emacs-lisp/checkdoc.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index a28bf414bd..075817a252 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -2100,7 +2100,8 @@ checkdoc-ispell-init
   (unless ispell-process
     (condition-case nil
 	(progn
-	  (ispell-buffer-local-words)
+          (ispell-set-spellchecker-params)    ; Initialize variables and dicts alists
+          (ispell-accept-buffer-local-defs)   ; use the correct dictionary
 	  ;; This code copied in part from ispell.el Emacs 19.34
 	  (dolist (w checkdoc-ispell-lisp-words)
 	    (process-send-string ispell-process (concat "@" w "\n"))))
-- 
2.23.0

>From a3ee18626be6ac564eef34128d50a006073aa130 Mon Sep 17 00:00:00 2001
From: Damien Cassou <[email-masked]>
Date: Thu, 12 Dec 2019 20:27:05 +0100
Subject: [PATCH 5/5] Add unattended spell-checking to checkdoc

This commit makes checkdoc capable of spell-checking even when the
user isn't using it interactively. When TAKE-NOTES is non-nil,
checkdoc will run spell-checking (with ispell) and report spelling
mistakes.

* lisp/emacs-lisp/checkdoc.el (checkdoc-current-buffer): Pass
TAKE-NOTES to `checkdoc-start`.
(checkdoc-continue): Pass TAKE-NOTES to `checkdoc-this-string-valid`.
(checkdoc-this-string-valid): Add optional argument TAKE-NOTES and
pass it to `checkdoc-this-string-valid-engine`.
(checkdoc-this-string-valid-engine): Add optional argument TAKE-NOTES and
pass it to `checkdoc-ispell-docstring-engine`.
(checkdoc-ispell-docstring-engine): Add optional argument TAKE-NOTES
to force reporting of spell-checking errors.
---
 lisp/emacs-lisp/checkdoc.el | 56 +++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 075817a252..3e6542bc80 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -849,7 +849,7 @@ checkdoc-current-buffer
     ;; every test is responsible for returning the cursor.
     (or (and buffer-file-name ;; only check comments in a file
 	     (checkdoc-comments))
-	(checkdoc-start)
+	(checkdoc-start take-notes)
 	(checkdoc-message-text)
 	(checkdoc-rogue-spaces)
         (when checkdoc-package-keywords-flag
@@ -902,7 +902,7 @@ checkdoc-continue
       ;; the user is navigating down through the buffer.
       (while (and (not wrong) (checkdoc-next-docstring))
 	;; OK, let's look at the doc string.
-	(setq msg (checkdoc-this-string-valid))
+	(setq msg (checkdoc-this-string-valid take-notes))
 	(if msg (setq wrong (point)))))
     (if wrong
 	(progn
@@ -1284,12 +1284,15 @@ checkdoc-create-common-verbs-regexp
 
 ;;; Checking engines
 ;;
-(defun checkdoc-this-string-valid ()
+(defun checkdoc-this-string-valid (&optional take-notes)
   "Return a message string if the current doc string is invalid.
 Check for style only, such as the first line always being a complete
 sentence, whitespace restrictions, and making sure there are no
 hard-coded key-codes such as C-[char] or mouse-[number] in the comment.
-See the style guide in the Emacs Lisp manual for more details."
+See the style guide in the Emacs Lisp manual for more details.
+
+With a non-nil TAKE-NOTES, store all errors found in a warnings
+buffer, otherwise stop after the first error."
 
   ;; Jump over comments between the last object and the doc string
   (while (looking-at "[ \t\n]*;")
@@ -1366,13 +1369,16 @@ checkdoc-this-string-valid
 	      (point) (+ (point) 1) t)))))
     (if (and (not err) (= (following-char) ?\"))
         (with-syntax-table checkdoc-syntax-table
-          (checkdoc-this-string-valid-engine fp))
+          (checkdoc-this-string-valid-engine fp take-notes))
       err)))
 
-(defun checkdoc-this-string-valid-engine (fp)
+(defun checkdoc-this-string-valid-engine (fp &optional take-notes)
   "Return an error list or string if the current doc string is invalid.
 Depends on `checkdoc-this-string-valid' to reset the syntax table so that
-regexp short cuts work.  FP is the function defun information."
+regexp short cuts work.  FP is the function defun information.
+
+With a non-nil TAKE-NOTES, store all errors found in a warnings
+buffer, otherwise stop after the first error."
   (let ((case-fold-search nil)
 	;; Use a marker so if an early check modifies the text,
 	;; we won't accidentally lose our place.  This could cause
@@ -1864,7 +1870,7 @@ checkdoc-this-string-valid-engine
      ;; Make sure the doc string has correctly spelled English words
      ;; in it.  This function is extracted due to its complexity,
      ;; and reliance on the Ispell program.
-     (checkdoc-ispell-docstring-engine e)
+     (checkdoc-ispell-docstring-engine e take-notes)
      ;; User supplied checks
      (save-excursion (checkdoc-run-hooks 'checkdoc-style-functions fp e))
      ;; Done!
@@ -2107,27 +2113,32 @@ checkdoc-ispell-init
 	    (process-send-string ispell-process (concat "@" w "\n"))))
       (error (setq checkdoc-spellcheck-documentation-flag nil)))))
 
-(defun checkdoc-ispell-docstring-engine (end)
+(defun checkdoc-ispell-docstring-engine (end &optional take-notes)
   "Run the Ispell tools on the doc string between point and END.
 Since Ispell isn't Lisp-smart, we must pre-process the doc string
-before using the Ispell engine on it."
+before using the Ispell engine on it.
+
+With a non-nil TAKE-NOTES, store all errors found in a warnings
+buffer, otherwise stop after the first error."
   (when (and checkdoc-spellcheck-documentation-flag
              ;; If the user wants no questions or fixing, then we must
              ;; disable spell checking as not useful.
-             checkdoc-autofix-flag
-             (not (eq checkdoc-autofix-flag 'never)))
+             (or take-notes
+                 (and checkdoc-autofix-flag
+                      (not (eq checkdoc-autofix-flag 'never)))))
     (checkdoc-ispell-init)
     (save-excursion
       (skip-chars-forward "^a-zA-Z")
-      (let (word sym case-fold-search err)
+      (let (word sym case-fold-search err word-beginning word-end)
         (while (and (not err) (< (point) end))
           (if (save-excursion (forward-char -1) (looking-at "[('`]"))
               ;; Skip lists describing meta-syntax, or bound variables
               (forward-sexp 1)
-            (setq word (buffer-substring-no-properties
-                        (point) (progn
-                                  (skip-chars-forward "a-zA-Z-")
-                                  (point)))
+            (setq word-beginning (point)
+                  word-end (progn
+                             (skip-chars-forward "a-zA-Z-")
+                             (point))
+                  word (buffer-substring-no-properties word-beginning word-end)
                   sym (intern-soft word))
             (unless (and sym (or (boundp sym) (fboundp sym)))
               ;; Find out how we spell-check this word.
@@ -2139,7 +2150,16 @@ checkdoc-ispell-docstring-engine
                        )
                 (save-excursion
                   (let ((lk last-input-event))
-                    (ispell-word nil t)
+                    (if take-notes
+                        (progn
+                          (unless (ispell-correct-p)
+                            (checkdoc-create-error
+                             (format "Error checking word %s using %s with %s dictionary"
+		                     (funcall ispell-format-word-function word)
+		                     (file-name-nondirectory ispell-program-name)
+		                     (or ispell-current-dictionary "default"))
+                             word-beginning word-end)))
+                      (ispell-word nil t))
                     (if (not (equal last-input-event lk))
                         (progn
                           (sit-for 0)
-- 
2.23.0