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

Add format_if_else_cond_comment config option #1619

Merged

Conversation

topecongiro
Copy link
Contributor

Closes #1575.
The option only supports comment starting with // and is set to false by default.

@topecongiro topecongiro force-pushed the config/format_if_else_cond_comment branch from cd94f2e to 889cd09 Compare June 1, 2017 15:33
@nrc
Copy link
Member

nrc commented Jun 1, 2017

The option only supports comment starting with //

What is the reasoning for this restriction? Seems to me that the change should apply equally to /* */ comments

@topecongiro
Copy link
Contributor Author

I agree with you. Moreover, the PR does work with /* */.
I will add tests and rebase.

@topecongiro topecongiro force-pushed the config/format_if_else_cond_comment branch from 889cd09 to 2f99a2b Compare June 1, 2017 23:20
Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

This all looks good, but I don't think we even really need an option for it - it can just be the default behaviour.

@topecongiro
Copy link
Contributor Author

topecongiro commented Jun 2, 2017

Thanks for you comment! I will remove an option and make it the default behaviour.

@topecongiro topecongiro force-pushed the config/format_if_else_cond_comment branch from 2f99a2b to aadd3e1 Compare June 3, 2017 09:19
@topecongiro
Copy link
Contributor Author

Removed an option and rebased.

@nrc nrc merged commit 3e25e62 into rust-lang:master Jun 4, 2017
@topecongiro topecongiro deleted the config/format_if_else_cond_comment branch June 4, 2017 04:18
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