Ignore .gitignored files?

NixOS / nixfmt

The official (but not yet stable) formatter for Nix code

https://nixfmt.serokell.io

Mozilla Public License 2.0

711 stars 27 forks source link

Ignore .gitignored files? #151

Open x10an14 opened 4 months ago

x10an14 commented 4 months ago

Context

Currently testing out the new pkgs.nixfmt-rfc-style, and it's taking forever + erroring out since it attempts to format my .gitignored .direnv/ folder:

-> $ nix fmt
nixfmt: ./.direnv/flake-inputs/5gc4ncnkzcgma8fl1qabm8v9kj7w68gl-source/doc/doc-support: openTempFileWithDefaultPermissions: permission denied (Read-only file system)

Expected behaviour

Don't format .gitignored/otherwise explicitly ignored files/files in ignored subfolders of repository.
Maybe (additionally, which would solve my issue w/the error, but not the runtime) ignore read-only files.

Things to consider:

only support git ignores?
support others?
add a default list of elements to ignore (like .direnv/, result/)?

x10an14 commented 4 months ago

"Taking forever" ~> 20 seconds, just to not mislead what I meant by that technical hyperbole (but then again, how many expect a linter/formatter to take more than sub two seconds).

piegamesde commented 4 months ago

While nixfmt as of recently has the capability of recursing into subdirectories, usually this should be the job of the program invoking it. Therefore, you should probably take your feature request to the nix fmt developers in your case. For reference, treefmt has a configuration file where you can specify ignores.

Handling gitignore semantics is a highly non-trivial task, and should not be re-implemented for every language formatter.

infinisil commented 4 months ago

@piegamesde I don't agree with this. It's really not that hard to support this, because we can use a library like vcs-ignore, which will be good enough for most cases.

infinisil commented 4 months ago

A especially good argument for needing this is that it doesn't make sense for a formatter to accept directory arguments when it doesn't make any effort to filter those directories, because at that point you need to use another tool for filtering anyways and you'd pass files to the formatter instead.

And at that point, nixfmt is essentially not an end-user CLI anymore, which might be fine, but then we should also deprecate passing directories. This needs to be discussed further.

Low priority, but let's keep this open for now.

piegamesde commented 4 months ago

Yeah, adding directories has just been added in the last release, and honestly I'm uncertain about that either. There's already issues with symlink handling IIRC.

acid-bong commented 4 months ago

Btw, vanilla nixfmt behaves the same

infinisil commented 4 months ago

(discussed in the team meeting today)

With nix fmt this becomes less important
There's also treefmt
Generally no need to manually call nixfmt
Let treefmt or nix take care of this instead.
Decision: nixfmt should not be in charge of recursing into directories. That functionality should be removed before the first official release.
Short-term can be kept around

So let's not worry about implementing this and instead focus on nix fmt/treefmt integration. We can keep the open issue for now though

nixos-discourse commented 4 months ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-03-19/41845/1