Skip to content

Fixed 'Incorrect comment indent inside if/else' issue.#4459

Merged
calebcartwright merged 8 commits intorust-lang:masterfrom
whizsid:comment_before_else_block
Oct 15, 2020
Merged

Fixed 'Incorrect comment indent inside if/else' issue.#4459
calebcartwright merged 8 commits intorust-lang:masterfrom
whizsid:comment_before_else_block

Conversation

@whizsid
Copy link
Contributor

@whizsid whizsid commented Oct 7, 2020

Fixes #4120 .

@calebcartwright
Copy link
Member

calebcartwright commented Oct 8, 2020

Could you elaborate on the removal of the existing test file?

@whizsid
Copy link
Contributor Author

whizsid commented Oct 8, 2020

@calebcartwright It was implemented to test #1575 and there are conflicts between the two cases. And already covered these cases in the new test file.

And no, this definitely should not be a comment on the else — actually,
it should not be rustfmt's decision whether this applies to the else or is a free comment after the 1.

Should we need to detect the user's indentation and format the last comment with the user's indentation?

@whizsid
Copy link
Contributor Author

whizsid commented Oct 10, 2020

@calebcartwright I changed the PR as you suggested in the comment #4120 (comment) . CI is failing because of a network error.

@calebcartwright
Copy link
Member

calebcartwright commented Oct 11, 2020

Excellent news @whizsid. I've restarted the GH run as it did indeed look spurious. I didn't get a chance to go through the implementation changes, but did a quick run through the tests added and they look great!

Will try to go through this in more detail tomorrow

topecongiro
topecongiro approved these changes Oct 14, 2020
View reviewed changes
Copy link
Contributor

@topecongiro topecongiro 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 the PR! LGTM.

calebcartwright
calebcartwright approved these changes Oct 15, 2020
View reviewed changes
Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Well done @whizsid, thank you!

@calebcartwright calebcartwright merged commit 4834547 into rust-lang:master Oct 15, 2020
@calebcartwright calebcartwright mentioned this pull request Oct 15, 2020
Closed
calebcartwright pushed a commit to calebcartwright/rustfmt that referenced this pull request Nov 14, 2020
@whizsid @calebcartwright
Fixed 'Incorrect comment indent inside if/else' issue. (rust-lang#4459)
b8f08f8 
* Added test cases

* Fixed if condition comment issue

* Fixed extern C issue

* Removed previous test case

* Removed tmp file

* honor the authors intent

* Changed the file name to its original name

* Removed extra whitespace
calebcartwright pushed a commit that referenced this pull request Nov 14, 2020
@whizsid @calebcartwright
Fixed 'Incorrect comment indent inside if/else' issue. (#4459)
faf97a6 
* Added test cases

* Fixed if condition comment issue

* Fixed extern C issue

* Removed previous test case

* Removed tmp file

* honor the authors intent

* Changed the file name to its original name

* Removed extra whitespace
@karyon
Copy link
Contributor

karyon commented Oct 26, 2021

Backported in #4525

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

Reviewers

@calebcartwright calebcartwright calebcartwright approved these changes

+1 more reviewer

@topecongiro topecongiro topecongiro approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

1x-backport:completed hacktoberfest-accepted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect comment indent inside if/else

4 participants

@whizsid @calebcartwright @karyon @topecongiro