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

Allow empty extras in pep508-rs and add more corner case to tests #2128

Merged
merged 4 commits into from
Mar 2, 2024

Conversation

SigureMo
Copy link
Contributor

@SigureMo SigureMo commented Mar 2, 2024

Summary

Fixes #2127, allow empty extras, and add more corner case to tests

References

@SigureMo SigureMo changed the title Allow empty extras in PEP508 and add more corner case to tests Allow empty extras in pep508-rs and add more corner case to tests Mar 2, 2024
@SigureMo SigureMo changed the title Allow empty extras in pep508-rs and add more corner case to tests Allow empty extras in pep508-rs and add more corner case to tests Mar 2, 2024
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

A small grammar nit regarding the error message:

crates/pep508-rs/src/lib.rs Outdated Show resolved Hide resolved
crates/requirements-txt/src/lib.rs Outdated Show resolved Hide resolved
@charliermarsh charliermarsh self-requested a review March 2, 2024 14:04
@charliermarsh charliermarsh self-assigned this Mar 2, 2024
@charliermarsh charliermarsh added bug Something isn't working compatibility Compatibility with a specification or another tool labels Mar 2, 2024
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -617,9 +617,48 @@ fn parse_extras(cursor: &mut Cursor) -> Result<Vec<ExtraName>, Pep508Error> {
let Some(bracket_pos) = cursor.eat_char('[') else {
return Ok(vec![]);
};
cursor.eat_whitespace();
Copy link
Member

Choose a reason for hiding this comment

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

Moved this out from the start of the loop, because it's also at the end of the loop.

@charliermarsh charliermarsh enabled auto-merge (squash) March 2, 2024 20:28
@charliermarsh charliermarsh merged commit d4f1973 into astral-sh:main Mar 2, 2024
7 checks passed
@SigureMo SigureMo deleted the pep508-allow-empty-extras branch March 3, 2024 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility Compatibility with a specification or another tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uv pip install does not accept empty extras
3 participants