Trailing commas

[jtojnar]

Describe the bug

I would like to use trailing commas in attrset pattern matches in function arguments to be consistent with regular attrsets (which have trailing semicolons):

let
  fn =
    {
      foo,
      bar,
    }:
    foo;
in
fn {
  foo = 1;
  bar = 2;
}

Currently, the trailing commas are moved to the next row, looking really ugly:

let
  fn =
    { foo
    , bar
    ,
    }:
    foo;
in
fn {
  foo = 1;
  bar = 2;
}

Would also help with the dilemma where to place a comma with regards to the argument comments by @Mindavi and @asymmetric.

To Reproduce

1. Run `nixpkgs-fmt' on the first code block contents.

Expected behavior
The function argument would keep commas at the same line when a trailing comma for the final attr is detected.

System information

• nixpkgs-fmt --version nixpkgs-fmt 1.2.0

[zimbatm]

I agree with the reasoning but I think this ship has sailed already. Most of the nixpkgs default.nix use the comma-first notation.

[jtojnar]

I would not say it is too late. Many (most?) nixpkgs packages still use the multiple-attributes-per-line style so they will need to be reformatted anyway.

If we consider this reasonable, the upcoming nixpkgs reformatting would be a great opportunity.

[grahamc]

I prefer the style proposed here, and it would be a great time to do it. Another option is to remove the final ,, but I like the final , demonstrated in the first code sample.

[zimbatm]

I am open to introducing the change, but I don't think it should be holding back the upstream effort. nixpkgs-fmt 1.0 has been released on Aug 17, 2020, so it's not like the hasn't been plenty of time to discover this.

[DavHau]

I started using nixpkgs-fmt integration for my editor a few days ago, but this issue is the reason I will disable it again.
The way nixpkgs-fmt formats function arguments is inconsistent. (The first line starts with { and subsiding ones with ,).
This complicates my editing workflow for two reasons:

• it makes re-ordering arguments more complicated, one cannot just shift lines around without braking stuff.
• inserting a new argument in front of the first one is more complicated, as one cannot just duplicate the first line to then edit it.

There doesn't seem to be any option to disable this behavior. Or is there?

I agree with the reasoning but I think this ship has sailed already. Most of the nixpkgs default.nix use the comma-first notation.

Why has the ship sailed? There is no enforced formatting in nixpkgs AFAIK and right now there is still a lot of chaos. I see all kinds of different formatting styles in nixpkgs. It doesn't seem like anything will get worse by introducing this change.

I am open to introducing the change.

What needs to be done to make it happen?

[MagicRB]

I personally prefer the way it is now, so if anything happens I'd like it to be configurable. Of course not in nixpkgs but for other projects.

[DavHau]

I personally prefer the way it is now, so if anything happens I'd like it to be configurable. Of course not in nixpkgs but for other projects.

Could you explain why you prefer the current style over the one that is proposed by the current issue?
It has clearly been shown that the current style has disadvantages in a number of scenarios where the proposed style has not.
Could you make an example that demonstrates a disadvantage of the proposed style?

[piegamesde]

I may have some motivation to take a shot at this. Would somebody familiar with the code base please give me some directions for a head start?

[andir]

I like the current implementation. After initial confusion (why would anyone do that?!?) I came to prefer that style even when writing code myself. It just looks nicer to me and is more consistent (within itself). I see that it isn't consistent with attrsets but that isn't an issue for me personally.

[mohe2015]

I'm not so sure but I think the proposed style also has the advantage that merging different code (potentially with conflicts) is easier as you can just remove the conflict markers / remove one part in the case that the

{ foo # first line
, bar
,
}

is removed and another one is added or similar changes.

[zimbatm]

It's interesting, and not surprising to see people having different preferences. There is not really a way to solve that without issuing a vote to all NixOS contributors.

If we were to make this change, it should go into a 2.x release as it would be very backwards-incompatible.

[nrdxp]

I think the leading comma is fine, and it's a pattern I do in other languages as well. I'm not sure I would directly correlate this with regular attribute sets and ; since the latter is actually defining values, while the former is just defining argument names.

That said, I don't think everyone is going to agree one way or another on this. I would say, in this case, to go with what most packages in nixpkgs already do, which is the leading , from what I've seen so far.

[DavHau]

I think we should optimize the style for practicality, not personal preferences.
@MagicRB @andir @nrdxp , All your statement lack logical explanation of why your preferred style of having a leading comma is more practical.
If I may summarize, your argumentation goes like:

• I am used to it
• It just looks nicer
• Most of the people are doing it
• I have done it in other languages before

It just looks nicer to me and is more consistent (within itself).

Could you please explain what you mean, maybe with an example?
The issue description and my comment above clearly demonstrate inconsistencies of this style and explain some resulting complications.

In what way is it more consistent if the first element starts with a { and the second with ,? How does this make your editing workflow simpler?

There is not really a way to solve that without issuing a vote to all NixOS contributors.

I don't really see why this is a hard decision to make. Multiple points have been brought up, demonstrating practical downsides of the current style, but basically no arguments other than i like it better have been made for keeping the old style.

[nrdxp]

@DavHau, I'm not sure if I can provide a compelling objective argument honestly. To me, this is just one of those things that boils down to preference. I also think the difficulties you mentioned are dependant on the editor you use. I have been in the two situations you mentioned myself, but didn't really find the difficulty you described in kakoune.

All I can say is that having the comma on the starting line makes the deliniation trivially clear to my eyes.

Also, for your second point in particular, it seems this would be an issue with either style. The only real way to simplify that would be if we forced a newline after the first {, that is, if I understand the issue correctly.

[mohe2015]

Also, for your second point in particular, it seems this would be an issue with either style. The only real way to simplify that would be if we forced a newline after the first {, that is, if I understand the issue correctly.

As far as I can tell that's what the issue creation comment does.

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/the-uncompromising-nix-code-formatter/17385/26