Ispell: Skipping part of text in texinfo-mode

Ispell: Skipping part of text in texinfo-mode

[Arash Esbati]

Hi all,

I wanted to spellcheck a .texi file with 'M-x ispell RET' (running
hunspell) and it occurred to me that skipping part of text like in .tex
files isn't available OOTB.  In my experiment, ispell didn't ignore
anything and checked every Texinfo macro.

Having no clue about ispell.el, I managed to ease the pain with this
change:

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 99f9e10a5a8..af12c99ce0a 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -1744,6 +1744,36 @@ ispell-html-skip-alists
  (e.g. \"<[tT][tT]/\" and \"<[^ \\t\\n>]\").")
 (put 'ispell-html-skip-alists 'risky-local-variable t)

+;;;###autoload
+(defvar ispell-texinfo-skip-alists
+  (let ((single-arg (regexp-opt '("acronym" "cite" "code" "command" "env"
+                                  "file" "key" "option" "url" "var")
+                                "@\\(?:"))
+        (skip-line (regexp-opt '("deffn" "defun" "defopt" "defvar"
+                                 "findex" "vindex" "kindex" "cindex"
+                                 "end "
+                                 "ifclear" "ifset" "include"
+                                 "setfilename" )
+                               "^@\\(?:")))
+    `(;; Macros with a single arg
+      (,single-arg ispell-tex-arg-end)
+      ;; Special arrangement for things like @kbd{C-(}
+      ("@\\(?:kbd\\|samp\\){" . "[^@]}")
+      ;; Envs to skip entirely
+      ("^@\\(?:\\(?:small\\)?example\\|lisp\\|verbatim\\)" .
+       "^@end \\(?:\\(?:small\\)?example\\|lisp\\|verbatim\\)")
+      ;; macros w/o arg
+      (,skip-line forward-line)
+      ;; This is for the first line:
+      ("\\\\input" forward-line)
+      ;; All other macros
+      ("@[a-zA-Z]+")))
+  "Lists of start and end keys to skip in texinfo buffers.
+Same format as `ispell-skip-region-alist'.
+Note - Match for general texinfo macros like @foo must come last, e.g.:
+  (\"@[a-zA-Z]+\").")
+(put 'ispell-texinfo-skip-alists 'risky-local-variable t)
+
 (defvar-local ispell-local-pdict ispell-personal-dictionary
   "A buffer local variable containing the current personal dictionary.
 If non-nil, the value must be a string, which is a file name.
@@ -1799,11 +1829,11 @@ ispell-accept-output
 and pass it the output of the last Ispell invocation."
   (if ispell-async-processp
       (if (process-live-p ispell-process)
-       (let ((timeout (if timeout-msecs
-			  (+ (or timeout-secs 0) (/ timeout-msecs 1000.0))
-		        timeout-secs)))
-	 (accept-process-output ispell-process timeout))
-       (error "No Ispell process to read output from!"))
+          (let ((timeout (if timeout-msecs
+			     (+ (or timeout-secs 0) (/ timeout-msecs 1000.0))
+		           timeout-secs)))
+	    (accept-process-output ispell-process timeout))
+        (error "No Ispell process to read output from!"))
     (if (null ispell-process)
 	(error "No Ispell process to read output from!")
       (let ((buf ispell-output-buffer)
@@ -3277,6 +3307,8 @@ ispell-begin-skip-region-regexp
           ;; tex
           (if (eq ispell-parser 'tex)
               (ispell-begin-tex-skip-regexp))
+          (if (eq ispell-parser 'texinfo)
+              (ispell-begin-skip-region ispell-texinfo-skip-alists))
           ;; html stuff
           (if ispell-skip-html
               (ispell-begin-skip-region ispell-html-skip-alists))
@@ -3341,6 +3373,9 @@ ispell-skip-region-list
 	      skip-alist (append (car ispell-tex-skip-alists)
 				 (car (cdr ispell-tex-skip-alists))
 				 skip-alist)))
+    (if (eq ispell-parser 'texinfo)
+        (setq case-fold-search nil
+              skip-alist (append ispell-texinfo-skip-alists skip-alist)))
     (if ispell-skip-html
 	(setq skip-alist (append ispell-html-skip-alists skip-alist)))
     (if (and ispell-checking-message
--8<---------------cut here---------------end--------------->8---

I presume there are some .texi writers here and I'd like to know how
others handle this.  Is the change above useful for ispell.el?
Otherwise I can put it in my init file and add it locally to
`ispell-skip-region-alist' in `texinfo-mode'.

Best, Arash

Re: Ispell: Skipping part of text in texinfo-mode

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Date: Thu, 29 Aug 2024 18:25:02 +0200
> 
> I wanted to spellcheck a .texi file with 'M-x ispell RET' (running
> hunspell) and it occurred to me that skipping part of text like in .tex
> files isn't available OOTB.  In my experiment, ispell didn't ignore
> anything and checked every Texinfo macro.
> 
> Having no clue about ispell.el, I managed to ease the pain with this
> change:

Thanks.  But there be dragons...

> +;;;###autoload
> +(defvar ispell-texinfo-skip-alists
> +  (let ((single-arg (regexp-opt '("acronym" "cite" "code" "command" "env"
> +                                  "file" "key" "option" "url" "var")

There are many more, no?  @samp, @sc, @item, @itemx, to mention just a
few.

> +        (skip-line (regexp-opt '("deffn" "defun" "defopt" "defvar"
> +                                 "findex" "vindex" "kindex" "cindex"
> +                                 "end "

Likewise here: @defmac, @defspec, and many others (see the Texinfo
manual).

> +      ;; Special arrangement for things like @kbd{C-(}
> +      ("@\\(?:kbd\\|samp\\){" . "[^@]}")

What about @kbd in general? and @key?

> +      ;; Envs to skip entirely
> +      ("^@\\(?:\\(?:small\\)?example\\|lisp\\|verbatim\\)" .
> +       "^@end \\(?:\\(?:small\\)?example\\|lisp\\|verbatim\\)")

Why "entirely"?  They frequently include comments, not just code.

> I presume there are some .texi writers here and I'd like to know how
> others handle this.

Very simple: press 'a' once on each directive you never want to look
at, and leave the rest for judgment calls.

> Is the change above useful for ispell.el?

To some degree, I guess.  I think we should take only the bare
minimum, the ones that we want _never_ to be marked as mis-spellings,
and leave the rest to the users.

Re: Ispell: Skipping part of text in texinfo-mode

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  But there be dragons...

I thought so, and I was hoping that someone could guide me through that
unknown water :-)

> There are many more, no?  @samp, @sc, @item, @itemx, to mention just a
> few.
> [...]
> Likewise here: @defmac, @defspec, and many others (see the Texinfo
> manual).

Yes, at some point, I have to go through the @-Command List section in
the Texinfo manual.

> What about @kbd in general? and @key?

See below, the first regexp approach was fragile anyway, I think I have
a better solution now.

> Why "entirely"?  They frequently include comments, not just code.

I think the environments contain more code to be ignored than comments
to check, but that's only me.

> Very simple: press 'a' once on each directive you never want to look
> at, and leave the rest for judgment calls.

Thanks, this was also my approach until now, but I didn't find it
satisfactory.

> To some degree, I guess.  I think we should take only the bare
> minimum, the ones that we want _never_ to be marked as mis-spellings,
> and leave the rest to the users.

We can discuss which macros to include and which to drop once the code
is working.  I'm currently facing the issue that when I open a .texi
file, eval (setq ispell-parser 'texinfo) and do M-x ispell RET, also
comments in the file are checked, the value of `ispell-check-comments'
is nil, though.  Do you have an idea what's going wrong?  I don't get
this in a .tex buffer.

Here the latest patch:

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 99f9e10a5a8..8da5681af1c 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -1744,6 +1744,37 @@ ispell-html-skip-alists
  (e.g. \"<[tT][tT]/\" and \"<[^ \\t\\n>]\").")
 (put 'ispell-html-skip-alists 'risky-local-variable t)

+;;;###autoload
+(defvar ispell-texinfo-skip-alists
+  (let ((single-arg (regexp-opt '("acronym" "cite" "code" "command" "env"
+                                  "file" "kbd" "key" "option" "samp"
+                                  "url" "var")
+                                "@\\(?:"))
+        (skip-line (regexp-opt '("deffn" "deffnx" "defmac" "defun"
+                                 "defopt" "defspec" "defvar"
+                                 "findex" "vindex" "kindex" "cindex"
+                                 "end "
+                                 "ifclear" "ifset" "include"
+                                 "item" "itemx"
+                                 "setfilename")
+                               "^@\\(?:")))
+    `(;; Macros with a single arg
+      (,single-arg ispell-texinfo-arg-end)
+      ;; Envs to skip entirely
+      ("^@\\(?:\\(?:small\\)?example\\|lisp\\|verbatim\\)" .
+       "^@end \\(?:\\(?:small\\)?example\\|lisp\\|verbatim\\)")
+      ;; macros w/o arg
+      (,skip-line forward-line)
+      ;; This is for the first line:
+      ("\\\\input" forward-line)
+      ;; All other macros
+      ("@[a-zA-Z]+")))
+  "Lists of start and end keys to skip in texinfo buffers.
+Same format as `ispell-skip-region-alist'.
+Note - Match for general texinfo macros like @foo must come last, e.g.:
+  (\"@[a-zA-Z]+\").")
+(put 'ispell-texinfo-skip-alists 'risky-local-variable t)
+
 (defvar-local ispell-local-pdict ispell-personal-dictionary
   "A buffer local variable containing the current personal dictionary.
 If non-nil, the value must be a string, which is a file name.
@@ -1799,11 +1830,11 @@ ispell-accept-output
 and pass it the output of the last Ispell invocation."
   (if ispell-async-processp
       (if (process-live-p ispell-process)
-       (let ((timeout (if timeout-msecs
-			  (+ (or timeout-secs 0) (/ timeout-msecs 1000.0))
-		        timeout-secs)))
-	 (accept-process-output ispell-process timeout))
-       (error "No Ispell process to read output from!"))
+          (let ((timeout (if timeout-msecs
+			     (+ (or timeout-secs 0) (/ timeout-msecs 1000.0))
+		           timeout-secs)))
+	    (accept-process-output ispell-process timeout))
+        (error "No Ispell process to read output from!"))
     (if (null ispell-process)
 	(error "No Ispell process to read output from!")
       (let ((buf ispell-output-buffer)
@@ -3277,6 +3308,8 @@ ispell-begin-skip-region-regexp
           ;; tex
           (if (eq ispell-parser 'tex)
               (ispell-begin-tex-skip-regexp))
+          (if (eq ispell-parser 'texinfo)
+              (ispell-begin-skip-region ispell-texinfo-skip-alists))
           ;; html stuff
           (if ispell-skip-html
               (ispell-begin-skip-region ispell-html-skip-alists))
@@ -3341,6 +3374,9 @@ ispell-skip-region-list
 	      skip-alist (append (car ispell-tex-skip-alists)
 				 (car (cdr ispell-tex-skip-alists))
 				 skip-alist)))
+    (if (eq ispell-parser 'texinfo)
+        (setq case-fold-search nil
+              skip-alist (append ispell-texinfo-skip-alists skip-alist)))
     (if ispell-skip-html
 	(setq skip-alist (append ispell-html-skip-alists skip-alist)))
     (if (and ispell-checking-message
@@ -3360,6 +3396,30 @@ ispell-tex-arg-end
      (beep)
      (sit-for 2))))

+(defvar ispell--texinfo-arg-end-syntax-table
+  (let ((table (make-syntax-table))
+        (chars '((?\f . ">") (?\n . ">")
+                 (?\" . " ") (?\@ . "/")
+                 (?\( . " ") (?\) . " ")
+                 (?\[ . " ") (?\] . " ")
+                 (?\< . " ") (?\> . " ")
+                 (?\\ . " ")
+                 (?\{ . "(}") (?\} . "){"))))
+    (dolist (elt chars)
+      (modify-syntax-entry (car elt) (cdr elt) table))
+    table))
+
+(defun ispell-texinfo-arg-end (&optional arg)
+  "Skip across ARG number of braces."
+  (condition-case nil
+      (with-syntax-table ispell--texinfo-arg-end-syntax-table
+        (unless (= (following-char) ?\{)
+          (skip-chars-forward "^{"))
+        (forward-sexp (or arg 1)))
+    (error
+     (message "Error skipping s-expressions at point %d." (point))
+     (beep)
+     (sit-for 2))))

 (defun ispell-ignore-fcc (start end)
   "Query whether to delete Fcc header due to attachment between START and END.
--8<---------------cut here---------------end--------------->8---

Re: Ispell: Skipping part of text in texinfo-mode

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Fri, 30 Aug 2024 10:13:59 +0200
> 
> Yes, at some point, I have to go through the @-Command List section in
> the Texinfo manual.

Then let's discuss them one by one, based on the Texinfo manual (which
documents much more than you can find in our manuals).  Some commands
need to ignore their arguments, some don't.  For example, @var's
argument is more often than not be a real word or a phrase made out of
words, like @var{next-file}.

> > Very simple: press 'a' once on each directive you never want to look
> > at, and leave the rest for judgment calls.
> 
> Thanks, this was also my approach until now, but I didn't find it
> satisfactory.

It's definitely a possibility, I think, so we IMO should support it,
at least as an option.

> We can discuss which macros to include and which to drop once the code
> is working.  I'm currently facing the issue that when I open a .texi
> file, eval (setq ispell-parser 'texinfo) and do M-x ispell RET, also
> comments in the file are checked, the value of `ispell-check-comments'
> is nil, though.  Do you have an idea what's going wrong?  I don't get
> this in a .tex buffer.

Doesn't ispell-check-comments only relevant to
ispell-comments-and-strings?

> +(defvar ispell--texinfo-arg-end-syntax-table
> +  (let ((table (make-syntax-table))
> +        (chars '((?\f . ">") (?\n . ">")
> +                 (?\" . " ") (?\@ . "/")
> +                 (?\( . " ") (?\) . " ")
> +                 (?\[ . " ") (?\] . " ")
> +                 (?\< . " ") (?\> . " ")
> +                 (?\\ . " ")
> +                 (?\{ . "(}") (?\} . "){"))))
> +    (dolist (elt chars)
> +      (modify-syntax-entry (car elt) (cdr elt) table))
> +    table))
> +
> +(defun ispell-texinfo-arg-end (&optional arg)
> +  "Skip across ARG number of braces."
> +  (condition-case nil
> +      (with-syntax-table ispell--texinfo-arg-end-syntax-table
> +        (unless (= (following-char) ?\{)
> +          (skip-chars-forward "^{"))
> +        (forward-sexp (or arg 1)))
> +    (error
> +     (message "Error skipping s-expressions at point %d." (point))
> +     (beep)
> +     (sit-for 2))))

Does this support nested markup, as in

  @w{@code{(@var{file} . @var{buffer})}}

in a way that will allow to spell-check "file" and "buffer"?  I'm not
sure syntax-table feature supports nesting.

Re: Ispell: Skipping part of text in texinfo-mode

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

> Then let's discuss them one by one, based on the Texinfo manual (which
> documents much more than you can find in our manuals).  Some commands
> need to ignore their arguments, some don't.

Will do so, no problem.  My first choice was by driven by auctex.texi
which I tested with.

> For example, @var's argument is more often than not be a real word or
> a phrase made out of words, like @var{next-file}.

I will remove 'var' from the skip list.

> It's definitely a possibility, I think, so we IMO should support it,
> at least as an option.

Hmm, we could say that people who want to use this must enable it with:

(add-hook 'texinfo-mode-hook (lambda ()
                               (setq ispell-parser 'texinfo)))

otherwise things will be as before.

> Doesn't ispell-check-comments only relevant to
> ispell-comments-and-strings?

Might be, then I still don't understand why my change instructs ispell
to spell-check comments.  Something is still off and I don't know what.

> Does this support nested markup, as in
>
>   @w{@code{(@var{file} . @var{buffer})}}
>
> in a way that will allow to spell-check "file" and "buffer"?  I'm not
> sure syntax-table feature supports nesting.

No, my code ignores everything inside @code{...}.  This is actually what
ispell does for .tex files.  The syntax-table stuff is to skip over
things like @kbd{C-c @key{@}} @key{RET}} robustly.

Best, Arash