unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
@ 2019-12-12 21:26 Damien Cassou
  2019-12-13  9:05 ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2019-12-12 21:26 UTC (permalink / raw)
  To: 38583
  Cc: Alex Branham, Paul Eggert, Ken Stevens, Lars Ingebrigtsen,
	Eric M. Ludlam

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-function-ispell-correct-p.patch --]
[-- Type: text/x-patch, Size: 3274 bytes --]

From aca6b5bc1571018efa3f05e972882738ddb3a271 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Fix-indentation-of-checkdoc-ispell-docstring-engine.patch --]
[-- Type: text/x-patch, Size: 4049 bytes --]

From 2e78244d3da3796dbf960ce5acbea82ba476bd12 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Cleanup-of-checkdoc-ispell-docstring-engine.patch --]
[-- Type: text/x-patch, Size: 3948 bytes --]

From b3044d5068fa17d1626a2345ffaadf81e6b4e86b Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Properly-initialize-ispell-in-checkdoc.patch --]
[-- Type: text/x-patch, Size: 1309 bytes --]

From 58eff3a38959479691ede0d42b841d6696d64a29 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0005-Add-unattended-spell-checking-to-checkdoc.patch --]
[-- Type: text/x-patch, Size: 7160 bytes --]

From a3ee18626be6ac564eef34128d50a006073aa130 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
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


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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-12 21:26 bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc Damien Cassou
@ 2019-12-13  9:05 ` Eli Zaretskii
  2019-12-13 19:53   ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-12-13  9:05 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

> From: Damien Cassou <damien@cassou.me>
> Cc: "Ken Stevens" <k.stevens@ieee.org>, "Eric M. Ludlam" <zappo@gnu.org>,
>  "Alex Branham" <alex.branham@gmail.com>, "Lars Ingebrigtsen"
>  <larsi@gnus.org>, "Paul Eggert" <eggert@cs.ucla.edu>, "Eli Zaretskii"
>  <eliz@gnu.org>
> Date: Thu, 12 Dec 2019 22:26:35 +0100
> 
> 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.

Thanks.

What happens if none of the spellers supported by ispell.el is
installed on the user's system?





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-13  9:05 ` Eli Zaretskii
@ 2019-12-13 19:53   ` Damien Cassou
  2019-12-13 19:59     ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2019-12-13 19:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

Eli Zaretskii <eliz@gnu.org> writes:
> What happens if none of the spellers supported by ispell.el is
> installed on the user's system?

Starting new Ispell process ispell with default dictionary... \ 
Buffer xxx has no process

Emacs exit status is 255 in this case.


-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-13 19:53   ` Damien Cassou
@ 2019-12-13 19:59     ` Eli Zaretskii
  2019-12-27 11:51       ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-12-13 19:59 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

> From: Damien Cassou <damien@cassou.me>
> Cc: bug-gnu-emacs@gnu.org, k.stevens@ieee.org, zappo@gnu.org, alex.branham@gmail.com, larsi@gnus.org, eggert@cs.ucla.edu
> Date: Fri, 13 Dec 2019 20:53:43 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > What happens if none of the spellers supported by ispell.el is
> > installed on the user's system?
> 
> Starting new Ispell process ispell with default dictionary... \ 
> Buffer xxx has no process
> 
> Emacs exit status is 255 in this case.

Can we check this up front, and display some user-friendly message to
the effect that spell-checking cannot be done?





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-13 19:59     ` Eli Zaretskii
@ 2019-12-27 11:51       ` Damien Cassou
  2019-12-27 14:08         ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2019-12-27 11:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

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

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:
>> Eli Zaretskii <eliz@gnu.org> writes:
>> > What happens if none of the spellers supported by ispell.el is
>> > installed on the user's system?
>> 
>> Starting new Ispell process ispell with default dictionary... \ 
>> Buffer xxx has no process
>> 
>> Emacs exit status is 255 in this case.
>
> Can we check this up front, and display some user-friendly message to
> the effect that spell-checking cannot be done?

Here is an additional patch for the series to answer your request.

If you believe the patch series is too large to be reviewed, I can split
it in several smaller ones.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0006-Improve-error-message-when-no-spellchecker-can-be-fo.patch --]
[-- Type: text/x-patch, Size: 1131 bytes --]

From 754a6ed8e9bbe7dd46e0d4595d8e46b76dc5b606 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
Date: Fri, 27 Dec 2019 12:48:55 +0100
Subject: [PATCH] Improve error message when no spellchecker can be found

* lisp/emacs-lisp/checkdoc.el: Throw error when (checkdoc-ispell-init)
fails configuring ispell.
---
 lisp/emacs-lisp/checkdoc.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 3e6542bc80..0269b89790 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -2127,6 +2127,9 @@ checkdoc-ispell-docstring-engine
                  (and checkdoc-autofix-flag
                       (not (eq checkdoc-autofix-flag 'never)))))
     (checkdoc-ispell-init)
+    (unless checkdoc-spellcheck-documentation-flag
+      ;; this happens when (checkdoc-ispell-init) can't start `ispell-program-name'
+      (user-error "No spellchecker installed: check the variable `ispell-program-name'."))
     (save-excursion
       (skip-chars-forward "^a-zA-Z")
       (let (word sym case-fold-search err word-beginning word-end)
-- 
2.24.1


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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-27 11:51       ` Damien Cassou
@ 2019-12-27 14:08         ` Eli Zaretskii
  2019-12-27 14:43           ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-12-27 14:08 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

> From: Damien Cassou <damien@cassou.me>
> Cc: bug-gnu-emacs@gnu.org, k.stevens@ieee.org, zappo@gnu.org,
>  alex.branham@gmail.com, larsi@gnus.org, eggert@cs.ucla.edu
> Date: Fri, 27 Dec 2019 12:51:42 +0100
> 
> > Can we check this up front, and display some user-friendly message to
> > the effect that spell-checking cannot be done?
> 
> Here is an additional patch for the series to answer your request.

You mean, this is in addition to the previous patches?  If so, I'd
prefer a single patch.

Thanks.





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-27 14:08         ` Eli Zaretskii
@ 2019-12-27 14:43           ` Damien Cassou
  2020-01-11 11:49             ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2019-12-27 14:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

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

Eli Zaretskii <eliz@gnu.org> writes:
> You mean, this is in addition to the previous patches?  If so, I'd
> prefer a single patch.

you want all the changes merged into a single patch? This seems much
harder to review but here it is.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-unattended-spell-checking-to-checkdoc.patch --]
[-- Type: text/x-patch, Size: 12781 bytes --]

From 71bf337ce06411f4951ec3a6e3b2a7c7f5dca55b Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
Date: Fri, 27 Dec 2019 15:35:52 +0100
Subject: [PATCH] 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/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/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-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.
(checkdoc-ispell-docstring-engine): Add optional argument TAKE-NOTES
to force reporting of spell-checking errors.  Throw error
when (checkdoc-ispell-init) fails configuring ispell.  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 | 117 +++++++++++++++++++++---------------
 lisp/textmodes/ispell.el    |  46 ++++++++++----
 2 files changed, 101 insertions(+), 62 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 6c40bdf632..0269b89790 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!
@@ -2100,58 +2106,69 @@ 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"))))
       (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."
-  (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
+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.
+             (or take-notes
+                 (and checkdoc-autofix-flag
+                      (not (eq checkdoc-autofix-flag 'never)))))
     (checkdoc-ispell-init)
+    (unless checkdoc-spellcheck-documentation-flag
+      ;; this happens when (checkdoc-ispell-init) can't start `ispell-program-name'
+      (user-error "No spellchecker installed: check the variable `ispell-program-name'."))
     (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))))
+      (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-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.
+              (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
+                  (let ((lk last-input-event))
+                    (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)
+                          (message "Continuing..."))))))))
+          (skip-chars-forward "^a-zA-Z"))
+        err))))
 
 ;;; Rogue space checking engine
 ;;
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.24.1


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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2019-12-27 14:43           ` Damien Cassou
@ 2020-01-11 11:49             ` Damien Cassou
  2020-01-11 12:26               ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2020-01-11 11:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

