Skip to content

Add formatter to default template#5

Merged
lucperkins merged 16 commits intomainfrom
default-formatter
Oct 24, 2025
Merged

Add formatter to default template#5
lucperkins merged 16 commits intomainfrom
default-formatter

Conversation

@lucperkins
Copy link
Member

@lucperkins lucperkins commented Aug 31, 2025

Summary by CodeRabbit

  • New Features

    • Built-in Nix formatter exposed per supported system and published as a public output.
    • Development shells now include the system-specific formatter and a small demo package by default.
  • Documentation

    • Added activation hints for dev environments and RFC‑166 formatter usage, including format and check commands.
  • Chores

    • Per-system generators now receive system context; per-system configuration adjusted (parameterized system outputs).
    • CI formatting check step adjusted for quoting and obsolete structure-check step removed.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 5, 2025

Walkthrough

Adds a per-system formatter output and exposes it in per-system devShells by changing the per-system generator to accept system. Dev shells include self.formatter.${system}, ponysay, and use pkgs.mkShellNoCC. CI removes a nix flake show step and adjusts quoting in the formatting check.

Changes

Cohort / File(s) Summary of changes
Default flake outputs & dev shells
default/flake.nix
- Added public output formatter = forEachSupportedSystem ({ pkgs, ... }: pkgs.nixfmt-rfc-style).
- Changed per-system devShells generator signature to accept { pkgs, system }.
- Added self.formatter.${system} and ponysay to devShells packages; switched to pkgs.mkShellNoCC.
- Added documentation comments for activation and RFC-166 formatting commands.
nix-darwin flake outputs & dev shells
nix-darwin/flake.nix
- Added formatter.${system} = inputs.nixpkgs.legacyPackages.${system}.nixfmt-rfc-style.
- Injected self.formatter.${system} into devShells.${system}.default packages.
- Added RFC-166 formatting usage comments.
nixos flake outputs
nixos/flake.nix
- Parameterized nixosConfigurations.${name} (was my-system) and introduced let-bound system and name.
- Added public formatter.${system} output configured to nixfmt-rfc-style.
- Updated nixosSystem to inherit system.
CI workflow
.github/workflows/ci.yaml
- Removed the "Ensure flake is properly structured" step that ran nix flake show.
- Changed quoting in "Check Nix formatting" step from single to double quotes around the nix run URL.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer
  participant Flake as flake.outputs
  participant PerSys as per-system generator
  participant Formatter as formatter.${system}
  participant DevShell as devShells.${system}

  Note over Flake,PerSys: forEachSupportedSystem now receives { pkgs, system }
  Dev->>Flake: inspect flake outputs
  Flake->>PerSys: instantiate per-system outputs (pkgs, system)
  PerSys->>Formatter: produce system-specific formatter (nixfmt-rfc-style)
  PerSys->>DevShell: build devShell including self.formatter.${system} and ponysay
  Dev->>DevShell: nix develop / activate
  Dev->>Formatter: run formatter inside devShell (format/check)
  Note right of Formatter #DFF2E1: formatter exposed as public per-system output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Add nix-darwin template #7 — touches nix-darwin/flake.nix and devShell wiring; strongly related to per-system formatter addition.
  • Provide a NixOS template #8 — modifies NixOS flake outputs and configuration naming; related to nixos/flake.nix parameterization and formatter output.

Suggested reviewers

  • edolstra
  • grahamc

Poem

I hop through flakes with tidy paws,
I add a formatter for all your OSs,
Shells now carry ponies and style,
CI trims a step and quotes a while,
A rabbit cheers: formats in a mile. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Add formatter to default template" is directly related to the main change in the changeset. The title accurately describes the addition of a formatter configuration to the default template flake.nix file, which is demonstrated by the changes including formatter output, devShells updates, and documentation. While the changeset also updates the nix-darwin and nixos templates with similar formatter additions, the title appropriately focuses on the default template as the primary subject. The title is concise, specific, and clearly conveys the main purpose of the pull request without vague terminology.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch default-formatter

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 289dd46 and 0cacbb2.

