Conversation
infinisil
left a comment
infinisil
left a comment
There was a problem hiding this comment.
Neat, I didn't know about git mergetool! While this way of merging isn't fool-proof against syntactic errors, it is quicker than the auto-rebase script, and there's no problem with having both, so I say let's have this. Will need some work to integrate into the build, but we can do this in follow-up PRs.
For reference, here's an example of a syntactic error that can be introduced. Consider this file:
{
foo
# , bar
}:
nullFormatting this on one branch turns it into
{
foo,
# , bar
}:
nullBut if on another branch the extra argument gets uncommented:
{
foo
, bar
}:
nullYou'll end up with this invalid merge result (no merge conflict):
{
foo,
, bar
}:
nullThis is an easily catchable syntactic error though. I can't imagine such a failure changing semantics, but would love to be proven wrong.
| # Copyright (c) 2025 Jan Malakhovski <[email protected]> | ||
| # | ||
| # This file can be redistributed under the terms of Unlicense | ||
| # <https://unlicense.org> license. |
There was a problem hiding this comment.
The only worry I have is the re-introduction of separately licensed files, which we moved away from in the past: #171. Would you be okay with you to re-license it under the MPL-2.0 license and assign copyright to the generic "Nixfmt contributors" as defined in
Lines 3 to 6 in e825e95
| # Copyright (c) 2025 Jan Malakhovski <[email protected]> | |
| # | |
| # This file can be redistributed under the terms of Unlicense | |
| # <https://unlicense.org> license. |
Inspired by oxij's #277, but implemented in Haskell, therefore faster (less process spawning) and trivially integrated into the build and distribution.
|
Because it's a bit tedious to distribute a shell script along with the build, I made a PR to implement this directly in the Haskell codebase: #283. This also benefits from improved speed, because three less processes need to be spawned. |
|
On license: yes, feel free to re-license under whatever, I selected unlicense exactly for the reason it allows doing that.
On builtin `--mergetool`: nice!
|
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
Also just opened numtide/treefmt#523, because this functionality would be useful for all formatters :) |
Makes resolving formatting-related conflicts easier. Idea and docs are from @oxij's #277. Code and tests are from @infinisil. By implementing it in Haskell, it's faster (less process spawning) and trivially integrated into build and distribution. Co-Authored-By: Jan Malakhovski <[email protected]>
Makes resolving formatting-related conflicts easier. Idea and docs are from @oxij's #277. Code and tests are from @infinisil. By implementing it in Haskell, it's faster (less process spawning) and trivially integrated into build and distribution. Co-Authored-By: Jan Malakhovski <[email protected]>
|
Done in #283 |
Makes resolving formatting-related conflicts easier. Idea and docs are from @oxij's NixOS#277. Code and tests are from @infinisil. By implementing it in Haskell, it's faster (less process spawning) and trivially integrated into build and distribution. Co-Authored-By: Jan Malakhovski <[email protected]>
3 participants
The script is self-documenting.
IMHO, this script should also be added to the output of
nixfmt-rfc-styleof Nixpkgs. Otherwise, after recent treewide changes, things became impossible to merge manually.