bug#21129: 24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word

bug#21129: 24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word

[N. Jackson]

flyspell-check-previous-highlighted-word (f-c-p-h-w) seems to me to have
(at least) two bugs (see below) and has typos in (and other problems
with) its docstring.

The function (in flyspell.el) appears thus:

    (defun flyspell-check-previous-highlighted-word (&optional arg)
      "Correct the closer misspelled word.
    This function scans a mis-spelled word before the cursor. If it finds one
    it proposes replacement for that word. With prefix arg, count that many
    misspelled words backwards."
      (interactive)
      (let ((pos1 (point))
            (pos  (point))
            (arg  (if (or (not (numberp arg)) (< arg 1)) 1 arg))
            ov ovs)
        (if (catch 'exit
              (while (and (setq pos (previous-overlay-change pos))
                          (not (= pos pos1)))
                (setq pos1 pos)
                (if (> pos (point-min))
                    (progn
                      (setq ovs (overlays-at (1- pos)))
                      (while (consp ovs)
                        (setq ov (car ovs))
                        (setq ovs (cdr ovs))
                        (if (and (flyspell-overlay-p ov)
                                 (= 0 (setq arg (1- arg))))
                            (throw 'exit t)))))))
            (save-excursion
              (goto-char pos)
              (ispell-word)
              (setq flyspell-word-cache-word nil) ;; Force flyspell-word re-check
              (flyspell-word))
          (error "No word to correct before point"))))

1. The docstring fails to state that, to be found, the misspelled word
must be highlighted by Flyspell. (It might not be if Flyspell has not
yet run over the buffer or region in question.)

2. Some other problems/typos with the docstring are pointed out with
carets below:

    Correct the closer misspelled word.
                    ^^  
    This function scans a mis-spelled word before the cursor. If it
                       ^     ^                    ^^^^^^^^^^
    finds one it proposes replacement for that word. With prefix arg,
                         ^                               ^       
    count that many misspelled words backwards.

Perhaps a better docstring might be something like:

    Correct the closest previous word that is highlighted as misspelled.
    This function scans for a word before point that has been
    highlighted by Flyspell as misspelled. If it finds one it proposes a
    replacement for that word. With a prefix arg, it scans for the nth
    misspelled word before point, for n equal to ARG.

3. Strangely, f-c-p-h-w uses the Ispell user interface rather than
the Flyspell user interface. This suits me fine as I like a
non-graphical interface, but it seems to me to be a bug. This is
Flyspell after all, and I think one would expect the same interface as
with, say, `M-x flyspell-correct-word-before-point'.

4. Contrary to the docstring, if point is immediately after the last
character of a misspelled (and Flyspell-highlighted) word, f-c-p-h-w
skips to the previous misspelled (and Flyspell-highlighted) word, even
though the closest previous misspelled word was the one directly before
point.

5. Executing f-c-p-h-w with a prefix argument does not appear to work;
the behaviour is as if there was no prefix argument. E.g. with several
misspelled words in the buffer before point, `C-u 3 M-x
flyspell-check-previous-highlighted-word' acts on the final one just as
`M-x flyspell-check-previous-highlighted-word' does, rather than on the
3rd from last one.


In GNU Emacs 24.5.1 (x86_64-redhat-linux-gnu, GTK+ Version 3.14.12)
 of 2015-05-07 on buildvm-08.phx2.fedoraproject.org
Windowing system distributor `Fedora Project', version 11.0.11603000
System Description:	Fedora release 21 (Twenty One)

bug#21129: 24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word

[Eli Zaretskii]

> From: nljlistbox2@gmail.com (N. Jackson)
> Date: Fri, 24 Jul 2015 14:35:47 -0300
> 
> flyspell-check-previous-highlighted-word (f-c-p-h-w) seems to me to have
> (at least) two bugs (see below) and has typos in (and other problems
> with) its docstring.

Indeed; thanks for pointing that out.

> 1. The docstring fails to state that, to be found, the misspelled word
> must be highlighted by Flyspell. (It might not be if Flyspell has not
> yet run over the buffer or region in question.)
> 
> 2. Some other problems/typos with the docstring are pointed out with
> carets below:
> 
>     Correct the closer misspelled word.
>                     ^^  
>     This function scans a mis-spelled word before the cursor. If it
>                        ^     ^                    ^^^^^^^^^^
>     finds one it proposes replacement for that word. With prefix arg,
>                          ^                               ^       
>     count that many misspelled words backwards.
> 
> Perhaps a better docstring might be something like:
> 
>     Correct the closest previous word that is highlighted as misspelled.
>     This function scans for a word before point that has been
>     highlighted by Flyspell as misspelled. If it finds one it proposes a
>     replacement for that word. With a prefix arg, it scans for the nth
>     misspelled word before point, for n equal to ARG.

I used this text with minor stylistic corrections, to make it use the
same style as in other doc strings.

> 3. Strangely, f-c-p-h-w uses the Ispell user interface rather than
> the Flyspell user interface. This suits me fine as I like a
> non-graphical interface, but it seems to me to be a bug. This is
> Flyspell after all, and I think one would expect the same interface as
> with, say, `M-x flyspell-correct-word-before-point'.

I did nothing about this one, as this appears a deliberate design
decision of the original author.

> 4. Contrary to the docstring, if point is immediately after the last
> character of a misspelled (and Flyspell-highlighted) word, f-c-p-h-w
> skips to the previous misspelled (and Flyspell-highlighted) word, even
> though the closest previous misspelled word was the one directly before
> point.

Fixed.

> 5. Executing f-c-p-h-w with a prefix argument does not appear to work;
> the behaviour is as if there was no prefix argument. E.g. with several
> misspelled words in the buffer before point, `C-u 3 M-x
> flyspell-check-previous-highlighted-word' acts on the final one just as
> `M-x flyspell-check-previous-highlighted-word' does, rather than on the
> 3rd from last one.

A simple oversight in how the function's 'interactive' form was used;
fixed.

The patch appears below, so you could try it in Emacs 24.5.

Thanks.

diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index a5dff07..2329f29 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -1827,11 +1827,12 @@ (make-variable-buffer-local 'flyspell-auto-correct-word)
 ;;*    flyspell-check-previous-highlighted-word ...                     */
 ;;*---------------------------------------------------------------------*/
 (defun flyspell-check-previous-highlighted-word (&optional arg)
-  "Correct the closer misspelled word.
-This function scans a mis-spelled word before the cursor. If it finds one
-it proposes replacement for that word. With prefix arg, count that many
-misspelled words backwards."
-  (interactive)
+  "Correct the closest previous word that is highlighted as misspelled.
+This function scans for a word which starts before point that has been
+highlighted by Flyspell as misspelled.  If it finds one, it proposes
+a replacement for that word.  With prefix arg N, check the Nth word
+before point that's highlighted as misspelled."
+  (interactive "P")
   (let ((pos1 (point))
 	(pos  (point))
 	(arg  (if (or (not (numberp arg)) (< arg 1)) 1 arg))
@@ -1842,7 +1843,7 @@ (defun flyspell-check-previous-highlighted-word (&optional arg)
 	    (setq pos1 pos)
 	    (if (> pos (point-min))
 		(progn
-		  (setq ovs (overlays-at (1- pos)))
+		  (setq ovs (overlays-at pos))
 		  (while (consp ovs)
 		    (setq ov (car ovs))
 		    (setq ovs (cdr ovs))

bug#21129: 24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word

[N. Jackson]

At 06:40 -0300 on Saturday 2015-07-25, Eli Zaretskii wrote:

>> 3. Strangely, f-c-p-h-w uses the Ispell user interface rather than
>> the Flyspell user interface. This suits me fine as I like a
>> non-graphical interface
>
> I did nothing about this one, as this appears a deliberate design
> decision of the original author.

That's good news for me. (Otherwise I would have been needing a
flyspell-goto-previous-error function so that I could use the Ispell
interface!)

> The patch appears below, so you could try it in Emacs 24.5.

The patch works for me as advertised, and I see you have already closed
the bug. Thank you very much.