Damien Cassou <damien@cassou.me> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>> You mean, this is in addition to the previous patches?  If so, I'd
>> prefer a single patch.
>
> you want all the changes merged into a single patch? This seems much
> harder to review but here it is.

any news?

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-11 11:49             ` Damien Cassou
@ 2020-01-11 12:26               ` Eli Zaretskii
  2020-01-13  5:19                 ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2020-01-11 12:26 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

> From: Damien Cassou <damien@cassou.me>
> Cc: bug-gnu-emacs@gnu.org, k.stevens@ieee.org, zappo@gnu.org,
>  alex.branham@gmail.com, larsi@gnus.org, eggert@cs.ucla.edu
> Date: Sat, 11 Jan 2020 12:49:17 +0100
> 
> Damien Cassou <damien@cassou.me> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >> You mean, this is in addition to the previous patches?  If so, I'd
> >> prefer a single patch.
> >
> > you want all the changes merged into a single patch? This seems much
> > harder to review but here it is.
> 
> any news?

Sorry, it fell through the cracks.

However, I was about to push it now, but compiling the modified
version generates warnings:

    ELC      emacs-lisp/checkdoc.elc

  In checkdoc-ispell-docstring-engine:
  emacs-lisp/checkdoc.el:2161:47:Warning: reference to free variable
      `ispell-format-word-function'
  emacs-lisp/checkdoc.el:2162:62:Warning: reference to free variable
      `ispell-program-name'
  emacs-lisp/checkdoc.el:2163:42:Warning: reference to free variable
      `ispell-current-dictionary'

  In end of data:
  emacs-lisp/checkdoc.el:2714:1:Warning: the following functions are not known
      to be defined: ispell-set-spellchecker-params,
      ispell-accept-buffer-local-defs, ispell-correct-p

Could you please augment the patch so that these warnings are avoided?

Also, please mention the bug number in the commit log message.

TIA





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-11 12:26               ` Eli Zaretskii
@ 2020-01-13  5:19                 ` Damien Cassou
  2020-01-16 18:43                   ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2020-01-13  5:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

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

Eli Zaretskii <eliz@gnu.org> writes:
> Sorry, it fell through the cracks.


no problem.


> However, I was about to push it now, but compiling the modified
> version generates warnings: […]
>
> Could you please augment the patch so that these warnings are avoided?


fixed


> Also, please mention the bug number in the commit log message.


fixed

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-unattended-spell-checking-to-checkdoc.patch --]
[-- Type: text/x-patch, Size: 13297 bytes --]

From 46df39d2f7ddfe79dd55d8342ea20a2dc17ac31c Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
Date: Fri, 27 Dec 2019 15:35:52 +0100
Subject: [PATCH] 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.

Fixes: (bug#38583).

* 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-error-checking-word): New function.
(ispell-correct-p): New function.  Use `ispell--run-on-word` and
`ispell-error-checking-word`.
* 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-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.
(checkdoc-ispell-docstring-engine): Add optional argument TAKE-NOTES
to force reporting of spell-checking errors.  Throw error
when (checkdoc-ispell-init) fails configuring ispell.  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 | 118 +++++++++++++++++++++---------------
 lisp/textmodes/ispell.el    |  50 +++++++++++----
 2 files changed, 106 insertions(+), 62 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 93b9ffbe38..cbad6f0554 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!
@@ -2090,6 +2096,10 @@ checkdoc-sentencespace-region-engine
 ;;
 (defvar ispell-process)
 (declare-function ispell-buffer-local-words "ispell" ())
+(declare-function ispell-correct-p "ispell" ())
+(declare-function ispell-set-spellchecker-params "ispell" ())
+(declare-function ispell-accept-buffer-local-defs "ispell" ())
+(declare-function ispell-error-checking-word "ispell" (word))
 
 (defun checkdoc-ispell-init ()
   "Initialize Ispell process (default version) with Lisp words.
@@ -2100,58 +2110,66 @@ 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"))))
       (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."
-  (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
+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.
+             (or take-notes
+                 (and checkdoc-autofix-flag
+                      (not (eq checkdoc-autofix-flag 'never)))))
     (checkdoc-ispell-init)
+    (unless checkdoc-spellcheck-documentation-flag
+      ;; this happens when (checkdoc-ispell-init) can't start `ispell-program-name'
+      (user-error "No spellchecker installed: check the variable `ispell-program-name'."))
     (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))))
