Skip to content

Conversation

epage
Copy link
Contributor

@epage epage commented Sep 4, 2025

What does this PR try to resolve?

Currently, if you have a manifest parse error for a cargo script, the line number will be relative to the start of the frontmatter and not the source file. This changes it so we get the line number relative to the start of the source file.

How to test and review this PR?

Notes

To do this, I added span tracking to our frontmatter parser. To make this easier, I used some helpers from winnow which is an existing transitive dependency.

This span tracking will also come into use when I change the frontmatter parser to start reporting spans in errors so we get annotated snippets of source in the error messages to users.

@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2025

r? @weihanglo

rustbot has assigned @weihanglo.
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-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2025
@epage epage changed the title ix(manifest): Report script manifest errors for the right line number fix(manifest): Report script manifest errors for the right line number Sep 4, 2025
@epage epage force-pushed the script-line-num branch 2 times, most recently from 4898269 to 44389cb Compare September 5, 2025 15:35
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Looks good. Feel free to address nits or merge it :)

@epage epage enabled auto-merge September 8, 2025 19:27
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the comment updates!

@epage epage added this pull request to the merge queue Sep 8, 2025
Merged via the queue into rust-lang:master with commit 0e07ceb Sep 8, 2025
27 checks passed
@epage epage deleted the script-line-num branch September 8, 2025 20:31
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2025
bors added a commit to rust-lang/rust that referenced this pull request Sep 10, 2025
Update cargo submodule

10 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..98402ac7a41dd0f564d3e56d58180d325d0417a0
2025-09-04 01:25:01 +0000 to 2025-09-09 20:19:39 +0000
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)

r? ghost
bors added a commit to rust-lang/rust that referenced this pull request Sep 11, 2025
Update cargo submodule

13 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..24bb93c388fb8c211a37986539f24a819dc669d3
2025-09-04 01:25:01 +0000 to 2025-09-10 23:16:07 +0000
- Bump miow to 0.60.1 (rust-lang/cargo#15950)
- test(help): Ensure consistent behavior regardless of rustup use (rust-lang/cargo#15949)
- docs(changelog): Clarify how manifest paths are used (rust-lang/cargo#15946)
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)
@rustbot rustbot added this to the 1.91.0 milestone Sep 11, 2025
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 12, 2025
Update cargo submodule

13 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..24bb93c388fb8c211a37986539f24a819dc669d3
2025-09-04 01:25:01 +0000 to 2025-09-10 23:16:07 +0000
- Bump miow to 0.60.1 (rust-lang/cargo#15950)
- test(help): Ensure consistent behavior regardless of rustup use (rust-lang/cargo#15949)
- docs(changelog): Clarify how manifest paths are used (rust-lang/cargo#15946)
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-manifest Area: Cargo.toml issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants