Skip to content

chore: Use origin rendering code for suggestions#388

Open
scrabsha wants to merge 2 commits intorust-lang:mainfrom
scrabsha:push-qlzuvwwlvuwr
Open

chore: Use origin rendering code for suggestions#388
scrabsha wants to merge 2 commits intorust-lang:mainfrom
scrabsha:push-qlzuvwwlvuwr

Conversation

@scrabsha
Copy link
Copy Markdown
Contributor

@scrabsha scrabsha commented Mar 14, 2026

it turns out emit_suggestion_default has its own code to display the origin at which a suggestion should be applied.

i think this is not optimal because some code is duplicated, and keeping it as leads to even more duplication being added as part of my work on #386.

this commit removes the custom origin display code in emit_suggestion_default and uses render_origin (which AFAIK is used everywhere else in annotate_snippets) instead.

i ran rustc's test/ui tests against this change and it resulted in no oracle change. while i cannot assert rendering hasn't changed at all, i am reasonably confident that this commit does not break anything.

first commit adds what is described above. second commit uses let chains (which IMO makes the code clearer) and bumps the MSRV to 1.88.0 (released on 26 June, 2025), which is required for let chains. happy to drop the second commit if y'all think this does not warrant an MSRV bump. to be clear, having let chains would help a lot for #386 as well. it's not just this PR.

@scrabsha scrabsha force-pushed the push-qlzuvwwlvuwr branch 4 times, most recently from ebad973 to 533a86d Compare March 14, 2026 13:11
@scrabsha scrabsha force-pushed the push-qlzuvwwlvuwr branch from 533a86d to c3c0622 Compare March 14, 2026 13:16
@scrabsha scrabsha marked this pull request as ready for review March 14, 2026 13:25
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

chore: Bump MSRV to 1.88.0, use let chains

As noted at #378 (comment), as we don't have an MSRV policy MSRV bumps should not be done without discussion.

Also, this commit seems unrelated to this PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, this commit seems unrelated to this PR

i wasn't satisfied with what the code looked like when i wrote the first commit and figured it would be a good idea to see how it renders if let chains were used. i tried and was much happier with my changes. i still kept it as a separate commit so it's easier review (and remove if y'all are ok with the code as it is at f4a9639). the other edits in c3c0622 are required to make CI pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants