oantolin / embark

Emacs Mini-Buffer Actions Rooted in Keymaps
GNU General Public License v3.0
952 stars 58 forks source link

Cycle candidates in embark-live-occur? #31

Closed JSDurand closed 2 years ago

JSDurand commented 4 years ago

Hello: I think it would be nice to be able to go forward or go backward completion candidates in the embark live occur buffer, like what icomplete-forward-completions and icomplete-backward-completions do.

So I wrote two functions and modified embark-minibuffer-candidates to provide this functionality. I am wondering what do you think of this?

Thanks for the nice package in any case. :)

(defun embark-forward-completions (&optional n)
  "Step forward completions by N entry.
The N-th entry becomes the first and can be selected with
`minibuffer-force-complete-and-exit'."
  (interactive "p")
  (let* ((beg (if (window-minibuffer-p)
                  (minibuffer-prompt-end)
                (nth 0 completion-in-region--data)))
         (end (if (window-minibuffer-p)
                  (point-max)
                (nth 1 completion-in-region--data)))
         (comps (completion-all-sorted-completions beg end))
         (tail (cdr (last comps))))
    (when comps
      ;; make it a real list
      (setf (cdr (last comps)) nil)
      ;; Rotating an entire round is the same as no rotation at all.
      (setf n (mod n (length comps)))
      ;; rotating
      (setf comps (append (nthcdr n comps)
                          (butlast comps (- (length comps) n)))
            ;; adjoin the tail back
            (cdr (last comps)) tail)
      (completion--cache-all-sorted-completions
       beg end comps))
    (embark-occur--update-linked)))

(defun embark-backward-completions (&optional n)
  "Step backward completions by N entry.
The N-th-to-last entry becomes the first and can be selected with
`minibuffer-force-complete-and-exit'."
  (interactive "p")
  (let* ((beg (if (window-minibuffer-p)
                  (minibuffer-prompt-end)
                (nth 0 completion-in-region--data)))
         (end (if (window-minibuffer-p)
                  (point-max)
                (nth 1 completion-in-region--data)))
         (comps (completion-all-sorted-completions beg end))
           (tail (cdr (last comps))))
    (when comps
      ;; make it a real list
      (setf (cdr (last comps)) nil)
      ;; Rotating an entire round is the same as no rotation at all.
      (setf n (mod n (length comps)))
      ;; rotate
      (setf comps (append (nthcdr (- (length comps) n) comps)
                          (butlast comps n))
            ;; adjoin the tail back
            (cdr (last comps)) tail)
      (completion--cache-all-sorted-completions beg end comps))
    (embark-occur--update-linked)))

(defun embark-minibuffer-candidates ()
  "Return all current completion candidates from the minibuffer.
Modified by Durand."
  (when (minibufferp)
    (let* ((all (completion-all-sorted-completions
                 (minibuffer-prompt-end)
                 (point-max)))
           (last (last all)))
      (when last (setcdr last nil))
      all)))
oantolin commented 4 years ago

Thanks! This is an interesting suggestion.

You can of course already use minibuffer-force-complete and cycle forward through candidates, but I guess it's not a great experience because:

  1. The order of the candidate cycling doesn't match the order of the candidates in the embark occur buffer.

  2. When you start cycling the embark live buffer updates to display only the items that match the current completion (and usually only it matches it, so you cannot see any other candidates!).

  3. In particular, you get no indication of which candidate comes next in the cycle.

  4. Finally, you might want to cycle backwards sometimes.

Points 1 and 2 are fixed by your modified embark-minibuffer-candidates.

But I don't think I would want to impose completion-all-sorted-completions's preferred ordering on all users (I quite like that embark-occur preserves the order chosen by the completion table, and I usually don't want candidates sorted by length ---plus it's faster not to sort them at all!). One way to deal with this would be to call your function by a different name, maybe embark-sorted-minibuffer-candidates, and tell users to replace embark-minibuffer-candidates in embark-candidate-collectors with embark-sorted-minibuffer-candidates if they wish to use this ordering. Maybe if people like this ordering, it could even be the default and I can just set the old one in my personal configuration.

Points 3 and 4 are dealt with by the other two functions you wrote. (By the way, I would implement the second one by calling the first: (embark-forward-completions (- n)) ---after all, you do call mod and (mod -2 10) = 8.)

