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

fix(nix fmt): remove the default "." argument #11438

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Sep 5, 2024

Motivation

Allow treefmt to function as expected.

Context

When nix fmt is called without an argument, Nix appends the "." argument before calling the formatter. The comment in the code is:

Format the current flake out of the box

This also happens when formatting sub-folders.

This means that the formatter is now unable to distinguish, as an interface, whether the "." argument is coming from the flake or the user's intent to format the current folder. This decision should be up to the formatter.

Treefmt, for example, will automatically look up the project's root and format all the files. This is the desired behaviour. But because the "." argument is passed, it cannot function as expected.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@zimbatm zimbatm requested a review from edolstra as a code owner September 5, 2024 08:46
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Sep 5, 2024
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

I think this is a good idea.

@roberth
Copy link
Member

roberth commented Sep 7, 2024

Could you add a brief release note, documentation and a test case?

@github-actions github-actions bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Sep 10, 2024
@zimbatm
Copy link
Member Author

zimbatm commented Sep 10, 2024

thanks, updated the tests and added a release note

When `nix fmt` is called without an argument, Nix appends the "." argument before calling the formatter. The comment in the code is:
> Format the current flake out of the box

This also happens when formatting sub-folders.

This means that the formatter is now unable to distinguish, as an interface, whether the "." argument is coming from the flake or the user's intent to format the current folder. This decision should be up to the formatter.

Treefmt, for example, will automatically look up the project's root and format all the files. This is the desired behaviour. But because the "." argument is passed, it cannot function as expected.
@tomberek
Copy link
Contributor

Was also discussed in Formatter team meeting.

@tomberek tomberek merged commit c60e1be into NixOS:master Sep 11, 2024
11 checks passed
@zimbatm zimbatm deleted the nix-fmt-pwd branch September 11, 2024 11:10
@zimbatm
Copy link
Member Author

zimbatm commented Sep 11, 2024

Thanks, I didn't expect this to be merged so quickly!

For visitors:

nixfmt, nixpkgs-fmt and alejandra are currently formatting stdin when no arguments are passed, so this change will cause some back-compat issues.

I would recommend those projects change their default to format the current folder instead (and format stdin on --stdin instead).

For visitors hitting this, here is a small wrapper example that will restore the previous behaviour:

{
  outputs = { self, nixpkgs, systems }:
    let
      eachSystem = nixpkgs.lib.genAttrs (import systems) (system: nixpkgs.legacyPackages.${system});
    in
    {
      formatter = eachSystem (pkgs: 
        pkgs.writeShellScriptBin "formatter" ''
          if [[ $# = 0 ]]; set -- .; fi
          exec "${pkgs.nixfmt-rfc-style}/bin/nixfmt "$@"
        '');
    };
}

zimbatm added a commit to zimbatm/nixfmt that referenced this pull request Sep 11, 2024
The common case is to format folders, not stdin.

This changes the default to format the current directory, and introduces
a `--stdin` flag for users that want to wrap `nixfmt` into editors and
other tools.

See also NixOS/nix#11438
lf- pushed a commit to lix-project/lix that referenced this pull request Sep 30, 2024
When `nix fmt` is called without an argument, Nix appends the "." argument before calling the formatter. The comment in the code is:
> Format the current flake out of the box

This also happens when formatting sub-folders.

This means that the formatter is now unable to distinguish, as an interface, whether the "." argument is coming from the flake or the user's intent to format the current folder. This decision should be up to the formatter.

Treefmt, for example, will automatically look up the project's root and format all the files. This is the desired behaviour. But because the "." argument is passed, it cannot function as expected.

Upstream-PR: NixOS/nix#11438

Change-Id: I60fb6b3ed4ec1b24f81b5f0d76c0be98470817ce
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/nix-2-25-released/55994/1

Cynerd added a commit to Cynerd/libshv that referenced this pull request Nov 18, 2024
The new version of Nix no longer provides path to the source to the
formatter and thus alejandra uses stdin instead and thus run fails.

NixOS/nix#11438
fvacek pushed a commit to silicon-heaven/libshv that referenced this pull request Nov 18, 2024
The new version of Nix no longer provides path to the source to the
formatter and thus alejandra uses stdin instead and thus run fails.

NixOS/nix#11438
zimbatm added a commit to numtide/blueprint that referenced this pull request Dec 5, 2024
zimbatm added a commit to numtide/blueprint that referenced this pull request Dec 5, 2024
This is now possible thanks to <NixOS/nix#11438>

In older versions of Nix, "." is still passed as a default argument.
accelbread added a commit to nix-community/flakelight that referenced this pull request Jan 15, 2025
Nix 2.25 changes the args passed to the formatter, so we now need to
handle an empty args.

See NixOS/nix#11438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants