Add NVIMF program implementation#186
Conversation
Implements a standalone nvimf program that provides interactive fuzzy file selection with syntax-highlighted preview and opens files in Neovim. Changes: - Created modules/programs/nvimf/ with shell script and Nix module - Implemented comprehensive error handling and dependency management - Added cross-platform support (NixOS and Darwin) - Removed legacy nvimf alias from .zshrc (line 53) - Removed nvimf function from nushell config.nu - Integrated nvimf into engineer feature for automatic availability - Updated tasks.md to reflect completion of all implementation tasks Key features: - Interactive fuzzy finder with fzf - Syntax-highlighted file preview using bat - Graceful cancellation handling (ESC/Ctrl-C) - Smart filtering of build artifacts (.git, node_modules, etc.) - Optional fd support with fallback to find - Comprehensive help system (nvimf --help) The program is now available system-wide when engineer feature is enabled, replacing the previous shell-specific alias approach with a proper Nix derivation that manages all dependencies correctly.
WalkthroughThe pull request removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
modules/programs/nvimf/nvimf (1)
93-95: Align documented “smart filtering” with actual fd exclusionsDocs say you exclude
.git/,node_modules/,.cache/,dist/, andbuild/by default, but:
- fd branch (lines 125–127) excludes only
.git,node_modules,.cache.- find branch (lines 136–143) adds
.next,dist,build.To keep behavior predictable, consider making the fd exclusions match the find path (or updating the docs to describe the actual behavior). For example:
- if command_exists fd; then - selected_file=$(fd --type f --hidden --exclude .git --exclude node_modules --exclude .cache \ + if command_exists fd; then + selected_file=$(fd --type f --hidden \ + --exclude .git \ + --exclude node_modules \ + --exclude .cache \ + --exclude .next \ + --exclude dist \ + --exclude build \ | fzf ...Also applies to: 124-152
modules/programs/nvimf/default.nix (1)
1-209: Nix module and packaging look solid; only minor dependency nitThe derivation and
delib.modulewiring look correct: enablingprograms.nvimfadds a wrappednvimfwith all required tools on PATH for both NixOS and Darwin, and the large doc block nicely documents behavior and usage.One minor point: the comments describe
fdas “optional”, but the derivation always bundlespkgs.fdand the wrapper unconditionally adds it to PATH. That’s fine in practice, but you might either:
- Keep
fdalways present and just tweak the docs to say “preferred, with find fallback for non‑Nix environments”; or- Move
fdout of the default build inputs / wrapper list if you truly want it to be optional at the package level.Given this is purely stylistic/clarity, it can be deferred.
If you rely on a specific
delib.module/singleEnableOptioncontract or Nixpkgs version, please double‑check that this pattern matches your existing modules (e.g., otherprograms.*modules in this repo) for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.config/nushell/config.nu(0 hunks).zshrc(0 hunks)modules/features/engineer.nix(2 hunks)modules/programs/nvimf/default.nix(1 hunks)modules/programs/nvimf/nvimf(1 hunks)spectr/changes/add-nvimf-program/tasks.md(1 hunks)
💤 Files with no reviewable changes (2)
- .config/nushell/config.nu
- .zshrc
⏰ 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). (4)
- GitHub Check: Build (linux-x86_64)
- GitHub Check: Build (macos-aarch64)
- GitHub Check: Validate Templates (linux-x86_64)
- GitHub Check: Validate Templates (macos-aarch64)
🔇 Additional comments (2)
spectr/changes/add-nvimf-program/tasks.md (1)
3-27: Task completions match the implemented changesAll items toggled to
[x]line up with the added module, script, feature wiring, and alias removals. Nothing actionable from a code perspective; this is consistent bookkeeping.modules/features/engineer.nix (1)
25-43: Engineer feature wiring correctly enablesprograms.nvimfHooking
nvimf.enable = trueinto the NixOS and Darwinmyconfig.programssets cleanly ties the new program into the existing engineer feature. This matches theprograms.nvimfmodule name exposed in the new Nix module and should make the tool available wherever the feature is enabled.Please rebuild once with
features.engineer.enable = trueon both NixOS and Darwin to confirmnvimfappears on PATH and respects the wrapped dependencies as expected.Also applies to: 202-211
| set -euo pipefail | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
set -euo pipefail breaks graceful cancel handling from fzf
With set -euo pipefail in effect, the fd|fzf / find|fzf pipelines inside the command substitutions will cause the script to exit immediately when fzf returns a non‑zero status (e.g., ESC/Ctrl‑C). That means the later check:
- Lines 155–158:
if [ -z "${selected_file:-}" ]; then ...
is never reached on cancellation, so you don’t actually get the intended “graceful exit” path.
A simple fix is to absorb the non‑zero status inside the substitution so the outer shell doesn’t treat it as an error, while still letting you detect “no selection” via the empty variable:
- if command_exists fd; then
- selected_file=$(fd --type f --hidden --exclude .git --exclude node_modules --exclude .cache \
- | fzf --preview 'bat --color=always --style=numbers,changes --line-range :500 {}' \
- --preview-window=right:60%:wrap \
- --height=100% \
- --border \
- --prompt='Select file to edit: ' \
- --header='Enter=open | ESC=cancel | Ctrl-/=toggle preview' \
- --bind='ctrl-/:toggle-preview' \
- --color='header:italic:underline')
+ if command_exists fd; then
+ selected_file=$(fd --type f --hidden --exclude .git --exclude node_modules --exclude .cache \
+ | fzf --preview 'bat --color=always --style=numbers,changes --line-range :500 {}' \
+ --preview-window=right:60%:wrap \
+ --height=100% \
+ --border \
+ --prompt='Select file to edit: ' \
+ --header='Enter=open | ESC=cancel | Ctrl-/=toggle preview' \
+ --bind='ctrl-/:toggle-preview' \
+ --color='header:italic:underline' || true)
else
- selected_file=$(find . -type f \
+ selected_file=$(find . -type f \
-not -path '*/\.git/*' \
-not -path '*/node_modules/*' \
-not -path '*/\.cache/*' \
-not -path '*/.next/*' \
-not -path '*/dist/*' \
-not -path '*/build/*' \
| sed 's|^\./||' \
| fzf --preview 'bat --color=always --style=numbers,changes --line-range :500 {}' \
--preview-window=right:60%:wrap \
--height=100% \
--border \
--prompt='Select file to edit: ' \
--header='Enter=open | ESC=cancel | Ctrl-/=toggle preview' \
- --bind='ctrl-/:toggle-preview' \
- --color='header:italic:underline')
+ --bind='ctrl-/:toggle-preview' \
+ --color='header:italic:underline' || true)This keeps cancellation from tripping set -e while preserving your explicit “no selection” handling.
Please verify in a shell with set -euo pipefail that cancelling fzf (ESC/Ctrl‑C) currently exits the script with a non‑zero status before the “No file selected” message, and that applying the above change restores the intended behavior.
Also applies to: 124-152, 155-158
🏁 Script executed:
#!/bin/bash
# Find and inspect the nvimf script
find . -name "nvimf" -type f 2>/dev/null | head -5Length of output: 30
🏁 Script executed:
#!/bin/bash
# Show first 200 lines of the nvimf script with line numbers
if [ -f "modules/programs/nvimf/nvimf" ]; then
head -200 "modules/programs/nvimf/nvimf" | cat -n
fiLength of output: 6039
🏁 Script executed:
#!/bin/bash
# Test bash set -e + pipefail behavior: does it exit before the check?
cat > /tmp/test_fzf_cancel.sh << 'EOF'
#!/bin/bash
set -euo pipefail
echo "Before command substitution"
# Simulate fzf returning non-zero (no selection)
selected_file=$( (exit 1) || true )
echo "After command substitution with || true"
if [ -z "${selected_file:-}" ]; then
echo "Graceful: No file selected"
fi
echo "Script completed successfully"
EOF
bash /tmp/test_fzf_cancel.shLength of output: 124
🏁 Script executed:
#!/bin/bash
# Test WITHOUT the || true fix to show the problem
cat > /tmp/test_fzf_problem.sh << 'EOF'
#!/bin/bash
set -euo pipefail
echo "Before command substitution"
# This will exit the script because (exit 1) fails and pipefail is set
selected_file=$( (exit 1) )
echo "After command substitution (this line never executes)"
if [ -z "${selected_file:-}" ]; then
echo "No file selected"
fi
EOF
bash /tmp/test_fzf_problem.sh 2>&1 || echo "Script exited with status: $?"Length of output: 56
Add || true to fzf command substitutions to permit graceful cancellation under set -e
Your review comment is correct. Testing confirms that with set -euo pipefail, when fzf exits non-zero (ESC/Ctrl-C), the script terminates immediately instead of reaching the "no selection" check at lines 155–158.
The fix is straightforward and verified to work: append || true to both fzf invocations to absorb the non-zero status.
Apply the || true fix to:
- Line 134: end of fd + fzf pipeline
- Line 151: end of find + fzf pipeline
- --color='header:italic:underline')
+ --color='header:italic:underline' || true)(for both branches)
This preserves the ability to detect "no selection" via the empty variable check while preventing set -e from prematurely exiting the script.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In modules/programs/nvimf/nvimf around lines 22 and specifically at lines 134
and 151, the fzf command substitutions can return non-zero when cancelled which
causes the script to exit under set -euo pipefail; append "|| true" to the end
of each fzf pipeline (both branches) so the command substitution always returns
success, allowing subsequent "no selection" empty-variable checks at lines
155–158 to run and handle cancellations gracefully.
connerohnesorge
deleted the
claude/add-nvimf-program-01KevmNuHcPuRfZcqdxHFked
branch
2 participants
Implements a standalone nvimf program that provides interactive fuzzy file selection with syntax-highlighted preview and opens files in Neovim.
Changes:
Key features:
The program is now available system-wide when engineer feature is enabled, replacing the previous shell-specific alias approach with a proper Nix derivation that manages all dependencies correctly.
Summary by CodeRabbit
New Features
nvimf: an interactive fuzzy file finder with syntax-highlighted previews that opens selected files in Neovim. Supports directory navigation with fast file listing and comprehensive error handling.Refactor
✏️ Tip: You can customize this high-level summary in your review settings.