Now, these rotate the entire list in the embark occur buffer, right? I'm not sure that's the best way to indicate visually which candidate comes next. Wouldn't it be nicer to keep the order in the buffer fixed and highlight the current candidate instead? Look at Emacs completion UIs: most use the fixed-order, move-highlight strategy rather than the rotate-entire-list strategy and I think that's a reflection of what most people prefer. (Although, I myself don't really have a preference between those two UX choices: I've been a happy user of each of icomplete, ivy and helm.)

oantolin commented 4 years ago

Also, I should have said: there is already a very different mechanism for moving among the candidates and selecting one (a mechanism not involving minibuffer-force-complete-and-exit). You can use embark-switch-to-live-occur and navigate normally, then press RET when you reach the desired completion.

To make this very convenient, in my configuration I bind <down> in the minibuffer to embark-switch-to-live-occur ---and I also bind <right> to this function:

(defun embark-forward-char-or-switch-to-live-occur ()
  "Move forward one char if possible, else switch to Embark Occur buffer."
  (interactive)
  (if (eobp) (embark-switch-to-live-occur) (forward-char)))

So if I feel the urge to move amongst completions I shift a hand to the arrow keys, navigate to the completion and press RET. In list view this is pretty much like using your embark-forward/backward-completions functions and then minibuffer-force-complete-and-exit (well, there is one advantage of switching to the embark occur buffer in list view: you can use isearch to search even among the annotations!). In grid view, I actually think switching to the embark occur buffer is even better, because you don't just move forward and back: you also have up and down, so it's faster to reach any given completion.

JSDurand commented 4 years ago
  1. After I opened the issue, I read more in depth into the codes of embark, and realized that the intended interaction model of live-occur is to switch to the occur buffer and then select the candidates. I think I need to work with it for some time to see how it works out for me. BTW, the reason I tried to go forward/backward the candidate list is because I don't see the grid view has any effect. Maybe I shall open another issue for that?

  2. Yes, I should have called forward with a negative argument for the backward function. :)

  3. Currently I have two interaction models in mind for an occur buffer:

    1. occur-buffer-centric: the cursor stays in the occur buffer the whole time, and remap self-insert-command to embark-self-insert, roughly equivalent to (progn (select-window (active-minibuffer-window)) (self-insert-command) (embark-switch-to-live-occur)).
    2. minibuffer-centric: the opposite of the above: the cursor stays completely inside the mini buffer, and bind keys to move highlight in the occur buffer.

It seems like the current interaction model is a mixture of the above two? Maybe we can add bindings from both models to facilitate the completion process?

Also I have no particular preference on sorted v.s. all completions, though sometimes I want icomplete to keep the original completion table intact. Maybe one should bring this to emacs-devel to add an option, to sort or not to sort. :)

I am looking forward to how this package is going to be after it is stabilizes, since it feels like a simpler version of helm, and one might use it to do similar things, like interactive incremental rg searches, and org-heading-searches (ala org-rifle). 👍

oantolin commented 4 years ago
  1. I don't see the grid view has any effect.

What do you mean? Does the command embark-occur-toggle-view not work with your configuration? You can call it from the embark occur buffer or from the buffer it is linked to. You can also control whether the initial view is list or grid with embark-occur-initial-view-alist and embark-occur-default-initial-view (those variable names might be slightly wrong, I'm on my phone and didn't check).

Also, yet another way to interact with the candidates that you might want to try is the avy-embark-occur package in this same repo. (This can be used in a minibuffer-centric way.)

JSDurand commented 4 years ago

Ah, I see the reason. I was playing around with something like this:

(let ((completing-read-function 'embark-completing-read)
      (embark-occur-view 'grid)
      icomplete-mode)
  (completing-read "test: "
                   (append
                    '("apple" "approval" "acapella" "aladin")
                    (mapcar 'number-to-string
                            (number-sequence 1 101)))))

And it didn't work (I got the prototype of this snippet from the dotemacs files of Protesilaos). But after I change to bind embark-occur-initial-view-alist instead, it works great. :D

I shall experiment with ivy-embark-occur to see how it fits in. ;)

minad commented 2 years ago

@oantolin It seems to be okay to close this one, since it hasn't been updated for a long time.

oantolin commented 2 years ago

Yes, I'm closing as WONTFIX, I guess.