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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add brew integration description #415

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c64b72d
Add brew integration description
gonzalom May 25, 2019
b873a55
Merge branch 'master' into my-master
gonzalom Dec 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ RUBIES+=(~/.rbenv/versions/*)
RUBIES+=(~/.rbfu/rubies/*)
```

#### brew

``` bash
RUBIES+=(/usr/local/Cellar/ruby/*)
Copy link
Collaborator

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.

Copy link
Author

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.

Copy link
Collaborator

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.

Copy link
Collaborator

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

Copy link
Author

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

Copy link
Author

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:

  1. I don't see the point of adding a for loop, I think that it would be better to use just the * wildcard.
  2. 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.

```

### Auto-Switching

If you want chruby to auto-switch the current version of Ruby when you `cd`
Expand Down