GNU bug report logs - #50443
Fwd: Flyspell error traversal additions

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 50443 in the body.
You can then email your comments to 50443 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: Fwd: Flyspell error traversal additions
Date: Tue, 7 Sep 2021 02:22:29 +0200

[Message part 1 (text/plain, inline)]

X-Debbugs-CC: Roni Kallio <roni <at> kallio.app>

This was sent to emacs-devel, but never followed up.  I'm forwarding
to the bug tracker so that we don't lose track of it.

---------- Forwarded message ---------
Från: Roni Kallio <roni <at> kallio.app>
Date: tors 28 jan. 2021 kl 14:59
Subject: Flyspell error traversal additions
To: <emacs-devel <at> gnu.org>


While consolidating my spell/syntax checking setup, I noticed that
flyspell lacks a command to jump to the nearest error that is before the
point in a buffer.  There exists `flyspell-goto-next-error', which of
course loops back to beginning when the end of the buffer is reached,
but no equivalent command for going backwards.

I have attached a patch that implements backwards traversal, making sure
it acts analogous to `flyspell-goto-next-error'.  I'd like to open a
discussion on whether it would be feasible to have a key binding in
`flyspell-mode-map' for this command.  For reference, the bindings
currently are:

C-;   -- flyspell-auto-correct-previous-word
C-,   -- flyspell-goto-next-error
C-.   -- flyspell-auto-correct-word
C-c $ -- flyspell-correct-word-before-point

IMO the best course of action would be to modify
`flyspell-goto-next-error' to accept a prefix argument.  The prefix
would control the direction and number of jumps performed; negative
arguments would jump backwards -ARG errors (by calling
flyspell-goto-previous-error), while positive arguments would jump
forwards ARG errors.  This would be similar to how commands like
`forward-word' handle prefix arguments.  This would allow us to leave
the mode-map unchanged, but still distribute the improvement to all
users.

--
Roni Kallio

[0001-Add-flyspell-goto-previous-error.patch (application/x-patch, attachment)]

Message #8 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50443 <at> debbugs.gnu.org, roni <at> kallio.app
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Tue, 07 Sep 2021 08:57:04 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Tue, 7 Sep 2021 02:22:29 +0200
> Cc: roni kallio <roni <at> kallio.app>
> 
> This was sent to emacs-devel, but never followed up.  I'm forwarding
> to the bug tracker so that we don't lose track of it.

Thanks.

> Från: Roni Kallio <roni <at> kallio.app>
> Date: tors 28 jan. 2021 kl 14:59
> Subject: Flyspell error traversal additions
> To: <emacs-devel <at> gnu.org>
> 
> 
> While consolidating my spell/syntax checking setup, I noticed that
> flyspell lacks a command to jump to the nearest error that is before the
> point in a buffer.  There exists `flyspell-goto-next-error', which of
> course loops back to beginning when the end of the buffer is reached,
> but no equivalent command for going backwards.
> 
> I have attached a patch that implements backwards traversal, making sure
> it acts analogous to `flyspell-goto-next-error'.

Thanks.

> IMO the best course of action would be to modify
> `flyspell-goto-next-error' to accept a prefix argument.  The prefix
> would control the direction and number of jumps performed; negative
> arguments would jump backwards -ARG errors (by calling
> flyspell-goto-previous-error), while positive arguments would jump
> forwards ARG errors.  This would be similar to how commands like
> `forward-word' handle prefix arguments.  This would allow us to leave
> the mode-map unchanged, but still distribute the improvement to all
> users.

Yes, I agree.  Can you change the patch to work this way?

Also, the patch is long enough to require a copyright assignment from
you.  Would you be willing to start your legal paperwork at this time,
so we could accept your contribution when it is complete?

> I'd like to open a discussion on whether it would be feasible to
> have a key binding in `flyspell-mode-map' for this command.  For
> reference, the bindings currently are:
> 
> C-;   -- flyspell-auto-correct-previous-word
> C-,   -- flyspell-goto-next-error
> C-.   -- flyspell-auto-correct-word
> C-c $ -- flyspell-correct-word-before-point

I'm not sure these won't conflict with other minor modes.  And these
keys are not available on text-mode frames, which is a disadvantage.
But let's hear what others think.

Message #11 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50443 <at> debbugs.gnu.org, roni kallio <roni <at> kallio.app>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Thu, 09 Sep 2021 20:24:44 +0300

> IMO the best course of action would be to modify
> `flyspell-goto-next-error' to accept a prefix argument.  The prefix
> would control the direction and number of jumps performed; negative
> arguments would jump backwards -ARG errors (by calling
> flyspell-goto-previous-error), while positive arguments would jump
> forwards ARG errors.  This would be similar to how commands like
> `forward-word' handle prefix arguments.  This would allow us to leave
> the mode-map unchanged, but still distribute the improvement to all
> users.

