d12frosted / flyspell-correct

Distraction-free words correction with flyspell via selected interface.
Other
200 stars 14 forks source link

A hydra for flyspell with flyspell-correct #69

Open gusbrs opened 4 years ago

gusbrs commented 4 years ago

Hi,

following the discussion at #58, I share here a hydra I cooked for flyspell spellchecking making use of flyspell-correct-at-point as basic correction interface. As I mentioned there, it is not strictly within the logic of flyspell-correct but I believe it may complement it in the particular task of whole buffer checking, so it might be useful. So, this is not a "request" nor a "bug report". It is meant only as sharing some ideas I found worked well for me, for your use in flyspell-correct as you please, if you please. Of course, if some of the ideas or code are deemed interesting enough to be somehow incorporated to flyspell-correct, I'll be more than happy with that.

If I understand the design choice correctly, flyspell-correct is mainly targeted at making fast spell corrections on a local scope, as you type, and excels at that. This is pretty much the same logic of flyspell itself. But sometimes we also need to do a thorough "whole buffer" check, particularly when we reach a revision stage of a given document. The standard Emacs tool for this task is ispell. Here, in the sense of being designed for it: it keeps track of a ispell session, it defaults to ispell-buffer if there is no region, etc. Of course, we can use ispell-word to correct a word at point or go about a whole buffer with flyspell-goto-next-error and then using flyspell-correct-at-point (or vanilla flyspell facilities for that), but in either case things are less than optimal. (I probably wouldn't start flyspell-correct-wrapper for a whole buffer spell-checking task myself, but it is of course possible, and seems to be so used, e.g. https://github.com/d12frosted/flyspell-correct/issues/60#issue-559228687).

Sure, we could use either tool where it is most appropriate, and probably this is what many people actually do. But I wanted to keep a single UI for spellchecking to reduce the cognitive strain of switching between quite different interfaces. Flyspell would be then the natural candidate, for it is hard to beat for the "spell-checking as you type" task. I've also been reaching best results for LaTeX with it. And I use flyspell-correct(-ivy) as my Flyspell interface, and like it very much.

So I wanted to improve upon flyspell for this particular task of "whole buffer checking" while keeping the goodness of flyspell-correct. This is what this hydra is about.

That given, a couple of things stood out as differences with respect to the behavior of flyspell-correct for this particular task I wanted to address:

One thing I wanted to keep from flyspell-correct is it's bidirectional functioning, which is not really granted by either flyspell or ispell. True, this is probably less important for a "whole buffer" kind of task, but I like this flexibility and, in my experience, sometimes things that show up further down the road of a checking session makes us regret a previous decision of skipping or accepting and I want to go back. So it's a nice flexibility I wanted to have: navigating back and forth the spelling mistakes.

Those things given, I came up with the hydra plus auxiliary functions below. It admittedly is not within the logic of flyspell-correct-move, and just makes use of flyspell-correct-at-point when needed, which grants the UI experience in line with flyspell-correct in general. The core are the skip functions (just adapted versions of flyspell-goto-next-error). The result is more akin to the ispell logic (given the task envisaged), but with even more flexibility and nicer interface granted by flyspell-correct.

See how you like it and, as said before, if there's anything you can take from it (including all of it) for flyspell-correct, I'd be very glad. If not, it is fine too, the fun in sharing and discussing is more than good enough.

(defhydra hydra-flyspell (:color amaranth
                                 :body-pre
                                 (progn
                                   (when mark-active
                                     (deactivate-mark))
                                   (when (or (not (mark t))
                                             (/= (mark t) (point)))
                                     (push-mark (point) t)))
                                 :hint nil)
  "
 ^Flyspell^         ^Errors^            ^Word^
---------------------------------------------------------
 _b_ check buffer   _c_ correct         _s_ save (buffer)
 _d_ change dict    _n_ goto next       _l_ lowercase (buffer)
 _u_ undo           _p_ goto previous   _a_ accept (session)
 _q_ quit
"
  ("b" flyspell-buffer)
  ("d" ispell-change-dictionary)
  ("u" undo-tree-undo)
  ("q" nil :color blue)
  ("C-/" undo-tree-undo)

  ("c" my/flyspell-correct-at-point-maybe-next)
  ("n" my/flyspell-goto-next-error)
  ("p" my/flyspell-goto-previous-error)
  ("." my/flyspell-correct-at-point-maybe-next)
  ("SPC" my/flyspell-goto-next-error)
  ("DEL" my/flyspell-goto-previous-error)

  ("s" my/flyspell-accept-word-buffer)
  ("l" my/flyspell-accept-lowercased-buffer)
  ("a" my/flyspell-accept-word)

  ("M->" end-of-buffer)
  ("M-<" beginning-of-buffer)
  ("C-v" scroll-up-command)
  ("M-v" scroll-down-command))

(defun my/flyspell-error-p (&optional position)
  "Return non-nil if at a flyspell misspelling, and nil otherwise."
  ;; The check technique comes from 'flyspell-goto-next-error'.
  (let* ((pos (or position (point)))
         (ovs (overlays-at pos))
         r)
    (while (and (not r) (consp ovs))
      (if (flyspell-overlay-p (car ovs))
          (setq r t)
        (setq ovs (cdr ovs))))
    r))

(defvar my/hydra-flyspell-direction 'forward)
(defun my/flyspell-correct-at-point-maybe-next ()
  (interactive)
  (cond ((my/flyspell-error-p)
         (save-excursion
           (flyspell-correct-at-point)))
        ((equal my/hydra-flyspell-direction 'forward)
         (my/flyspell-goto-next-error)
         ;; recheck, for 'my/flyspell-goto-next-error' can legitimately stop
         ;; at the end of buffer
         (when (my/flyspell-error-p)
           (save-excursion
             (flyspell-correct-at-point))))
        ((equal my/hydra-flyspell-direction 'backward)
         (my/flyspell-goto-previous-error)
         ;; recheck, for 'my/flyspell-goto-previous-error' can legitimately
         ;; stop at the beginning of buffer
         (when (my/flyspell-error-p)
           (save-excursion
             (flyspell-correct-at-point))))))

;; Just an adapted version of 'flyspell-goto-next-error'.
(defun my/flyspell-goto-previous-error ()
  "Go to the previous previously detected error.
In general FLYSPELL-GOTO-PREVIOUS-ERROR must be used after
FLYSPELL-BUFFER."
  (interactive)
  (setq my/hydra-flyspell-direction 'backward)
  (let ((pos (point))
        (min (point-min)))
    (if (and (eq (current-buffer) flyspell-old-buffer-error)
             (eq pos flyspell-old-pos-error))
        (progn
          (if (= flyspell-old-pos-error min)
              ;; goto end of buffer
              (progn
                (message "Restarting from end of buffer")
                (goto-char (point-max)))
            (backward-word 1))
          (setq pos (point))))
    ;; seek the previous error
    (while (and (> pos min)
                (not (my/flyspell-error-p pos)))
      (setq pos (1- pos)))
    (goto-char pos)
    (when (eq (char-syntax (preceding-char)) ?w)
      (backward-word 1))
    ;; save the current location for next invocation
    (setq flyspell-old-pos-error (point))
    (setq flyspell-old-buffer-error (current-buffer))
    (if (= pos min)
        (message "No more miss-spelled word!")))
  ;; After moving, check again if we are at a misspelling (accepting a word
  ;; might have changed this, since the last check).  If not, go to the next
  ;; error again, unless we are at point-min (otherwise we might enter into
  ;; infinite loop, if there are no remaining errors).
  (flyspell-word)
  (unless (or (= (point) (point-min))
              (my/flyspell-error-p))
    (my/flyspell-goto-previous-error))
  (when (my/flyspell-error-p)
    (swiper--ensure-visible)))

(defun my/flyspell-goto-next-error ()
  "Go to the next previously detected error.
In general FLYSPELL-GOTO-NEXT-ERROR must be used after
FLYSPELL-BUFFER."
  ;; Just a recursive wrapper on the original flyspell function, which takes
  ;; into account possible changes of accepted words since the last check.
  (interactive)
  (setq my/hydra-flyspell-direction 'forward)
  (flyspell-goto-next-error)
  ;; After moving, check again if we are at a misspelling.  If not, go to
  ;; the next error again.
  (flyspell-word)
  (unless (or (= (point) (point-max))
              (my/flyspell-error-p))
    (my/flyspell-goto-next-error))
  (when (my/flyspell-error-p)
    (swiper--ensure-visible)))

(defun my/flyspell-accept-word (&optional local-dict lowercase)
  "Accept word at point for this session.
If LOCAL-DICT is non-nil, also add it to the buffer-local
dictionary. And if LOWERCASE is non-nil, do so with the word
lower-cased."
  (interactive)
  (let ((word (flyspell-get-word)))
    (if (not (and (consp word)
                  ;; Check if we are actually at a flyspell error.
                  (my/flyspell-error-p (car (cdr word)))))
        (message "Point is not at a misspelling.")
      (let ((start (car (cdr word)))
            (word (car word)))
        (when (and local-dict lowercase)
          (setq word (downcase word)))
        ;; This is just taken (slightly adjusted) from
        ;; 'flyspell-do-correct', which is essentially what
        ;; 'ispell-command-loop' also does.
        (ispell-send-string (concat "@" word "\n"))
        (add-to-list 'ispell-buffer-session-localwords word)
        (or ispell-buffer-local-name ; session localwords might conflict
            (setq ispell-buffer-local-name (buffer-name)))
        (flyspell-unhighlight-at start)
        (if (null ispell-pdict-modified-p)
            (setq ispell-pdict-modified-p
                  (list ispell-pdict-modified-p)))
        (if local-dict
            (ispell-add-per-file-word-list word))))))

(defun my/flyspell-accept-word-buffer ()
  "See `my/flyspell-accept-word'."
  (interactive)
  (my/flyspell-accept-word 'local-dict))
(defun my/flyspell-accept-lowercased-buffer ()
  "See `my/flyspell-accept-word'."
  (interactive)
  (my/flyspell-accept-word 'local-dict 'lowercase))

Notes: 1) I use undo-tree, place there whatever you use; 2) this lacks the facility of saving the word to the "global" dictionary, which I don't use, but it could easily be added; 3) this is an amaranth hydra, so the session will be mostly "sticky", which is meant, unless you explicitly want to leave it; 4) the hydra includes some navigation commands and alternative bindings for base actions which are not in explicit in the "menu".

d12frosted commented 4 years ago

@gusbrs thank you very much for sharing the code and your thoughts about spell checking! It's valuable, because you bring use cases that I never though about and because you describe it all in details.

I don't personally use hydra but I see how it can be useful when one needs to correct multiple mistakes in one run. Especially when you compare it to the ivy interface, where the functional focus is on the corrections list and not on the actions, which are hidden under M-o (in my experience not all users know about these actions).

So, I am more than happy to provide hydra wrapper for spell correction.

In order for it to function flyspell-correct needs to:

  1. [ ] Be able to work outside of visible part of window (e.g. on whole buffer in one of the directions). This is limitation that we have right now is somewhat artificial, and I placed it for performance reasons only.
  2. [x] Be able to stop correction flow on a specific word and leave the cursor there (just a new action, nothing fancy). But when you (re)start the hydra, how should it behave? Should it continue the previous flow or start a new one? The later is not good for ergonomics and user experience. On the other hand, @gusbrs, why does anyone need to stop at the misspelled word? If it's just to change the word completely, then it can be done as part of #68. For example, I misspelled the word, but my dictionary doesn't have the correction version. For example, in the sentence 'I poured water in gaivan.' I just want to change 'gaivan' to 'gaiwan'. helm interface already allows it. But if I want to change not only the misspelled word, but also it's surroundings, e.g. 'in gaivan' -> 'into gaiwan', then user needs a mechanism to stop hydra/flyspell-correct-wrapper.
  3. [ ] Be able to jump to the next misspelled word even when rapid mode is not enabled on certain action (skip or maybe separate action) - this is discussed in #58.

I haven't tested the code itself, so maybe you solved (2) in a fancy way, it's just not obvious to me by looking at code.

I think that flyspell-correct-wrapper can benefit from these 3 functional improvements as well. In my sense, hydra is a separate wrapper with different focus. While flyspell-correct-wrapper is great for quick and local correction (as you pointed out), hydra is great for review step.

This is fascinating :)

gusbrs commented 4 years ago

@d12frosted I thought you'd like it! :-) And I'm glad you did.

Do try it out. You'll see that most, if not all, of your workflow concerns are indeed covered. (This is not to say the way this is currently implemented there is ready to be somehow incorporated to flyspell-correct as they are).

If you are not much into hydra, you can abstract it for the time being. Just bind the body (hydra-flyspell/body) to some key of your liking (I use "C-c k", good mneumonic) and consider this as a sort of prefix key to those which are defined in the hydra. So "C-c k b" will call flyspell-buffer, "C-c k n" will call my/flyspell-goto-next-error and so on.

As to how it works (UI side) the starting or quitting the hydra will not move point by itself at all, point will only be moved if this is explicitly asked (with my/flyspell-goto-next-error, my/flyspell-goto-previous-error or with some of the navigation commands made available there). But starting the hydra pushes the mark, so we can return to the starting point by popping it.

A typical spell-checking with it will be: call the hydra with "C-c k", normally call "flyspell-buffer" with "b" (unless already done before), if I want to start from BoB "M-<", otherwise just stay put, then start navigating through misspellings on the desired direction with either "n/SPC" or "p/DEL", when correction is desired "c/.", the same with the saving commands, and so on. When finished "q" or "C-g". If I want to return point "C-u C-SPC". Done.

An important difference on the working of the hydra wrt flyspell-correct-wrapper is that the direction can be changed at any time during the process. One can be doing a forward job from the BoB, find that you just made a mistake, go back, do your thing, and then continue forward again.

And yes you are quite correct that the difference is subtle:

Especially when you compare it to the ivy interface, where the functional focus is on the corrections list and not on the actions, which are hidden under M-o.

But it is a critical one in that the hydra does not call the correction interface automatically. You essentially navigate through the misspellings until you decide to do so explicitly. That makes a difference on fluidity for this use case. As said, here I wanted to "mostly skip, and sometimes correct".

On the implementation side, note that as it stands the hydra uses flyspell-correct-at-point, and not flyspell-correct-move. I did try first to get the behaviour I was aiming at through flyspell-correct-wrapper, but I couldn't make it. I could not go around the while loop through the overlays in flyspell-correct-move. The best thing I could come up with from my experience with this hydra within the logic of flyspell-correct-wrapper was my suggestion at #58.

So I did not "solve (2) in a fancy way", but by a really simple one: by not entering a loop, by not moving point by itself, so the issue does not arise. So there is also no "rapid" mode at play here.

As to what to do with it, I'm not sure this functionality should be placed directly in flyspell-correct-wrapper (you might see something I don't though). flyspell-correct-wrapper is brilliant for a local scope kind of task. Note as well that this hydra does not substitute whatever flyspell-correct-interface is in use, it runs on top of it. When from the hydra I call flyspell-correct-at-point I get the Ivy interface (the one I use). So I think of this more being useful as an added layer of functionality rather than substituting anything. Specially because I really like flyspell-correct-wrapper as it is for the task where it shines (local scope). ;-)

As to:

So, I am more than happy to provide hydra wrapper for spell correction.

Thank you very much! I'm at your disposal to think this through and, within my capabilities, try my hands on helping implement this in a way which fits properly into flyspell-correct.

My suggestion is that you try it out with a look for the user interface first, and see how it feels/works. As I initially said, I think most of your concerns as to functionality should be addressed. How to fit this into flyspell-correct is probably trickier. I see two general approaches: try to incorporate some of this concepts organically into flyspell-correct-wapper or just layer this upon flyspell-correct as added functionality. I admit that (as an user) I find the later more appealing, but I do understand flyspell-correct is being developed with a keen awareness of certain design choices, something which I also value, so I do fear this might not "feel right". Anyway, up for discussion and, of course, from my side just suggestions.

d12frosted commented 4 years ago

Hey @gusbrs,

Just wanted to say that I do remember about this issue! Don't worry. I just need to find time to tackle it.

gusbrs commented 4 years ago

Hi @d12frosted , take your time, of course! And if you need something from me with this, let me know.