-
-
Notifications
You must be signed in to change notification settings - Fork 192
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 brew integration description #415
base: master
Are you sure you want to change the base?
Conversation
#### brew | ||
|
||
``` bash | ||
RUBIES+=(/usr/local/Cellar/ruby/*) |
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.
I wonder if it's worth showing brew --cellar
to accommodate alternative brew prefixes?
RUBIES+=("$(brew --cellar ruby)"/*)
On the other hand, the hardcoded /usr/local
version is prettier.
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.
I didn't think about that option.
Not sure if it could be a custom path for it. However, as it's documentation, not a script, it could have both options documented.
In my case, I rather set the path, as this would be one command less to execute every time I start a terminal session.
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.
Those are good points. I think it's fine to hard code to the brew prefix.
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.
It'd be nice to add engine-version
to RUBIES
rather than just version
. With Ruby 2.6.3 for example, it'd currently be 2.6.3
instead of ruby-2.6.3
.
I also wonder about other Ruby engines that brew has packages for. Is it worth adding those too, or maybe just separate examples for Ruby and JRuby?
for ruby_engine in jruby mruby ruby; do
for ruby in /usr/local/Cellar/$ruby_engine/*; do
RUBIES+=("${ruby%/*}/$ruby_engine-${ruby##/*/}")
done
done
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.
I can't tell about the jruby ones, as I just use homebrew for it. Sounds good to me, but as I can't test it, I wouldn't dear :D
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.
I was going to propose you this:
RUBIES+=(
/usr/local/Cellar/jruby/*
/usr/local/Cellar/mruby/*
/usr/local/Cellar/ruby/*
)
But here is my experience...
First of all, I installed (throw homebrew) jruby and mruby to check it out.
The binary directories would be then:
/usr/local/Cellar/ruby/2.6.5/bin/
/usr/local/Cellar/jruby/9.2.9.0/bin/
/usr/local/Cellar/mruby/2.0.1/bin/
If I run your loop, it would check for this:
/usr/local/Cellar/jruby/jruby-9.2.9.0
/usr/local/Cellar/mruby/mruby-2.0.1
/usr/local/Cellar/ruby/ruby-2.6.5
Which is not right, as you can see, so you can just run it like:
for ruby_engine in jruby mruby ruby; do
for ruby in /usr/local/Cellar/$ruby_engine/*; do
echo "${ruby%/*}/${ruby##/*/}"
done
done
But what would be the point? It would do the same as what I propose:
RUBIES+=(
/usr/local/Cellar/jruby/*
/usr/local/Cellar/mruby/*
/usr/local/Cellar/ruby/*
)
But there is a problem here:
> chruby
2.6.5
9.2.9.0
2.0.1
> chruby 2.6.5
> ruby --version
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin19]
> chruby 2.0.1
chruby: /usr/local/Cellar/mruby/2.0.1/bin/ruby not executable
> chruby 9.2.9.0
chruby: /usr/local/Cellar/jruby/9.2.9.0/bin/ruby not executable
You see what it's doing? and why it fails? is because it is trying to get the {path}/bin/ruby
binary, but in the case of jruby and mruby, the binaries are called jruby
and mruby
, so it fails.
In conclusion:
- I don't see the point of adding a for loop, I think that it would be better to use just the
*
wildcard. - It will not work anyways, unless there is a way to tell chruby the name of the binary (I don't now if there is a way or not), in which case, this is not the purpose of this PR, I think.
So I suggest to get stick with the current proposal.
Merged with master in order to pass the tests. In any case, this PR is just documentation, it does not touch the code really, but it looks better this way ;) |
Just adding a brew integration description.