emacs-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[debbugs-tracker] bug#21129: closed (24.5; Behaviour and docstring of fl


From: GNU bug Tracking System
Subject: [debbugs-tracker] bug#21129: closed (24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word)
Date: Sat, 25 Jul 2015 09:41:01 +0000

Your message dated Sat, 25 Jul 2015 12:40:15 +0300
with message-id <address@hidden>
and subject line Re: bug#21129: 24.5; Behaviour and docstring of 
flyspell-check-previous-highlighted-word
has caused the debbugs.gnu.org bug report #21129,
regarding 24.5; Behaviour and docstring of 
flyspell-check-previous-highlighted-word
to be marked as done.

(If you believe you have received this mail in error, please contact
address@hidden)


-- 
21129: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21129
GNU Bug Tracking System
Contact address@hidden with problems
--- Begin Message --- Subject: 24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word 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.

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)



--- End Message ---
--- Begin Message --- Subject: Re: bug#21129: 24.5; Behaviour and docstring of flyspell-check-previous-highlighted-word Date: Sat, 25 Jul 2015 12:40:15 +0300
> From: address@hidden (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))


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]