-
Notifications
You must be signed in to change notification settings - Fork 615
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
Rework reformatting in PyProjectTomlMut to respect original indentation #5075
Rework reformatting in PyProjectTomlMut to respect original indentation #5075
Conversation
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.
Thanks for doing this!
Can we add a test to ensure this is working as expected? You can see https://github.com/astral-sh/uv/blob/main/crates/uv/tests/edit.rs#L14 for an example (you can probably omit the part checking uv.lock
).
Sure, will do |
Updated, please re-review 🙏 |
let decor_prefix = decor | ||
.prefix() | ||
.and_then(|s| s.as_str()) | ||
.map(|s| s.split('#').next().unwrap_or("").to_string()) |
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.
Hmm I'm a little unclear how this works if the line doesn't have a comment?
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.
If the decor.prefix()
does not have a comment this whole expression will return the String
that is in decor.prefix()
trimmed by "\n"
from the left
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.
If you're asking about this particular line
.map(|s| s.split('#').next().unwrap_or("").to_string())
in case there is no '#' in the string it well return the whole string
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.
Ah that makes sense.
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.
Nice work!
Summary
So this PR introduces change to how
Array
of dependencies representation is reformatted whilePyProjectTomlMut
is manipulated. These changes are here for it to respect the original indentation.Closes #5009
Test Plan
Using
pyproject.toml
likeExecuted
And expected in
pyproject.toml
Preserving original indentation