A handier skip for flyspell-correct-ivy

[gusbrs]

Hi,

I've been a happy user of flyspell-correct, and of flyspell-correct-ivy in particular, for some time. Thank you very much.

Since it came around, I also rely mainly on flyspell-correct-wrapper for my "on the spot" typing mistakes. However, I see myself somewhat frequently stumbling around the prefixes, and wishing I had thought all the way through before I started the command. Alas, I mostly don't... Commonly, I'd start flyspell-correct-wrapper just to find out I was actually aiming for the second (or more) spelling mistake before point. If so, the alternative would be to cancel the current operation, start it all over, now with the prefix, "M-o k" to skip the first (again, if it's more), to then reach the desired typo.

Also, I was also thinking the "Skip" action could be more useful when not in "rapid", for it just quits. Finally, ivy-actions are neat for eventual operations, but "M-o k" can be too much for an operation I want to be fast and might want repeatedly. So I had an itch to scratch, I did scratch it today, and thought I might as well share.

The result is bellow, with main features:

• Action "Skip" does not leave the loop even when rapid is nil, that is, when the command is called without prefix.
• It introduces command my/flyspell-correct-ivy-skip, bound in flyspell-correct-ivy-map (which had to be created), so that we can skip in succession by simply repeating the key initially used to call flyspell-correct-wrapper (or -previous or -next). This makes it easier to adjust the task after starting a flyspell-correct operation, at least to some degree.

This is still only lightly tested, but so far I'm quite glad with the results. I'd be even happier if you guys also like it, and to eventually find something along these lines incorporated to flyspell-correct.

The code uses el-patch syntax, because that's what I'm using here, and also because it makes it easy to see where I changed stuff. But if that's a problem, I can make it otherwise.

(defvar my/flyspell-correct--skip nil)
(el-patch-defun flyspell-correct-move (position &optional forward rapid)
  "Correct the first misspelled word that occurs before POSITION.

Uses `flyspell-correct-at-point' function for correction.

With FORWARD set non-nil, check forward instead of backward.

With RAPID set non-nil, automatically continues in direction
until all errors in buffer have been addressed."
  ;; NOTE: The way I may be pushing the mark may possibly be more
  ;; idiomatically done using the opoint arg of
  ;; `flyspell-correct-word-before-point'.
  (interactive "d")
  (el-patch-add
    ;; this is done in 'flyspell-correct-wrapper', but not in
    ;; 'flyspell-correct-next' or 'flyspell-correct-previous', which thus
    ;; result in error if there is no mark.
    (when (or (not (mark t))
              (/= (mark t) (point)))
      (push-mark (point) t)))
  (save-excursion
    (let ((incorrect-word-pos))

      ;; narrow the region
      (overlay-recenter (point))

      (let ((overlay-list
             (if forward
                 (overlays-in (- position 1) (point-max))
               (overlays-in (point-min) (+ position 1))))
            (overlay 'dummy-value))
        (while overlay
          (setq overlay (car-safe overlay-list))
          (setq overlay-list (cdr-safe overlay-list))
          (when (and overlay
                     (flyspell-overlay-p overlay))
            (setq incorrect-word-pos (overlay-start overlay))
            (let ((scroll (> incorrect-word-pos (window-end))))
              (goto-char incorrect-word-pos)
              (when scroll (ignore-errors (recenter))))

            ;; try to correct word `flyspell-correct-at-point' returns t when
            ;; there is nothing to correct. In such case we just skip current
            ;; word.
            (unless (flyspell-correct-at-point)
              (when (/= (mark t) (point)) (push-mark (point) t))
              (when (el-patch-swap
                      (not rapid)
                      (and (not my/flyspell-correct--skip)
                           (not rapid)))
                (setq overlay nil))))))

      (when incorrect-word-pos
        (goto-char incorrect-word-pos)
        (forward-word)
        (when (= (mark t) (point)) (pop-mark))))))

(defun my/flyspell-correct-ivy-skip ()
  (interactive)
  (setq my/flyspell-correct--skip t)
  (setq ivy-exit 'done)
  (exit-minibuffer))

;; These are the same keys I use for `flyspell-correct-wrapper',
;; `flyspell-correct-previous' and `flyspell-correct-next'.
(defvar flyspell-correct-ivy-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "C-;") #'my/flyspell-correct-ivy-skip)
    (define-key map (kbd "C-.") #'my/flyspell-correct-ivy-skip)
    (define-key map (kbd "C-,") #'my/flyspell-correct-ivy-skip)
    map))

(el-patch-defun flyspell-correct-ivy (candidates word)
  "Run `ivy-read' for the given CANDIDATES.

List of CANDIDATES is given by flyspell for the WORD.

Return a selected word to use as a replacement or a tuple
of (command, word) to be used by `flyspell-do-correct'."
  (el-patch-add
    (setq my/flyspell-correct--skip nil))
  (let* (result
         (action-default (lambda (x) (setq result x)))
         (action-save-word (lambda (_) (setq result (cons 'save word))))
         (action-accept-session (lambda (_) (setq result (cons 'session word))))
         (action-accept-buffer (lambda (_) (setq result (cons 'buffer word))))
         (action-skip-word (lambda (_)
                             (el-patch-swap
                               (setq result (cons 'skip word))
                               (setq my/flyspell-correct--skip t))))
         (action `(1
                   ("o" ,action-default "correct")
                   ("s" ,action-save-word "Save")
                   ("S" ,action-accept-session "Accept (session)")
                   ("b" ,action-accept-buffer "Accept (buffer)")
                   ("k" ,action-skip-word "Skip"))))
    (ivy-read (format "Suggestions for \"%s\" in dictionary \"%s\": "
                      word (or ispell-local-dictionary
                               ispell-dictionary
                               "Default"))
              candidates
              :action action
              (el-patch-add :keymap flyspell-correct-ivy-map)
              :caller 'flyspell-correct-ivy)
    (el-patch-add
      (when my/flyspell-correct--skip
        (setq result (cons 'skip word))))
    result))

[d12frosted]

@gusbrs thank you very much for sharing your feedback and ideas on how to improve usability of flyspell-correct. I haven't read your comment very thoughtfully, but on the high level it sounds amazing! I also quite often found myself in a situation when skip imitating rapid mode would help me. I like your focus on ergonomics!

I will play with this code as soon as possible (hopefully, on Sunday).

[d12frosted]

@gusbrs it seems that I won't get a chance to deeply look into your proposal. Will focus on it during the next week. Sorry for delay.

[gusbrs]

@d12frosted Please be at ease with this. Take your time, there is absolutely no rush from my side.

PS: Further testing/playing here suggests me you will most probably like it too. :-)

[d12frosted]

Nice. I played a bit with your version and I like it. I think that solution can be simplified after #61. Because skip command is now explicitly returned by flyspell-correct-word-at. Which means that I can introduce a variable (for the sake of the users) flyspell-correct-skip-as-rapid (set to t by default) and treat skip result as attempt to jump to the next word when flyspell-correct-skip-as-rapid is set to t (even if rapid is nil). WDYT?

This doesn't solve the keybinding improvement. Will check later if it's possible to do it in a generic way.

[gusbrs]

I'm glad you liked it!

As to how to implement it, I have no particular attachment to the way I came up with it. I didn't examine #61 thoroughly, but a cursory look suggests that indeed the approach can be simplified as you describe. And I'm all for it, if there is a way to implement the same functionality in a more organic way to the code.

Of course I'm only at the suggesting side here, but I think the biggest treat is the keybinding. Sure it's great already to have a skip that won't quit, but I find "M-o k" in the Ivy interface demanding for this particular task (it is perfectly fine for, e.g. saving the word to the dictionary). My elisp-fu is no big deal, but as far as I understand, we will need an interactive function for that. If so, that function will have to be able to receive/pass the variable's value. Perhaps the return value of each of flyspell-correct-interface functions can be tweaked to return this alongside what they already do (and is required for things to work), but this sounds tricky to me. So I wonder if simply passing a defvar is not the easiest way to achieve this after all. I also can't see how this function might be made interface independent, but I might be wrong, of course. Anyway, I'm just a toddler at this, so and I'm more than willing to watch and learn.

[gusbrs]

Hi @d12frosted , I see you set a milestone for v1.0, and I assume it means you intend to think more carefully on how to implement this feature. As I said before, it's all fine for me.

But I actually have another suggestion to make to flyspell-correct and I didn't want to hit your issue tracker again while this one is pending, without checking it with you. Also, this is the kind of suggestion which would probably fit better before the milestone.

This idea I had of improving the skip of flyspell-correct came from an hydra I made some time ago for flyspell. This hydra is not strictly within the logic of flyspell-correct but is rather aimed at making whole buffer spell checking, and uses flyspell-correct-at-point to do the actual correction job. It resulted in a very smooth user experience for this task. I'm really happy with it.

So, if you'd like to take a look at it (no commitment in so doing), to see if any other ideas come out of it, or if you'd be interested in incorporating it, just let me know, and I'll be glad to share it (in another issue, I suppose).

[d12frosted]

@gusbrs please do share it! 😸

[gusbrs]

Done at #69 . Have fun! :-)

[d12frosted]

Thank you!

[d12frosted]

@gusbrs I went ahead and implemented a functionality that treats skip action as a one-time enabler for rapid mode. Which means that even if you didn't enable rapid mode, but used action 'skip', flyspell-correct-wrapper will jump to previous (or next) misspelled word. Here again you can use skip action to go even further. But if you chose any other action, rapid mode will be stopped. There is a separate test for this.

I didn't introduce any configuration variables, as I think that this is super intuitive. If you don't want to do anything, just C-g.

Now the only part left here is the keymap. I am not sure if this feature makes sense in the flyspell-correct-ivy package, but I want at least to make it easy to implement. As far as I can see, the only bit that is left is to create a flyspell-correct-ivy-map, so you can use it.

[d12frosted]

Ok, and I actually implemented the last bit. So now you can simply use flyspell-correct-ivy-map for your needs.

(defun flyspell-correct-ivy-skip ()
  (interactive)
  (ivy-exit-with-action
   (lambda (_) (setq flyspell-correct-ivy--result (cons 'skip "")))))

(define-key flyspell-correct-ivy-map (kbd "C-;") #'flyspell-correct-ivy-skip)

Well, this looks like much less code than what you have right now ;) Hope you enjoy it! Let me know if there are any problems with current implementation.

P. S. Even though I share a code where 'private' variable is used (flyspell-correct-ivy--result), be careful if you are going to use it for other purposes. flyspell-correct-interface is your friend in terms of documentation for flyspell-correct-ivy--result format.

[gusbrs]

I went ahead and implemented a functionality that treats skip action as a one-time enabler for rapid mode. Which means that even if you didn't enable rapid mode, but used action 'skip', flyspell-correct-wrapper will jump to previous (or next) misspelled word. Here again you can use skip action to go even further.

That's brilliant! Thank you very very much!

I've tested it here and it looks great. I also took a look at the corresponding commits, and the way you chose to do it is both more general and better integrated to the machinery of flyspell-correct than what I had come up with. In terms of functionality, they are equivalent, so the suggestion here is fully taken care of.
(I will test it further though, and report if I find anything. But as far as I see thus far, I don't expect to find any trouble).

I didn't introduce any configuration variables, as I think that this is super intuitive. If you don't want to do anything, just C-g.

I do agree. I'm suspect in that of course though. ;)
I don't know as you do the users base, but I think this should be fine, and anyway is easy to adjust if we misjudged this.

I am not sure if this feature makes sense in the flyspell-correct-ivy package, but I want at least to make it easy to implement.

I'd like to argue in favour of the inclusion of flyspell-correct-ivy-skip. Reasons: 1) I believe it has the potential of wide use; 2) it allows you to not need to recommend the use of an internal variable by the end users (and I do agree with you that flyspell-correct-ivy--result should be internal); 3) I don't see any significant inconvenience in this inclusion, so why not? I do concur though that any keybindings should be left to users here.

Of course, things are also great if this is left for users to define themselves. And for me personally it is already perfect, as it is very much true that:

Well, this looks like much less code than what you have right now ;)

So there's no pushing, just sharing my thoughts on this choice.

There is only one thing I think is worth suggesting further in this. You've chosen to expose the variable flyspell-correct-ivy--result for the Ivy interface only. I'd expect this functionality to be useful to other flyspell-correct interfaces (at least any that has a keymap?, and I presume some of them do). So, why not expose flyspell-correct--result instead, which can then be used by any of the interfaces in equal grounds in case its useful there?

Hope you enjoy it!

I already do! And I hope you do too, alongside other users of flyspell-correct.

Again, thank you very much for this!