bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Arash Esbati]

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

Hi all,

please consider this .tex file which is valid LaTeX code:

--8<---------------cut here---------------start------------->8---
\documentclass{article}

\newenvironment{spacetest }{\itshape}{}
\newenvironment{spacetest}{\ttfamily}{}

\begin{document}

\begin{verbatim}
(setq ispell-tex-skip-alists
      '((("\\\\document\\(class\\|style\\)" . "\\\\begin[ \t\n]*{document}"))
        (("spacetest"    . "\\\\end[ \t]*{spacetest}")
         ("verbatim\\*?" . "\\\\end[ \t]*{verbatim\\*?}"))))
\end{verbatim}

\begin{spacetest }
Thiz iz nott to be ignorrd.
\end{spacetest }

\begin{spacetest}
  Thiz iz to be ignorrd.
\end{spacetest}

\end{document}
--8<---------------cut here---------------end--------------->8---

Eval the form in the verbatim environment and do 'M-x ispell RET'.  It
doesn't find any misspelled words at all and ignores both environments
'spacetest ' and 'spacetest' where it should only ignore the latter.

This is due to unnecessary matching of whitespaces in the function
`ispell-begin-tex-skip-regexp'.  The patch attached fixes the issue.
The patch is against the master branch, but should also apply to the
release branch.  I can update it once a number is assigned to the
report.

Best, Arash

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Delete-matching-of-whitespaces-in-LaTeX-env-names.patch --]
[-- Type: text/x-patch, Size: 1013 bytes --]

From 2dc603c32c106aa77cff23fc60a1500fdde4a5e1 Mon Sep 17 00:00:00 2001
From: Arash Esbati <arash@gnu.org>
Date: Tue, 23 Jul 2024 22:40:41 +0200
Subject: [PATCH] Delete matching of whitespaces in LaTeX env names

* lisp/textmodes/ispell.el (ispell-begin-tex-skip-regexp): Remove
matching of arbitrary whitespaces in LaTeX environment names when
wrapping them inside \begin{}.
---
 lisp/textmodes/ispell.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 667da10d7a3..99f9e10a5a8 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -3320,9 +3320,7 @@ ispell-begin-tex-skip-regexp
    "\\|"
    ;; keys wrapped in begin{}
    (mapconcat (lambda (lst)
-                (concat "\\\\begin[ \t\n]*{[ \t\n]*"
-                        (car lst)
-                        "[ \t\n]*}"))
+                (concat "\\\\begin[ \t\n]*{" (car lst) "}"))
 	      (car (cdr ispell-tex-skip-alists))
 	      "\\|")))
 
-- 
2.45.2

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Arash Esbati]

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

Arash Esbati <arash@gnu.org> writes:

> I can update it once a number is assigned to the report.

Here it is.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Delete-matching-of-whitespaces-in-LaTeX-env-names.patch --]
[-- Type: text/x-patch, Size: 1026 bytes --]

From c5dea065783d08b181c77cf83d1ede57df141466 Mon Sep 17 00:00:00 2001
From: Arash Esbati <arash@gnu.org>
Date: Tue, 23 Jul 2024 22:40:41 +0200
Subject: [PATCH] Delete matching of whitespaces in LaTeX env names

* lisp/textmodes/ispell.el (ispell-begin-tex-skip-regexp): Remove
matching of arbitrary whitespaces in LaTeX environment names when
wrapping them inside \begin{}.  (bug#72262)
---
 lisp/textmodes/ispell.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 667da10d7a3..99f9e10a5a8 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -3320,9 +3320,7 @@ ispell-begin-tex-skip-regexp
    "\\|"
    ;; keys wrapped in begin{}
    (mapconcat (lambda (lst)
-                (concat "\\\\begin[ \t\n]*{[ \t\n]*"
-                        (car lst)
-                        "[ \t\n]*}"))
+                (concat "\\\\begin[ \t\n]*{" (car lst) "}"))
 	      (car (cdr ispell-tex-skip-alists))
 	      "\\|")))
 
-- 
2.45.2


[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


Best, Arash

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Date: Sat, 27 Jul 2024 09:21:44 +0200
> 
> Arash Esbati <arash@gnu.org> writes:
> 
> > I can update it once a number is assigned to the report.
> 
> Here it is.

Thanks, but couldn't this change break someone's setup or workflows?
The matching of whitespace there seems to be on purpose, and your use
case involves customization of ispell-tex-skip-alists.  What about
users who didn't customize that or customized it in a different way?

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, but couldn't this change break someone's setup or workflows?

I can't currently think of a scenario where the change breaks a working
setup.

> The matching of whitespace there seems to be on purpose,

In LaTeX2e, spaces in environment names are significant, so \begin{foo}
is not equivalent to \begin{foo } or \begin{ foo }, therefore I think
the current code is wrong.

> and your use case involves customization of ispell-tex-skip-alists.
> What about users who didn't customize that or customized it in a
> different way?

The former group is not affected at all since LaTeX has no environment
names with spaces.  The latter will actually get what they're asking for
with the change I proposed, which is a follow-up to 808917b3fc.

Best, Arash

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Cc: 72262@debbugs.gnu.org
> Date: Sun, 04 Aug 2024 22:01:07 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, but couldn't this change break someone's setup or workflows?
> 
> I can't currently think of a scenario where the change breaks a working
> setup.
> 
> > The matching of whitespace there seems to be on purpose,
> 
> In LaTeX2e, spaces in environment names are significant, so \begin{foo}
> is not equivalent to \begin{foo } or \begin{ foo }, therefore I think
> the current code is wrong.
> 
> > and your use case involves customization of ispell-tex-skip-alists.
> > What about users who didn't customize that or customized it in a
> > different way?
> 
> The former group is not affected at all since LaTeX has no environment
> names with spaces.  The latter will actually get what they're asking for
> with the change I proposed, which is a follow-up to 808917b3fc.

I know nothing about TeX, so I added two people who do, in the hope
that they could provide a more meaningful review.

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Tassilo Horn]

Eli Zaretskii <eliz@gnu.org> writes:

Hi all,

> I know nothing about TeX, so I added two people who do, in the hope
> that they could provide a more meaningful review.

Sorry for being so late.  Actually, Arash knows (La)TeX much better than
me.  I just validated his claim that spaces in environment names are
significant and its true.  So the patch is correct.

Bye,
  Tassilo

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Eli Zaretskii]

> From: Tassilo Horn <tsdh@gnu.org>
> Cc: Arash Esbati <arash@gnu.org>,  Stefan Monnier
>  <monnier@iro.umontreal.ca>,  72262@debbugs.gnu.org
> Date: Mon, 19 Aug 2024 10:05:43 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Hi all,
> 
> > I know nothing about TeX, so I added two people who do, in the hope
> > that they could provide a more meaningful review.
> 
> Sorry for being so late.  Actually, Arash knows (La)TeX much better than
> me.  I just validated his claim that spaces in environment names are
> significant and its true.  So the patch is correct.

Thanks for chiming in.

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The matching of whitespace there seems to be on purpose,
>
> In LaTeX2e, spaces in environment names are significant, so \begin{foo}
> is not equivalent to \begin{foo } or \begin{ foo }, therefore I think
> the current code is wrong.

Indeed, this is puzzling: I can't see why the code explicitly ignored
whitespace there.  I've never seen environment names with leading or
trailing whitespace, so it's probably not a big deal wither way, but
your patch looks right to me.


        Stefan

bug#72262: 31.0.50; [PATCH] Bug in `ispell-begin-tex-skip-regexp'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  72262@debbugs.gnu.org
> Date: Wed, 14 Aug 2024 08:11:45 -0400
> 
> >> The matching of whitespace there seems to be on purpose,
> >
> > In LaTeX2e, spaces in environment names are significant, so \begin{foo}
> > is not equivalent to \begin{foo } or \begin{ foo }, therefore I think
> > the current code is wrong.
> 
> Indeed, this is puzzling: I can't see why the code explicitly ignored
> whitespace there.  I've never seen environment names with leading or
> trailing whitespace, so it's probably not a big deal wither way, but
> your patch looks right to me.

Thanks, installed on master, and closing the bug.