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

SH dispatch #57

Merged
merged 3 commits into from Sep 7, 2011
Merged

SH dispatch #57

merged 3 commits into from Sep 7, 2011

Conversation

josh
Copy link
Contributor

@josh josh commented Aug 23, 2011

Adding a collection of required sh functions isn't going to fly for rbenv. We are making some exceptions for stuff like shell completion where the functionality isn't essential.

I think this is a really nice lightweight convention for defining custom shell helpers. A small rbenv command dispatcher is installed at init. Regular commands pass though as you'd expect. But special shell commands prefixed with sh- are called, but its result is evaled in the current shell allowing them to modify environment variables. Whats really cool is this is now a simple plugin framework for anyone to write custom rbenv shell commands.

I think we'll start with rbenv-use for now. Its basically an alias for export RBENV_VERSION.

@metaskills
Copy link

Sexy!

@tmking
Copy link
Contributor

tmking commented Aug 23, 2011

I like this. Very clever and elegant.

@tmking
Copy link
Contributor

tmking commented Aug 23, 2011

@josh,

I added some error handling and a few other minor things. Should I open a pull request against the sh branch? Or should I wait for this pull request to be accepted and open it against master?

@josh
Copy link
Contributor Author

josh commented Aug 23, 2011

I need to see what Sam thinks.

The completion stuff needs to be fixed for this. Thinking rbenv commands should list everything (local, version, use). Then add some flags for restricting it to sh commands rbenv commands --sh and rbenv commands --not-sh-bad-name.

@josh
Copy link
Contributor Author

josh commented Sep 2, 2011

@sstephenson ping!!!

Probably just going to merge this soon :)

josh added a commit that referenced this pull request Sep 7, 2011
@josh josh merged commit 75f65a9 into master Sep 7, 2011
@sstephenson
Copy link
Contributor

I know I'm late... but this is great. Thanks @josh.

@josh
Copy link
Contributor Author

josh commented Sep 7, 2011

Ah, yeah I forgot about the shell change.

cehoffman pushed a commit to cehoffman/rbenv that referenced this pull request Apr 8, 2014
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.

None yet

4 participants