-
Notifications
You must be signed in to change notification settings - Fork 88
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
feat: add treefmt_nix to builtins #189
base: main
Are you sure you want to change the base?
Conversation
Is it possible for you to use .with() for this? Some integration might detect the diagnostics source and this will break them with this approach. |
I'm excited about this! But the direction smells a little off to me: there's already a treefmt entrypoint with the config embedded, it's IMO, we should instead be adding a generic "run |
I've polished up my "nix flake fmt" builtin: #192. |
@Lassulus, do you think there's value in merging this PR up? Or does #192 look like a good alternative? I was sort of hoping we could remove treefmt-nix (revert numtide/treefmt-nix#243). |
command = "treefmt-nix", | ||
args = { "--allow-missing-formatter", "--stdin", "$FILENAME" }, | ||
to_stdin = true, | ||
-- treefmt-nix wrapper needs to be available |
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 comment has drifted from its code.
@Lassulus, now that #192 has landed, I propose that we close this PR. (Related: numtide/treefmt-nix#291) |
depends on numtide/treefmt-nix#243
not sure if there is a policy before creating a builtin. but I didn't want to break usage with the treefmt plugin. treefmt-nix allows configuring treefmt with nix and there is an optional wrapper that can be included in a devshell. this builtin is expected to be used together with devenv and the treefmt-nix wrapper inside the devshell.
@Mic92 maybe has some feedback on this approach :)