-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 chruby support to Ruby layer #2321
Conversation
LGTM 👍 I am currently using |
Any movement on getting this merged into develop @syl20bnr? |
:defer t | ||
:init (chruby) | ||
:config (add-hook 'enh-ruby-mode-hook | ||
(lambda () (chruby-use-corresponding))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need the lambda expression here you can replace it by 'chruby-use-corresponding
. The lambda expression (or a named function) is needed only when you have to pass arguments to the function to hook.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup I can change that, I was trying to be consistent with the rbenv
expression directly above this.
@syl20bnr bump for the questions above so I can fix this up and have it be merged. |
@bjeanes Take a look at documentation of use-package to understand more about One more thing to improve would be to only install/load the package when the binary is found on the system. Something like following should help in |
@nixmaniack I have read those docs. My still-unanswered question is that
That does sound good. But, likewise, this is not being done in the case of the other ruby managers. Why not consistency first? |
@bjeanes That as well needs to be fixed then. I'm not sure why it may have skipped from the review but one reason I see is that the Right now we're aiming for correctness rather than consistency but yes we should definitely fix the other ruby-version-managers sooner. |
@nixmaniack OK fair enough. I'll try to have a look at this again in a few days |
#4065 Is about |
Thank you ! |
Tested locally (I use Chruby personally). I'm not sure how
chruby.el
fairs in other configurations as I've not used it much before. It seems closely modelled afterrbenv.el
.