Repository navigation
Conversation
53d9b8c to
ad63aa3
Compare
| manifest: &Manifest, | ||
| cfg: &Cfg<'_>, | ||
| ) -> Option<Self> { | ||
| Self::for_action( |
There was a problem hiding this comment.
IMO this callback solution isn't much better than the previous unabstracted solution. Suggest factoring out some helper functions instead.
39a0a0c to
951efbb
Compare
| candidate_name == component.name() || candidate_name == component.wildcard().name() | ||
| } | ||
|
|
||
| fn toolchain_has_installed_component(toolchain: &Toolchain<'_>, component: &Component) -> bool { |
There was a problem hiding this comment.
This has a single caller, just inline it?
| .any(|installed| component_matches(component, installed.name())) | ||
| } | ||
|
|
||
| fn toolchain_has_available_component( |
53b8a1a to
0c480f7
Compare
| fn other_channel_toolchains<'a>( | ||
| cfg: &'a Cfg<'a>, | ||
| desc: &ChannelToolchainName, | ||
| ) -> impl Iterator<Item = (ChannelToolchainName, ChannelToolchain<'a>)> { |
There was a problem hiding this comment.
Nit: It looks like you are mimicking Cfg::list_channels() but that function is probably not the best abstraction, because:
Cfg::list_channels()is choosing only tracking channels, which might make its current name imprecise.ChannelToolchainNamecan be fetched fromChannelToolchain::desc(), and there should be no point in storing both?other_channel_toolchains()is doing another kind of filtering but over mostly the same collection.
Could we do some refactoring actions upfront so this looks even nicer?
There was a problem hiding this comment.
What do you think about making Cfg::list_channels() return all installed channel toolchains, then reusing it in both Cfg::list_tracking_channels() and other_channel_toolchains() to apply their respective filters? on top of PR #5175.
There was a problem hiding this comment.
@14elias Yeah something like that would be cool, probably list_channels() returning an iterator could be worth investigating in the meantime.
0c480f7 to
119efc3
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
571a7ab to
d166b47
Compare
d166b47 to
a0d2415
Compare
related #5098
This change improves component suggestions when a requested component is not available in the active toolchain.
Previously, rustup could suggest a component removal from another toolchain, but it did not handle the add case. This patch makes component suggestions action-aware and adds a toolchain-based suggestion for rustup component add.