+      (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-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.
+              (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
+                  (let ((lk last-input-event))
+                    (if take-notes
+                        (progn
+                          (unless (ispell-correct-p)
+                            (checkdoc-create-error
+                             (ispell-error-checking-word word)
+                             word-beginning word-end)))
+                      (ispell-word nil t))
+                    (if (not (equal last-input-event lk))
+                        (progn
+                          (sit-for 0)
+                          (message "Continuing..."))))))))
+          (skip-chars-forward "^a-zA-Z"))
+        err))))
 
 ;;; Rogue space checking engine
 ;;
diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 53a4543308..c06f3915fa 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 word))
       (cond ((eq poss t)
 	     (or quietly
 		 (message "%s is correct"
@@ -2024,6 +2013,43 @@ 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-error-checking-word (word)
+  "Return a string describing that checking for WORD failed."
+  (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")))
+
+(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))
+           (word (car word-and-boundaries))
+           (poss (ispell--run-on-word word)))
+      (unless poss (error (ispell-error-checking-word word)))
+      (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.24.1


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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-13  5:19                 ` Damien Cassou
@ 2020-01-16 18:43                   ` Eli Zaretskii
  2020-01-16 21:07                     ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2020-01-16 18:43 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583-done, larsi, zappo

> From: Damien Cassou <damien@cassou.me>
> Cc: bug-gnu-emacs@gnu.org, k.stevens@ieee.org, zappo@gnu.org,
>  alex.branham@gmail.com, larsi@gnus.org, eggert@cs.ucla.edu
> Date: Mon, 13 Jan 2020 06:19:56 +0100
> 
> >From 46df39d2f7ddfe79dd55d8342ea20a2dc17ac31c Mon Sep 17 00:00:00 2001
> From: Damien Cassou <damien@cassou.me>
> 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.





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-16 18:43                   ` Eli Zaretskii
@ 2020-01-16 21:07                     ` Damien Cassou
  2020-01-17  7:59                       ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2020-01-16 21:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583-done, larsi, zappo

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:
> Thanks, I've now pushed this to the master branch.
>
> Sorry for the delays and thanks for persevering.

thank you very much. Is it too late for Emacs 27?

Best,

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-16 21:07                     ` Damien Cassou
@ 2020-01-17  7:59                       ` Eli Zaretskii
  2020-01-17  9:06                         ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2020-01-17  7:59 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583-done, larsi, zappo

> From: Damien Cassou <damien@cassou.me>
> Cc: 38583-done@debbugs.gnu.org, k.stevens@ieee.org, zappo@gnu.org,
>  alex.branham@gmail.com, larsi@gnus.org, eggert@cs.ucla.edu
> Date: Thu, 16 Jan 2020 22:07:33 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Thanks, I've now pushed this to the master branch.
> >
> > Sorry for the delays and thanks for persevering.
> 
> thank you very much. Is it too late for Emacs 27?

In general, yes.  But you can try convincing me that it is so
important to have in Emacs 27 that we should make an exception.  I
considered this, and concluded that it's a minor convenience feature.
But maybe I'm missing something?





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-17  7:59                       ` Eli Zaretskii
@ 2020-01-17  9:06                         ` Damien Cassou
  2020-01-17  9:49                           ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Cassou @ 2020-01-17  9:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583-done, larsi, zappo

Eli Zaretskii <eliz@gnu.org> writes:
> In general, yes.  But you can try convincing me that it is so
> important to have in Emacs 27 that we should make an exception.  I
> considered this, and concluded that it's a minor convenience feature.
> But maybe I'm missing something?

Integrating into Emacs 27 late in the process is risky because my code
could have introduced subtle bugs inside checkdoc and I would not like
to be responsible for a delay in the release.  That being said and
checkdoc being automatically activated by flycheck (which I believe many
people use), I think we will find bugs in my code quite fast if there
are any.  Moreover, I think that improving the overall quality of Emacs
code is important and every little step in this direction would be
helpful.

I'm not sure this is convincing enough but I would have tried :-D.

FYI, this work on checkdoc is part of a larger piece of work I've
started some years ago on improving the quality of Emacs packages.  Here
are some of my activities in this area:

- opening issues and sending patches on Emacs core regarding checkdoc
  (e.g., this one, bug#37063, bug#37034);

- giving very detailed reviews of packages sent to melpa (e.g.,
  https://github.com/melpa/melpa/pull/5451);

- rewriting melpa's CONTRIBUTING.org from scratch
  (https://github.com/melpa/melpa/blob/master/CONTRIBUTING.org);

- collaborating with emake's developer
  (https://github.com/vermiculus/emake.el/pull/8);

- writing makel (https://gitea.petton.fr/DamienCassou/makel);

- doing a conference on the subject at EmacsConf 2019
  (https://media.emacsconf.org/2019/16.html);

- prototyping a linting engine for everyone to easily write lint rules
  (https://gitlab.com/lintel/lintel).



-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-17  9:06                         ` Damien Cassou
@ 2020-01-17  9:49                           ` Eli Zaretskii
  2020-01-17 10:35                             ` Damien Cassou
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2020-01-17  9:49 UTC (permalink / raw)
  To: Damien Cassou; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

> From: Damien Cassou <damien@cassou.me>
> Cc: 38583-done@debbugs.gnu.org, k.stevens@ieee.org, zappo@gnu.org,
>  alex.branham@gmail.com, larsi@gnus.org, eggert@cs.ucla.edu
> Date: Fri, 17 Jan 2020 10:06:09 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > In general, yes.  But you can try convincing me that it is so
> > important to have in Emacs 27 that we should make an exception.  I
> > considered this, and concluded that it's a minor convenience feature.
> > But maybe I'm missing something?
> 
> Integrating into Emacs 27 late in the process is risky because my code
> could have introduced subtle bugs inside checkdoc and I would not like
> to be responsible for a delay in the release.  That being said and
> checkdoc being automatically activated by flycheck (which I believe many
> people use), I think we will find bugs in my code quite fast if there
> are any.  Moreover, I think that improving the overall quality of Emacs
> code is important and every little step in this direction would be
> helpful.
> 
> I'm not sure this is convincing enough but I would have tried :-D.

OK, I've cherry-picked this to the release branch, mainly because it
narrowly missed the branching due to delays in our review of the
patch.

> FYI, this work on checkdoc is part of a larger piece of work I've
> started some years ago on improving the quality of Emacs packages.  Here
> are some of my activities in this area:

Thank you for your work on this.





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

* bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc
  2020-01-17  9:49                           ` Eli Zaretskii
@ 2020-01-17 10:35                             ` Damien Cassou
  0 siblings, 0 replies; 16+ messages in thread
From: Damien Cassou @ 2020-01-17 10:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alex.branham, eggert, k.stevens, 38583, zappo, larsi

Eli Zaretskii <eliz@gnu.org> writes:
> OK, I've cherry-picked this to the release branch, mainly because it
> narrowly missed the branching due to delays in our review of the
> patch.

thank you!

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill





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

end of thread, other threads:[~2020-01-17 10:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12 21:26 bug#38583: [PATCH] 27.0.50; Add unattended spell-checking to checkdoc Damien Cassou
2019-12-13  9:05 ` Eli Zaretskii
2019-12-13 19:53   ` Damien Cassou
2019-12-13 19:59     ` Eli Zaretskii
2019-12-27 11:51       ` Damien Cassou
2019-12-27 14:08         ` Eli Zaretskii
2019-12-27 14:43           ` Damien Cassou
2020-01-11 11:49             ` Damien Cassou
2020-01-11 12:26               ` Eli Zaretskii
2020-01-13  5:19                 ` Damien Cassou
2020-01-16 18:43                   ` Eli Zaretskii
2020-01-16 21:07                     ` Damien Cassou
2020-01-17  7:59                       ` Eli Zaretskii
2020-01-17  9:06                         ` Damien Cassou
2020-01-17  9:49                           ` Eli Zaretskii
2020-01-17 10:35                             ` Damien Cassou

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