Skip to content
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

[unstable option] trailing_comma #3379

Open
Tracked by #1895
scampi opened this issue Feb 13, 2019 · 11 comments
Open
Tracked by #1895

[unstable option] trailing_comma #3379

scampi opened this issue Feb 13, 2019 · 11 comments
Labels
unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for unstable option: trailing_comma

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@scampi scampi changed the title [unstable option] unstable option: trailing_comma [unstable option] trailing_comma Feb 18, 2019
@c02y

This comment has been minimized.

@calebcartwright

This comment has been minimized.

@c02y

This comment has been minimized.

@nab-os
Copy link

nab-os commented May 3, 2022

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

@dcow
Copy link

dcow commented May 19, 2022

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.

@calebcartwright
Copy link
Member

@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.

@LollipopFt
Copy link

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

@calebcartwright
Copy link
Member

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

#3377 (comment)

@externl
Copy link

externl commented Jun 3, 2022

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
Copy link

externl commented Jun 3, 2022

Just found this relevant comment: #4956 (comment).

@calebcartwright
Copy link
Member

You beat me to it @externl 😄 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unstable option tracking issue of an unstable option
Projects
None yet
Development

No branches or pull requests

7 participants