Skip to content

fix(completions): account for active toolchain in target completions - #5163

Open
14elias wants to merge 8 commits into
rust-lang:mainfrom
14elias:fix/toolchain-aware-target-completion
Open

14elias wants to merge 8 commits into
rust-lang:mainfrom
14elias:fix/toolchain-aware-target-completion

Conversation

@14elias

@14elias 14elias commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #5144

Currently, rustup target add always pulls targets from the first installed channel toolchain alphabetically, ignoring any +toolchain override passed on the command line.

This causes a couple of issues:

  • If you use a custom toolchain like rustup +stage1 target add , it still suggests targets even though custom toolchains don't have rustup-managed targets.
  • If you specify a toolchain like +nightly, it might end up suggesting targets from another installed toolchain instead of nightly.

This changes fixes this by resolving the toolchain in this order:

  1. Check if a +toolchain override was passed (if it's a custom toolchain, suggest nothing).
  2. If not, check the active toolchain for the directory/default.
  3. Fall back to the first installed toolchain if no active toolchain is set.

@14elias
14elias force-pushed the fix/toolchain-aware-target-completion branch from c7492bd to 54ab6c3 Compare October 24, 2026 20:34
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
@14elias
14elias force-pushed the fix/toolchain-aware-target-completion branch from 54ab6c3 to 10b6316 Compare October 31, 2026 09:52
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
Comment thread src/cli/completions.rs Resolved
@14elias
14elias force-pushed the fix/toolchain-aware-target-completion branch from 10b6316 to ad36b48 Compare November 8, 2026 11:20

@rami3l rami3l left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello, thank you for the ping!

My apologies but I am disappointed with the quality of your work. The changes that you are making only scratch the surface of the problem and are done without further analysis of the codebase. If I keep accepting your patches without further cleanup I am afraid the quality of the outcome will not be ensured.

Please bear with me and allow me to take it from here.

View changes since this review

@14elias

14elias commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Hello, thank you for the ping!

My apologies but I am disappointed with the quality of your work. The changes that you are making only scratch the surface of the problem and are done without further analysis of the codebase. If I keep accepting your patches without further cleanup I am afraid the quality of the outcome will not be ensured.

Please bear with me and allow me to take it from here.

View changes since this review

Thank you for the honest feedback and i apologize for the disappointment i caused. please go ahead and take it from here i completely understand your wanting to ensure the quality. i will learn from the changes you make. i will do my best to improve the quality of my contribution going forward.

@rami3l
rami3l force-pushed the fix/toolchain-aware-target-completion branch from ad36b48 to 30f9196 Compare November 9, 2026 16:27
@rami3l rami3l changed the title fix(completions): account for +toolchain override in target completions fix(completions): account for active toolchain in target completions Nov 9, 2026
@rami3l
rami3l force-pushed the fix/toolchain-aware-target-completion branch from 30f9196 to fe01c78 Compare November 9, 2026 16:35
@rami3l

rami3l commented Nov 9, 2026

Copy link
Copy Markdown
Member

@FranciscoTGouveia @14elias Patch updated via force push. Would you mind giving it another look?

This branch has not been deployed

No deployments
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.

Wrong completions when using the +toolchain syntax

3 participants