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

impl rewrite_result for ast::Expr #6311

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

ding-young
Copy link
Contributor

tracked by #6206

Description

This pr impl rewrite_result for Expr by updating format_expr to return RewriteResult.
Now we don't call .ok() after calling rewrite_*** functions in format_expr :)

Comment on lines -370 to -377
macro_rules! skip_out_of_file_lines_range {
($self:ident, $span:expr) => {
if out_of_file_lines_range!($self, $span) {
return None;
}
};
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now this is totally replaced with skip_out_of_file_lines_range! which returns RewriteError::SkipFormatting instead of None

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean skip_out_of_file_lines_range_err? In a follow up PR I think it would be nice to rename skip_out_of_file_lines_range_err back to skip_out_of_file_lines_range! Now that we know we've replaced all the invocations to return RewriteResult. I'm not in a rush to make this change though.

@ytmimi ytmimi added the GSoC Google Summer of Code label Sep 3, 2024
Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

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

Nice to see format_expr return RewriteResult 👍

@ytmimi
Copy link
Contributor

ytmimi commented Sep 5, 2024

Running the Diff-Check

Edit: Job passed ✅

@ytmimi ytmimi merged commit d720a7e into rust-lang:master Sep 5, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Google Summer of Code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants