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

More maintainable solution for Selectrum-based commands #234

Closed
minad opened this issue Nov 24, 2020 · 20 comments
Closed

More maintainable solution for Selectrum-based commands #234

minad opened this issue Nov 24, 2020 · 20 comments

Comments

@minad
Copy link
Contributor

minad commented Nov 24, 2020

There are many useful commands in the wiki which might profit from a more maintained and packaged solution. Basically it would be nice to have an alternative to the ivy counsel package, which uses the completing-read mechanism and maybe only falls back to selectrum if more functionality is required. Ideally the package should not have a hard dependency on selectrum. Hopefully such a package would be much much smaller than counsel.el, since many Emacs functions already work out of the box with completing-read and don't need counseling. However there are still some others which do more than only completion, e.g. the switch-buffer function with support for virtual buffers (recent files, bookmarks etc). Things like this could go into such a new and simple "consult" package ;)

@raxod502 How are you organizing such things? Do you keep such things in your radian Emacs config? And you @clemera?

@clemera
Copy link
Collaborator

clemera commented Nov 24, 2020

I think having a package which provides and maintains useful completing-read commands (like we have on the wiki) would be great! BTW is there something that needs to be aware of regarding the license of code contributed to the wiki?

However there are still some others which do more than only completion, e.g. the switch-buffer function with support for virtual buffers (recent files, bookmarks etc).

I think the only cases which currently require Selectrum are those which use dynamic swapping of candidate sets by your input pattern and those which gather data from text properties of the result:

The former could probably also be replicated passing a dynamic completion function to completing-read. One problem is that Selectrum currently does not fully support such dynamic completion functions as it currently expects that all the candidates are returned initially, see #114. The feature of Selectrum to let the completion function determine which part of the input should be used for matching the candidates is a bit harder to replicate and could only be done if the completion function handles the filtering itself, I think. There are not many command which need to make use of this and generally I think those should probably live in user configurations anyway.

The latter will be possible to some extend in Emacs 28 where we get minibuffer-allow-text-properties.

@minad
Copy link
Contributor Author

minad commented Nov 24, 2020

I think having a package which provides and maintains useful completing-read commands (like we have on the wiki) would be great!

Cool! The problem I am seeing with something like this is only that it is some kind of kitchen sink package, which is not clearly limited. However if some functionality grows larger, it could be moved to another separate package, in order to avoid that such a thing grows beyond any bounds.

BTW is there something that needs to be aware of regarding the license of code contributed to the wiki?

In my understanding, the license of the wiki/project documentation coincides with the project license. But we should clearly state if some work is derived from something else, e.g. ivy.

I think the only cases which currently require Selectrum are those which use dynamic swapping of candidate sets by your input pattern and those which gather data from text properties of the result...

What I find interesting about this, is that given more data regarding this, one could propose an improved completing-read api for upstream, if it is necessary after all. I thought about this before in #225.

Regarding the details you wrote, I am not yet such familiar with all those. I think we will see case-by-case if completing-read is sufficient or not. Only one thing, I wonder, what about this selectrum-switch-buffer+ function? Would something like this be out of reach or is it possible with completing-read?

I am a bit divided what to do about functions which would need more than completing-read. I see the following options:

  1. Only include stuff which works with completing-read and nothing beyond. I would like that but it might be too limited.
  2. Also include additional functions which use selectrum and which are only available if selectrum is available. This is probably acceptable and a pragmatic solution.
  3. Implement functions, such that they are enhanced if selectrum is available and fall back to a more basic completing-read implementation. I am not sure that this is a good option due to test coverage, doubled maintenance effort and inconsistent behavior of such functions.
  4. Implement some kind of extended-completing-read shim, which falls back to selectrum/helm/ivy. This is an api which could potentially go upstream a some far point in the future. For this option I don't know enough and I also think it is pointless for now, since we don't have enough functions yet in order to shape such a thing. Furthermore, I think helm and ivy people will and should continue to use their counsel or other extension libraries.

It seems to me either option 1 or 2 are acceptable. What do you think, @clemera?

@clemera
Copy link
Collaborator

clemera commented Nov 24, 2020

Only one thing, I wonder, what about this selectrum-switch-buffer+ function? Would something like this be out of reach or is it possible with completing-read?

I'm not sure, maybe it would be possible with some hacks and removing the issues I mentioned with dynamic candidate sets.

It seems to me either option 1 or 2 are acceptable. What do you think, @clemera?

I think I would prefer to only include completing-read commands, maybe there could be a separate package for commands which currently require selectrum.

@hpdeifel
Copy link

