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

Adding show-toolchain for command line prompts. #148

Merged
merged 1 commit into from
Apr 15, 2016

Conversation

aetherknight
Copy link
Contributor

This implements a command to satisfy #147 .

It adds multirust show-toolchain, which prints just the name of the current toolchain. Eg, "nightly", or "stable".

elif find_default; then
_toolchain="$RETVAL_TOOLCHAIN"
else
_toolchain="No default"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it print something if it can't find a default, or should it print nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For comparison, rbenv local prints nothing to STDOUT, but it does print an error to STDERR if it cannot determine the local version of ruby that should apply.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's print nothing then to match rbenv.

@brson
Copy link
Owner

brson commented Mar 21, 2016

I hate that show-override, show-default and show-toolchain are all slightly different. rbenv has just one version subcommand, and that's probably what I'm going to implement in multirust-rs.

I guess this is consistent with the existing multirust interface though.

For your purposes could you use sed to parse this out of the show-override command? Edit: nevermind I see you already addressed that in the op.

@brson brson merged commit 0d98a59 into brson:master Apr 15, 2016
@brson
Copy link
Owner

brson commented Apr 15, 2016

I went ahead and merged as-is. Thanks @aetherknight!

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.

2 participants