treefmt: add configuration wrappers #390147
Conversation
github-actions
bot
added
10.rebuild-darwin: 0
There was a problem hiding this comment.
Interesting! I'll take a detailed look later, but my initial reaction is that I don't totally understand the motivation here. My thinking is that people can already get a nice, modular configuration of treefmt if they use treefmt-nix with flake-parts. Just thinking out loud, here are some reasons that this PR could be nice:
- As you mention in your OP, this could simplify the implementation nixfmt-tree package, which by virtue of living in nixpkgs, cannot depend on treefmt-nix.
- There are folks out there who might not be using flakes or flake-parts, and this would let them modularize their configuration.
- Wouldn't it be better to address this in treefmt-nix itself, though?
- Anything else?
|
Thanks for your initial thoughts!
treefmt-nix has a much larger scope than what is proposed in this PR. For example, it provides sane configuration for many formatting programs. On the other hand, this PR is focused solely on defining treefmt-nix is great at what it does, and I personally use it whenever I need a non-trivial treefmt config. Offering simple wrappers and structural settings feels within scope for nixpkgs and (imo) shouldn't necessarily require external dependencies like treefmt-nix or flake-parts. Perhaps at some point this could be done in such a way that treefmt-nix can also build on top of the impl here? Although I don't think that should be our first consideration. |
| { | ||
| freeformType = settingsFormat.type; | ||
| } | ||
| { | ||
| _file = "<treefmt2.buildConfig args>"; | ||
| imports = lib.toList module; | ||
| } |
There was a problem hiding this comment.
Can't those two be merged together?
There was a problem hiding this comment.
They could, but I think that'd be more confusing.
The first module is definitions by us (this file) while the second is wrapping user-supplied module(s) and setting a default file location.
| freeformType = settingsFormat.type; | ||
| } | ||
| { | ||
| _file = "<treefmt2.buildConfig args>"; |
There was a problem hiding this comment.
Best to use ./build-config.nix to provide the actual context.
There was a problem hiding this comment.
Normally I'd agree, but in this case we don't want to specify _file = ./build-config.nix, because we are wrapping the user-supplied modules. Kinda like how deferredModule wraps definitions with _file = "${def.file}, via option ${showOption loc}".
I don't think we have access to the actual location here,1 so the default _file only indicates the function name, to which the user's modules were passed.
I've added _file = ./build-config.nix to the other module, along with some clarifying comments and a typo fix (treefmt2 -> treefmt).
Footnotes
-
I tried playing with
builtins.unsafeGetAttrPos, but it always returnednull. I'm guessingmakeOverridableturningbuildConfiginto a functor is messing with attr locations? ↩
|
I like it; we should also port this into the package exposed by |
MattSturgeon
force-pushed
the
treefmt-config
branch
from
029d7db to
5619851
Compare
- `treefmt.withConfig` builds a treefmt package wrapped with a config - `treefmt.buildConfig` builds a treefmt.toml file using a freeform module
MattSturgeon
force-pushed
the
treefmt-config
branch
from
5619851 to
447f275
Compare
Would it make sense for treefmt's flake to inherit and/or override the impl directly from nixpkgs, instead of re-implementing or copy-pasting it? Something like: {
passthru = {
withConfig = pkgs.treefmt.withConfig.override {
treefmt = finalAttrs.finalPackage;
};
buildConfig = pkgs.treefmt.buildConfig.override {
treefmt = finalAttrs.finalPackage;
};
};
}I like the idea or re-using/sharing implementations wherever possible, to ease maintenance and keep related projects in sync with each other. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/formatting-team-meeting-2025-03-18/61868/1 |
Inherit the `withConfig` and `buildConfig` wrapper functions added in NixOS/nixpkgs#390147 If the current nixpkgs revision does not have these attrs, a sane error message is thrown when attempting to use them. The `treefmt` argument passed is overridden to refer to `perSystem.self.treefmt`.
Summary
This PR adds some functions to the treefmt package that allow users to produce a treefmt config file and/or a wrapped treefmt package from structured settings modules.
treefmt.buildConfigbuilds a treefmt.toml file using a freeform moduletreefmt.withConfigbuilds a treefmt package wrapped with a configAlso:
Motivation
It is inspired by treefmt-nix and the new nixfmt-tree wrapper package recently added by @infinisil.
Hopefully, the implementation here could be used by the new nixfmt-tree package, ideally simplifying its implementation. To keep this PR's scope minimal, I've not done that here.
Freeform options module
I also experimented locally with a
settings.nixmodule that defined some options & defaults for the treefmt.toml file. I've not included that in this PR to keep the scope minimal. It also sees somewhat pointless to add module options without a clear way to document them; currently the nixpkgs manual does use nixos-render-docs, but it is hard-coded to render only the top-level pkgsconfigoptions.Local
settings.nixcommitcommit f23edc22568005eae80e3a4ca411b4c4cb20af46 Author: Matt Sturgeon <matt@sturgeon.me.uk> Date: Wed Mar 5 03:12:47 2025 +0000 treefmt: add default settings module Based on the config scheme in treefmt-nix: https://github.com/numtide/treefmt-nix/blob/94ae23/module-options.nix diff --git a/pkgs/by-name/tr/treefmt/build-config.nix b/pkgs/by-name/tr/treefmt/build-config.nix index 6f8b672d8e54..8a37961f3233 100644 --- a/pkgs/by-name/tr/treefmt/build-config.nix +++ b/pkgs/by-name/tr/treefmt/build-config.nix @@ -7,6 +7,8 @@ let settingsFormat = formats.toml { }; configuration = lib.evalModules { modules = [ + # TODO: document the settings options in the nixpkgs manual + ./settings.nix { freeformType = settingsFormat.type; } diff --git a/pkgs/by-name/tr/treefmt/settings.nix b/pkgs/by-name/tr/treefmt/settings.nix new file mode 100644 index 000000000000..cb8b24f5903e --- /dev/null +++ b/pkgs/by-name/tr/treefmt/settings.nix @@ -0,0 +1,59 @@ +{ lib, config, ... }: +let + inherit (lib) types; + + # Coerce paths and packages into exe strings + toExeStr = p: if builtins.isPath p then "${p}" else lib.getExe p; + exeType = types.coercedTo (types.either types.package types.path) toExeStr types.str; + + # Type for `formatter.*` + formatterType = types.submodule { + inherit (config._module) freeformType; + options = { + command = lib.mkOption { + description = "Executable obeying the treefmt formatter spec"; + type = exeType; + }; + options = lib.mkOption { + description = "List of arguments to pass to the command"; + type = types.listOf types.str; + default = [ ]; + }; + includes = lib.mkOption { + description = "List of files to include for formatting. Supports globbing."; + type = types.listOf types.str; + }; + excludes = lib.mkOption { + description = "List of files to exclude for formatting. Supports globbing. Takes precedence over the includes."; + type = types.listOf types.str; + default = [ ]; + }; + }; + }; +in +{ + options = { + excludes = lib.mkOption { + description = "A global list of paths to exclude. Supports glob."; + type = types.listOf types.str; + default = [ ]; + example = [ "./node_modules/**" ]; + }; + on-unmatched = lib.mkOption { + description = "Log paths that did not match any formatters at the specified log level."; + type = types.enum [ + "debug" + "info" + "warn" + "error" + "fatal" + ]; + default = "warn"; + }; + formatter = lib.mkOption { + type = types.attrsOf formatterType; + default = { }; + description = "Set of formatters to use"; + }; + }; +}Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.