Preserve more newlines #201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The argument was never used for anything else
This was referenced May 14, 2024
Closed
infinisil
force-pushed
the
preserve-more-newlines
branch
3 times, most recently
from
2ed49bb to
1b4093a
Compare
Previously, newlines after the last list/attrs/let-in item would not be
preserved. So e.g.
[
"foo"
]
would wrongly turn into
[
"foo"
]
Subsequently it would turn into
[ "foo" ]
This commit fixes it, such that the original is preserved in this case.
Whereas previously all the newlines were stripped already during
parsing, in this new commit all newlines are preserved, which both fixes
the above problem and simplifies the code.
This does introduce one odd edge case due to interaction with another
feature. That feature is that comments before a let-in's `in` part, get
moved below it. The following:
let
x = 10;
# X
in
x
Gets reformatted into
let
x = 10;
in
# X
x
Due to how the new code preserves newlines, those newly also get moved
further down. So whereas previously the following:
let
x = 10;
in
x
Would wrongly remove the newline, turning it into:
let
x = 10;
in
x
With this change it won't anymore, but it will move the newline down:
let
x = 10;
in
x
While this could be handled with a special case, I'm not sure if that's
worth it. It might be better to rethink this moving of comments further
down idea, I'm not sure if that's necessary anymore.
infinisil
force-pushed
the
preserve-more-newlines
branch
from
1b4093a to
7071b1a
Compare
This fixes the odd case introduced in the parent commit
infinisil
force-pushed
the
preserve-more-newlines
branch
from
7071b1a to
84b3b6c
Compare
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/formatting-team-meeting-2024-05-14/45433/1 |
|
I am not really a fan of the diff and think that the format specification should be changed instead. The rule of not preserving trailing empty lines should be easy to formalize and IMO is a good heuristic for desired behavior. |
|
@piegamesde The diff is a bit confusing, because all the extra lines it "adds" are ones that were already there before. E.g. the newline in this section was previously removed, but is now preserved. So while this causes the result to have more lines, the diff is actually smaller!
Reasons for having this:
|
This was referenced May 15, 2024
Sereja313
approved these changes
May 28, 2024
tomberek
approved these changes
May 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #188 in a better way, as an alternative to #193
Previously, newlines after the last list/attrs/let-in item would not be preserved. So e.g.
would wrongly turn into
Subsequently it would turn into
This PR fixes it, such that the original is preserved in this case.
Whereas previously all the newlines were stripped already during parsing, in this new commit all newlines are preserved, which both fixes the above problem and simplifies the code.