csharpier
csharpier copied to clipboard
Enforce trailing commas in object and collection initializer
Take the following code:
var foos = new[]
{
new Foo(),
new Bar()
};
If later a new item is added:
var foos = new[]
{
new Foo(),
new Bar(),
new Baz()
};
This causes the following issues:
- A misleading commit diff of
+2/-1
even though there isn't really any code deletion. - Polluting git blame.
These issues also occur when deleting/reordering the last item in initializers.
These issues can be fixed by enforcing trailing commas. Relevant Prettier option: https://prettier.io/docs/en/options.html#trailing-commas
I had considered this early on - #23. I think it was closed because we decided to not have csharpier changing anything besides whitespace. But there are good arguments for transforming code in some cases, and there are good arguments for forcing tailing commas. The discussions I found around making trailing commas the default in prettier - https://github.com/prettier/prettier/issues/9369 and https://github.com/prettier/prettier/issues/68
I think I am in favor of adding this. If it were to follow the prettier way, it would only include the trailing comma when the initializer breaks.
Conversely, it would also be nice if CSharpier removed the trailing comma when the object initializer fits on single line.
I think it's a common style guide to not have the final comma on a single line, and include it on multiple lines, i.e. this format:
Dictionary<string, string> headers;
// No trailing comma after If-Match
headers = new() { { "OData-Version", "4.0" }, { "If-Match", "*" } };
// Trailing comma after If-Match
headers = new()
{
{ "OData-Version", "4.0" },
{ "Accept", "application/json;odata.metadata=minimal" },
{ "If-Match", "*" },
};
I think this makes a strong argument for CSharpier adding/removing that comma automatically:
- Depending on the length of initializers, CSharpier will reflow on single or multiple lines.
- Currently if you want to adhere to the style above, you need to manually add/remove that comma based on CSharpier reflow, so after CSharpier runs.
- This is not doable if you run CSharpier automatically, e.g. as part of a git commit hook.
- Even if you Format on Save, it's easy to miss.
Also csharpier could act as Black (popular python code formatter) about trailing commas. It forces multiline output if sees trailing comma. So this code in python
def foo(a: int, b: str,) -> None:
will be formatted to
def foo(
a: int,
b: str,
) -> None
And vice versa this code
def foo(
a: int,
b: str
) -> None:
will be formatted to
def foo(a: int, b: str) -> None:
because it doesn't have trailing comma and statement fits to one line.
It is really useful behaviour and gives power to users to force multiline output when it improves readability.
@tkukushkin I'm personally against that. The biggest benefit of Prettier is to simply stop people from arguing about style and just get things done. Introducing choices especially to the degree of being able to vary place by place, will devolve it back into arguing about when should we break into multiple lines or not.
I understand your point and I can say from my experience, that I've never seen any arguments over someone splitting one statement into several lines when it fits into one. Nobody cares about such things. But this is my personal experience and maybe your experience is different.
I think Black was created with the same principle in mind, only few basic configuration options, it is uncompromising, opinionated. And this behaviour works for Black, then why wouldn't it work for csharpier?
I'm not sure that's a good reasoning, on the flip side "Prettier doesn't allow you the force a line split at all, why wouldn't it work for CSharpier?" It's one less thing to worry about: if the line was too long when I originally wrote it, formatter should split it and insert the trailing comma; if later a parameter or two was removed and the line is no longer too long to fit in one line, formatter should just change it back to one line and also remove that trailing comma in the process, rather than keeping it split. The fact that you can arrive at the same code in two different ways and end up with two different formatting, is imo not desirable.
I'm gonna side with @NonSpicyBurrito on this one.
I like that CSharpier (or Prettier) handles like length for me. Managing line breaks manually just feels wrong.
Consider: after I (globally) rename a type in my solution, some method declaration may have significantly changed length. I don't want to review each one manually to make them (un-)wrap based on the last comma. I want CSharpier to just reformat them appropriately.
Consider: after I (globally) rename a type in my solution, some method declaration may have significantly changed length. I don't want to review each one manually to make them (un-)wrap based on the last comma.
If new name is longer and some statement (method call for example) doesn't fit in one line anymore, it will be formatted as multiline, no changes to current behaviour here.
If new name is shorter and some statement is already multiline, has trailing comma and could fit in one line, it will not be unwrapped, yes. And it is not common case IMHO, I have never reviewed each change manually, because it is not so important. There is no problem for me that some statements remain multiline.
I think we can stop arguing about this, I understand your point and respect your decision.
It would be really nice to have this as a configuration option.