Adding a negative argument to `flyspell-goto-next-error'
is the most non-conflicting solution.  There was an idea
to use 'M-g n' and 'M-g p' to navigate flyspell errors.
But I don't believe this is workable since often 'M-g n'
and 'M-g p' are needed to navigate errors or search hits
displayed in another window with a list of results.

Message #14 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Roni Kallio <roni <at> kallio.app>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 50443 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Thu, 09 Sep 2021 23:58:19 +0300

>> IMO the best course of action would be to modify
>> `flyspell-goto-next-error' to accept a prefix argument.  The prefix
>> would control the direction and number of jumps performed; negative
>> arguments would jump backwards -ARG errors (by calling
>> flyspell-goto-previous-error), while positive arguments would jump
>> forwards ARG errors.  This would be similar to how commands like
>> `forward-word' handle prefix arguments.  This would allow us to leave
>> the mode-map unchanged, but still distribute the improvement to all
>> users.
>
> Yes, I agree.  Can you change the patch to work this way?

I've been using this updated patch for a while now, hope it comes
through ok.

diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index 975f540936..b80626bb12 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -1708,39 +1708,77 @@ flyspell-old-pos-error
 ;;*---------------------------------------------------------------------*/
 ;;*    flyspell-goto-next-error ...                                     */
 ;;*---------------------------------------------------------------------*/
-(defun flyspell-goto-next-error ()
-  "Go to the next previously detected error.
-In general FLYSPELL-GOTO-NEXT-ERROR must be used after
-FLYSPELL-BUFFER."
-  (interactive)
-  (let ((pos (point))
-	(max (point-max)))
-    (if (and (eq (current-buffer) flyspell-old-buffer-error)
-	     (eq pos flyspell-old-pos-error))
-	(progn
-	  (if (= flyspell-old-pos-error max)
-	      ;; goto beginning of buffer
-	      (progn
-		(message "Restarting from beginning of buffer")
-		(goto-char (point-min)))
-	    (forward-word 1))
-	  (setq pos (point))))
-    ;; seek the next error
-    (while (and (< pos max)
-		(let ((ovs (overlays-at pos))
-		      (r '()))
-		  (while (and (not r) (consp ovs))
-		    (if (flyspell-overlay-p (car ovs))
-			(setq r t)
-		      (setq ovs (cdr ovs))))
-		  (not r)))
-      (setq pos (1+ pos)))
-    ;; save the current location for next invocation
-    (setq flyspell-old-pos-error pos)
-    (setq flyspell-old-buffer-error (current-buffer))
-    (goto-char pos)
-    (if (= pos max)
-	(message "No more miss-spelled word!"))))
+(defun flyspell-goto-next-error (&optional arg)
+  "Go to the detected error that is ARG errors forward.
+In general `flyspell-goto-next-error' must be used after
+`flyspell-buffer'."
+  (interactive "p")
+  (cond
+   ((= arg 0) nil)
+   ((< arg 0) (flyspell-goto-previous-error (abs arg)))
+   (t
+    (let ((pos (point))
+	  (max (point-max)))
+      (if (and (eq (current-buffer) flyspell-old-buffer-error)
+	       (eq pos flyspell-old-pos-error))
+	  (progn
+	    (if (= flyspell-old-pos-error max)
+	        ;; goto beginning of buffer
+	        (progn
+		  (message "Restarting from beginning of buffer")
+		  (goto-char (point-min)))
+	      (forward-word 1))
+	    (setq pos (point))))
+      ;; seek the next error
+      (while (and (< pos max)
+		  (let ((ovs (overlays-at pos))
+		        (r '()))
+		    (while (and (not r) (consp ovs))
+		      (if (flyspell-overlay-p (car ovs))
+			  (setq r t)
+		        (setq ovs (cdr ovs))))
+		    (not r)))
+        (setq pos (1+ pos)))
+      ;; save the current location for next invocation
+      (setq flyspell-old-pos-error pos)
+      (setq flyspell-old-buffer-error (current-buffer))
+      (goto-char pos)
+      (if (= pos max)
+	  (message "No more miss-spelled word!")))
+    (flyspell-goto-next-error (1- arg)))))
+
+(defun flyspell-goto-previous-error (&optional arg)
+  "Go to the detected error ARG errors backward.
+In general `flyspell-goto-previous-error' must be used after
+`flyspell-buffer'."
+  (interactive "p")
+  (cond
+   ((= arg 0) nil)
+   ((< arg 0) (flyspell-goto-next-error (abs arg)))
+   (t
+    (let ((min (point-min)))
+      (when (and (eq (current-buffer) flyspell-old-buffer-error)
+                 (eq (point) flyspell-old-pos-error))
+        (when (= (point) min)
+          (message "Restarting from end of buffer")
+          (goto-char (point-max)))
+        (backward-word 1))
+      (while (and (> (point) min)
+                  (let ((ovs (overlays-at (point)))
+                        (r nil))
+                    ;; look for a flyspell overlay
+                    (while (and (not r) (consp ovs))
+                      (if (flyspell-overlay-p (car ovs))
+                          (setq r t)
+                        (setq ovs (cdr ovs))))
+                    (not r)))
+        ;; go to previous word if no overlay was found
+        (backward-word 1))
+      (setq flyspell-old-pos-error (point))
+      (setq flyspell-old-buffer-error (current-buffer))
+      (when (= (point) min)
+        (message "No more miss-spelled word!")))
+    (flyspell-goto-previous-error (1- arg)))))

 ;;*---------------------------------------------------------------------*/
 ;;*    flyspell-overlay-p ...                                           */

> Also, the patch is long enough to require a copyright assignment from
> you.  Would you be willing to start your legal paperwork at this time,
> so we could accept your contribution when it is complete?

Sure, can you direct me to what I need to do to sign?
--
Roni Kallio

Message #17 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50443 <at> debbugs.gnu.org, roni kallio <roni <at> kallio.app>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Fri, 10 Sep 2021 09:39:37 +0300

> Adding a negative argument to `flyspell-goto-next-error'
> is the most non-conflicting solution.  There was an idea
> to use 'M-g n' and 'M-g p' to navigate flyspell errors.
> But I don't believe this is workable since often 'M-g n'
> and 'M-g p' are needed to navigate errors or search hits
> displayed in another window with a list of results.

Maybe we could add another standard keybinding localized for navigation
in the current buffer only, e.g. 'M-g N' and 'M-g P' that could be used
by flyspell, flymake, etc.

Message #20 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Juri Linkov <juri <at> linkov.net>
Cc: 50443 <at> debbugs.gnu.org, roni kallio <roni <at> kallio.app>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Fri, 10 Sep 2021 08:49:53 +0200

Juri Linkov <juri <at> linkov.net> writes:

> Maybe we could add another standard keybinding localized for navigation
> in the current buffer only, e.g. 'M-g N' and 'M-g P' that could be used
> by flyspell, flymake, etc.

Works for me, but it seems unfortunate to put this on keys that are
somewhat inconvenient to type (switching between modifiers is rarely
fun).  What about 'M-g b' and 'M-g f'?

Message #25 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50443 <at> debbugs.gnu.org, roni kallio <roni <at> kallio.app>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Fri, 10 Sep 2021 19:12:45 +0300

>> Maybe we could add another standard keybinding localized for navigation
>> in the current buffer only, e.g. 'M-g N' and 'M-g P' that could be used
>> by flyspell, flymake, etc.
>
> Works for me, but it seems unfortunate to put this on keys that are
> somewhat inconvenient to type (switching between modifiers is rarely
> fun).  What about 'M-g b' and 'M-g f'?

If no one proposes a better use of these keys.

Message #28 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Roni Kallio <roni <at> kallio.app>
Cc: 50443 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Sun, 19 Sep 2021 09:05:29 -0700

Roni Kallio <roni <at> kallio.app> writes:

>> Also, the patch is long enough to require a copyright assignment from
>> you.  Would you be willing to start your legal paperwork at this time,
>> so we could accept your contribution when it is complete?
>
> Sure, can you direct me to what I need to do to sign?

Please email the following information to assign <at> gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]


[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]


[For the copyright registration, what country are you a citizen of?]


[What year were you born?]


[Please write your email address here.]


[Please write your postal address here.]


[Which files have you changed so far, and which new files have you written
so far?]

Message #33 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Roni Kallio <roni <at> kallio.app>
Cc: 50443 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Sun, 26 Dec 2021 19:12:11 -0800

Stefan Kangas <stefan <at> marxist.se> writes:

> Roni Kallio <roni <at> kallio.app> writes:
>
>>> Also, the patch is long enough to require a copyright assignment from
>>> you.  Would you be willing to start your legal paperwork at this time,
>>> so we could accept your contribution when it is complete?
>>
>> Sure, can you direct me to what I need to do to sign?
>
> Please email the following information to assign <at> gnu.org, and we
> will send you the assignment form for your past and future changes.

Any news regarding the paperwork?

Message #36 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50443 <at> debbugs.gnu.org, Roni Kallio <roni <at> kallio.app>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Thu, 24 Mar 2022 10:06:03 +0100

Stefan Kangas <stefan <at> marxist.se> writes:

>> Please email the following information to assign <at> gnu.org, and we
>> will send you the assignment form for your past and future changes.
>
> Any news regarding the paperwork?

This was three months ago, and I can't see the paperwork on file, so I
guess it's not going to happen?

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

Message #41 received at 50443 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50443 <at> debbugs.gnu.org, Roni Kallio <roni <at> kallio.app>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#50443: Fwd: Flyspell error traversal additions
Date: Thu, 25 Aug 2022 16:20:33 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> This was three months ago, and I can't see the paperwork on file, so I
> guess it's not going to happen?

So I've now implemented this in Emacs 29 -- `C-u M-x
flyspell-goto-next-error' now goes to the previous error.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.