rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

[unstable option] trailing_comma

Open scampi opened this issue 6 years ago • 11 comments
trafficstars

Tracking issue for unstable option: trailing_comma

scampi avatar Feb 13 '19 22:02 scampi

The three code snippets ("Always", "Never", "Vertical") of https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#trailing_comma are exactly the same.

c02y avatar Aug 16 '20 10:08 c02y

Thank you for sharing @c02y. However, that is not correct, as those three snippets do indeed have variance with trailing commas as designed (check for the absence/presence of the trailing comma after the sit in the first line and after adipiscing).

calebcartwright avatar Aug 16 '20 18:08 calebcartwright

Yeah, sorry, I didn't notice that, it will be better to highlight or add an arrow on those differences inside the code snippet.

c02y avatar Aug 17 '20 02:08 c02y

Hi, is there any reason why this feature is still considered unstable ? I can't find any opened issue mentioning it.

nab-os avatar May 03 '22 13:05 nab-os

I just tried using this feature and it adds trailing commas to function argument lists at the call site.

let foo = Foo::new("foo",);

It also adds trailing commas to match de-structuring even when there's no possibility of more bindings, like:

let action = match get_req_scope(req,).await {
    Ok(a,) => ...,
    Err(s,) => return Response::new(s,),
};

Adding commas in these instances seems way too broad. Is this intended?

What I'd expect when reaching for a feature like this is that a comma should be added to every instance of a "variable size" list where it would be expected that new items can be added without modifying the type (unless it's a type declaration itself) or functions (unless it's a function declaration) involved.

vec!, use, Derive, struct declarations, function declarations would be examples of what I'd limit this to and remove function call sites, struct creations call sites, and match arm destructuring.

dcow avatar May 19 '22 04:05 dcow

@dcow - You didn't specify, but I'm going to assume you are using the Always variant of the trailing_comma option. In which case, the answer is yes, that is intended. Feel free to suggest changes to the associated documentation for that variant if you think it can be improved or found it unclear, but Always indeed means always and I can't imagine changing that.

You're also free to open a request for an additional variant if you'd like to try to codify the heuristics, though I'll go ahead and note upfront that's not something we'd be able to allocate any of our bandwidth to working on, and would be reliant upon the community to implement.

However, I'm not really sure I follow the logic of how a function signature differs from a call site of said function in this context. One of the most common, but not exclusive, reasons people like utilizing trailing commas is to avoid extra diff noise in version control when something changes particularly in multiline cases, regardless of whether that's an additional parameter in a function (which impacts both the sig and call sites), new members in tuples or structs, variants, etc.. That's why the default value is Vertical, but Always exists for those that really want the commas everywhere.

calebcartwright avatar May 20 '22 03:05 calebcartwright

hi, may I know when this will be stabilised? just asking.

LollipopFt avatar May 28 '22 06:05 LollipopFt

hi, may I know when this will be stabilised? just asking.

https://github.com/rust-lang/rustfmt/issues/3377#issuecomment-1135281548

calebcartwright avatar May 28 '22 23:05 calebcartwright

I'm getting the same behavior as @dcow with the default "Vertical" setting when calling macros. For example I see this

let message = format!(
     "file is using {} by default", 
    "LF",
);

is formatted to:

let message = format!("file is using {} by default", "LF",);

Something like:

let message = my_format(
    "file is using ", 
    "LF by default",
);

formats correctly to:

let message = my_format("file is using ", "LF by default");

Not sure if this is a bug or another settings I'm missing?

externl avatar Jun 03 '22 16:06 externl

Just found this relevant comment: https://github.com/rust-lang/rustfmt/issues/4956#issuecomment-914822481.

externl avatar Jun 03 '22 16:06 externl

You beat me to it @externl :smile: but yes macro calls are an entirely different story, and it's intentional that rustfmt isn't stripping user-added commas in those cases for reasons detailed in some of those (transitively) linked issues

calebcartwright avatar Jun 03 '22 16:06 calebcartwright