minad / consult

:mag: consult.el - Consulting completing-read
GNU General Public License v3.0
1.14k stars 99 forks source link

Dynamic candidates - Is it possible to implement consult-buffer more generically? #10

Closed minad closed 3 years ago

minad commented 3 years ago

Right now there are two implementations:

minad commented 3 years ago

The dynamic candidate list issue also applies to:

Any ideas would be welcome here, regarding on how to implement dynamic candidate lists.

There has been some discussion on the selectrum tracker:

minad commented 3 years ago

There is the async-completing-read package by oantolin, see the comment https://github.com/minad/consult/issues/29#issuecomment-738119635

minad commented 3 years ago

This minimal example from the selectrum tracker works for me in "emacs -Q".

(icomplete-mode)

(let ((colla '("this" "is" "the" "colla"))
      (collb '("now" "showing" "collb")))
  (defun test (str pred action)
    (if (eq action 'metadata)
        `(metadata
          (cycle-sort-function . identity)
          (display-sort-function . identity))
      (let ((coll (cond ((string-prefix-p "a " str)
             colla)
            ((string-prefix-p "b " str)
             collb)
            (t
             (append colla collb)))))
    (cond ((null action)
           (cond ((or (string-prefix-p "a " str)
              (string-prefix-p "b " str))
              (concat (substring str 0 2)
                  (try-completion (substring str 2) coll pred)))
             (t (try-completion str coll pred))))
          ((eq action t)
           (all-completions
        (cond ((or (string-prefix-p "a " str)
               (string-prefix-p "b " str))
               (substring str 2))
              (t str))
        coll pred))
          (t nil))))))

(define-key minibuffer-local-completion-map (kbd "SPC") 'self-insert-command)

(completing-read "Check: " 'test)
oantolin commented 3 years ago

I just want to point for people reading this that haven't followed the link to the orderless issue, that I think I've shown there that it isn't orderless's fault this example doesn't work, as it doesn't work with the substring or partial-completion styles either.

minad commented 3 years ago

@oantolin Sorry, I should have made this more clear. This is example is something where we tried out on how to implement this - I am kind of clueless here how to do the dynamic candidate completion correctly.

s-kostyaev commented 3 years ago

Why not just add some *magic* candidates like *recent files* to buffer list and show counsult-recentf if user chose it? I would like to use it with icomplete-vertical. Maybe we should add a customizable option for a user to chose what magic candidates to add in that list.

Now imenu works like this. Actually I prefer to flat imenu before show to avoid one step, but we can use the base idea.

P.S. This is more about consult-buffer and less about dynamic candidates from other processes.

minad commented 3 years ago

Yes, this would be possible but it introduces an intermediate step. The way it is now allows faster switching. And you can always bind consult-recent-file to another key, e.g. C-x C-r.

s-kostyaev commented 3 years ago

Yes, this would be possible but it introduces an intermediate step. The way it is now allows faster switching. And you can always bind consult-recent-file to another key, e.g. C-x C-r.

I know. But if I restart emacs and continue working on my project I've tried to switch to that project's buffer. There is no buffer from that project. I should press C-g and then C-x C-r. It breaks my stream. Personally I prefer recent files to be in the buffer list without any intermediate steps (like it was in helm). But one step is better than all this stuttering.

Now I use desktop-mode for that purpose. But it reads saved desktop too long.

oantolin commented 3 years ago

I think you might be interested in embark-become, @s-kostyaev. It let's you switch from one command to another without losing what you have typed so far in the minibuffer.

s-kostyaev commented 3 years ago

Will try it, thank you.

s-kostyaev commented 3 years ago

@oantolin unfortunately it doesn't work for me. I can see:

Become Switch to:   [-  which-key: There are no keys to show]
oantolin commented 3 years ago

Sorry, I should have explained how to use it. First of all, you don't need to setup any extra bindings. I think you mentioned trying to switch to some buffer, and realizing you don't have it open and needed to do C-x C-r (your binding for consult-recentf) instead, right? In that case, while you are in your buffer switcher you can run embark-become (I like to bind it to C-> because > looks like an arrow), and then, even though which-key shows nothing!, you can still use your C-x C-r binding.

For even more convenience you can setup a keymap containing short bindings for all the commads you think you might want to jump between. For example, embark comes with an embark-file+buffer-map that has bindings b for switch-to-buffer and r for recentf-open-files. So if you were using plain old switch-to-buffer you could jump to recentf-open-files by just C-> r.

You could replace the keybindings in embark-file+buffer-map with the consult versions if you prefer, or make a new keymap, store in a variable, and add the variable name to the list embark-become-keymaps.

Hope that's understandable. Please ask me anything, if you have further questions.

oantolin commented 3 years ago

(I know embark's documentation is pretty sparse, I will work on it soon.)

s-kostyaev commented 3 years ago

Thank you for explanation. I will try to realize it tomorrow :)

oantolin commented 3 years ago

Hey, @minad, how about something vanilla like this for consult-buffer:

(defvar buffer-candidates '("asasdd" "elkfjq" "slfjdsk"))
(defvar file-candidates '("lerjkdd" "lskfjdq" "iuyewn"))
(defvar all-candidates (append buffer-candidates file-candidates))
(defvar current-candidates all-candidates)

;; the following 3 commands should also refresh the completion UI

(defun narrow-to-buffers ()
  (interactive)
  (setq current-candidates buffer-candidates))

(defun narrow-to-files ()
  (interactive)
  (setq current-candidates file-candidates))

(defun widen-to-all ()
  (interactive)
  (setq current-candidates all-candidates))

(defvar narrow-candidates-map
  (let ((map (make-sparse-keymap "Candidates: ")))
    (define-key map (kbd "a") '("all" . widen-to-all))
    (define-key map (kbd "f") '("files" . narrow-to-files))
    (define-key map (kbd "b") '("buffers" . narrow-to-buffers))
    map))

(define-key minibuffer-local-completion-map (kbd "^") #'narrow-candidates-map)

(defun mock-switcher ()
  (interactive)
  (completing-read "Switch to: "
                   ;; the following looks a little like it should be
                   ;; equivalent to passing just current-candidates,
                   ;; but the lambda delays evaluation of
                   ;; current-candidates until completion time
                   (lambda (string predicate action)
                     (complete-with-action action current-candidates
                                           string predicate))))

An alternative would be to make the narrow-candidates-map a command instead of a keymap.

oantolin commented 3 years ago

We can figure out a way to make the key binding to the narrow-candidates-map local to mock-switcher.

oantolin commented 3 years ago

To be concrete, for the command version I mean:

(defvar buffer-candidates '("asasdd" "elkfjq" "slfjdsk"))
(defvar file-candidates '("lerjkdd" "lskfjdq" "iuyewn"))
(defvar all-candidates (append buffer-candidates file-candidates))
(defvar current-candidates all-candidates)

;; the following command should also refresh the completion UI

(defun choose-candidates (ch)
  (interactive "cCandidates: [a]ll, [b]uffers, [f]iles")
  (setq current-candidates
        (pcase ch
          (?a all-candidates)
          (?b buffer-candidates)
          (?f file-candidates))))

(define-key minibuffer-local-completion-map (kbd "^") #'choose-candidates)

(defun mock-switcher ()
  (interactive)
  (completing-read "Switch to: "
                   ;; the following looks a little like it should be
                   ;; equivalent to passing just current-candidates,
                   ;; but the lambda delays evaluation of
                   ;; current-candidates until completion time
                   (lambda (string predicate action)
                     (complete-with-action action current-candidates
                                           string predicate))))
oantolin commented 3 years ago

And as a little nicety, one could update the prompt to indicate what candidates are being considered.

minad commented 3 years ago

@oantolin These are nice ideas, with the ^ prefix. I will try it!

minad commented 3 years ago

@oantolin @clemera How do you change the prompt? By messing with the minibuffer content?

But regarding this I guess I have to wait a bit until Selectrum gets support for dynamic candidates via the completing-read API, or I have to do something about it. But given my non-familiarity with the Selectrum code...

When looking at this example, I get the feeling that the dynamic aspect of the completing-read API is badly designed. For example if you pass a lambda as completion table and only overwrite sorting, but the candidates are still static, you force expensive reevaluation. Right now selectrum makes the choice to never reevaluate which is fast, but wrong. I think there should be some way to tell the completion-system that candidate reevaluation is not desired. Something like a metadata property 'candidates-are-actually-static. As far as I understood the current plan in Selectrum was to hard code a list of static completion tables (or maybe make this configurable). Alternatively one could offer a Selectrum option which allows to configure the behavior. I could configure that setting in consult as I already do for other things:

https://github.com/minad/consult/blob/b02c1916d76714982bca204e866a0ff9a4f49ba1/consult.el#L327-L329

Regarding icomplete, I am not sure what can be done here. Maybe propose something for upstream

clemera commented 3 years ago

How do you change the prompt? By messing with the minibuffer content?

Could you elaborate?

When looking at this example, I get the feeling that the dynamic aspect of the completing-read API is badly designed.

I think this because dynamic completion was probably added as an afterthought. The default completion mechanism only computes completions when you press TAB and there where no "continued completion" frameworks back then. Generally bringing the fruits of the discussion around this up to Emacs devel would be great!

@oantolin Nice workaround, I like that, too! You still need to trigger an update of the UI I think, for icomplete it seems the candidates don't update automatically, too?

With Selectrum you can make the example currently work like this:

(define-key selectrum-minibuffer-map (kbd "^") #'choose-candidates)

(defun choose-candidates (ch)
  (interactive "cCandidates: [a]ll, [b]uffers, [f]iles")
  (setq current-candidates
        (pcase ch
          (?a all-candidates)
          (?b buffer-candidates)
          (?f file-candidates)))
  (setq-local selectrum--previous-input-string nil)
  (setq-local selectrum--preprocessed-candidates
               current-candidates))
minad commented 3 years ago

How do you change the prompt? By messing with the minibuffer content?

Could you elaborate?

@oantolin proposed to change the prompt during an active completing-read session. I don't know how to do it. Therefore my question: How do you change the prompt?

I think this because dynamic completion was probably added as an afterthought. The default completion mechanism only computes completions when you press TAB and there where no "continued completion" frameworks back then. Generally bringing the fruits of the discussion around this up to Emacs devel would be great!

Yes, we should do that. I also mentioned something like that in https://github.com/raxod502/selectrum/issues/225. By improving the standard API we can improve the situation for everyone and we (or rather you as selectrum developer) are in the best position to make well-informed proposals to upstream. This should be somehow backwards compatible, ideally with graceful degradation. If we can make things work in icomplete and in selectrum with additional selectrum-specific non-recompute optimizations it would be great. These selectrum-specifics could then be integrated into the official API at some point.

clemera commented 3 years ago

proposed to change the prompt during an active completing-read session. I don't know how to do it. Therefore my question: How do you change the prompt?

Thanks, I missed that. You could use an overlay for that, see for example selectrum--count-overlay.

clemera commented 3 years ago

Alternatively one could offer a Selectrum option which allows to configure the behavior. I could configure that setting in consult as I already do for other things

Maybe we could allow recomputation of small tables by default (less then 1000 cands returned initially or something similar, could be configureable), that would probably work out for most cases and if not you can set it to most positive fix num to force recomputation even for bigger ones.

oantolin commented 3 years ago

@minad said:

When looking at this example, I get the feeling that the dynamic aspect of the completing-read API is badly designed. For example if you pass a lambda as completion table and only overwrite sorting, but the candidates are still static, you force expensive reevaluation.

You can do whatever you want in a lambda! If you don't want to recompute, cache. There is even a built-in completion-table-with-cache function to help with this.

Right now selectrum makes the choice to never reevaluate which is fast, but wrong. I think there should be some way to tell the completion-system that candidate reevaluation is not desired. Something like a metadata property 'candidates-are-actually-static.

I agree that Selectrum is wrong about this, but don't judge. Orderless is wrong too! It only calls the completion table on the prefix (usually empty or, for file completion, the directory) and does all filtering via completion-regexp-list. So basically it does what Selectrum does, too.

@clemera said:

You still need to trigger an update of the UI I think, for icomplete it seems the candidates don't update automatically, too?

Yes, I was too lazy to include the code so I just added a comment to the example saying it should be done:

@minad asked how to change the prompt, and @clemera said:

You could use an overlay for that

I forgot at the time I wrote "we can figure out" what the trick was, but yes an overlay is the way to go. The builtin modes minibuffer-electric-default-mode, minibuffer-depth-indicate-mode and minibuffer-eldef-shorten-default, use overlays, as does Embark to add an indicator to the minibuffer prompt, for example.

clemera commented 3 years ago

I agree that Selectrum is wrong about this, but don't judge. Orderless is wrong too! It only calls the completion table on the prefix (usually empty or, for file completion, the directory) and does all filtering via completion-regexp-list. So basically it does what Selectrum does, too.

Technically yes, but UI experience is more important to me in this case. Though I would be really happy if there would be better compromise than what we have currently.

minad commented 3 years ago

You can do whatever you want in a lambda! If you don't want to recompute, cache. There is even a built-in completion-table-with-cache function to help with this.

I agree, and I did things like this. But there is just one thing which confuses me - I perceive icomplete as slower than selectrum, it also seems to create more load/computations. And I don't get why. In particular if you use M-x which is a large table. As if it is always lagging a bit behind my keypresses. I set the delay to 0.

I agree that Selectrum is wrong about this, but don't judge. Orderless is wrong too! It only calls the completion table on the prefix (usually empty or, for file completion, the directory) and does all filtering via completion-regexp-list. So basically it does what Selectrum does, too.

What does that mean? So if the candidates change at some point, orderless won't see the new candidates?

oantolin commented 3 years ago

What does that mean? So if the candidates change at some point, orderless won't see the new candidates?

No, no, don't worry. I'm not quite as reckless as the selectrum people 😛 Orderless does call the completion table (with the prefix as argument) on every keystroke, so it does pick up changes (I actually only tested the above code that changes current-candidates with orderless).

But orderless is still wrong since a completion table can do whatever the hell it wants. It can decide that the empty string matches apple and artichoke but that a only matches artichoke. Orderless would give you both apple and artichoke because it actually calls the completion table with the empty string and then matches the a. 🤷

My sincere hope is that nobody will write a completion table orderless gives wrong answers on.

clemera commented 3 years ago

No risk now fun :sunglasses: There are not too many dynamic tables which need recomputation on every key press so usually we get away with it. But as I said I would also like to support all cases, just in a way they don't destroy the general UI experience we currently have.

clemera commented 3 years ago

@oantolin What I wonder about when the table uses complete-with-action the candidates are passed to all-completions for the t action. Doesn't this bypass completion-styles as all-completions prefilters the candidates based on current string prefix? So complete-with-action isn't actually something you should use unless you want that prefiltering...or do I get something wrong?

oantolin commented 3 years ago

all-completions does what you say if you call it on a list, hash table or obarray, but if the completion table is a function, then all-completions delegates to that function completely, it just returns whatever the function says.

clemera commented 3 years ago

Ah yes, but that means if you have a sequence you are better off not passing it to complete-with-action unless you want that prefiltering to happen right? For a long time I did not look at the code and just assumed it handles the API for me as intended.

oantolin commented 3 years ago

Ah yes, but that means if you have a sequence you are better off not passing it to complete-with-action unless you want that prefiltering to happen right? For a long time I did not look at the code and just assumed it handles the API for me as intended.

Yes, I think that's correct.

clemera commented 3 years ago

Okay thanks!

clemera commented 3 years ago

I noticed many tables use all-completions with the passed string. Either directly or indidrectly via complete-with-action. For example (help--symbol-completion-table "eval" nil t) gives you all the symbols with "eval" prefix. This is a problem if you want to have the last word when it comes to filtering (like selectrum or orderless). On the other hand not passing the string breaks applications where the returned candidate set depends on the input. But I think passing the empty string is more robust and covers the more important use case (that we do all the filtering and not the function), so I think it makes sense to keep it that way.

minad commented 3 years ago

I have a working solution in #55.