-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add an error when no crates are found from cargo search
#11038
Conversation
r? @epage (rust-highfive has picked a reviewer for you, use r? to override) |
if total_crates == 0 { | ||
return Err(anyhow!( | ||
"could not find any crates matching `{}` from the registry at {}", | ||
query, | ||
registry.host() | ||
)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you expand on why you feel this behavior change is acceptable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely certain that an error is the correct answer, I mostly did that as it was requested. I think an error might be too harsh since there really wasn't an "error", just no matches. Keeping that in mind, an error is the easiest way to change the output code (1.61.0 does allow main
to return different codes which could be a solution).
That being said, something should probably be shown when no crates were found. Having it output nothing seems like a misstep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.61.0 does allow main to return different codes which could be a solution).
There is also std::process::exit
(generally not good to do in a library call though)
Having it output nothing seems like a misstep.
At least apt search
doesn't really say anything.
@@ -984,6 +984,14 @@ pub fn search( | |||
) | |||
})?; | |||
|
|||
if total_crates == 0 { | |||
return Err(anyhow!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The motivation for this is scripting but we don't provide any programmatic output which makes me feel like we are providing a false promise by improving scripting support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the programmatic output is the error code in this case. It should be fairly trivial to check what the code is in most scripting languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is speculation. I'm concerned that there is also parsing of the output (why do a cargo search
and only care about the exit code?). We should wait on an actual reply from the reporter
return Err(anyhow!( | ||
"could not find any crates matching `{}` from the registry at {}", | ||
query, | ||
registry.host() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally we elide the registry if its crates.io
Since I closed the issue, I'm going to go ahead and close out this PR |
#11037 requested that an error code be added when no results are found when running
cargo search
.This makes it so
cargo search
returns an error when no results are found and adds a test to ensure consistency in the future.close #11037