Format code with rustfmt

[jrgarcia]

Used make format to format code according to project's rustfmt settings.

[andrewjstone]

Unfortunately this still contains trailing commas, which is the thing I dislike the most in the defaults. It looks like it happened because rustfmt changed the options to be non-backward compatible and ignores the now unknown options. I'll fix the options up the way I like them in another PR.

[jrgarcia]

Whoa, I somehow missed that. Sorry about that!

[andrewjstone]

Totally not your fault. I'll still let you run it once I merge in the update. That way you can be the second most active contributor :D

[jrgarcia]

Haha, sounds good! I'm re-reading the VRR paper and the docs in this repo. Hopefully I'll soon be contributing more than little stylistic tweaks here and there.

[andrewjstone]

@jrgarcia What's your opinion about https://github.com/rust-lang-nursery/rustfmt/blob/master/Configurations.md#chain_indent ?

[jrgarcia]

I think I'd prefer "Visual", though I've always had stuff set to "Block" in the past.

[andrewjstone]

I defaulted to writing in visual style, so let's stick with that. Thanks!

[jrgarcia]

I tried reformatting the code, but setting trailing_comma to "Never" breaks when there are RangeFulls (..) at the end. For example, here it gets reformatted to

        if let VrMsg::ClientReply {                                                                                                                               
                   request_num: reply_req_num,
                   value
                   ..
               } = msg {

(note the missing comma after value). Running rustfmt on that single file doesn't cause the problem nor does it happen if you remove trailing_comma from the .rustfmt.toml. Not sure what do here other than possibly reporting it to the rustfmt project.

[andrewjstone]

This does indeed seem like a bug. I would suggest opening an issue on rustfmt. It's really weird that it doesn't happen when run against the individual file. I have a workaround suggestion for now also that won't require changing he code to not use ...

You could just rm the problem files, run cargo fmt, do a git checkout of the deleted files then run rustfmt against those. If that sounds like too much of a pain in the ass I can give it a whirl. But I'd appreciate if you opened the issue on rustfmt as you've disovered it.

[jrgarcia]

I was actually in the middle of filing a bug there and I'll definitely do that and see how it turns out.

[andrewjstone]

Great Thanks!.

[jrgarcia]

It appears to happen with cargo fmt, make format, and rustfmt. It just only reports an error immediately after running when using cargo. I opened up an issue rust-lang-nursery/rustfmt#1544 for this.

[jrgarcia]

They fixed the issue with rustfmt, but it isn't released yet. I compiled it locally and it ran just fine. Here is the reformatted code. I ran a build and test and they both ran successfully.

That said, make format will still have this issue until rustfmt releases a new version and that's updated on anyone's computer that uses it.

[andrewjstone]

Thanks for doing this work @jrgarcia . I'm still kind of on the fence on merging it. On net I feel like it makes the code look worse than before. I like the consistency of chain operations, and the conversion of try to ? but the match statements pretty much all look worse. It's weird to force an indent on some matches just because they are a few characters long but still shorter than max line length. I specifically tightened up a bunch manually to look the way they do for quick scanning. Maybe some of the width variables could be enlarged to fix this, but there are still other things that still look off to me. And without adding braces to the last match arm, there is an unavoidable trailing comma which I don't like.

Write now I'm leaning toward not using rustfmt at all, Although I may do a search/replace to at least change the try prefixes to ? suffixes. What are your thoughts?

[jrgarcia]

On one hand, I think that having a tool like this is great, because it prevents any unnecessary debates over stylistic choices. I think it's been great for the Go community for that reason. There are plenty of more important things in a project than where you put indentation.

On the other hand, I think this works so well for the Go community because it's such a simple language. Rust is quite a bit more complex syntactically.

I'm fine with abandoning this. Even if this doesn't make it in, it lead to a bug fix in rustfmt and a good discussion.

[andrewjstone]

Let's punt on this for now. If you want to do a search replace of try with ? that'd be a good first step toward consistent formatting. You definitely don't have to do that though...