-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Fix confusing error message for comma typo in multiline statement #72348
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
Thank you very much for taking the time to look into this!
I have a couple of comments I would like you to address before this is merged.
Thank you very much for your review Esteban. I'll have a look at these issues. Do you happen to know what is going on with the new UI test fail? I ran it locally with |
You did the right thing. By running We care about both because the comments highlight intent, while we look at the |
I can confirm that this fixes the test. Ty! |
CI is failing but it looks like this is an issue with tidy checks on source unrelated to this PR https://github.com/rust-lang/rust/pull/72348/checks?check_run_id=700945715#step:24:1208 |
@estebank I think that we are gtg here. I pushed a new commit and CI is passing. |
LGTM. Can we squash all commits into a single commit for log cleanliness sake? Other than that, r=me |
confusing diagnostics, issue rust-lang#72253 add test for confusing error message, issue-72253 remove is_multiline check, refactor to self.expect(&token:Semi) update issue-72253 tests return Ok
Squashed to f384cdc |
@bors r=estebank |
@chrissimpkins: 🔑 Insufficient privileges: Not in reviewers |
@estebank ^^ |
@bors r+ rollup |
📌 Commit f384cdc has been approved by |
Fix confusing error message for comma typo in multiline statement Fixes rust-lang#72253. Expands on the issue with a colon typo check. r? @estebank cc @ehuss
Rollup of 6 pull requests Successful merges: - rust-lang#72348 (Fix confusing error message for comma typo in multiline statement) - rust-lang#72533 (Resolve UB in Arc/Weak interaction (2)) - rust-lang#72548 (Add test for old compiler ICE when using `Borrow`) - rust-lang#72606 (Small cell example update) - rust-lang#72610 (Remove font-display settings) - rust-lang#72626 (Add remark regarding DoubleEndedIterator) Failed merges: r? @ghost
Fixes #72253. Expands on the issue with a colon typo check.
r? @estebank
cc @ehuss