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

'default' and 'update' heuristics for bare triples #516

Merged
merged 1 commit into from
Jun 17, 2016

Conversation

inejge
Copy link
Contributor

@inejge inejge commented Jun 5, 2016

[For issue #411]

[Whoops, I started working on this before @moosingin3space left his/her comment. Here goes anyway, sorry if it led to wasted effort.]

This patch lets you use any recognizable target triple with default or update. The code will scan all installed toolchains matching the given (partial) triple and offer the user to choose one, or, if there is just one found, to confirm its use. If the command is default, the current default toolchain is not included in the list of candidates. The user must choose or confirm the inferred choice; there is no automatic operation. An empty list of candidates is an error. So is not choosing/confirming an offer.

@brson
Copy link
Contributor

brson commented Jun 7, 2016

Thanks @inejge!

I see that your logic is not quite the same as what I suggested on the issue. From my reading, for both the 'default' and 'update' cases this considers all installed toolchains with a matching triple to be candidates (with the exception of discarding the existing default for 'rustup default').

I think the logic is right for the 'update' case but needs to be different for 'default': the main reason I expect people to typo this when running 'rustup default' is because they are switching their host platform. So e.g. I start off on nightly-x86_64-pc-windows-gnu but want to be on nightly-x86_64-pc-windows-msvc so I type rustup default x86_64-pc-windows-msvc. If that is the most likely scenario, then we can assume they meant nightly-x86_64-pc-windows-msvc and suggest that. The logic for this would be: read the current default and pull out the channel name; prepend that channel name to the specified triple to get the suggested triple.

I'm also not sure I want to automatically use the channel name, nor prompt for it, though I can see why one might, and I can see why that's consistent with other areas of rustup that try to just 'do the right thing', notably the command synonyms. I've previously imagined printing out an error suggestion but forcing the user to type it correctly. Have to think about it some.

@inejge
Copy link
Contributor Author

inejge commented Jun 7, 2016

I think the logic is right for the 'update' case but needs to be different for 'default': the main reason I expect people to typo this when running 'rustup default' is because they are switching their host platform.

Oh, I see -- I misunderstood the example in the original comment on the issue. I'll change the logic for 'default' accordingly, just let me know whether you think that rustup should prompt for a toolchain name or error out.

When a bare (partial) triple is given instead of a toolchain name, try to
figure out user's intention. For 'update', generate a list of toolchains
with the matching triple. For 'default', find out if there's a non-default
toolchain on the same channel/date and the given triple, and suggest its
use. In both cases, terminate with an error, so the user has to reissue
the command with a valid toolchain spec.
@inejge
Copy link
Contributor Author

inejge commented Jun 9, 2016

Here's a version of the patch which implements the 'default' logic as it should have in the first place, and terminates with an error in all cases. The triple provided to the subcommand doesn't have to be complete, i.e., it should be possible to enter something like rustup default msvc and get switched to nightly-x86_64-pc-windows-msvc.

@brson brson merged commit c7fc118 into rust-lang:master Jun 17, 2016
@brson
Copy link
Contributor

brson commented Jun 17, 2016

Thanks @inejge! I'm so sorry I've been slow to respond to this.

@inejge inejge deleted the dflt-heur branch June 18, 2016 20:13
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