Get all members as available targets even though default-members was specified.
#15199
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
rustbot
added
A-cli
| [ERROR] "--bin" takes one argument. | ||
| Available binaries: | ||
| crate1 | ||
| crate2 |
There was a problem hiding this comment.
Fixes #14544
If the fix only adds bins from all targets to this error theo it will lead to user frustration as they then add --bin crate2 and it fails without saying why.
The core problem in the Issue is the --bin crate2 error message. It only suggested touching this error message as an addendum and suggested it show what the needed package is. I agree with that with the caveat that we need to double check if our completion or fish's completion script uses the output from this. While this is meant for humans and can change, we still want to play nice with these.
There was a problem hiding this comment.
While the focus of #14544 is now included, two of the 3 improvements still require investigating their impact on cargo's and fish's completions and one still has the issue of showing values that can't be used.
linyihai
force-pushed
the
available-packages
branch
from
3c409c9 to
ee13fd6
Compare
rustbot
added
the
A-cargo-targets
linyihai
force-pushed
the
available-packages
branch
from
ee13fd6 to
d1ed4c8
Compare
linyihai
force-pushed
the
available-packages
branch
from
d1ed4c8 to
7389aff
Compare
| fn get_targets_from_other_packages( | ||
| &self, | ||
| filter_fn: impl Fn(&Target) -> bool, | ||
| ) -> CargoResult<Option<(String, String)>> { | ||
| let packages = Packages::All(Vec::new()).get_packages(self.ws)?; | ||
| let targets = packages.into_iter().find_map(|pkg| { | ||
| if let Some(target) = pkg | ||
| .manifest() | ||
| .targets() | ||
| .iter() | ||
| .find(|target| filter_fn(target)) | ||
| { | ||
| Some((pkg.name().to_string(), target.name().to_string())) | ||
| } else { | ||
| None | ||
| } | ||
| }); | ||
|
|
||
| Ok(targets) | ||
| } |
There was a problem hiding this comment.
The name implies this returns multiple but it only returns the first.
What should we do when there are multiple?
linyihai
force-pushed
the
available-packages
branch
from
7389aff to
3d78767
Compare
…ave the `Target`.
linyihai
force-pushed
the
available-packages
branch
from
3d78767 to
7735088
Compare
| [HELP] a target with a similar name exists: `a` | ||
|
|
||
|
|
||
| "#]]) |
There was a problem hiding this comment.
This extra newline doesn't seem right
tests/testsuite/workspaces.rs
Outdated
| [ERROR] no bin target named `crate2`. | ||
| Available bin targets: | ||
| another | ||
| Available bin targets in crate2: |
There was a problem hiding this comment.
Should we say
no bin target named
crate2in crate1 package.
or
no bin target named
crate2in default-run packages
Without that extra context, I worry users might get confused because they know might know these targets come from other packages but not realize it
(this should probably be its own commit)
tests/testsuite/workspaces.rs
Outdated
| [ERROR] no bin target named `crate2`. | ||
| Available bin targets: | ||
| another | ||
| Available bin targets in crate2: |
There was a problem hiding this comment.
For cases like this where the bin and target name are redundant, should we clarify this
Available bin targets in package
crate2:
What does this PR try to resolve?
The old behavior gets members from
default-membersif specified in a virtual workspace, this PR gets all workspace members forAvailabletarget hints.Fixes #14544
How should we test and review this PR?
The first commit added the test, the second commit addressed the issue.
Additional information