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

Avoid list absorbtion when appended by two more non-list-literal expressions #198

Closed
ShamrockLee opened this issue Apr 25, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@ShamrockLee
Copy link

ShamrockLee commented Apr 25, 2024

Description

"Absorption" is an "an alternative layout" that reduces indentation level, according to NixOS/rfcs#166. That is, the specification allows such "alternative" alongside the indent-after-newline right-hand-side style.

Given a chain of evaluated-to-list expressions chained by list concatenation operators (++) consisting of at least one of which is a multi-line literal list and two adjacent non-list-literal expressions, if the multi-line list literal gets absorbed, the adjacent non-list-literal terms will be squashed into the same line, producing a long, hard-to-read, and git-diff-unfriendly line of code.

Current implementation of nixfmt sometimes perform the above-mentioned absoption. In my opinion, such behavior should be avoided.

Small example input

{
  buildInputs = [
    "ee"
    "fff"
    "gggggggg"
    "hhh"
    "iiiii"
    "jjjjj"
    "kkkkkkkkkkk"
  ]
  ++ lib.optional true "lllll-lllll"
  ++ lib.optional false "mmmmmmmm"
  ;
}

Expected output

{
  buildInputs =
    [
      "ee"
      "fff"
      "gggggggg"
      "hhh"
      "iiiii"
      "jjjjj"
      "kkkkkkkkkkk"
    ]
    ++ lib.optional true "lllll-lllll"
    ++ lib.optional false "mmmmmmmm";
}

Actual output

{
  buildInputs = [
    "ee" # To patch /bin/sh shebangs.
    "fff"
    "gggggggg"
    "hhh"
    "iiiii"
    "jjjjj"
    "kkkkkkkkkkk" # Required at build time by SingularityCE
  ] ++ lib.optional true "lllll-lllll" ++ lib.optional false "mmmmmmmm";
}
@infinisil infinisil moved this to Todo in Nix formatting May 28, 2024
@dasJ dasJ added the bug Something isn't working label Jun 26, 2024
piegamesde added a commit to piegamesde/nixfmt that referenced this issue Dec 2, 2024
Fixes NixOS#198.

This is a hotfix that should exclude the most common ugly case people
might run into, if this comes up again we should more thoroughly rethink
the rules for this case.
@piegamesde
Copy link
Member

Hotfix in #269. The rule for this special case is that anything goes on the RHS of the ++/// as long as it fits onto the rest of the line. This probably should be reworked in a more structured and stricter way.

For now, I've simply excluded the RHS to be another operation in order to expand operation chains as desired. I think that for now this should be sufficient, I don't think there are many other scenarios where people might run into this.

@infinisil infinisil mentioned this issue Dec 4, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Nix formatting Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants