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

Disable trailing comma by default #42

Closed
perlun opened this issue Nov 29, 2016 · 58 comments · Fixed by #101
Closed

Disable trailing comma by default #42

perlun opened this issue Nov 29, 2016 · 58 comments · Fixed by #101
Labels

Comments

@perlun
Copy link

perlun commented Nov 29, 2016

Introduction and background

When I ran rustfmt yesterday on some code I was writing, I was once again reminded by the fact that our current defaults are the following:

             where_trailing_comma <boolean> Default: false
                                  Put a trailing comma on where clauses
            struct_trailing_comma [Always|Never|Vertical] Default: Vertical
                                  If there is a trailing comma on structs
        struct_lit_trailing_comma [Always|Never|Vertical] Default: Vertical
                                  If there is a trailing comma on literal structs
              enum_trailing_comma <boolean> Default: true
                                  Put a trailing comma on enum declarations
       match_block_trailing_comma <boolean> Default: false
                                  Put a trailing comma after a block based match arm (non-block arms are not affected)
    match_wildcard_trailing_comma <boolean> Default: true
                                  Put a trailing comma after a wildcard arm

I know that some of this is ongoing discussion in #34, since it specifically mentions the trailing comma in some situations:

The last match arm should have a comma if the body is not a block:

I want to take the chance before #34 is merged to broaden that discussion a bit more, and try to reach consensus on trailing commas in general. (I know that they are sometimes needed; I consider that a bug and this discussion is more related to the areas where it is syntactically optional, but a common de-facto standard in the Rust community and/or enforced by rustfmt.)

I don't claim to be unbiased; I have a specific opinion that I am voicing in this RFC. However, what I do claim is to represent the different standpoints in a factual way. If you feel that I am missing or misrepresenting some argument for either position, please write a comment with the missing part and I'll try to adjust the proposal continously.

Basic presumption

I hope that (almost) everyone can agree that our current position - to sometimes advocate a trailing comma and sometimes not, as shown above - is not optimal. It feels counterintuitive and also like something you'd have to teach people about ("the style guide recommends it here, but not there, for whatever reason" - not saying that there aren't reasons for the inconsistency, just saying that an inconsistent set of rules in an area is much harder to defend and also harder for people to remember.)

Arguments used to support the addition of trailing commas

It's commonly used within the rust community

This is a fact. Servo uses it, rustc uses it, probably a bunch of other projects as well. The precedent is there, there's not so much to say about that. There are probably a number of people who like this style, otherwise it wouldn't have been started to be used anyway. Moving away from this style would risk discouraging those people from contributing to the community.

Counter-argument

  • It's not that common outside the Rust community.
  • The current Rust code in existance will hopefully eventually be a very small minority of the cumulative amount of rust code in the years to come. In other words, the vast majority of the rust code eventually written does not yet exist.
  • Once rustfmt is stable enough, we should be able to reformat larger codebases (like servo and rustc) with it anyway.
  • Regarding the "people might like it" part, it's a fact regarding any part of the style guide. The style guide cannot emphasise individual people so much, but should rather focus on the greater good for the community as a whole.

It makes changing the code easier

This argument is also factually correct; when adding a new entry to an array, you never have to remember adding a trailing comma. Perhaps more importantly so, when rearranging items in an array you don't have to visually scan the array to find which line now has the missing comma (so you can add it and make the code compile again).

Counter-argument

While the argument is factually correct, it's pretty much a matter of what you are used to. Good tooling (i.e. instant feedback on the validity of the code, perhaps powered by rls or simiilar) can make this be more of a moot point. If the editor you are using instantly shows you (or hints at) where the missing comma is, this is less of a point as before. (but of course, not all people can be expected to use a single editor/a RLS-powered editor etc.)

It produces shorter, less verbose diffs

This argument is based on the simple fact that if the trailing comma is already present on the last line of an {array|enum|match|etc}, that line will not have to be edited if you add a new element to the {array|enum|match}. In other words, the diff produced when changing the code will only include the relevant line (the new functionality being added), you won't have to touch an adjacent line to make it compile.

This is a very relevant and factual argument, since it avoids the cognitive load put on the brain when parsing the diff. You can focus on the actual change, and have to think less about the noise.

Counter-argument

While this is a a factually correct assertion, @bbatsov (the author of Rubocop, a linter for Ruby), wrote that:

The defaults will not be changed [to enable trailing commas by default] - using VCS diff limitations to justify a coding convention doesn't make sense

Of course, here is where facts end and opinions start: I personally agree with Boris, that this is too weak an argument to justify the current position. Others might, and will, say the opposite (that this is such a strong argument that the current position should be retained).

It breaks git blame

This is actually a pretty good argument. It will appear as if the person who added the comma was the author of that line even though his/her contribution was only the single character. Can't really say much about this, this is a reasonable, factual argument. (However, it's again letting the limitations of a particular tool mandate the style choices)

Arguments used to reject the addition of trailing commas

Opinionated: It looks like a typo

One of my early encounters with the Rust community was actually because of this; I read some code in the Rust compiler source code which struck me as "looking weird", because of the trailing comma. I have been doing programming for about 20 years, of which 14 years of it has been "for a living". Never during these years have I encountered a community that was advocating trailing commas.

