Rustfmt-ing liblog. #28898
Rustfmt-ing liblog. #28898
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| for s in m.split(',') { | ||
| if s.is_empty() { | ||
| continue | ||
| } |
There was a problem hiding this comment.
I think that there's a bug to leave this one alone, can't remember exactly though.
There was a problem hiding this comment.
I think this is OK (apart from a possible ; on the continue. I filed something about one line if expressions, but that would only be in expression position in any case.
|
r? @nrc |
| let mut logger: Box<Logger + Send> = LOCAL_LOGGER.with(|s| s.borrow_mut().take()) | ||
| .unwrap_or_else(|| { | ||
| box DefaultLogger { handle: io::stderr() } | ||
| }); |
There was a problem hiding this comment.
Hmm, I'd be interested to see what rustfmt makes of those larger blocks. I bet it's awkward formatting them with visual indent like this.
I used to prefer the original formatting, but the new one has grown on me (it was kind of unintended, but having the higher order functions aligned makes things very readable for me). It could be a difference between large and small blocks, which would be sad, because that is the kind of thing rustfmt finds hard to differentiate.
There was a problem hiding this comment.
Ah yeah that's a good point about the size of the blocks. The benefit I've seen from not aligning-on-period is that indentation works a little better inside a multi-line closure. For example if you always indent 4 spaces beyond the period that may be some non-multiple-of-4 spaces from the left-hand side, which sometimes causes editors confusion (or people!)
|
@goyox86 could you separate out the rustfmt changes from your fixups into two separate commits please? I'd like to have rustfmt issues for everywhere it screws up. I don't think we should land this until we fix the trailing comma issues etc., if they are rustfmt bugs, but it's hard to tell what is a rustfmt problem at the moment. |
|
Hey @nrc I think I should close this PR and open a new on with the separation made. What do you think? |
|
@goyox86 sure, or you can force push a new branch over the remote branch and keep the PR, up to you. |
|
Closing this on favor of #28991 |
…I, r=alexcrichton Hi Rustaceans! This is the second version of running rustfmt on liblog rust-lang#28898. There are only rustfmt suggested changes. Because I think that some patterns here (that I fixed up in the prev PR need to be discussed in detail. //cc @nrc
Hi Rustaceans!
I'm not Rust coder, I just ran rustfmt on liblog following @nrc's blog post and this is the result (Along some fixes I've done to rustfmt's work). As other PR's I think we should run rustfmt again on this crate in the future and see if we accept more of it's changes.
//cc @nrc