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

Nixfmt classic formatter #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

spikespaz
Copy link

This change sets the flake's formatter output to nixfmt-classic which is pretty close to your style.

By submiting this PR, I agree to license this contribution under Creative Commons’ CC0 license.

Copy link
Owner

@hraban hraban left a comment

Choose a reason for hiding this comment

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

Thanks for this. I should probably just bite the bullet and adopt the a standard formatter across all my nix code, this project included. I'll take a quick dive into the world of nix formatters this month because I'd like to get it right, but this PR will be a good starting point.

The actual result of this specific formatter feels a bit grating but I guess that's machine formatting for ya. It's worth it if it means everyone is aligned (basically like the golang ecosystem)

@@ -15,7 +15,8 @@
{
inputs = {
# This has SBCL 2.4.10 and docktuil 3.1.3 which are known to work
nixpkgs.url = "github:NixOS/nixpkgs/af51545ec9a44eadf3fe3547610a5cdd882bc34e";
nixpkgs.url =
"github:NixOS/nixpkgs/af51545ec9a44eadf3fe3547610a5cdd882bc34e";
Copy link
Owner

Choose a reason for hiding this comment

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

I must say this feels like a regression..

Copy link
Author

Choose a reason for hiding this comment

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

Sometimes it does break lines that I wish it wouldn't. I will leave justification for why this one in a regular comment.

@spikespaz
Copy link
Author

spikespaz commented Jan 8, 2025

I prefer nixfmt-classic based on the original serokell/nixfmt version 0.6.0. Infinisil has spearheaded the adaptation of the formatter's codebase for the RFC style.

I dislike Alejandra and RFC style for a few similar reasons, where the RFC style is far worse.

  • They both break lines that they should not.
  • Both break arguments at odd positions that harms readability.
  • The code is formatted in such a way that simple changes on individual lines cause large indentations across larger blocks.
  • Comments are sometimes formatted oddly if they are in a position which was not anticipated.

All of the above means code diffs are super noisy and change irrelevant lines per-commit.

Alejandra has these problems, though to a much lesser degree compared to Infinisil's adopted project, so it would be my second choice.

This PR is a prerequisite to other changes I would like to contribute.

Copy link
Owner

@hraban hraban left a comment

Choose a reason for hiding this comment

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

You had me at Infinisil 👍

@hraban
Copy link
Owner

hraban commented Jan 9, 2025

Hmm he does seem to advocate for -rfc though, not -classic. And I see this on their readme https://github.com/NixOS/nixfmt :

nixfmt was used as the basis for the official Nix formatter with a standardized formatting. The new formatting differs considerably from the original one. A recent nixfmt version is available as pkgs.nixfmt-rfc-style in Nixpkgs. The formatting of this version differs considerably from the original nixfmt that was used as the basis for the standardised official formatter, which is also still available as pkgs.nixfmt-classic for now, though unmaintained.

And apparently nixpkgs is going rfc style, I even commented on that discourse thread last year and completely forgot https://discourse.nixos.org/t/overview-of-nix-formatters-ecosystem/38880 😂

It seems like the numbers are in, and rfc-style has it. Do you see any significant adoption of -classic on the horizon? Because if I have to choose a formatter, and all of nixpkgs is going rfc, I'd rather follow them.

If I'm going against the grain I'd rather not have a formatter at all. I don't find this one an improvement on its own right, the only benefit I see to this PR is to bring it in line with everyone else.

@spikespaz
Copy link
Author

spikespaz commented Jan 9, 2025

All of this is true. I did not say "infinisil supports -classic".

I suggest running both on a large portion of your own nix code and choosing for yourself.


nixfmt-rfc-style originally took the nixfmt name, and forced the creation of nixfmt-classic. Later, it was decided to rename nixfmt to nixfmt-rfc-style and put a deprecation warning on using pkgs.nixfmt.

@hraban
Copy link
Owner

hraban commented Jan 9, 2025

gotcha thanks I'll try both and see

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.

2 participants