fix(upgrade): Preserve more formatting #725
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The main focus is a refactor to make it easier to change code rather than fighting with the combinators and the batching. Fixes killercup#717
I've wondered why things weren't upgraded before and it made me wonder if `cargo-upgrade` was broken. This provides a way to find out why and helps us verify our tests are doing what is intended.
I had missed that this was skipped when the user explicitly set a version
This was referenced Jul 13, 2022
epage
added a commit
to epage/cargo
that referenced
this pull request
Oct 17, 2023
Huh, overlooked the cargo side when I fixed this in killercup/cargo-edit#725 Fixes rust-lang#10850
epage
added a commit
to epage/cargo
that referenced
this pull request
Oct 18, 2023
Huh, overlooked the cargo side when I fixed this in killercup/cargo-edit#725 Fixes rust-lang#10850
epage
added a commit
to epage/cargo
that referenced
this pull request
Oct 19, 2023
Huh, overlooked the cargo side when I fixed this in killercup/cargo-edit#725 Fixes rust-lang#10850
bors
added a commit
to rust-lang/cargo
that referenced
this pull request
Oct 19, 2023
fix(add): Preserve more comments ### What does this PR try to resolve? This was fixed in killercup/cargo-edit#725 and I forgot to carry it over to here. I kept in some auto-formatting because there is little to preserve until the TOML 1.1 spec is out which will allow mult-line inline-tables which means there might be comments interspersed. We'll see which comes first, `cargo fmt` support for `Cargo.toml` or the 1.1 spec. Fixes #10850 ### How should we test and review this PR? First commit adds a test demonstrating the problem. The second makes a lot of noise that the third cleans up, so its a mixed bag as to which commit to look at. ### Additional information
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves us away from the cargo-add approach of generating a
Depedencyand overwriting the entry with just setting theversion, allowing us to preserve more formatting.I also feel like this overall simplified things and made it easier to consider what the behavior should be.
In doing so, I noticed that we didn't process preserve precision with
foo@versionbut we did process skipping compatibility. Now, we always trust the user.