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

Add ivy-info. #2310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add ivy-info. #2310

wants to merge 1 commit into from

Conversation

Luis-Henriquez-Perez
Copy link

This is the initial pull request for adding ivy-info, an ivy equivalent of helm-info. There are probably many things that need to be changed for example perhaps you'd prefer it to be called counsel-info. Please let me know.

@abo-abo
Copy link
Owner

abo-abo commented Nov 1, 2019

Thanks. It works for me. And the code isn't very long.

  1. Please rename it to counsel-info and put it in counsel.el.
  2. For changes over 15 lines you need an Emacs Copyright Assignment. See CONTRIBUTING.org for more info.
  3. Looks like some code is copy pasted from somewhere. There's even a reference to issue #1503. Please clean up and rewrite the code so that we can put it in GNU ELPA.

@Luis-Henriquez-Perez
Copy link
Author

I've done #1 and #3. Please let me know if there's sometime more you'd like.

I will now work on the commit assignment.

Oh and here's a question:

helm-info' has the side-effect of creating a function for each info node (ie. elisp-info, emacs-info, org-mode-info, etc.). I ported in to counsel` in such a way that this is no longer the case.

An argument could be made that this behavior is desirable. I suspect that like me most pople vast majority of the time only visit a handful of info nodes (elisp, emacs and org-mode). So perhaps it could be convenient to the specialized functions around. Wha t do you think?

@abo-abo
Copy link
Owner

abo-abo commented Nov 5, 2019

I ported in to counsel in such a way that this is no longer the case.

Thanks. I like it this way more. The user can define a custom function in their config if they want. I'd rather keep the counsel namespace cleaner.

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

Successfully merging this pull request may close these issues.

2 participants