-
Notifications
You must be signed in to change notification settings - Fork 33
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
Handling of dynamic minibuffer-completion-table #114
Comments
Hmmm... how often are dynamic completion tables used? There was one in |
I don't know, I stumbled across this issue when someone on reddit shared a command for locate.
It is probably rare but I think it's kind of bad/annoying if commands using default An other easier workaround would be to provide a command similar the recently introduced |
Your suggestions are good. Any of that would be fine. Another option would be to simply recompute the table on every input change, but have a customizable list of table symbols that are known to be static. |
I have experimented and while the timer approach works I don't like that the filtering behaves like helm (the output is noticeable delayed). It feels much more snappy with the current approach. I think dynamic minibuffer tables where the candidates can change with input are to rare to justify this decline in UI experience. I would prefer to default to the current behaviour and then maybe having the option to declare some special tables as needing to use the timer approach. But maybe having a command to recompute the current table on demand would suffice already (this is also how those tables where designed to be used because with default completion they are only recomputed on TAB press, too). When a candidates function gets passed to |
Ok, good to know about the timer approach. I agree we don't want to go the Helm way. There are a couple of good options, as you point out -- it's a balance between making things work out of the box / without extra keystrokes being required, allowing extensibility for people to deal with cases not supported properly, and not having more options than are really needed. I think you've got more context than me about what the right thing to do is. |
What is needed to support completing-read with a dynamic generation function in selectrum? I cannot implement consult-buffer based on completing-read it seems, since the generation function is only called once. Independent of that, you discuss timer based solutions - I would prefer not to do such a thing on the level of the selection framework. I think rather the generating function should take care of caching. But I don't know of current uses of generation functions, are they all fast or should the completion system handle potential slowness of the generation function? |
Essentially the dynamic function needs to be called on each input change. This is what we do when you pass a function to
Some built-in tables like |
I haven't tried helm for a long time. But slowness was certainly a big issue. For now I am not concentrating on consult-buffer. |
I suggested it as it should work with all dynamic completion tables AFAIK. But I probably should have suggested icomplete which is probably be the most API compliant incremental narrowing framework as of today. |
I'm trying to implement something like helm-projectile-ag (or rg/grep, etc), and I believe having being able to dynamically generate the completion list would be nice too! For now, I have something like this (I'm calling it projectile-ag because it depends on projectile to find within the project, and my understanding is that one of selectrum's tenets is that people should just be able to use
And then I realized that
And although the function is executed every time (the messages appear) the completion options in the minibuffer don't change. Also, I agree with @minad that the generating function should take care of the caching if necessary. |
Yes, but note that the completion table gets called multiple times to query information from it, so you need to take care that the process isn't invoked unnecessarily. To help with this there is the helper function (completing-read
"Locate: "
(completion-table-with-cache
(lambda (str)
(process-lines
"locate" "-i" "-l" "10" (concat "*" (replace-regexp-in-string " +" "*" str) "*"))))) But there is a problem with this as I just realized recently. Selectrum wants do its own matching on the results while the completion code in Emacs very much assumes the default completion UI (with the assumption that your input is a prefix of the matches). (funcall
(completion-table-with-cache
(lambda (str)
(process-lines
"locate" "-i" "-l" "10" (concat "*" (replace-regexp-in-string " +" "*" str) "*"))))
"bin" nil t) But calling it with the empty string will call the process with the empty string which is not what you would want. With Selectrum we would want to get the candidates returned by the process and then filter them afterwards according to user settings. I don't have a solution for this currently :( |
There is also the problem that there exists no real (defun check-try (string _table _pred point &optional _metadata)
(cons string point))
(defun check-all (string table pred _point)
(all-completions string table pred))
(defun test (str pred action)
(setq-local completion-styles '(check))
(setq-local completion-styles-alist
'((check check-try check-all "Checking")))
(when (eq action t)
(all-completions
""
(process-lines
"locate" "-i" "-l" "10" (concat "*" (replace-regexp-in-string " +" "*" str) "*"))
pred)))
(completing-read "Test: " 'test) But this is ugly and also for other cases we wouldn't want to pass the string like for example |
@clemera I think we could try to first get the simple cases to work? I thought about writing a consult-match function which expects the user to enter a regexp/string, then the candidate set is generated dynamically. Maybe after pressing space, the dynamic generation is stopped and the normal filtering continues - or do everything completely dynamic. I started something here https://github.com/minad/consult/tree/consult-match, but it is static as of now. |
I don't have any simple examples I could test with, do you have any tables at hand that work with default completion but not with selectrum? |
No, but I would for example try to implement this consult-match thing I mentioned above. And then we could see if that is possible to support in selectrum. |
Sounds good, I will look out for more examples, too. |
BTW I just noticed the completion-styles hack would also be an alternative way to implement |
You mean to fix the completion style such that it has special support for the prefix used by consult--buffer? I thought about something like this in this old discussion oantolin/orderless#24 (comment) But I like the solution we are having now for narrowing very much - did you see the improvements I added regarding backspace and showing a fancier narrowing indicator. I don't think there is much to be improved here. The only thing missing from the current implementation is the feature to show invisible buffers by pressing SPC - this will require dynamic candidates. But I don't think I will add support for that. How often do you switch to an invisible buffer? I rather concentrate on a good solution for the most common cases. |
No I did not, I have to check. I also like the current implementation its simple and robust, I think, also less expensive because the table does not need to be requeried. |
Yes, I agree. The more things we can do statically, the more robust it will be. And yes, also potentially more performant. But for example for icomplete I don't except a difference, even if there is a recomputation it will just return the same cached list. But still - I think at some point we need a solution for dynamic candidates, e.g. for consult-match and consult-rg/grep. There are not that many other commands on the wish list of consult which would need it, but these two I think are reasonably useful. Furthermore the question is also what other dynamic commands are there in the Emacs code base or inside commonly used packages. |
It does? I thought it only caches the very last computation of completions but I may be wrong.
Yes, we should collect some examples here and then go from there. |
My experiments above indicate that it might be quite hard to get such a dynamic completion table working with the default API. Maybe a different interaction model could also make sense for these commands. You could read the string passed to the process first and then get the candidates which you can then narrow with the completion UI. This would also make sense regarding the search syntax which is different between completion UI and the external process. As long as it is easy to edit the search string passed to the process I think I would like this. |
Here's my implementation, which prompts for a search input if it there isn't anything marked or at the cursor https://gist.github.com/ackerleytng/290c29ac951c18d859593a9414f88fe7 I think in terms of UX it isn't that bad since most people know what they're searching for. It's probably a nice tradeoff for simplicity and performance.
|
@ackerleytng Just to be clear - you ask first for the search string and then you present the options as a static list of candidates? This is a good first step and certainly an option when we cannot get dynamic candidates to work.
Please contribute this to Consult. But we have to wait a bit until we merge this since it makes sense to first explore if we can make it work with dynamic tables. |
Yes, the search string step (first step) is completely separate from the searching and filtering (second step). I'm not actually using anything from consult though - the magic of consult is in "previewing" the search with context in the buffer, right? The implementation doesn't actually do any previewing - how will consult handle opening files that have not previously been opened by emacs? |
@ackerleytng We have to figure that out - currently I avoided that and only preview already open buffers in order to avoid expensive loading. But technically there is no problem to open files and preview them. If you don't want to make it part of consult that's also okay, but the plan is to add a consult-rg command at some point so maybe you can help with that. And maybe some of the other consult commands turn out to be useful for you. |
I'd be happy to contribute - let's continue the discussion at minad/consult#68 |
@minad |
From my perspective it is not necessary to implement this. We manually trigger refreshing of the ui in consult for asynchronous commands. We do this for both icomplete and selectrum. It is solid and works well. |
This bug is affecting the wordnut package. The relevant call is:
I'd like to insist that this bug should be fixed. One of the main advantage of selectrum wrt. similar packages is that it respects the standard emacs interface. But this bug shows otherwise. |
@clemera This is off-topic, but thanks to this example you posted, I was finally able to make a dynamic completion command for Org QL: alphapapa/org-ql@4f5fbc4 Thank you very much for sharing it. May I suggest that you contribute it more permanently somewhere, like in the Elisp manual, or even just a blog post? I had to do a bit of Web searching before I finally found this issue (using Google). The minimal example was key to my being able to make it work; I would have had to dig through source code and Edebug for probably hours more before coming up with that, because the Elisp manual section on programmable completion just isn't enough. |
When
minibuffer-completion-table
is a function the candidates might be created dynamically. Currentlyselectrum--normalize-collection
gets the candidates from the table only once but this fails if the candidates are computed dynamically. On the other hand it is slow and unnecessary to recompute big tables likehelp--symbol-completion-table
on each input change.The text was updated successfully, but these errors were encountered: