unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#50443: Fwd: Flyspell error traversal additions
       [not found] ` <87r1m5ch9n.fsf@kallio.app>
@ 2021-09-07  0:22   ` Stefan Kangas
  2021-09-07  5:57     ` Eli Zaretskii
  2021-09-09 17:24     ` Juri Linkov
  0 siblings, 2 replies; 11+ messages in thread
From: Stefan Kangas @ 2021-09-07  0:22 UTC (permalink / raw)
  To: 50443; +Cc: roni kallio

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

X-Debbugs-CC: Roni Kallio <roni@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@kallio.app>
Date: tors 28 jan. 2021 kl 14:59
Subject: Flyspell error traversal additions
To: <emacs-devel@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

[-- Attachment #2: 0001-Add-flyspell-goto-previous-error.patch --]
[-- Type: application/x-patch, Size: 2284 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-07  0:22   ` bug#50443: Fwd: Flyspell error traversal additions Stefan Kangas
@ 2021-09-07  5:57     ` Eli Zaretskii
  2021-09-09 20:58       ` Roni Kallio
  2021-09-09 17:24     ` Juri Linkov
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2021-09-07  5:57 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 50443, roni

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 7 Sep 2021 02:22:29 +0200
> Cc: roni kallio <roni@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@kallio.app>
> Date: tors 28 jan. 2021 kl 14:59
> Subject: Flyspell error traversal additions
> To: <emacs-devel@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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-07  0:22   ` bug#50443: Fwd: Flyspell error traversal additions Stefan Kangas
  2021-09-07  5:57     ` Eli Zaretskii
@ 2021-09-09 17:24     ` Juri Linkov
  2021-09-10  6:39       ` Juri Linkov
  1 sibling, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2021-09-09 17:24 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 50443, roni kallio

> 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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-07  5:57     ` Eli Zaretskii
@ 2021-09-09 20:58       ` Roni Kallio
  2021-09-19 16:05         ` Stefan Kangas
  0 siblings, 1 reply; 11+ messages in thread
From: Roni Kallio @ 2021-09-09 20:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50443, Stefan Kangas

>> 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





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-09 17:24     ` Juri Linkov
@ 2021-09-10  6:39       ` Juri Linkov
  2021-09-10  6:49         ` Stefan Kangas
  0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2021-09-10  6:39 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 50443, roni kallio

> 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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-10  6:39       ` Juri Linkov
@ 2021-09-10  6:49         ` Stefan Kangas
  2021-09-10 16:12           ` Juri Linkov
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Kangas @ 2021-09-10  6:49 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 50443, roni kallio

Juri Linkov <juri@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'?





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-10  6:49         ` Stefan Kangas
@ 2021-09-10 16:12           ` Juri Linkov
  0 siblings, 0 replies; 11+ messages in thread
From: Juri Linkov @ 2021-09-10 16:12 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 50443, roni kallio

>> 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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-09 20:58       ` Roni Kallio
@ 2021-09-19 16:05         ` Stefan Kangas
  2021-12-27  3:12           ` Stefan Kangas
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Kangas @ 2021-09-19 16:05 UTC (permalink / raw)
  To: Roni Kallio; +Cc: 50443

Roni Kallio <roni@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@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?]





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-09-19 16:05         ` Stefan Kangas
@ 2021-12-27  3:12           ` Stefan Kangas
  2022-03-24  9:06             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Kangas @ 2021-12-27  3:12 UTC (permalink / raw)
  To: Roni Kallio; +Cc: 50443

Stefan Kangas <stefan@marxist.se> writes:

> Roni Kallio <roni@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@gnu.org, and we
> will send you the assignment form for your past and future changes.

Any news regarding the paperwork?





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2021-12-27  3:12           ` Stefan Kangas
@ 2022-03-24  9:06             ` Lars Ingebrigtsen
  2022-08-25 14:20               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 11+ messages in thread
From: Lars Ingebrigtsen @ 2022-03-24  9:06 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 50443, Roni Kallio

Stefan Kangas <stefan@marxist.se> writes:

>> Please email the following information to assign@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





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#50443: Fwd: Flyspell error traversal additions
  2022-03-24  9:06             ` Lars Ingebrigtsen
@ 2022-08-25 14:20               ` Lars Ingebrigtsen
  0 siblings, 0 replies; 11+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-25 14:20 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 50443, Roni Kallio, Eli Zaretskii

Lars Ingebrigtsen <larsi@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.






^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-08-25 14:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <87sg6lchgc.fsf@kallio.app>
     [not found] ` <87r1m5ch9n.fsf@kallio.app>
2021-09-07  0:22   ` bug#50443: Fwd: Flyspell error traversal additions Stefan Kangas
2021-09-07  5:57     ` Eli Zaretskii
2021-09-09 20:58       ` Roni Kallio
2021-09-19 16:05         ` Stefan Kangas
2021-12-27  3:12           ` Stefan Kangas
2022-03-24  9:06             ` Lars Ingebrigtsen
2022-08-25 14:20               ` Lars Ingebrigtsen
2021-09-09 17:24     ` Juri Linkov
2021-09-10  6:39       ` Juri Linkov
2021-09-10  6:49         ` Stefan Kangas
2021-09-10 16:12           ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).