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

Amx support for counsel #1557

Closed
Ergus opened this issue May 5, 2018 · 5 comments

Comments

@Ergus
Copy link
Contributor

commented May 5, 2018

I am wondering that smex is not actively maintained anymore and the new fork/substitution project is supposed to be amx that is very active right now.
Is it there any plan to support amx in a near/medium future? The interface is pretty much the same as I could see, so the implementation should be relatively simple.
Very thanks

@noctuid

This comment has been minimized.

Copy link

commented May 5, 2018

Amx supports using ivy already.

@Ergus

This comment has been minimized.

Copy link
Contributor Author

commented May 5, 2018

Yes, but not counsel. Amx-mode can use Ivy by itself, but most if Ivy people are using counsel-M-x, which optionally uses smex to sort properly (calling smex functions internally out of the box). counsel-M-x checks if smex is installed, if so it calls the functions. Switching to avx is something barely simple, just change the prefixes in counsel.el (I tried) and it worked, but I don't feel confident enough in elisp programming to insert the support for both and make a pull request.

@abo-abo

This comment has been minimized.

Copy link
Owner

commented May 8, 2018

@Ergus
If amx provides extra value compared to smex, I think it's a good idea to offer support for it. But keep in mind that, even though smex isn't actively maintained, it still does its job well.

but I don't feel confident enough in elisp programming to insert the support for both and make a pull request

If a change in your config was enough to make it work, it's good enough for a PR. I'd be happy to review the code, give pointers for improvement and merge it. No pressure, of course.

@manuel-uberti

This comment has been minimized.

Copy link
Contributor

commented May 17, 2018

@Ergus any update on this one? Looks really interesting :)

@Ergus

This comment has been minimized.

Copy link
Contributor Author

commented May 18, 2018

Hi abo-abo sorry for the delay, I was too busy the last days. I just added a pull requests with the changes I did (rebasing master and cleaning up). Please, any suggestion/change/critic/comment will be very welcome, it is my first lisp code ever.

basil-conto added a commit to basil-conto/swiper that referenced this issue Jun 6, 2018
counsel.el: Support amx in addition to smex
(counsel--M-x-externs): New function.
(counsel-M-x): Use it.  Conditionally call amx-rank or smex-rank,
when available, from the action function.

Borrowing heavily from and building on the work and suggestions of
Jimmy Aguilar Mena <kratsbinovish@gmail.com> (@Ergus) in abo-abo#1580.

Fixes abo-abo#1557

@abo-abo abo-abo closed this in 4e3eaf4 Jun 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.