-
-
Notifications
You must be signed in to change notification settings - Fork 208
stylix: switch to using treefmt #1076
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Consider sorting the entries by moving formatter
between checks
and packages
at line 192.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalidating the formatting in /flake.nix
and running nix fmt
results in the following error, causing the file not to be formatted back:
$ nix fmt
warning: Git tree '<STYLIX>' is dirty
2025/03/31 18:59:07 INFO using config file: /nix/store/8p4mscr4wwmcgfdk9h5ghd7r9rlb4psx-treefmt.toml
Error: path . not inside the tree root /nix/store
On a side note, should we rename the PR to something more explicit, like:
stylix: support 'nix fmt'
Seems like |
|
flake.nix
Outdated
@@ -190,6 +190,13 @@ | |||
}; | |||
}; | |||
|
|||
formatter = pkgs.nixfmt-tree.withConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should either be pkgs.nixfmt-tree.override
or pkgs.treefmt.withConfig
.
The first one uses the "simple" nixfmt+treefmt wrapper and overrides a few minor settings.
The second one allows you to configure treefmt with a blank slate; this is what nixfmt-tree uses under the hood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh oops misread the pr description. Not sure if we want to introduce treefmt into this project because it's functionality overlaps with git-hooks.nix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some context, treefmt is needed because nixfmt
doesn't support being run against a directory; it can only be run against a file (or a list of files). That's why the nixfmt-tree wrapper exists; it provides a treefmt pre-configured to run nixfmt.
Not sure if we want to introduce treefmt into this project because its functionality overlaps with git-hooks.nix
Git hooks can just run treefmt; in nixvim we use git-hook.nix's pre-commit.hooks.treefmt
options.
its functionality overlaps with git-hooks.nix
Maybe you're thinking of treefmt-nix, which is a set of modules for configuring treefmt. Essentially a much fancier version of pkgs.treefmt.withConfig
. That does have some overlap regarding checks
, but can be configured as needed.
d1c8e77
to
204f61a
Compare
allows for using
nix fmt
on the repoThings done
Notify maintainers