Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
minad opened this issue Nov 29, 2020 · 36 comments
Labels
question Further information is requested

Comments

@minad
Copy link
Owner

minad commented Nov 29, 2020

Right now there are two implementations:

  • consult--buffer-selectrum has narrowing support
  • consult--buffer-default does not have narrowing support
@minad minad added question Further information is requested dynamic candidates labels Nov 29, 2020
@minad
Copy link
Owner Author

minad commented Dec 2, 2020

The dynamic candidate list issue also applies to:

  • consult-match computes candidates based on matching with the current input regexp
  • consult-rg generates candidates by invoking rg with the current input

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

There has been some discussion on the selectrum tracker:

@minad
Copy link
Owner Author

minad commented Dec 4, 2020

There is the async-completing-read package by oantolin, see the comment #29 (comment)

@minad minad changed the title Is it possible to implement consult-buffer more generically? Dynamic candidates - Is it possible to implement consult-buffer more generically? Dec 4, 2020
@minad
Copy link
Owner Author

minad commented Dec 4, 2020

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
Copy link
Contributor

oantolin commented Dec 5, 2020

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
Copy link
Owner Author

minad commented Dec 5, 2020

@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
Copy link

s-kostyaev commented Dec 9, 2020

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
Copy link
Owner Author

minad commented Dec 9, 2020

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

s-kostyaev commented Dec 9, 2020

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
Copy link
Contributor

oantolin commented Dec 9, 2020

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

Will try it, thank you.

@s-kostyaev
Copy link

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

Become Switch to:   [-  which-key: There are no keys to show]

@oantolin
Copy link
Contributor

oantolin commented Dec 9, 2020

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
Copy link
Contributor

oantolin commented Dec 9, 2020

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

@s-kostyaev
Copy link

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

@oantolin
Copy link
Contributor

oantolin commented Dec 9, 2020

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
Copy link
Contributor

oantolin commented Dec 9, 2020

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

@oantolin
Copy link
Contributor

oantolin commented Dec 9, 2020

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
Copy link
Contributor

oantolin commented Dec 9, 2020

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

@minad
Copy link
Owner Author

minad commented Dec 10, 2020

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

@minad
Copy link
Owner Author

minad commented Dec 10, 2020

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

consult/consult.el

Lines 327 to 329 in b02c191

(consult--selectrum-config
`(,@(unless default-top '(:no-move-default-candidate t))
,@(when initial `(:initial-input ,initial))))

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

@clemera
Copy link
Contributor

clemera commented Dec 10, 2020

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
Copy link
Owner Author

minad commented Dec 10, 2020

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 radian-software/selectrum#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
Copy link
Contributor

clemera commented Dec 10, 2020

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
Copy link
Contributor

clemera commented Dec 10, 2020

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
Copy link
Contributor

oantolin commented Dec 10, 2020

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

@minad minad mentioned this issue Dec 10, 2020
@clemera
Copy link
Contributor

clemera commented Dec 10, 2020

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
Copy link
Owner Author

minad commented Dec 10, 2020

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
Copy link
Contributor

oantolin commented Dec 10, 2020

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
Copy link
Contributor

clemera commented Dec 10, 2020

No risk now fun 😎 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
Copy link
Contributor

clemera commented Dec 10, 2020

@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
Copy link
Contributor

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
Copy link
Contributor

clemera commented Dec 10, 2020

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
Copy link
Contributor

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
Copy link
Contributor

clemera commented Dec 10, 2020

Okay thanks!

@clemera
Copy link
Contributor

clemera commented Dec 10, 2020

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
Copy link
Owner Author

minad commented Dec 11, 2020

I have a working solution in #55.

@minad minad closed this as completed Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants