More nix nario list improvements

[edolstra]

Motivation

• Add a -l option to list details about files.
• Replace --no-contents by --recursive / -R to be consistent with nix {nar,store} ls.
• Simplify the output a bit.

Context

Summary by CodeRabbit

• New Features

  • Added --long / -l flag to display detailed file information including permissions, size, and symlink targets.
  • Added --recursive / -R flag to enable recursive directory traversal and listing.

• Tests

  • Updated test suite to verify new command-line options for long and recursive listing modes.

[coderabbitai]

Walkthrough

The PR introduces a new MixLongListing mixin to standardize long-format listing support across Nix commands. It refactors MixLs to inherit from this mixin, updates CmdNarioList to support detailed listing output (permissions, sizes, symlink targets), replaces the --no-contents flag with --recursive, and updates tests accordingly.

Changes

Cohort / File(s)	Summary
New mixin for long listing support src/nix/ls.hh	New file introducing MixLongListing struct with bool longListing member and flag registration for --long / -l command-line option.
Mixin inheritance refactoring src/nix/ls.cc	Updated MixLs to inherit from MixLongListing; removed verbose flag and associated registration; switched formatting logic from verbose to longListing mode; adjusted output width from %20d to %9d.
Nario listing refactoring src/nix/nario.cc	Updated renderNarListing() signature to accept const CanonPath& prefix and bool longListing parameter; added detailed formatting (permissions, sizes, symlink targets) in long listing mode. Updated CmdNarioList to inherit from MixLongListing; replaced --no-contents flag with --recursive flag; refactored ListingStore to store reference to parent CmdNarioList instead of local bool; updated rendering calls to pass cmd.longListing and new path type.
Test updates tests/functional/export.sh	Updated grep filter to remove bytes-specific expectation; added recursive listing checks with -lR flags to verify permissions and symlink targets; modified JSON test to use -R flag and updated assertions; removed --no-contents references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

The changes span four files with mixed complexity: a straightforward new mixin definition, a targeted refactoring of MixLs, significant restructuring in nario.cc involving signature changes and state plumbing, and updated test assertions. The heterogeneous nature across components and logic updates in the central nario.cc file demand separate reasoning for each area.

Possibly related PRs

• nix nario list improvements #232: Both PRs modify CmdNarioList, ListingStore, and renderNarListing to adjust listing modes and output behavior, making them closely related in the nario listing subsystem.

Poem

🐰 Long files are listed now with pride,
Permissions, sizes, symlinks side by side,
Recursive flags dance where no-contents stood,
A mixin's touch made the listing good!
–Signed, a very thorough rabbit ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title Check	❓ Inconclusive	The title "More nix nario list improvements" correctly identifies the target command being modified but relies on the vague and generic term "improvements" to describe the changes. While the title accurately references nix nario list, it fails to convey the specific nature of the modifications, such as adding the -l flag for long listing or replacing --no-contents with --recursive. A teammate scanning the commit history would not have a clear understanding of what was specifically improved without reviewing the full PR details. This level of non-specificity falls into the category of generic phrasing that doesn't convey meaningful information about the changeset.	Consider revising the title to be more specific about the key changes, such as "Add -l flag and --recursive option to nix nario list" or "Implement long listing and recursive options for nix nario list". This would help team members quickly understand the primary improvements without needing to examine the PR details.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch nario-list

---

📜 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 dd7140a and 70a08db.

📒 Files selected for processing (4)

• src/nix/ls.cc (2 hunks)
• src/nix/ls.hh (1 hunks)
• src/nix/nario.cc (7 hunks)
• tests/functional/export.sh (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/functional/export.sh (1)

src/nix/ls.cc (1)

• list (140-143)

🪛 Clang (14.0.6)

src/nix/ls.hh

[error] 3-3: 'nix/util/args.hh' file not found

(clang-diagnostic-error)

⏰ 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). (2)

• GitHub Check: build_x86_64-linux / build
• GitHub Check: build_aarch64-darwin / build

🔇 Additional comments (11)

src/nix/ls.cc (2)

7-11: LGTM! Clean refactoring to shared mixin.

The addition of MixLongListing as a base class successfully consolidates the long listing functionality into a reusable component, enabling consistent behavior across ls and nario commands.

---

38-44: LGTM! Formatting now consistent across commands.

The switch to longListing from the base mixin is correct, and the format width change from %20d to %9d aligns with the rendering in nario.cc (line 183), ensuring consistent output formatting across ls and nario list commands.

src/nix/ls.hh (1)

1-20: LGTM! Clean mixin design.

The MixLongListing mixin provides a well-structured, reusable component for long listing functionality. The virtual inheritance from Args follows the established pattern used by other Nix command mixins.

Note: The static analysis error about 'nix/util/args.hh' file not found is a false positive—the header exists in the codebase and will resolve during the actual build.

tests/functional/export.sh (2)

54-57: LGTM! Tests updated for new default behavior.

The test changes correctly reflect the new command behavior:

• Line 54: Pattern simplified to match the new non-recursive output format (no "bytes" suffix)
• Lines 55-57: Comprehensive validation of long listing format (-lR) including directories, symlinks, and regular files with expected permissions and paths

---

70-78: LGTM! JSON tests cover both recursive and non-recursive modes.

The tests correctly validate:

• Line 70: Recursive mode (-R) populates the contents field with directory structure
• Lines 76-78: Non-recursive mode (default) results in contents.type being null, confirming that NAR contents are not listed by default

This aligns with the behavioral change from opt-out (--no-contents) to opt-in (--recursive).

src/nix/nario.cc (6)

9-9: LGTM!

Include added for the new MixLongListing mixin.

---

173-198: LGTM! Long listing format implemented correctly.

The updated renderNarListing signature and implementation:

• Accepts CanonPath prefix and longListing flag
• Conditionally renders detailed information (permissions, size, symlink targets) when longListing is true
• Format string matches ls.cc line 44 for consistency: "%s %9d %s"
• Correctly handles directories, regular files, and symlinks

---

200-212: LGTM! Behavioral change aligns with PR objectives.

The changes implement the flag rename from --no-contents to --recursive / -R:

• Old behavior: listContents defaulted to true (opt-out with --no-contents)
• New behavior: listContents defaults to false (opt-in with --recursive)

This inverts the default behavior to align with nix {nar,store} ls and provides a simpler default output. The inheritance from MixLongListing enables the new -l option.

---

244-248: LGTM! Clean state management.

Storing a reference to CmdNarioList instead of duplicating state provides cleaner access to both listContents and longListing flags.

---

268-287: LGTM! Correct implementation of recursive and long listing modes.

The logic correctly handles all combinations:

• Uses cmd.listContents to determine whether to parse NAR contents (line 272)
• Passes cmd.longListing to control output format (line 284)
• Provides simplified summary output for non-recursive mode (line 286): "<path>: <size> bytes"

The behavior aligns with the test expectations in export.sh.

---

321-321: LGTM!

Correctly passes *this to provide ListingStore access to command state.

---

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

[github-actions]

🚀 Deployed on https://68f6492f3688a141ad67de74--determinate-nix.netlify.app