📒 Files selected for processing (1)
  • nixos/flake.nix (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: checks
🔇 Additional comments (3)
nixos/flake.nix (3)

16-23: Good parameterization for template customization.

The introduction of system and name variables makes this template easier for users to adapt to their specific needs. The explanatory comments are helpful.


26-27: LGTM! Clean use of parameterized values.

Using ${name} and inherit system; makes the code more maintainable and idiomatic.


50-60: Formatter output looks good; verify the dev shell command.

The formatter.${system} output is correctly implemented with nixfmt-rfc-style. However, line 59 references nix develop --command nixfmt --check, which assumes a dev shell exists with nixfmt available. This template doesn't define a devShells output, unlike the default and nix-darwin templates mentioned in the PR context.

Consider either:

  1. Adding a devShells.${system}.default output that includes the formatter (as done in the other templates)
  2. Updating the comment to use nix run .#formatter.${system} -- --check instead
  3. Removing the command if users aren't expected to have a dev shell in this template

Apply this diff if you want to update the comment to work without a dev shell:

-  # git ls-files -z '*.nix' | xargs -0 -r nix develop --command nixfmt --check
+  # git ls-files -z '*.nix' | xargs -0 -r nix run .#formatter.${system} -- --check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
default/flake.nix (1)

20-31: Fix: pass system into forEachSupportedSystem’s lambda (build is failing).

f is called without system, but downstream lambdas destructure { pkgs, system }, causing: “anonymous lambda called without required argument 'system'.” Update the helper to include system in the attrset you pass to f.

       forEachSupportedSystem =
         f:
         inputs.nixpkgs.lib.genAttrs supportedSystems (
           system:
-          f {
-            pkgs = import inputs.nixpkgs {
-              inherit system;
-              # Enable using unfree packages
-              config.allowUnfree = true;
-            };
-          }
+          f {
+            inherit system;
+            pkgs = import inputs.nixpkgs {
+              inherit system;
+              # Enable using unfree packages
+              config.allowUnfree = true;
+            };
+          }
         );
🧹 Nitpick comments (3)
default/flake.nix (3)

46-49: Nit: drop unnecessary with pkgs.

packages only references self.formatter.${system}, not pkgs. Remove with pkgs; to avoid scope pollution.

-            packages = with pkgs; [
+            packages = [
               # Add the flake's formatter to your project's environment
               self.formatter.${system}
             ];

65-68: Docs nit: simplify formatting commands.

If you’re on a recent Nix that supports nix fmt, users can just run nix fmt (and nix fmt --check) without piping file lists.

-      # To format all Nix files:
-      # git ls-files '*.nix' | xargs nix fmt
-      # To check formatting:
-      # git ls-files '*.nix' | xargs nixfmt --check
+      # To format all Nix files in the repo:
+      # nix fmt
+      # To check formatting (no changes written):
+      # nix fmt --check

69-69: Optional: add a fallback if nixfmt-rfc-style isn’t in the pinned nixpkgs.

Some channels may not expose pkgs.nixfmt-rfc-style. Using or prevents evaluations from failing and keeps the docs’ nixfmt commands working.

-      formatter = forEachSupportedSystem ({ pkgs, ... }: pkgs.nixfmt-rfc-style);
+      formatter = forEachSupportedSystem ({ pkgs, ... }:
+        pkgs.nixfmt-rfc-style or pkgs.nixfmt
+      );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 682a2bc and f6f0381.

📒 Files selected for processing (1)
  • default/flake.nix (2 hunks)
🧰 Additional context used
🪛 GitHub Actions: .github/workflows/ci.yaml
default/flake.nix

[error] 24-24: Nix flake evaluation error: anonymous lambda called without required argument 'system'. (flake.nix:24:11)


[error] 41-41: Nix flake evaluation error: anonymous lambda called without required argument 'system'. (flake.nix:41:9)

🔇 Additional comments (1)
default/flake.nix (1)

41-41: LGTM once helper passes system.

After the helper fix above, this { pkgs, system } destructuring will evaluate cleanly.

Re-run CI after the helper change to confirm both errors at Line 24 and Line 41 disappear.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
default/flake.nix (3)

48-51: Drop unnecessary with pkgs; and consider a lighter shell.

with pkgs isn’t used here; removing it avoids polluting scope. Optional: use mkShellNoCC to trim closure when the shell only adds tools.

Apply this diff:

-            packages = with pkgs; [
+            packages = [
               # Add the flake's formatter to your project's environment
               self.formatter.${system}
             ];

Optionally:

-          default = pkgs.mkShell {
+          default = pkgs.mkShellNoCC {

62-70: Tighten formatter docs to prefer nix fmt.

Since you provide a formatter, the simplest path is nix fmt; pass-through args work for checks.

Apply this diff:

-      # To format all Nix files:
-      # git ls-files '*.nix' | xargs nix fmt
-      # To check formatting:
-      # git ls-files '*.nix' | xargs nixfmt --check
+      # To format:
+      # nix fmt
+      # To check formatting (pass-through to nixfmt):
+      # nix fmt -- --check

71-71: Replace deprecated nixfmt-rfc-style with nixfmt and verify availability
Ensure your pinned nixpkgs provides the new pkgs.nixfmt attribute (which supersedes nixfmt-rfc-style); if it isn’t available, fall back to pkgs.alejandra.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 95e45f3 and cdb2664.

📒 Files selected for processing (1)
  • default/flake.nix (3 hunks)
🔇 Additional comments (3)
default/flake.nix (3)

25-25: Propagating system is correct.

This enables downstream lambdas to destructure system cleanly (needed for self.formatter.${system}). LGTM.


38-41: Activation docs look good.

Clear and accurate for a template. LGTM.


43-43: Destructuring { pkgs, system } is appropriate.

Matches the updated generator contract and usage below. LGTM.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
.github/workflows/ci.yaml (1)

41-42: Prefer the flake’s own formatter to avoid version skew.

Now that the flake exposes a per‑system formatter, call it instead of a pinned remote to keep CI/dev parity. Also add -- to git ls-files for pathspec safety.

-          git ls-files -z '*.nix' | \
-            xargs -0 -r nix run "https://flakehub.com/f/NixOS/nixpkgs/0.2505.808723#nixfmt-rfc-style" -- --check
+          git ls-files -z -- '*.nix' | \
+            xargs -0 -r nix run .#formatter -- --check

If your runner has a recent Nix with nix fmt, an alternative is:

-          git ls-files -z '*.nix' | \
-            xargs -0 -r nix run "https://flakehub.com/f/NixOS/nixpkgs/0.2505.808723#nixfmt-rfc-style" -- --check
+          git ls-files -z -- '*.nix' | \
+            xargs -0 -r nix fmt -- --check
default/flake.nix (2)

43-55: mkShellNoCC and package set look solid; tiny style nit.

Optional: avoid with pkgs; to reduce scope pollution and ease grepability.

-          default = pkgs.mkShellNoCC {
-            # The Nix packages provided in the environment
-            packages = with pkgs; [
-              # Add the flake's formatter to your project's environment
-              self.formatter.${system}
-
-              # Other packages
-              curl
-              jq
-            ];
+          default = pkgs.mkShellNoCC {
+            # The Nix packages provided in the environment
+            packages = [
+              # Add the flake's formatter to your project's environment
+              self.formatter.${system}
+
+              # Other packages
+              pkgs.curl
+              pkgs.jq
+            ];

66-76: Unify docs with CI: prefer flake‑local formatter; keep NUL‑safety.

Use the local formatter for checks and add -- to git ls-files for pathspec separation.

-      # To format all Nix files:
-      # git ls-files -z '*.nix' | xargs -0 -r nix fmt
-      # To check formatting:
-      # git ls-files -z '*.nix' | xargs -0 -r nixfmt --check
+      # To format all Nix files:
+      # git ls-files -z -- '*.nix' | xargs -0 -r nix fmt
+      # To check formatting (uses this flake's formatter):
+      # git ls-files -z -- '*.nix' | xargs -0 -r nix run .#formatter -- --check

The formatter = forEachSupportedSystem ({ pkgs, ... }: pkgs.nixfmt-rfc-style); definition itself looks correct and matches RFC‑166 conventions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a60806 and a8a94a1.

📒 Files selected for processing (2)
  • .github/workflows/ci.yaml (1 hunks)
  • default/flake.nix (3 hunks)
🔇 Additional comments (2)
default/flake.nix (2)

25-25: Good: pass system into the per‑system generator.

This unblocks using system‑aware outputs elsewhere (e.g., self.formatter.${system}).


38-41: Docs addition LGTM.

Clear activation hints for nix develop/direnv.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8a94a1 and 1c05df5.

📒 Files selected for processing (3)
  • .github/workflows/ci.yaml (1 hunks)
  • default/flake.nix (3 hunks)
  • nix-darwin/flake.nix (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: .github/workflows/ci.yaml
nix-darwin/flake.nix

[error] 153-153: nix flake check failed: undefined variable 'forEachSupportedSystem' at nix-darwin/flake.nix:153. Command 'nix flake check --all-systems "./${flake}"' exited with code 1.

🔇 Additional comments (7)
.github/workflows/ci.yaml (1)

27-27: Quote change has no functional impact.

The change from single quotes to double quotes is cosmetic and doesn't affect functionality in this context.

nix-darwin/flake.nix (2)

139-141: Good addition, but depends on fixing the formatter definition.

Adding the formatter to the dev shell packages is a good practice. However, this will only work once the formatter output definition at line 153 is fixed.


143-152: Excellent documentation for formatter usage.

The comments clearly explain the RFC 166 standard and provide practical examples for both formatting and checking. This will help users understand how to use the formatter.

default/flake.nix (4)

25-25: Proper system parameter propagation.

Adding inherit system; at line 25 and updating the devShells signature to accept { pkgs, system } at line 43 correctly enables per-system references like self.formatter.${system}. This is the right pattern for accessing per-system outputs.

Also applies to: 43-43


46-54: Good use of mkShellNoCC and formatter integration.

Using mkShellNoCC is appropriate for development shells that don't require C compilation tooling. Including self.formatter.${system} in the packages makes the formatter readily available in the dev environment, following best practices.

Note: ponysay appears to be added as an example package.


37-41: Helpful activation instructions.

The added comments clearly explain how to activate the development environment using both nix develop and direnv allow, making it easier for users to get started.


64-74: Formatter properly implemented with excellent documentation.

The formatter output is correctly defined using forEachSupportedSystem and pkgs.nixfmt-rfc-style, following RFC 166 standards. The documentation provides clear examples for both formatting and checking, which will help users adopt consistent formatting practices.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c05df5 and 289dd46.

📒 Files selected for processing (1)
  • nix-darwin/flake.nix (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: checks
🔇 Additional comments (1)
nix-darwin/flake.nix (1)

139-142: Good addition of formatter to devShell.

Making the formatter available in the development shell enables convenient formatting within the nix develop environment. The reference to self.formatter.${system} is valid due to Nix's lazy evaluation of attribute sets.

Comment on lines +144 to +153
# Nix formatter

# This applies the formatter that follows RFC 166, which defines a standard format:
# https://github.com/NixOS/rfcs/pull/166

# To format all Nix files:
# git ls-files -z '*.nix' | xargs -0 -r nix fmt
# To check formatting:
# git ls-files -z '*.nix' | xargs -0 -r nix develop --command nixfmt --check
formatter.${system} = inputs.nixpkgs.legacyPackages.${system}.nixfmt-rfc-style;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

❓ Verification inconclusive

Excellent fix for the previous undefined variable issue.

The formatter definition now correctly uses formatter.${system} instead of the previously undefined forEachSupportedSystem helper, resolving the critical pipeline failure. The implementation properly references nixfmt-rfc-style, which implements RFC 166.

However, please verify that the nixfmt binary provided by nixfmt-rfc-style supports the --check flag mentioned in the comment on line 152. Some formatters use different verification flags.

Run this script to verify the formatter's CLI options:


🏁 Script executed:

#!/bin/bash
# Verify that nixfmt-rfc-style supports the --check flag

# Get the formatter package and check its help output
nix eval --raw ".#formatter.aarch64-darwin" --apply 'p: p.outPath' | xargs -I {} cat {}/bin/nixfmt 2>/dev/null || echo "Binary check requires building the package"

# Alternative: Check the package's help text
nix shell ".#formatter.aarch64-darwin" --command nixfmt --help 2>&1 | grep -i "check\|verify"

Length of output: 41


Line 152 uses incorrect nixfmt flag; should use --verify instead of --check.

The code comment on line 152 suggests using nixfmt --check, but nixfmt includes a --verify flag to check idempotency, not --check. The --check flag is available in other formatters like nixpkgs-fmt, but not in nixfmt-rfc-style. Update the comment to:

# git ls-files -z '*.nix' | xargs -0 -r nix develop --command nixfmt --verify
🤖 Prompt for AI Agents
nix-darwin/flake.nix around lines 144 to 153: the comment shows using `nixfmt
--check` which is the wrong flag for nixfmt; change the example command in the
comment to use `--verify` instead (i.e. replace `nixfmt --check` with `nixfmt
--verify`) so the documentation matches nixfmt-rfc-style behavior.

@lucperkins lucperkins merged commit e030250 into main Oct 24, 2025
2 checks passed
@lucperkins lucperkins deleted the default-formatter branch October 24, 2025 17:49
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