Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Confusing attset merging #310

Open
layus opened this issue Jun 20, 2023 · 0 comments
Open

Confusing attset merging #310

layus opened this issue Jun 20, 2023 · 0 comments
Labels
formatting Issue with the output format needs triage

Comments

@layus
Copy link
Contributor

layus commented Jun 20, 2023

Input

foo test {
a = 1;
} // {
b = 2;
c = 3;
}

Output

foo test
  {
    a = 1;
  } // {
  b = 2;
  c = 3;
}

Desired output

I think the following output makes more sense, because it matches the implicit precedence of the merge operator.

foo test {
  a = 1;
} // {
  b = 2;
  c = 3;
}

When parentheses are added according to precedence, nixpkgs-fmt gives a result similar to the expected output above.

(foo test {
  a = 1;
}) // {
  b = 2;
  c = 3;
}

Hence, I think the current output is confusing readers into the wrong precedence for //, and may trick them into parsing the expression incorrectly.


Now, I understand this issue is a bit tricky. In particular, when parentheses are added to perform the merge first (force higher precedence than default), nixpkgs-fmt gives a result quite similar to the one above. So parenthesizing the expression about anywhere changes the output. And I do not have a general rule to apply here yet, just this snippet.

foo test ({
  a = 1;
} // {
  b = 2;
  c = 3;
})

PS: The online formatter with "submit code sample" is so nice to use !

@layus layus added formatting Issue with the output format needs triage labels Jun 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
formatting Issue with the output format needs triage
Projects
None yet
Development

No branches or pull requests

1 participant