bug#20543: 24.5; <SPC> in ispell-buffer accepts spelling for the whole line

bug#20543: 24.5; <SPC> in ispell-buffer accepts spelling for the whole line

[Jürgen Hartmann]

Using GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.2)--but
this applies to former Emacs versions also--I found the following puzzling
behavior of ispell-buffer:

If a suspicious word is accepted once by pressing <SPC> in an interactive
ispell-buffer session, all further occurrences of the same spelling on the
same line are skipped, i.e. considered as accepted too.

Here is an example:

Open an Emacs 24.5 session (it is the same with Emacs 24.4) by

   LC_ALL=C emacs -Q

and enter the following line (with or without newline does not matter) in the
*scratch* buffer:

   The term charset is short for charset.

Assume that the last word is a typo that should read "character set". Now
change the dictionary to american and run ispell-buffer. The first occurrence
of "charset" gets highlighted, but since it is correct here, we use <SPC> to
accept it once and proceed. But oops... the spell-check finishes immediately
without giving us the chance to correct the second occurrence of "charset" in
that line.

This finding was shortly discussed in help-gnu-emacs
(http://lists.gnu.org/archive/html/help-gnu-emacs/2015-05/threads.html#00134)
and it was Eli Zaretskii who indicated that there is the following comment in
line 3651 of emacs-24.5/lisp/textmodes/ispell.el:

   ;; Do not recheck accepted word on this line.

This suggests that there might be a reason for that behavior. If this is
true, what is it?

Apart from that, I can not imagine why such a behavior might be desirable.

Juergen

 		 	   		  

bug#20543: 24.5; <SPC> in ispell-buffer accepts spelling for the whole line

[Marcin Borkowski]

On 2015-05-10, at 18:05, Jürgen Hartmann <juergen_hartmann_@hotmail.com> wrote:

> Using GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.2)--but
> this applies to former Emacs versions also--I found the following puzzling
> behavior of ispell-buffer:
>
> If a suspicious word is accepted once by pressing <SPC> in an interactive
> ispell-buffer session, all further occurrences of the same spelling on the
> same line are skipped, i.e. considered as accepted too.
>
> Here is an example:
>
> Open an Emacs 24.5 session (it is the same with Emacs 24.4) by
>
>    LC_ALL=C emacs -Q
>
> and enter the following line (with or without newline does not matter) in the
> *scratch* buffer:
>
>    The term charset is short for charset.
>
> Assume that the last word is a typo that should read "character set". Now
> change the dictionary to american and run ispell-buffer. The first occurrence
> of "charset" gets highlighted, but since it is correct here, we use <SPC> to
> accept it once and proceed. But oops... the spell-check finishes immediately
> without giving us the chance to correct the second occurrence of "charset" in
> that line.
>
> This finding was shortly discussed in help-gnu-emacs
> (http://lists.gnu.org/archive/html/help-gnu-emacs/2015-05/threads.html#00134)
> and it was Eli Zaretskii who indicated that there is the following comment in
> line 3651 of emacs-24.5/lisp/textmodes/ispell.el:
>
>    ;; Do not recheck accepted word on this line.
>
> This suggests that there might be a reason for that behavior. If this is
> true, what is it?

I've just seen this report.  I'm also very curious about that reason.

> Apart from that, I can not imagine why such a behavior might be desirable.
>
> Juergen


-- 
Marcin

bug#20543: 24.5; <SPC> in ispell-buffer accepts spelling for the whole line

[Lars Ingebrigtsen]

Marcin Borkowski <mbork@mbork.pl> writes:

>> line 3651 of emacs-24.5/lisp/textmodes/ispell.el:
>>
>>    ;; Do not recheck accepted word on this line.

I think there's two meanings of "accepted" in that function -- one is
hitting SPC, and the other is when the word is already in the dictionary.

>> This suggests that there might be a reason for that behavior. If this is
>> true, what is it?
>
> I've just seen this report.  I'm also very curious about that reason.

It seems nonsensical to me -- just because you accept the word once on a
line, it might not be acceptable in the next instance.  So I've changed
this in Emacs 28.

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

bug#20543: 24.5; <SPC> in ispell-buffer accepts spelling for the whole line

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 28 May 2021 02:10:50 +0200
> Cc: Jürgen Hartmann <juergen_hartmann_@hotmail.com>,
>  20543@debbugs.gnu.org
> 
> Marcin Borkowski <mbork@mbork.pl> writes:
> 
> >> line 3651 of emacs-24.5/lisp/textmodes/ispell.el:
> >>
> >>    ;; Do not recheck accepted word on this line.
> 
> I think there's two meanings of "accepted" in that function -- one is
> hitting SPC, and the other is when the word is already in the dictionary.

No, the two meanings of "accepted" here are:

  . the user hits SPC to "leave the word unchanged"
  . the user hits 'a' to "accept the word for this session"

See below.

> >> This suggests that there might be a reason for that behavior. If this is
> >> true, what is it?
> >
> > I've just seen this report.  I'm also very curious about that reason.
> 
> It seems nonsensical to me -- just because you accept the word once on a
> line, it might not be acceptable in the next instance.  So I've changed
> this in Emacs 28.

This is wrong, because now 'a' doesn't work as expected.

Recipe:

  . emacs -Q
  . type into *scratch*:
    foobarical something foobarical something else
  . M-x ispell-buffer
  . a (to "accept" the first "foobarical")
  . observe Emacs stopping on the next "foobarical", instead of
    skipping it, as expected

The problem is that replace == nil means two things: either the user
pressed SPC or the user pressed 'a' (but NOT 'A').  However, the
"don't check the same line" logic should only be applied for 'a', not
for SPC.  So what we need to fix this is IMO adding a way to
distinguish between 'a' and SPC (perhaps by looking at
ispell-buffer-session-localwords?).

Note that the comments in ispell-process-line wrt the meaning of the
value of 'replace' are AFAICT inaccurate:

            ;; Insert correction if needed.
            (cond
             ((equal 0 replace)         ; INSERT
              (if (equal 0 replace)     ; BUFFER-LOCAL DICT ADD
                  (ispell-add-per-file-word-list (car poss)))
              ;; Do not recheck accepted word on this line.
              (setq accept-list (cons (car poss) accept-list)))
             (t
              ;; The user hit SPC, so accept this word, but keep
              ;; checking the rest of the line.
              (unless replace
                (setq accepted t)
                (setq replace (list (buffer-substring-no-properties
                                     (point) (+ word-len (point))))))

(This is also inelegant, as it tests 'replace' for being zero twice.)

Contrary to the comment, as can be seen from ispell-command-loop, the
value of 'replace' can be:

  nil if user pressed 'i' or 'u'
  nil if user pressed 'a'
  nil if user typed SPC
  0 if user pressed 'A'
  replacement word if user typed 'r' or 'R'
  t if the spelling session should end

So the above 'cond' is incorrect and should be fixed.

In any case, the change, as it is, is for the worse, because 'a' is by
far more important than SPC during spell-checking of technical text,
where there are many acronyms and jargon words unknown to the
dictionary.  If we don't have good ideas how to fix the SPC case, we
should revert the change and add a FIXME.

bug#20543: 24.5; <SPC> in ispell-buffer accepts spelling for the whole line

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> If we don't have good ideas how to fix the SPC case, we
> should revert the change and add a FIXME.

Right; I'll revert and reopen this bug report.  Fixing the problem
should be possible, though.

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