minad / consult

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

Better fix for the async filtering issue reported by @clemera #185

Closed minad closed 3 years ago

minad commented 3 years ago

See 0c9d6ab4fadfc3613880148bc1ed2c53cd851fbe

@clemera Can you please give it another quick test? It should work with the current Selectrum independent of selectrum-active-p. It seems to work for me in both cases, if the completion style is used by Selectrum and if it is not.

clemera commented 3 years ago

I think I would prefer using the flag approach (I changed it to be set early now), users might use other refinement functions which use completion styles, I know about one who adds ./ in file completions and calls selectrum-refine-candidates-using-completions-styles only in his customized version.

clemera commented 3 years ago

Another issue is when you check the refine/highlight function in early setup hook you won't get the values users might have set themselves via setup hook.

minad commented 3 years ago

Okay, I pushed the original fix again.

clemera commented 3 years ago

Thanks!

minad commented 3 years ago

@clemera ~It still does not work. I think your fix is bad, can you set somehow a higher priority? https://github.com/raxod502/selectrum/commit/257224bc7c5e97a52cc56cef2930f548af3a83e9~ disregard, I think I tested the wrong version...

EDIT: Okay, I confirmed that it works now. Please check.

clemera commented 3 years ago

Yes, things work for me now, too.

minad commented 3 years ago

Just to make it absolutely safe, did you try both with/without completion style?

clemera commented 3 years ago

Yes, although not in emacs -Q.

clemera commented 3 years ago

Hm, wait I'm using emacs -Q and still encounter some problems, I will report back.

clemera commented 3 years ago

I had some loading order issues apparently, filtering works now with both but when selecting a candidate it won't jump to the selected match.

minad commented 3 years ago

I just tried it again with emacs -Q. Seems to work for me.

minad commented 3 years ago

This should work:

(package-initialize)
(require 'consult)
(require 'selectrum)
(selectrum-mode)
(setq completion-styles '(substring))

Also this:

(load "consult.el")
(load "consult-selectrum.el")
(load "selectrum.el")
(selectrum-mode)
(setq completion-styles '(substring))
clemera commented 3 years ago

The filtering works but jumping to the match seems to be broken.

minad commented 3 years ago

For me it works. How did you load it?

clemera commented 3 years ago

I retried loading it as you described above, but I still can't jump to the matches, it seems @manuel-uberti has the same problem as reported here.

clemera commented 3 years ago

I used the manual loading you described second if that matters.

minad commented 3 years ago

@clemera @manuel-uberti Please try the newest main commit, see https://github.com/minad/consult/commit/eb72c233b34918346197602c0bf991e1da4aaa74. I tried to improve some flushing behavior but this seems to have broken the candidate lists, see #182.

clemera commented 3 years ago

Yes, this fixes it!

manuel-uberti commented 3 years ago

@clemera @manuel-uberti Please try the newest main commit, see eb72c23. I tried to improve some flushing behavior but this seems to have broken the candidate lists, see #182.

Thank you, consult-ripgrep works as expected now.

minad commented 3 years ago

@clemera @manuel-uberti Thank you both for checking again and the report!

manuel-uberti commented 3 years ago

Always glad to be of any help. [BTW: consult-ripgrep is just awesome.]

minad commented 3 years ago

Thank you very much, I am glad you like it!

minad commented 3 years ago

@manuel-uberti Btw, there is https://github.com/minad/consult/pull/170. What do you think about this approach? It will not be part of the Consult core package but I think there could be some additional consult-transient-grep package which allows to tweak the options via transient.

manuel-uberti commented 3 years ago

Interesting approach, it reminds me of rg.el which lets you refine the search criteria. Although, as per my use cases, consult-ripgrep suits me super well already.

minad commented 3 years ago

Yes, it would just be an add-on. You can summon the menu, it shows you the available options, you select and continue with the usual consult-ripgrep interface.