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

ivy: spacemacs-help: Fix candidate layers with no packages #10000

Closed
wants to merge 1 commit into from

Conversation

@nikital
Copy link

commented Dec 15, 2017

When a layer doesn't have a package, its name was added to the candidate list
as a symbol instead of a string, breaking actions such as
layer-action-open-packages which expect a string.

Spacemacs is great, keep up the good work :)

Edit: Just noticed the pull request number! Do I get eternal glory now? :P

Edit2:
Steps to reproduce:

  1. Start with a clean Spacemacs on develop branch and no .spacemacs.d
  2. Upon first start choose spacemacs-base for the distribution and vim for editing style
  3. Open the generated config (SPC f e d) and replace helm layer with ivy
  4. Restart Spacemacs to load Ivy
  5. Invoke ivy-spacemacs-help with SPC h SPC and search for vingar no packages (Or any other layer that doesn't own any packages)
  6. With Ivy still open and vinegar selected, use M-o p to open Vinegar's packages.el. You will get Wrong type argument: stringp, vinegar.
ivy: spacemacs-help: Fix candidate layers with no packages
When a layer doesn't have a package, its name was added to the candidate list as
a symbol instead of a string, breaking actions such as
layer-action-open-packages which expect a string.

@nikital nikital force-pushed the nikital:fix-ivy-help-candidates branch from e252b64 to 0d69da4 Dec 15, 2017

@nikital

This comment has been minimized.

Copy link
Author

commented Jan 3, 2018

@syl20bnr @JAremko Is there any change I can do to make this patch easier to accept?

@sdwolfz

This comment has been minimized.

Copy link
Collaborator

commented Jan 3, 2018

Not denying that what you did works, but I think it would help if you also provided a way to reproduce this bug in the description, so others can try it out for themselves and give you a 👍.

@nikital

This comment has been minimized.

Copy link
Author

commented Jan 4, 2018

@sdwolf That's a good point, thanks. Updated the pull request with steps to reproduce.

@syl20bnr syl20bnr added the Merged label Jan 7, 2018

@syl20bnr syl20bnr self-assigned this Jan 7, 2018

@syl20bnr

This comment has been minimized.

Copy link
Owner

commented Jan 8, 2018

Good catch, Thank you ! 👍
Cherry-picked into develop branch, you can safely delete your branch.

@syl20bnr syl20bnr closed this Jan 8, 2018

@syl20bnr syl20bnr removed the Merged label Jan 8, 2018

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