Skip to content

Run rustfmt #1259

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

Merged
merged 5 commits into from
Dec 20, 2016
Merged

Run rustfmt #1259

merged 5 commits into from
Dec 20, 2016

Conversation

mcarton
Copy link
Member

@mcarton mcarton commented Oct 3, 2016

So I was going to integrate coverage tests in Travis, but @Manishearth was to fast.
So I though “Hey, what other tool could I fight against? Oh I know, it's been a long time since we haven't run rustfmt!”.

⚠️ I offer one Internet point to anyone who can explain to me how rustfmt decides what to do with match arms and another one for all our |db| {} lambdas.

@mcarton
Copy link
Member Author

mcarton commented Oct 3, 2016

And of course now #1246 is calling and there will be horrible merge conflicts I'm sure \o/

@mcarton
Copy link
Member Author

mcarton commented Oct 3, 2016

No conflict whatsoever, where is the fun git?

span_lint_and_then(cx, USELESS_ATTRIBUTE, attr.span,
span_lint_and_then(cx,
USELESS_ATTRIBUTE,
attr.span,
"useless lint attribute",
|db| {
sugg.insert(1, '!');
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually like what rustfmt did with our closures. They look like this closure now.

Copy link
Member Author

Choose a reason for hiding this comment

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

The thing is it did not format them all the same. Sometimes pushing the content to the left like here, sometimes pushing it to the right 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

try closure_block_indent_threshold = -1 or closure_block_indent_threshold = 99

"useless lint attribute",
|db| {
sugg.insert(1, '!');
db.span_suggestion(attr.span, "if you just forgot a `!`, use", sugg);
});
}
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

set match_block_trailing_comma = true

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really have a strong opinion on that one. The default looks fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a strong opinion :P I really like the fact that you can distinguish a match arm block closing bracket from other closing brackets, but I can do my own PR when I get around to it.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 20, 2016

I reran rustfmt, there was a really degenerate case in one of our tests, but even I can't figure out how to format that properly, so I just slapped on a rustfmt_skip

@Manishearth Manishearth merged commit a248e50 into master Dec 20, 2016
@Manishearth Manishearth deleted the rustfmt branch December 20, 2016 14:42
@mcarton
Copy link
Member Author

mcarton commented Dec 20, 2016

@oli-obk You realize this only runs rustfmt on clippy but not clippy_lints right?
The original PR (which by now had no chance to be rebased anyway) ran it on clippy_lints too.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 20, 2016

whoops fixing

I was wondering why I got so few hits

@mcarton
Copy link
Member Author

mcarton commented Dec 20, 2016

I was wondering why I got so few hits

😄

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.

3 participants