Point in case, as to why a dedicated package would be nicer than a wiki: The counsel-rg replacement from the wiki has bitrotted (selectrum-minibuffer-bindings doesn't exist), I don't know how to fix it myself and there's nowhere to report an issue for it.

@minad
Copy link
Contributor Author

minad commented Nov 25, 2020

@hpdeifel Yes, better maintenance is certainly a reason. Furthermore we automatically get access to all the commands which are deemed good enough for such a package without having to manually copy them from the wiki.

@raxod502
Copy link
Member

Okay, seeing issues like #232, and a large number of previous ones containing code snippets with various minibuffer-with-setup-hook incantations... I'm on board. We should definitely have a better solution that will facilitate discoverability and reuse.

One option---and feel free to suggest others---would be to do a thing similar to straight.el, which has a file straight-x.el that contains user-contributed things like this. The idea is that the bar to add things to straight-x.el is much lower: it doesn't matter if the code isn't the cleanest, or we aren't sure whether it's a good idea to support the feature. Then, if people turn out to really like something that's added, it can be cleaned up a bit and then put into straight.el proper.

So we could perhaps have a selectrum-x.el, not loaded by default but available by (require 'selectrum-x), with the stuff from the wiki and things from various issues.

If we end up with a bunch of functions in selectrum-x.el that seem like good ideas to document and include, and it seems awkward to put them all in selectrum.el, then we could perhaps split out a new file just for user commands. That could be a part of the selectrum package, or a separate one, but it could nonetheless still be in this repository.

And I find the idea of calling such a package consult to be incredibly amusing :D

@raxod502
Copy link
Member

@raxod502 How are you organizing such things? Do you keep such things in your radian Emacs config?

You may be surprised to learn that I don't use any of them... which probably explains why I haven't pushed for a more maintainable solution until now, sorry :o

@raxod502 raxod502 changed the title Counsel alternative More maintainable solution for Selectrum-based commands Nov 25, 2020
@minad
Copy link
Contributor Author

minad commented Nov 25, 2020

@raxod502 No, I am not really surprised. I looked at your radian config before. And I generally like your more minimalist approach to things.

Instead of doing the selectrum-x file, I propose to have a separate package. This is cleaner and otherwise we end up with a monorepo like swiper which contains swiper, ivy and counsel. If we generally like the "consult" name I would just go with that, instead of chosing a more selectrum-related name.

What I've heard from @clemera, he prefers to have most commands not selectrum-specific, which would also point to having a separate package. It is one of the main reasons for me to use selectrum, since it integrates with the Emacs defaults. So maybe we should not deviate from that approach.

I am also willing to help maintain such a thing, this wasn't a request to have this in selectrum. However it would be ideal if some more elisp experienced folks like @clemera or you would co-maintain or somehow help out. I think the package should also be up to the quality standards of this package, otherwise there is not really a point in doing this.

@clemera
Copy link
Collaborator

clemera commented Nov 25, 2020

I think what @raxod502 suggested would be good fit for commands which require selectrum. For framework agnostic commands I agree with @minad that they are better placed in a separate package. I also like "consult", for searching it may be beneficial to also include the term "completion" as in "consult-completion" if that sounds still good. I don't want to spend to much time with such a package currently but I'm happy to help out if I can.

@minad
Copy link
Contributor Author

minad commented Nov 25, 2020

@clemera Regarding the name, I like "consult" more. I tend to prefer descriptive names like "completion-utils" or completely free names like "consult", but I am not in favor of mixing both naming schemes. This feels better if the package gets split up at some point, e.g., "consult-selectrum" and "consult". Regarding the maintenance it would be very nice to get a review and some consultation here and there ;)

@raxod502 Do you prefer to have a selectrum-x package and a non selectrum specific consult package? My preference would be to rather have consult and consult-selectrum. I would lobby a bit to keep selectrum lean and focused only on the completion aspect and rather having a consult package. This introduces a clean separation. I think the ivy/counsel relation is the right one.

@clemera
Copy link
Collaborator

clemera commented Nov 25, 2020

@minad Feel free to ping/consult me ;) BTW @okamsn has collected some nice info about completing-read commands on the wiki.

@minad
Copy link
Contributor Author

minad commented Nov 25, 2020

@clemera Thank you! Yes I've seen the wiki page!

@minad
Copy link
Contributor Author

minad commented Nov 25, 2020

I started something at https://github.com/minad/consult. I will step-by-step add the functions from the wiki when I have time. We will see how this goes. Please let me know if you want to be added to the repository, have some comments (@raxod502, @clemera, @okamsn).

@manuel-uberti
Copy link

Chiming in only to say that keeping selectrum as simple as possible is the best approach for me as well, but I am looking forward to seeing what consult ends up including. 👍

@minad
Copy link
Contributor Author

minad commented Nov 26, 2020

Update: I added a bunch of functions to the consult repository. If some of you are interested, it would be great to get some feedback or maybe you have time to test if the commands work in your setup. Maybe some things don't work or feel odd. Please let me know. Furthermore I would be interested in hearing about commands you would like to have added. Any kind of feedback is appreciated!

@minad
Copy link
Contributor Author

minad commented Nov 28, 2020

Update: Since consult is already quite usable, I opened a PR on melpa melpa/melpa#7255 and a PR here which links to consult #238. Now some of the commands also feature live preview+recursive editing: consult-buffer, consult-mark, consult-outline and consult-line. consult-theme also supports preview when scrolling through candidates.

Another small update: I improved the live preview support and consult-buffer such that the package is now fully compatible with icomplete (consult-buffer on icomplete is a bit less powerful however). The library is almost completion-system agnostic now, which is pretty nice.

@minad
Copy link
Contributor Author

minad commented Nov 30, 2020

Closing this. It would be great if we can get a link in the readme, see #238.

@raxod502
Copy link
Member

This looks awesome! Great work @minad!

@raxod502
Copy link
Member

Feel free to ping me if you need help with anything.

@minad
Copy link
Contributor Author

minad commented Dec 15, 2020

@raxod502 Thank you! If you have suggestions, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants