Skip to content
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

Use null delimiters in xargs to support space in file names #95

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

pbsds
Copy link
Contributor

@pbsds pbsds commented Mar 15, 2025

Alternatively we could have xargs use newline instead of space as the delimiter, but fd will also ignore hidden files and respect .gitignore if it finds such further down.

@pbsds
Copy link
Contributor Author

pbsds commented Mar 15, 2025

It seems fd only wants to list directories

@pbsds pbsds changed the title Use fd to traverse for nixfmt Use newline as delimiter in xargs Mar 15, 2025
@JumpIn-Git
Copy link

It seems fd only wants to list directories

what do you mean? if you want to use fd to format, you should do
fd -t f -e nix -x nixfmt
this should follow the gitignore

@pbsds
Copy link
Contributor Author

pbsds commented Mar 15, 2025

These are the failing tests where I used fd '\.nix$ "$@" --type f - nixfmt':
https://buildbot.numtide.com/#/builders/519/builds/166

It doesn't like files in $@

@phaer
Copy link
Member

phaer commented Mar 16, 2025

Could you add a rationale for this change to the description, or ideally, the commit?

Is it to support file names with spaces? If so git ls-files -z for \0-terminated file names might be an even more robust alternative (see e.g. man page)

@JumpIn-Git
Copy link

JumpIn-Git commented Mar 16, 2025

These are the failing tests where I used fd '\.nix$ "$@" --type f - nixfmt': https://buildbot.numtide.com/#/builders/519/builds/166

It doesn't like files in $@

use fd -t f -e nix -x nixfmt -- "${@}"

@pbsds pbsds changed the title Use newline as delimiter in xargs Use null delimiters in xargs to support space in file names Mar 20, 2025
@pbsds
Copy link
Contributor Author

pbsds commented Mar 20, 2025

use fd -t f -e nix -x nixfmt -- "${@}"

That would run nixfmt on the files mentioned in "${@}" multiplied by the number of files that fd otherwise finds:

$ myfmt() { fd -t f -e nix -x echo nixfmt -- "${@}"; }
$ myfmt file-i-want-to-format.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/modules/home/home-shared.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/modules/nixos/host-shared.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/hosts/my-darwin/users/me/home-configuration.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/hosts/my-nixos/users/me/home-configuration.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/hosts/my-darwin/darwin-configuration.nix
nixfmt -- file-i-want-to-format.nix ./templates/toml-devenvs/devshells/bye.nix
nixfmt -- file-i-want-to-format.nix ./templates/toml-devenvs/checks/default-devshell.nix
nixfmt -- file-i-want-to-format.nix ./templates/toml-devenvs/checks/bye-devshell.nix
nixfmt -- file-i-want-to-format.nix ./templates/system-manager/flake.nix
nixfmt -- file-i-want-to-format.nix ./templates/system-manager/hosts/myhost/system-configuration.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/hosts/my-nixos/configuration.nix
nixfmt -- file-i-want-to-format.nix ./formatter.nix
nixfmt -- file-i-want-to-format.nix ./templates/nixos-and-darwin-shared-homes/flake.nix
nixfmt -- file-i-want-to-format.nix ./templates/toml-devenvs/flake.nix
nixfmt -- file-i-want-to-format.nix ./templates/default/devshell.nix
nixfmt -- file-i-want-to-format.nix ./lib/default.nix
nixfmt -- file-i-want-to-format.nix ./templates/toml-devenvs/packages/bye.nix
nixfmt -- file-i-want-to-format.nix ./templates/default/flake.nix
nixfmt -- file-i-want-to-format.nix ./packages/docs/default.nix
nixfmt -- file-i-want-to-format.nix ./flake.nix

Copy link
Member

@phaer phaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@phaer phaer merged commit 7ae8756 into numtide:main Mar 20, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants