clog-tool / clog-cli

Generate beautiful changelogs from your Git commit history
MIT License
856 stars 43 forks source link

chore: reformat code using rustfmt #75

Closed cburgdorf closed 9 years ago

cburgdorf commented 9 years ago

Hey @kbknapp,

I wanted to start using rustfmt to align the source to a common style. I created a rustfmt.toml with some tweaks because I didn't like the default configs. It's debatable. You seem to be in favor for trailing commas in struct literals and match arms. I'm not a huge fan but if you strongly vote for them I can change the config ;)

kbknapp commented 9 years ago

Actually, I tend to not to put trailing commas...but rustfmt makes me since I haven't started a rustfmt.toml, or my editor puts them in by default :stuck_out_tongue_winking_eye: There are a few points which I at first didn't like about rustfmt, but have since really began to enjoy. I'd like your thoughts on them. Biggest issue is arguments and return values of functions. I'm now a big fan of placing them on their own line if there are more than one. (rustfmt default is only if they exceed the 100 char max per line...I just prefer to stay consistent throughout even if it's below 100 chars)

fn some_func(a: &mut SomeType, w: OtherType) -> Result<(), String> {

}

// Becomes

fn some_func(a: &mut SomeType, 
             w: OtherType) 
             -> Result<(), String> {

}

Which make for far less noisy commit diffs. Also if there is a where clause it gets indented by 4 spaces, and generic parameters get aligned to each other.

fn some_func<I, S>(a: &mut SomeType, 
                   i: I) 
                   -> Result<(), String>
    where I: IntoIterator<Item = A>,
          A: AsRef<str>
{

}

Took some getting used to, but now I'm a big fan. And the newline for opening brace is only if there is a where clause which really helps break up the noise of the function signature from the function body. Thoughts?

Since it's your project, I'm good with your choices ;)

cburgdorf commented 9 years ago

Actually, I tend to not to put trailing commas...but rustfmt makes me

Good to know! I also know the pros of cleaner diffs and I totally understand that it's the better decision from a technical standpoint. I just dislike it from a standpoint of aesthetics ;)

I haven't started a rustfmt.toml

I just helped on rustfmt to make it easier to have your own rustfmt.toml that only overwrites a subset of configs: https://github.com/nrc/rustfmt/commits/master?author=cburgdorf

Also notice my first commit there. It's actually a job for clap haha ;)

I'm now a big fan of placing them on their own line if there are more than one

My current thinking is that I prefer them to be in one line as long as it fits in the 100 char budget. But I could also live with going multiline if it's more than one argument.

Also if there is a where clause it gets indented by 4 spaces, and generic parameters get aligned to each other

:+1: yep, like I like that, too

And the newline for opening brace is only if there is a where clause which really helps break up the noise of the function signature from the function body

That's the thing I don't like!!!!11!!11!!!!111! haha

Let's face it. For everyone who's not truly in love with rust. A function signature like that one is just a mess of chars.

fn some_func<I, S>(a: &mut SomeType, 
                   i: I) 
                   -> Result<(), String>
    where I: IntoIterator<Item = A>,
          A: AsRef<str> {

}

I don't think putting the brace on a newline just for the case when there is a where cleans it up much.

fn some_func<I, S>(a: &mut SomeType, 
                   i: I) 
                   -> Result<(), String>
    where I: IntoIterator<Item = A>,
          A: AsRef<str> 
{

}

In fact, I think it looks really inconsistent when all the other functions don't have their opening brace on a new line.

Since it's your project, I'm good with your choices ;)

I consider clog to be as much your project as it is mine. Especially now that it's in it's own organization. I just happened to have kicked off the initial development ;)

kbknapp commented 9 years ago

Thanks for the kind words!

Ok, then I'd say we go with:

I'm good with the above list :+1:

cburgdorf commented 9 years ago

Great! For the trailing comma we still need to get rid of it in match arms (See https://github.com/nrc/rustfmt/issues/173). I tried to work on that today but haven't had much luck with it yet.

Btw, totally off topic: Did the thoughtram package arrive already?

kbknapp commented 9 years ago

@cburgdorf I'll try and knock some of them out and add some commits to this branch. Last time I looked there may be some 100+ char lines to shorten! And yep, it arrived :smile:! See here