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

Reopen no space for discussion #360

Open
jooooscha opened this issue Nov 6, 2022 · 4 comments
Open

Reopen no space for discussion #360

jooooscha opened this issue Nov 6, 2022 · 4 comments

Comments

@jooooscha
Copy link

I would like to reopen the "space"/"no space" discussion for this formatter.
There is already #181, which was closed with a link to #108 with the following argument:

After counting the pros and cons, going without spaces seemed the rational decision

However, the mentioned issue does not contain any discussion, but rather comments by kamadorueda alone stating that this would be the best choice. A comment by pinpox was just ignored.

There is also nix-community/nixpkgs-fmt#280, where kamadorueda tried to force "no space" in nixpkgs-fmt, which was not merged.
A similer issue is nix-community/nixpkgs-fmt#264, but this one only talks about empty lists.

It seems to me that the community does not have one clearly preferred style. Therefore, I would reopen this discussion for this formatter.

From my observations, it seems that most people can agree on having no space for empty lists, like [] {}, but having one for lists that contain at least one item, like [ "abc" ] { a = 1; }.
However, currently, even for non-empty lists, spaces are removed.

Therefore, to start the discussion, I would suggest changing the behavior to adding a space in non-empty lists.
In my personal experience, this increases readability and maintainability of lists.

@rummik
Copy link

rummik commented Dec 5, 2022

To be honest, it might be that most of the community uses spaces in non-empty lists, but most of the community doesn't currently use a formatter, so feedback just hasn't appeared

@rummik
Copy link

rummik commented Dec 5, 2022

Additionally, #108 only had discussion about empty containers, not containers with at least one item

@rummik
Copy link

rummik commented Dec 5, 2022

It looks like the changes for attrsets with one or more attributes wasn't added in #113, but was added in #140

@rummik
Copy link

rummik commented Dec 5, 2022

#140 was also where the changes to destructured arguments were made, which I believe should be part of this discussion

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

No branches or pull requests

2 participants