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

feat(toml): Allow adding/removing from cargo scripts #14857

Merged
merged 9 commits into from
Nov 26, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Nov 25, 2024

What does this PR try to resolve?

This adds support for cargo script for more cargo commands and is a part of #12207

Unfortunately, there is a double-warning for unspecified editions for cargo remove. Rather than addressing that here, I'll be noting it in the tracking issue.

How should we test and review this PR?

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Nov 25, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-manifest Area: Cargo.toml issues Command-remove S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2024
@epage epage force-pushed the script-mut branch 4 times, most recently from 2a5fdea to 6fa341d Compare November 25, 2024 20:43
Comment on lines +213 to +217
// Ok, this is a shebang but if the next non-whitespace token is `[`,
// then it may be valid Rust code, so consider it Rust code.
if rest.trim_start().starts_with('[') {
return Ok(source);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a comment: This isn't quite the same behavior, as the compiler allows comments before the opening [. I realize parsing that would be difficult, and that particular scenario is a bit silly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is existing code, just moved. Ok if we put this in the tracking issue or an issue and track separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to the checklist: #12207

Comment on lines +303 to +308
if !manifest.ends_with("\n") {
manifest.push_str("\n");
}
let fence = "---\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a comment: These look like it will cause mixed line endings on a CRLF file. However, that looks like it is an existing problem with cargo add in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As this is another manifestation of an existing issue, are we ok with deferring this out of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #14863 and noted this in it

src/cargo/util/toml_mut/manifest.rs Outdated Show resolved Hide resolved
src/cargo/util/toml_mut/manifest.rs Outdated Show resolved Hide resolved
src/cargo/util/toml_mut/manifest.rs Show resolved Hide resolved
@epage epage force-pushed the script-mut branch 3 times, most recently from a6a3dfe to 26844a3 Compare November 26, 2024 19:52
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss enabled auto-merge November 26, 2024 19:58
@ehuss ehuss added this pull request to the merge queue Nov 26, 2024
Merged via the queue into rust-lang:master with commit 92de1a2 Nov 26, 2024
22 checks passed
@epage epage deleted the script-mut branch November 27, 2024 19:03
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
Update cargo

8 commits in 4c39aaff66862cc0da52fe529aa1990bb8bb9a22..3908f64086a3d7b9af8d87b4da2bd100776c3e61
2024-11-25 16:36:17 +0000 to 2024-11-29 17:32:44 +0000
- chore(deps): update msrv (rust-lang/cargo#14867)
- fix(fix): Migrate cargo script manifests across editions (rust-lang/cargo#14864)
- feat(toml): Allow adding/removing from cargo scripts (rust-lang/cargo#14857)
- Add future-incompat warning against keywords in cfgs and add raw-idents (rust-lang/cargo#14671)
- test(build-std): download deps first (rust-lang/cargo#14861)
- test(pgo): ensure PGO works (rust-lang/cargo#14859)
- git-fetch-with-cli: Set `GIT_DIR` for bare repository compatibility (rust-lang/cargo#14860)
- fix(build-std): always link to std when testing proc-macros (rust-lang/cargo#14850)
@rustbot rustbot added this to the 1.85.0 milestone Nov 30, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2024
Update cargo

17 commits in 4c39aaff66862cc0da52fe529aa1990bb8bb9a22..05f54fdc34310f458033af8a63ce1d699fae8bf6
2024-11-25 16:36:17 +0000 to 2024-12-03 03:14:12 +0000
- test(pgo): only run on nightly (rust-lang/cargo#14887)
- chore: Bump to 0.86.0; update changelog (rust-lang/cargo#14885)
- docs(ref): Finish a sentence on rust-versions (rust-lang/cargo#14884)
- chore(deps): update rust crate cargo_metadata to 0.19.0 (rust-lang/cargo#14878)
- chore(deps): update rust crate gix to 0.68.0 (rust-lang/cargo#14879)
- fix: Remove default registry reference in `info` cmd docs (rust-lang/cargo#14880)
- test(pgo): determine test runnability at compile time (rust-lang/cargo#14874)
- test: `requires` attribute accepts string literals for cmds (rust-lang/cargo#14875)
- chore(deps): update msrv (3 versions) to v1.81 (rust-lang/cargo#14871)
- chore(deps): update msrv (rust-lang/cargo#14867)
- fix(fix): Migrate cargo script manifests across editions (rust-lang/cargo#14864)
- feat(toml): Allow adding/removing from cargo scripts (rust-lang/cargo#14857)
- Add future-incompat warning against keywords in cfgs and add raw-idents (rust-lang/cargo#14671)
- test(build-std): download deps first (rust-lang/cargo#14861)
- test(pgo): ensure PGO works (rust-lang/cargo#14859)
- git-fetch-with-cli: Set `GIT_DIR` for bare repository compatibility (rust-lang/cargo#14860)
- fix(build-std): always link to std when testing proc-macros (rust-lang/cargo#14850)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 6, 2024
Update cargo

17 commits in 4c39aaff66862cc0da52fe529aa1990bb8bb9a22..05f54fdc34310f458033af8a63ce1d699fae8bf6
2024-11-25 16:36:17 +0000 to 2024-12-03 03:14:12 +0000
- test(pgo): only run on nightly (rust-lang/cargo#14887)
- chore: Bump to 0.86.0; update changelog (rust-lang/cargo#14885)
- docs(ref): Finish a sentence on rust-versions (rust-lang/cargo#14884)
- chore(deps): update rust crate cargo_metadata to 0.19.0 (rust-lang/cargo#14878)
- chore(deps): update rust crate gix to 0.68.0 (rust-lang/cargo#14879)
- fix: Remove default registry reference in `info` cmd docs (rust-lang/cargo#14880)
- test(pgo): determine test runnability at compile time (rust-lang/cargo#14874)
- test: `requires` attribute accepts string literals for cmds (rust-lang/cargo#14875)
- chore(deps): update msrv (3 versions) to v1.81 (rust-lang/cargo#14871)
- chore(deps): update msrv (rust-lang/cargo#14867)
- fix(fix): Migrate cargo script manifests across editions (rust-lang/cargo#14864)
- feat(toml): Allow adding/removing from cargo scripts (rust-lang/cargo#14857)
- Add future-incompat warning against keywords in cfgs and add raw-idents (rust-lang/cargo#14671)
- test(build-std): download deps first (rust-lang/cargo#14861)
- test(pgo): ensure PGO works (rust-lang/cargo#14859)
- git-fetch-with-cli: Set `GIT_DIR` for bare repository compatibility (rust-lang/cargo#14860)
- fix(build-std): always link to std when testing proc-macros (rust-lang/cargo#14850)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-manifest Area: Cargo.toml issues Command-remove S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants