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

refactor rewrite_literal & combine_strs_with_missing_comments #6250

Merged
merged 3 commits into from
Aug 3, 2024

Conversation

ding-young
Copy link
Contributor

@ding-young ding-young commented Jul 25, 2024

Tracked by #6206

Description

  1. modify rewrite_literal to return RewriteResult
  2. refactor combine_strs_with_missing_comments and its call sites

@ytmimi ytmimi added the GSoC Google Summer of Code label Jul 25, 2024
src/attr.rs Outdated Show resolved Hide resolved
Comment on lines 2148 to +2151
let shape = if contains_comment {
shape.block_left(context.config.tab_spaces())?
shape
.block_left(context.config.tab_spaces())
.max_width_error(shape.width, between_span.with_hi(ex.span().hi()))?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

... = // comment 
       Expr

Since between_span corresponds to the span of // comment but we are calculating the shape for the expr, I think the span for max_width_error should start from between_span.lo() and end with expr.span.hi()

@ding-young ding-young marked this pull request as ready for review July 29, 2024 06:00
src/chains.rs Outdated
@@ -267,8 +267,15 @@ impl ChainItemKind {
}

impl Rewrite for ChainItem {
// TODO impl rewrite_result after rebase
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering what this comment is referencing. Can this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, since this pr also implements rewrite_result for ChainItem, I removed the comment.

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.

Thank you for another rewrite_result PR. Running the Diff-Check Job now

Edit: Job passed ✅

@ytmimi ytmimi merged commit b8a5b21 into rust-lang:master Aug 3, 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