I'm quite certain I'm not alone with this experience (which the SO links further down in this RFC indicates; people tend to get confused when they read code written like this). In my eyes, the current position definitely is not "mainstream" enough for a language with high ambitions to become a broad competitor to languages like C, C++ and perhaps Java, C# and others as well. If that's really what we're aiming for, we should try to aim for a style that "feels natural" to most people reading code without having seen what the style guide advocates.

Our language is so great because of the other features it provides (immmutability, the whole borrowing concept, proper concurrency etc) that I think that is what should be sticking out when people read our code: our superiority in these areas. Not that we write code that looks like an array element is missing from the end; that someone has forgotten to finish his sentence. Again, extremely opinionated, but it makes people question the code: is this really what the author intended to write? Is there a bug here, did I just discover an unpleasant surprise?

It is contrary to the recommended style guide/linter defaults for other programming languages

A non-comprehensive on some of the findings I could gather:

  • rubocop, a linting tool for Ruby, produces warnings for this syntax with its default config. The issue is discussed more in Trailing commas rubocop/rubocop#713, TrailingComma cop rubocop/rubocop#735 and at length in trailing commas should be encouraged in multiline literals rubocop/ruby-style-guide#273. Other style guides like the Thoughtbot one advocates trailing comma, but I'd say the former one is the more popular one (and definitely a more comprehensive guide with much more details about the recommendations).
  • eslint, a linting tool for Javascript, has it disabled by default but with options for enforcing it for various scenarios (multi-line only, always, etc).
  • tslint, a linter for Typescript, seems to take the approach to let you configure it either way, but don't seem to advocate a particular default. (tslint is not a reformatter in that sense, so it can take a more relaxed stance on the matter)
  • The PHP manual says that "[h]aving a trailing comma after the last defined array entry, while unusual, is a valid syntax."
  • It has been said to be common in Python, but I couldn't find a style guide to back this claim.

Other languages/style guides don't typically disallow it, but it's a common source for people to scratch their heads and wonder "why is this valid C#/Python/etc code?". Take a look at this and this link (Python), this link (C#), this link (Java) etc. It is definitely not a universally accepted style, recommended by a large majority of the style guides for programming languages in general or anything like that.

To put it bluntly: if we want to keep making people confused, we should keep the current recommendation as it is.

It is invalid code in some languages/environments:

  • JSON: Valid code

    {
      "foo": "bar",
      "baz": "zot"
    }

    Invalid code, parsers will reject this:

    {
      "foo": "bar",
      "baz": "zot",
    }

Opinionated: It's not really intended for humans writing code

The fact that virtually all programming languages (except from JSON, which isn't really a "programming language") allows it seems to be generally motivated by the fact that it's intended for code generation, where it's obviously a whole lot easier for the code generating code to not have to always check "is this the last element of the array being printed to stdout" and so forth. C# 2005 Programmer's Reference (p. 208) seems to imply this, and this SO thread gives the same impression. It was initially allowed in C back from the days of K&R, and many other languages have included support for it since.

One could argue that if this "feature" is designed for code generators, let the code generators use it and let humans produce code that is more aesthetically pleasing and straightforward.

Conclusion and proposed adjustment

I find the communities for various programming languages a bit scattered on the subject. Some people prefer this style very strongly, others (like me) dislike it equally strongly and would never ever want to run a tool to reformat our code with the default settings as they are right now. Others are just confused by reading code formatted like this and start to ask questions about it. That, to me, indicates that our current defaults are not so intuitive and hence I suggest that the following settings should be the new default:

             where_trailing_comma <boolean> Default: false
                                  Put a trailing comma on where clauses
            struct_trailing_comma [Always|Never|Vertical] Default: Never
                                  If there is a trailing comma on structs
        struct_lit_trailing_comma [Always|Never|Vertical] Default: Never
                                  If there is a trailing comma on literal structs
              enum_trailing_comma <boolean> Default: false
                                  Put a trailing comma on enum declarations
       match_block_trailing_comma <boolean> Default: false
                                  Put a trailing comma after a block based match arm (non-block arms are not affected)
    match_wildcard_trailing_comma <boolean> Default: false
                                  Put a trailing comma after a wildcard arm

IMHO, this would be very consistent and easy to understand for anyone: experts and new Rust enthusiasts alike.

Now, you may send the expected flames... 😉

@strega-nil
Copy link

strega-nil commented Nov 29, 2016

It's more consistent to have the trailing comma, and, imho, it's prettier. I hate that some languages cough C++ cough don't support it.

foo(
  x,
  y,
  z,
);

is much prettier than the alternatives

foo(
  x,
  y,
  z
)

or

foo(
  x,
  y,
  z)

In short, I completely disagree.

@solson
Copy link
Member

solson commented Nov 29, 2016

There's a lot of text here, so I'll try to respond point-by-point.

our current position - to sometimes advocate a trailing comma and sometimes not, as shown above

There isn't a current position. Most of rustfmt's defaults haven't been decided yet, so it doesn't represent the community's preference. In my experience the current majority preference is for consistently using trailing commas everywhere.

It's not that common outside the Rust community.

I disagree, I've seen a lot of non-Rust codebases that use trailing commas when their languages allow it.

Good tooling (i.e. instant feedback on the validity of the code, perhaps powered by rls or simiilar) can make this be more of a moot point. If the editor you are using instantly shows you (or hints at) where the missing comma is, this is less of a point as before. (but of course, not all people can be expected to use a single editor/a RLS-powered editor etc.)

The "sufficiently smart tooling" argument is not a good one. Even when I have something like an RLS-powered editor, I get in situations where they're not available, like editing code on GitHub, in kdiff3, or basically anywhere outside my main editor on my personal computer.

VCS diff limitations

This is blaming tools for not being smart enough again. They never end up as smart as we'd like, so it's easier to keep things simple when we can manage it, so the tools don't need smarts.

However, it's again letting the limitations of a particular tool mandate the style choices

As we should, when it's an easy win.

It looks like a typo

Eh, it's easy to get used to. Maybe it helps to think of it like this: when lists are formatted in one line, comma acts as a separator, but when lists are formatted across lines, commas acts as a terminator. At this point it looks like a typo to me when you omit the trailing comma.

It is invalid code in some languages/environments

Because of this, JSON has always been a big pain for me to edit by hand. Adding a new item constantly requires backtracking to add commas to previous lines.

It's not really intended for humans writing code

Strongly, strongly disagree. If I was designing a "better JSON" intended for human editing, then comments and trailing commas would be step 1. And it's not just me, since both YAML and TOML support trailing commas.

@petrochenkov
Copy link

Regarding aesthetics of structs and similar cases, I don't see much difference between Rust's , and other field separators like ; used by C-like languages.

// Rust
struct S {
    field1: u8,
    field2: u8, // <- trailing separator
}

// C++
struct S {
    uint8_t field1;
    uint8_t field2; // <- mandatory trailing separator
};

using VCS diff limitations to justify a coding convention doesn't make sense

Blaming other tools doesn't make the problems go away.

@nrc
Copy link
Member

nrc commented Nov 30, 2016

My preference is for a trailing comma before a newline:

Foo {
    f: a,
    g: b,
}

foo(
    a: T,
    b: T,
)

// but

foo(a, b);
Foo { f: a, g: b };

I think this is just subjective preference, but the diff thing is an argument in favour

@perlun
Copy link
Author

perlun commented Nov 30, 2016

I disagree, I've seen a lot of non-Rust codebases that use trailing commas when their languages allow it.

It would be interesting to see some examples of this. Also, if there are other style guides out there (apart from the Thoughtbot one which I already linked) that advocates it, please let us know.

@solson
Copy link
Member

solson commented Nov 30, 2016

@perlun I could try taking a look around later. Ultimately, that point is not very important, though. There is reason enough to use trailing commas in Rust without reference to external precedent.

@steveklabnik
Copy link
Member

It would be interesting to see some examples of this.

First few random JS/Ruby ones:

@bruno-medeiros
Copy link

While the argument is factually correct, it's pretty much a matter of what you are used to. Good tooling (i.e. instant feedback on the validity of the code, perhaps powered by rls or simiilar) can make this be more of a moot point. If the editor you are using instantly shows you (or hints at) where the missing comma is, this is less of a point as before. (but of course, not all people can be expected to use a single editor/a RLS-powered editor etc.)

Counter-Counter-Argument:
If the editor just shows where the error is, you still have to go there and fix it, so the editing operation is not as easy for the user as the trailing comma case. It would only be as easy if the editor automatically corrected your code - which would likely be extremely complicated, and IMO, not worth it.

@bruno-medeiros
Copy link

@perlun
Ultimately it's a subjective thing: However, you can't generally claim that users familiar with other languages that don't support or recommend trailing commas would prefer things to remain that way. I've been programming in Java for nearly all my programming career, and every once in a while I get annoyed it doesn't support trailing commas, especially since I use block indentation a lot. (These annoyances came into realization after first learning the D language many years ago, which for me was the first language I learned that supported trailing commas)

My preference is for a trailing comma before a newline:

Agreed. It's worth noting that distinction regarding the newline.

@joshtriplett
Copy link
Member

@bruno-medeiros Agreed. As an example, I attempted to get Haskell to support trailing commas a while back (with little success at that time).

@est31
Copy link
Member

est31 commented Dec 6, 2016

When I came to Rust from C++, I instantly fell in love with trailing commas being possible, and being used. I always wanted to do this on C++ but never were able to. For me its one of the many things where Rust improves over C++ (not saying that Rust only improves over C++, but here clearly it does).

Trailing commas don't break git blame and git diff, and countering your counter-argument, I think VCS is an important part of the developer's workflow these days (do you know of any big software not inside a VCS?), so a language should be VCS friendly.

That being said, if trailing commas are to be removed, there is a good style that still is compatible with git diff:

let a = Struct { a: "a"
               , b: "b"
               , c: "c"
               , d: "d"
               };

Its used for example by the elm language. But note that I clearly prefer to keep trailing commas.

@solson
Copy link
Member

solson commented Dec 6, 2016

@est31 The leading-comma style has the same problems as no-trailing-comma. It merely shifts the problem to the initial element. On top of that it introduces visual alignment which is also VCS-unfriendly, and per #8 we generally prefer block indent.

@est31
Copy link
Member

est31 commented Dec 6, 2016

Ah you are right. Indeed, the problem is still there. Its less bad though, as I think most times you add stuff to the end of a list, not the beginning.

@perlun
Copy link
Author

perlun commented Dec 6, 2016

Thanks to all (@solson, @est31 and others) for your feedback thus far. I am currently taking care of our newborn child so time for engaging further in the discussion is a bit sparse... ☺️

Just some quick comments for now; I intend to provide more feedback as soon as I can find some time. We are not speaking about "removing" trailing comma support here, but more discussing what should be the proposed "strong set of defaults" for the Rust programming language. Obviously, some people feel very strongly about the matter and I have great respect for that.

Personally, I also feel very strongly about it (which is kind of obvious since I spent hours writing a 3-page text about it..), but from the other POV. I really dislike it, the VCS argument has never ever been a problem for me (and like everyone else, I definitely use git and live my life using git diff all the time). Clearly, this is a matter of personal preference to a large degree, and what you are used to. I'm much more bothered by the unpleasant way it looks: it confused me, before I knew why it was done like this on purpose, and I'm quite sure it will confuse others new to this particular style as well. That's one of my key arguments against it.

It's a bit like the if (100 == bar) style which has sometimes been common in certain communities; looks weird at first glance, but once you realize why someone wrote it like that, it makes a lot more sense. But I still have never opted for that style either (there are other, better ways to deal with the error of assignment vs comparison), and I likely will never ever opt for the trailing comma style either. IMHO, the case for it is simply much weaker than the case against it. Now, not trying to shove this opinion down anyone's throat or anything; just stating very clearly that I don't like it and would likely configure any projects where I can to not use this style, regardless of what the final outcome of this discussion will be (which is one of the reasons why I find #33 to be important).

@solson
Copy link
Member

solson commented Dec 6, 2016

I have not seen people confused by trailing commas aside from a trivial "oh, you can do that?" realization. There are a lot of things that are hard about learning Rust, but this doesn't seem like one of them.

Besides, every long-term Rust user spends a small time as a beginner and a long time as an experienced user, so I give a lot more weight to making editing pleasant than to avoiding minor surprises for beginners. In terms of the "language strangeness budget", the trailing comma is an extremely cheap thing to add, with real benefits.

Given the already common use of trailing commas in the community, I think they should be the default style.

@Keats
Copy link

Keats commented Dec 6, 2016

Go forces you to put a trailing command for maps before newlines:

	commits := map[string]int{
		"rsc": 3711,
		"r":   2138,
		"gri": 1908,
		"adg": 912 // <- syntax error: unexpected semicolon or newline, expecting comma or } 
	}

Overall I agree with @nrc preferences and it matches the codestyle I've used in various codebases in Python.

@scottmcm
Copy link
Member

scottmcm commented Dec 7, 2016

FWIW, I find the Vertical setting to be more consistent.

For example, if you have this you have terminators on all the lines:

let mut x = MyStruct::default();
x.foo = 4;
x.bar = 13;

So I like having all the terminators in

let s = MyStruct {
    foo: 4,
    bar: 13,
};

Though MyStruct { foo: 4, bar: 13, } does look weird to me.

The VCS argument is compelling to me as well. I don't see getting away from line-based tooling any time soon, and especially for bug-fixes I prefer as small a diff as possible, even when it's something simple like commas.

@perlun
Copy link
Author

perlun commented Dec 9, 2016

@solson:

Besides, every long-term Rust user spends a small time as a beginner and a long time as an experienced user, so I give a lot more weight to making editing pleasant than to avoiding minor surprises for beginners.

Fair enough. It's just that not all of us find it pleasant to edit code where redundant, arbitrary characters have been added. 😜 In fact, some of us even find it unpleasant to look at and edit. This is really a matter of personal preference.

If we really really feel the comma should be there, I think using the approach mentioned by @Keats as used by Go, enforcing it in the compiler would be the way to go. I mean: make it a compile-time error if it is missing. I would find it annoying for sure, but at least it would be very consistent and stringent. This would no longer be a "style argument" but rather a "grammar argument" for the basic syntax of the language.

@solson also wrote, much earlier:

Maybe it helps to think of it like this: when lists are formatted in one line, comma acts as a separator, but when lists are formatted across lines, commas acts as a terminator.

My main problem with this argument is that commas are really not terminators. Not in Rust, nor in C, C++, C# or any other language I can think of. Not in English either. You wouldn't write: "apples, pears, bananas,", you would write either "apples, pears and bananas" or "apples, pears, and bananas" (with or without "Oxford comma") 😉 But the point is: comma is not commonly perceived as a terminator. (and yes, I know that the significant difference here is the "one field per line" use case)


Maybe that's the key problem here, that we (the Rust language) sometimes use a comma when a semicolon would be more appropriate? I wouldn't fight this, it looks reasonably sane:

let s = MyStruct {
    foo: 4; // Semicolons as terminator for each individual field value.
    bar: 13;
};

I would also accept this:

let s = MyStruct {
    foo: 4 // No trailing separator.
    bar: 13
};

...but this would have the downside in that it would make whitespace be the separator, since you must be able to specify more complex field values as well as these simple cases. And we also have the other cases, like enums and arrays (where semicolon would work but be different from pretty much any other language on the face of the earth) and match arms (where semicolons would probably not be very trivial to support).

So, anyway, what I will simply have to do is to invent my own language on top of Rust, which transpiles semicolons into the "trailing comma on each line" syntax and everybody could be happy. :trollface: 😆

@joshtriplett
Copy link
Member

@perlun Worth noting that English doesn't treat semicolons as terminators either; that convention comes from programming languages as well. (Some programming languages did use semicolons as separators rather than terminators; it worked very badly.) it's just a matter of what you're used to from programming languages.

@solson
Copy link
Member

solson commented Dec 9, 2016

Fair enough. It's just that not all of us find it pleasant to edit code where redundant, arbitrary characters have been added. 😜 In fact, some of us even find it unpleasant to look at and edit. This is really a matter of personal preference.

I can't quite agree with this. The trailing comma isn't redundant or arbitrary. It's a very intentional method for making editing operations easier and diffs cleaner. Whether you like to look at it or not is personal preference, but its utility is more objective than that, given that we have so many dumb line-based tools that aren't going away any time soon.

@Ixrec
Copy link

Ixrec commented Dec 9, 2016

For some of us the trailing comma is also more pleasant to edit. In most programming languages I often find myself editing the end of a list of items which does not have a trailing comma, and inevitably I'll forget to move the comma at some point and get a syntax error. I've also experienced at least a dozen git rebases where the only conflict was two commits that both appended a new item to the same list, and which would not have conflicted at all were trailing commas in use.

Since this is a hard syntax error that takes a few seconds to fix rather than a subtle runtime error like use-after-free, I don't have any strong opinion on whether it should be the default or official style, but in my subjective anecdotal experience, trailing commas results in less of my time being wasted, so I would object to removing the feature.

@perlun
Copy link
Author

perlun commented Dec 9, 2016

@joshtriplett

Worth noting that English doesn't treat semicolons as terminators either; that convention comes from programming languages as well.

That, of course, is true.


@solson

I can't quite agree with this. The trailing comma isn't redundant or arbitrary. It's a very intentional method for making editing operations easier and diffs cleaner. Whether you like to look at it or not is personal preference, but its utility is more objective than that, given that we have so many dumb line-based tools that aren't going away any time soon.

Well, it is in fact redundant, since the compiler doesn't need it to produce a valid binary. It's also not needed for people who are used to working with code formatted this way; we simply find it to be noise.

Arbitrary: OK, I can agree that it's not. That I can give you, it's a syntacically useful thing added with a clear purpose, that a number of people feel is very valuable.

@Ixrec

I've also experienced at least a dozen git rebases where the only conflict was two commits that both appended a new item to the same list, and which would not have conflicted at all were trailing commas in use.

This is a good argument to support the trailing comma. Thanks for that.

@jplatte
Copy link

jplatte commented Dec 9, 2016

It's also not needed for people who are used to working with code formatted this way; we simply find it to be noise.

I am very much used to no trailing commas, as I can't have them in C++ AFAIK. But I was very happy to see Rust supporting the trailing comma style so the last line in multiline lists is consistent with the preceding ones. Also, it might be worth noting that basically everyone already adds redundant separators in the form of semicolons in CSS.

And one final note, about the look of trailing separators: I personally actually dislike the look of the C / C++ / Json style multiline lists, because the last line is always inconsistent with the other ones. I always go for YAML when it comes to loading human-written data, because having to think about whether or not to put a comma after a list entry, even though it's such a minor thing, annoys me every time.

@perlun
Copy link
Author

perlun commented Dec 10, 2016

I am very much used to no trailing commas, as I can't have them in C++ AFAIK.

I have heard others say the same, and I think the answer is "it depends". This is fully valid code:

enum some_enum {
    Foo,
    Bar,
    Baz,
};

int some_array[] = {
    1,
    2,
    3,
};

...whereas this is not (as @ubsan pointed out early in this thread):

int main() {
    printf("%d %d %d", 1, 2, 3,); // Gives the error "foo.cpp:14:32: error: expected expression" with my Apple LLVM version 8.0.0 (clang-800.0.42.1)
}

And one final note, about the look of trailing separators: I personally actually dislike the look of the C / C++ / Json style multiline lists, because the last line is always inconsistent with the other ones. I always go for YAML when it comes to loading human-written data, because having to think about whether or not to put a comma after a list entry, even though it's such a minor thing, annoys me every time.

That's fine, and I don't claim that everyone using "non-trailing comma" is happy about that; sometimes they are limited by the languages they are using, but would prefer to not be. Sorry if I made it sound like that.


However, my point that there are certainly those of us who like to not add the trailing comma, who feels that the "editing argument" and the "VCS argument" is not compelling enough. We like the "traditional" way, with no extra comma at the end, and feel that the perceived "extra work" it causes isn't really a problem for us.

I think it all boils down to personal preference at the end, just like Linus Torvalds once said it:

Coding style is very personal, and I won't force my views on anybody

(quoting from linux/CodingStyle)

@est31
Copy link
Member

est31 commented Dec 10, 2016

@perlun this is only a discussion about what the default should be, rustfmt will (most likely) be customizable. So you can set it to some value you like. I too don't like some of rustfmt's choices, but I know that I can work towards adding an option in rustfmt for them.

@killercup
Copy link
Member

killercup commented Dec 10, 2016

I have not read all comments made here, but I think one pro-trailing-comma point is missing:

You can more easily copy-paste/comment out lines of code.

Maybe I write code in a weird way, but copying/commenting out struct fields, where clauses, match arms, etc. is something I often do when fixing/refactoring stuff. (I also use trailing commas in other languages and so far it has never made my code worse to work with.)

@nrc
Copy link
Member

nrc commented Dec 12, 2016

If we really really feel the comma should be there, I think using the approach mentioned by @Keats as used by Go, enforcing it in the compiler would be the way to go. I mean: make it a compile-time error if it is missing. I would find it annoying for sure, but at least it would be very consistent and stringent. This would no longer be a "style argument" but rather a "grammar argument" for the basic syntax of the language.

This approach has been somewhat rejected in Rust - generally, we try to be fairly flexible in the compiler (except where it matters, of course) and allow the user to opt-in to stricter checks using external tools such as lints or rustfmt.

@perlun
Copy link
Author

perlun commented Dec 15, 2016

@nrc

This approach has been somewhat rejected in Rust - generally, we try to be fairly flexible in the compiler (except where it matters, of course) and allow the user to opt-in to stricter checks using external tools such as lints or rustfmt.

That's nice, I like that approach. Being dogmatic with things that are really subjective (like this specific thing) can have adverse effects, like them choosing some other tool, language or platform instead. I think one reason for the success of C and C++ has been that they are quite relaxed in terms of style. (Then of course, the downside of that is that there is a plethora of various indentation styles etc over there...)

Java and C# has the advantage of a reasonable de-facto standard for style choices, where many (most) people follow the suggested style.


Back to this specific issue: should the Rust Style Guide and/or rustfmt recommend/enforce a particular choice (like "enforce trailing comma in vertical cases" which seems to be a fairly common choice among people commenting on this issue)? Or should it be one of the cases where the guide & tool is agnostic? (Please motivate either position.)

@scottmcm
Copy link
Member

@perlun It should, so that people don't have to care what they type, and refactoring/codegen tools can output whatever, trusting that they'll get normalized.

@nrc
Copy link
Member

nrc commented Jan 10, 2017

There's been no new discussion here for a while, so I'm putting this into FCP. I propose that we adopt the guiding principle of using a trailing comma if it is followed by a newline (see, e.g, #42 (comment) and #42 (comment)). I feel like this should be formally handled on a case by case basis, so there is no need for a PR here. But we could add this to the guiding principles in README.md, if someone wanted to do that.

@craftytrickster
Copy link

Is this issue still open for debate? I really believe that requiring a trailing comma by default is the wrong decision. I do not mind opening up an issue in a different location, etc if desired/required.

A comma in a list implies that an item will be followed by said comma. For example, if one views [1,2, .... after seeing the comma, it is natural for them to assume that content will come afterwards.

It seems incorrect to require an erroneous (or in the very least, completely unnecessary) trailing comma in these situations to be part of the formatting rules as default. I understand that some people do have a preference of adding the unnecessary comma and I respect their preference, but I strongly believe that it is a mistake to force this upon everyone. Essentially, a trailing comma to an indicator that maybe perhaps some additional content could theoretically appear here in the future, but it is not guaranteed. Given how loose that guarantee is, why would it be considered the default? It just seems to add syntactic noise for no benefit.

@joshtriplett
Copy link
Member

joshtriplett commented Aug 28, 2017

@craftytrickster In a certain sense, any issue is always theoretically open for new arguments, but this issue isn't open to further debate on the existing arguments that have already been considered. If you have a new point that hasn't previously been raised, please by all means raise it.

This is something where we understand that some people's mental parsers will (hopefully only temporarily) stumble on the trailing comma, but we believe that's something that people will get used to, in exchange for the various benefits already discussed elsewhere. It's common in a variety of languages, and we intend that Rust be one of them.

That said, I also believe it would be worthwhile for us to explicitly explain this, and the reasons for it, in Rust documentation somewhere, rather than assuming that people will simply pick it up by osmosis.

@craftytrickster
Copy link

@joshtriplett thank you for the explanation. I think that is a fair process, regardless of my opinion of the outcome.

@perlun
Copy link
Author

perlun commented Sep 12, 2017

@joshtriplett Would you or anyone else have a chance to add a list of languages/linters where the trailing comma is recommended by default? It would be valuable to me and this discussion. I think some of it has been mentioned already, but it would be super if someone could make a bullet list of it.

@joshtriplett
Copy link
Member

@perlun C, in many contexts. Python, in PEP 8.

@chriskrycho
Copy link

Also a fair number of modern JS styles are defaulting that way. It's one of the common configuration options in ESLint, TSLint, and Prettier.

@Mange
Copy link

Mange commented Sep 12, 2017

Go requires a trailing comma in some cases. Yes, the compiler regards it as a syntax error otherwise, that's how hard they enforce this style.

@perlun
Copy link
Author

perlun commented Sep 14, 2017

So the bullet list of other languages/environments advocating a trailing comma is for now:

  • C, in many context (which ones? Sorry to be annoying, but as many "hard facts" as possible here is best)
  • Python, in PEP 8
  • "A fair number of modern JS styles" (again, hard facts would be useful here - ESLint actually goes the other way, having it disabled by default. Please provide links to JS styles advocating its usage, for completeness. TSLint has a setting for it, but it's not enforced by the default as is suggested in this thread. It's enabled in the tslint:recommended ruleset: https://github.com/palantir/tslint/blob/master/src/configs/recommended.ts#L156-L161)
  • Go enforces it for certain scenarios.

Yes, the compiler regards it as a syntax error otherwise, that's how hard they enforce this style.

I don't think that's as crazy at it seems. If we really think that advocating this should be the default (which I'm already stated that I'm against, for reasons already discussed and which closely resembles those shared by @craftytrickster), making the compiler yell about it is at least extremely consistent. You cannot even compile lint-erroneous code in that case, which is one perfectly valid standpoint - it will at least make everyone abide by the linting rules. 😉

(I am not saying that I advocate making this a compiler error, I am just saying it is at least very consistent. If we would start incorporating rustfmt rules into the compiler, I think such things should be a --strict flag or something that can be turned on and off, ideally per linting rule. But it also adds significant complexity to the compiler I guess, so it might not be the best idea to implement/suggest at the moment.)

nrc added a commit that referenced this issue Nov 15, 2017
Includes discussion of trailing commas which closes #42
@nrc nrc added has-PR and removed ready-for-PR labels Nov 20, 2017
@nrc nrc closed this as completed in #101 Dec 8, 2017
@simonwep
Copy link

simonwep commented Feb 4, 2020

I'm a bit confused, if , should act as separator then I would expect after each , another values as this is the meaning of a separator. I I'd write (a, b, ) then technically a value after b, is missing since the comma after it doesn't separate anything.

Compared with ; which is commonly used to terminate statements:

let a = 100; 
let b = 200;

whereby , is used to separate stuff, e.g. it should be put between expressions:

let tupel = (a, b, c, d)

The comma is, in linguistics also used as separator and regarding programming languages it's described as separator:

The use of the comma token as an operator is distinct from its use in function calls and definitions, variable declarations, enum declarations, and similar constructs, where it acts as a separator.

IMO The semicolon acts most of the time as statement-terminator, but could be a separator (as seen in loops) but the semicolon had always the meaning of separating values where trailing commas wouldn't be allowed and wont make any sense since they won't separate anymore.

@joshtriplett
Copy link
Member

@simonwep This isn't really the right place for further help on formatting/style questions; if you'd like to ask questions about Rust style, users.rust-lang.org is probably a better place.

I would suggest not thinking of , as strictly a separator, or alternatively, thinking of it as separating a previous expression from a possible subsequent expression.

Please see the history of this thread and the resulting documentation for the various benefits of using trailing commas, and the tradeoffs we considered.

@sweihub
Copy link

sweihub commented Nov 26, 2022

Hi, I am looking for a solution to remove the trailing comma of function arguments? It bothers me much, can anyone help? I don't find any related settings.

fn on_rsp_user_login(
    &mut self,
    pRspUserLogin: *mut CThostFtdcRspUserLoginField,
    pRspInfo: *mut CThostFtdcRspInfoField,
    nRequestID: ::std::os::raw::c_int,
    bIsLast: bool,     <==== HERE, I DON'T WHAT THIS COMMA!
) {
    debug!("user login");
    self.tx.send(Event::UserLogin).unwrap();
}

@perlun
Copy link
Author

perlun commented Nov 27, 2022

@sweihub If I interpret databendlabs/databend#613 correctly, this was the behavior of some previous rustfmt version. The current version does not use a trailing comma in such cases. (caveat: not currently using Rust myself, others can verify or falsify this accordingly)

@bjorn3
Copy link
Member

bjorn3 commented Nov 27, 2022

Current rustfmt still uses the trailing comma. Databend uses an unstable rustfmt option to disable trailing commas.

@sweihub
Copy link

sweihub commented Nov 29, 2022

Yes, I am using the nightly build of rustfmt, the trailing comma is a phantom there! Is this anti-human comma as-design, or a bug? I would like to be self-aid to change the rustfmt code, but it'll never be accepted to RFC.

@sweihub
Copy link

sweihub commented Nov 29, 2022

Current rustfmt still uses the trailing comma. Databend uses an unstable rustfmt option to disable trailing commas.

@bjorn3 Would you share with me the magic-hidden-unstable option? I need a workaroud, thanks!

@Mange
Copy link

Mange commented Nov 29, 2022

Is this anti-human comma as-design, or a bug?

Considering it takes significant effort to make parsers able to work with it, and it being almost only modern languages that have added it, I don't think it's anti-human at all. It's more like anti-compiler and pro-human.

I don't think you need this kind of aggressive take on it. It is definitely intentional, but your preference is also valid.

@bjorn3
Copy link
Member

bjorn3 commented Nov 29, 2022

This one I think: https://github.com/datafuselabs/databend/blob/712782d5c39ea3fe27bd0ef0389067ff03354483/rustfmt.toml#L7 Note that trying to format using stable rustfmt will ignore unstable settings like this.

@perlun
Copy link
Author

perlun commented Dec 7, 2022

Is this anti-human comma as-design, or a bug?

Considering it takes significant effort to make parsers able to work with it, and it being almost only modern languages that have added it, I don't think it's anti-human at all. It's more like anti-compiler and pro-human.

Not trying to re-open this age-old flamewar again @Mange, but I think there's a big difference here between:

  • accepting a trailing comma in e.g. method calls (in addition to other places like arrays where other languages also tend to be liberal), and
  • advocating the use of a trailing comma in such scenarios, by e.g. making it the rustfmt default (without even a chance to turn it off, it seems - at least not yet)

The former can quite clearly be considered a "feature", since it can make certain forms of code-generation easier (you don't have to care about whether you are adding the last parameter or not, but can easily just append value + "," regardless of parameter position.

The latter, OTOH, is an incredibly opinionated thing. Some people like it (as evidenced by the discussions in this issue 😅) and others like me see it more as anti-feature. But who knows? Perhaps you can get used to this weird thing, just like you can get used to many other things which seem weird at first. 🙂

@Mange
Copy link

Mange commented Dec 7, 2022

Not trying to re-open this age-old flamewar again

Me neither, so this will be my last reply. Just to contextualize my statement a bit more, since it was apparently not clear enough. 🙂

The former can quite clearly be considered a "feature", […].

The latter, OTOH, is an incredibly opinionated thing. Some people like it […] and others like me see it more as anti-feature.

That just illustrates what I was trying to say. I was trying to say that effort was put in to make it appear, so it's not a bug. It's intentional. Because some people (no comment on how big this set is of the whole) like it, in order to make them happy.

It's not "anti-human", and using that kind of language about something some people spent time and energy trying to implement is a bit disrespectful, IMO.
I would wager it would be much more constructive with some respect on both sides, and to not complain too aggressively about something that other spend their free time working on.

I think it's a bit like someone throwing a party and then a party-goer complains about the food being "anti-human" because they don't like olives on their pizza. Someone cooked that pizza for humans and gave it away for free as a nice gesture.

All-in-all, this is just bike-shedding and I think everyone must accept that a language will always have made some decisions that oneself find distasteful. One just have to consider if they:

  • Want to help out and provide more options (give away their own free pizza in the party).
  • Should accept that it's probably not the biggest of deals and focus on the other things (like enjoying the other food or the company at the party - it's still a nice party, I hope, even without pizza without olives).
  • Respectfully ask for changes, in a way that just tries to reinforce that more people would be happy with other alternatives, but still accepting that no one is obligated to do it just because you asked.

Anyway, that's my last take on the matter. I sincerely hope I didn't kickstart even more meta-discussion.

@perlun
Copy link
Author

perlun commented Dec 13, 2022

Me neither, so this will be my last reply. Just to contextualize my statement a bit more, since it was apparently not clear enough. 🙂

👍 I seem to fall for the trap of replying once more... 😂

That just illustrates what I was trying to say. I was trying to say that effort was put in to make it appear, so it's not a bug. It's intentional. Because some people (no comment on how big this set is of the whole) like it, in order to make them happy.

It's not "anti-human", and using that kind of language about something some people spent time and energy trying to implement is a bit disrespectful, IMO.

I agree. But I think the Internet & the whole western society at large is getting far too sensitive in terms of how people express themselves (sorry to bring in yet another meta-discussion into the picture. 😁). Why did the person put it that way? It was probably because the issue at hand (enforcing trailing commas) is something which affects people's emotions => emotional responses is what you get.

Instead of focusing on "what on earth are you saying??!?", I think it's often more fruitful to understand why this is happening.

I think it's a bit like someone throwing a party and then a party-goer complains about the food being "anti-human" because they don't like olives on their pizza. Someone cooked that pizza for humans and gave it away for free as a nice gesture.

This analogy is a bit incomplete, IMO. More something like this: someone is throwing a party and says that if you are to attend this party you must prefer olives on their pizza. If not, you are still welcome to attend the party, but you won't get any pizza.

That more resembles the kind of attitude I think I've experienced in this issue. "Rustfmt should not be configurable" was one of the ideas advocated for earlier. In other words: olives on the pizza, whether you like it or not. "How can you be so narrow-minded to not want to have olives on your pizza!"

I know I am taking it a bit to the extreme here, but the point is that I think the community went too far in this case. The fact that rustfmt seems configurable these days, and the trailing comma is (almost) possible to disable seems like the community is, slowly but steadily, maturing. I like that; I like what I see. 👍 Perhaps Rust will one day be a language for the masses and not just a little tool for the right-thinking elite. :trollface: 😎

  • Respectfully ask for changes, in a way that just tries to reinforce that more people would be happy with other alternatives, but still accepting that no one is obligated to do it just because you asked.

That's what I tried to do about 6 years ago. 🙂 As if whether I did it respectfully or not, I'll leave to others to judge.

Anyway, that's my last take on the matter. I sincerely hope I didn't kickstart even more meta-discussion.

Sorry... 😇😂

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

Successfully merging a pull request may close this issue.