feat(fish): add global dev command for nix devshell - #540
Conversation
Add a `dev` fish abbreviation that enters a Nix development shell from anywhere. The function searches for the nearest flake.nix in parent directories and uses that devshell, or falls back to ~/dotfiles if no flake is found.
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR introduces a new Fish shell function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Fish shell experience by introducing a convenient Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRAdded a global What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request introduces a convenient dev command for entering Nix development shells. The implementation correctly searches for a flake.nix file in parent directories and falls back to a default. My review includes one high-severity finding related to a logic bug in the directory traversal, which could cause it to miss a flake.nix file in the root directory. I've provided a code suggestion to fix this.
| set -l check_dir (pwd) | ||
| while test "$check_dir" != "/" | ||
| if test -f "$check_dir/flake.nix" | ||
| set target_dir $check_dir | ||
| break | ||
| end | ||
| set check_dir (dirname $check_dir) | ||
| end |
There was a problem hiding this comment.
The current loop for finding flake.nix will not check the root directory /. The loop condition test "$check_dir" != "/" causes the loop to terminate when check_dir becomes /, without checking for flake.nix in that directory. This means if a flake.nix exists only at the filesystem root, it won't be found, and the function will incorrectly fall back to the default. The suggested change fixes this by adjusting the loop to correctly handle traversal up to and including the root directory.
set -l check_dir (pwd)
while true
if test -f "$check_dir/flake.nix"
set target_dir $check_dir
break
end
if test "$check_dir" = "/"; break; end
set check_dir (dirname $check_dir)
end
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
home-manager/programs/fish/functions/_dev_function.fish (1)
6-13: Consider checking the root directory.The loop exits when
check_dirreaches"/"without checking forflake.nixat the root itself. While having aflake.nixat/is extremely unlikely in practice, you could make the traversal complete by adjusting the loop condition.♻️ Optional fix to include root directory check
- set -l check_dir (pwd) - while test "$check_dir" != "/" + set -l check_dir (pwd) + while true if test -f "$check_dir/flake.nix" set target_dir $check_dir break end + test "$check_dir" = "/" && break set check_dir (dirname $check_dir) end
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
home-manager/programs/fish/default.nixhome-manager/programs/fish/functions/_dev_function.fish
🧰 Additional context used
📓 Path-based instructions (6)
**/*.nix
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.nix: Use nixfmt for formatting all Nix files
Document complex configurations with comments in Nix files
**/*.nix: Use 2 spaces for indentation in Nix files
Keep line length under 100 characters in Nix files
Sort attribute sets alphabetically in Nix files
Use consistent spacing around operators in Nix files
Format lists and sets consistently in Nix filesUse treefmt.toml for formatting Nix files
**/*.nix: UsemkOptionfor configurable options in Nix modules
Implement proper typing for all options in Nix modules
Follow the Nix expression language style guide
Files:
home-manager/programs/fish/default.nix
**/default.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Use
default.nixfiles for module exports
Files:
home-manager/programs/fish/default.nix
home-manager/programs/*/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Program configurations should be located in
home-manager/programs/<name>/and use home-manager's built-in modules when available
Files:
home-manager/programs/fish/default.nix
home-manager/programs/**/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Program configurations must include all necessary dependencies in their configuration
Files:
home-manager/programs/fish/default.nix
home-manager/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
home-manager/**/*.nix: Use typed options whenever possible in Nix configurations
Document all configuration options in Nix modules and programs
Follow home-manager's module structure and keep configurations modular
Use proper indentation and formatting in Nix configuration files
Files:
home-manager/programs/fish/default.nix
home-manager/programs/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Follow program-specific best practices in program configuration files
Program configurations in
home-manager/programs/should be organized by program name, include all necessary dependencies, usehome.packagesfor package installations, and useprograms.<name>when available in home-manager
Files:
home-manager/programs/fish/default.nix
⏰ 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). (13)
- GitHub Check: Agent
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Mesa Review
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: shell-lint
- GitHub Check: shell-test
- GitHub Check: nix-nixos
- GitHub Check: lua-neovim
- GitHub Check: lua-neovim-test
- GitHub Check: nix-linux
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
🔇 Additional comments (4)
home-manager/programs/fish/default.nix (1)
68-68: LGTM! Clean integration of the new dev function.The
_dev_functionis properly registered as a shell abbreviation and correctly included in the function source list, following the established pattern for other function-based abbreviations.Also applies to: 141-141
home-manager/programs/fish/functions/_dev_function.fish (3)
1-3: LGTM! Proper function definition.The function follows Fish best practices with a descriptive flag and proper local variable initialization.
15-18: Verify fallback behavior when $HOME/dotfiles doesn't exist.The function assumes
$HOME/dotfilesexists and contains a validflake.nix. If the directory doesn't exist or lacks a flake,nix developwill fail with an error message. Consider whether you want to add validation before invokingnix develop, or if relying on nix's error messages is acceptable.♻️ Optional: Add validation for fallback directory
# Fallback to dotfiles if no flake found if test -z "$target_dir" set target_dir "$HOME/dotfiles" + if not test -d "$target_dir" + echo "Error: Fallback directory $target_dir does not exist" >&2 + return 1 + end end
20-22: LGTM! Correct devshell invocation.The command properly:
- Sets
DEVENV_ROOTfor devenv integration- Passes the target directory to
nix develop- Forwards additional arguments via
$argvThe informational message provides good UX feedback to the user.
There was a problem hiding this comment.
Performed full review of aec622a...880efe7
Analysis
-
Environment variable propagation issue - The
DEVENV_ROOTsetting may not correctly propagate to spawned shell sessions, which could cause unexpected behavior. -
Hardcoded fallback path (
~/dotfiles) reduces portability and flexibility of the solution for different environments or users. -
Missing validation for valid devShell definitions in found flakes, which could lead to runtime errors when attempting to use invalid configurations.
-
Performance concerns with the directory traversal implementation - always walking up to the root directory could be inefficient on deeply nested paths or network filesystems.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 1 comments | Edit Agent Settings • Read Docs
| # Check if current directory or any parent has flake.nix | ||
| set -l check_dir (pwd) | ||
| while test "$check_dir" != "/" | ||
| if test -f "$check_dir/flake.nix" |
There was a problem hiding this comment.
The function doesn't verify if the found flake.nix actually contains a valid devShell definition. Consider adding a check or handling the error case when nix develop fails on an incompatible flake. A simple approach could be catching the error and displaying a helpful message.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#540
File: home-manager/programs/fish/functions/_dev_function.fish#L8
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The function doesn't verify if the found `flake.nix` actually contains a valid devShell definition. Consider adding a check or handling the error case when `nix develop` fails on an incompatible flake. A simple approach could be catching the error and displaying a helpful message.
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/programs/fish/functions/_dev_function.fish">
<violation number="1" location="home-manager/programs/fish/functions/_dev_function.fish:7">
P2: The loop condition exits when `check_dir` becomes `/` without checking if `flake.nix` exists at the root directory. If a `flake.nix` exists only at `/`, it won't be found and the function will incorrectly fall back to `~/dotfiles`. Consider restructuring the loop to check the root directory before exiting.</violation>
<violation number="2" location="home-manager/programs/fish/functions/_dev_function.fish:22">
P1: Fish does not support `VAR=value command` syntax, so this line tries to run a command named `DEVENV_ROOT=…` and never sets the variable before invoking `nix develop`. Use `env` or `set -lx` to export DEVENV_ROOT for the command.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| # Enter the devshell | ||
| echo "Entering devshell in $target_dir" | ||
| DEVENV_ROOT=$target_dir nix develop $target_dir $argv |
There was a problem hiding this comment.
P1: Fish does not support VAR=value command syntax, so this line tries to run a command named DEVENV_ROOT=… and never sets the variable before invoking nix develop. Use env or set -lx to export DEVENV_ROOT for the command.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_dev_function.fish, line 22:
<comment>Fish does not support `VAR=value command` syntax, so this line tries to run a command named `DEVENV_ROOT=…` and never sets the variable before invoking `nix develop`. Use `env` or `set -lx` to export DEVENV_ROOT for the command.</comment>
<file context>
@@ -0,0 +1,23 @@
+
+ # Enter the devshell
+ echo "Entering devshell in $target_dir"
+ DEVENV_ROOT=$target_dir nix develop $target_dir $argv
+end
</file context>
| DEVENV_ROOT=$target_dir nix develop $target_dir $argv | |
| env DEVENV_ROOT=$target_dir nix develop $target_dir $argv |
|
|
||
| # Check if current directory or any parent has flake.nix | ||
| set -l check_dir (pwd) | ||
| while test "$check_dir" != "/" |
There was a problem hiding this comment.
P2: The loop condition exits when check_dir becomes / without checking if flake.nix exists at the root directory. If a flake.nix exists only at /, it won't be found and the function will incorrectly fall back to ~/dotfiles. Consider restructuring the loop to check the root directory before exiting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_dev_function.fish, line 7:
<comment>The loop condition exits when `check_dir` becomes `/` without checking if `flake.nix` exists at the root directory. If a `flake.nix` exists only at `/`, it won't be found and the function will incorrectly fall back to `~/dotfiles`. Consider restructuring the loop to check the root directory before exiting.</comment>
<file context>
@@ -0,0 +1,23 @@
+
+ # Check if current directory or any parent has flake.nix
+ set -l check_dir (pwd)
+ while test "$check_dir" != "/"
+ if test -f "$check_dir/flake.nix"
+ set target_dir $check_dir
</file context>
There was a problem hiding this comment.
Pull request overview
This PR adds a convenient dev abbreviation for entering Nix development shells from any directory. The function searches upward through parent directories for a flake.nix file and falls back to the dotfiles directory if none is found.
Changes:
- Added
_dev_function.fishthat implements directory tree traversal to find nearestflake.nix - Registered
devabbreviation in Fish configuration - Integrated function into the Fish functions list
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| home-manager/programs/fish/functions/_dev_function.fish | New function implementing upward directory search and devshell entry logic |
| home-manager/programs/fish/default.nix | Registered dev abbreviation and added function to config file mapping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Enter the devshell | ||
| echo "Entering devshell in $target_dir" | ||
| DEVENV_ROOT=$target_dir nix develop $target_dir $argv |
There was a problem hiding this comment.
The POSIX-style environment variable syntax works in Fish but is not idiomatic. Consider using the Fish-native approach with begin; set -lx DEVENV_ROOT $target_dir; nix develop $target_dir $argv; end or using env DEVENV_ROOT=$target_dir nix develop $target_dir $argv for better clarity and consistency with Fish conventions.
| DEVENV_ROOT=$target_dir nix develop $target_dir $argv | |
| env DEVENV_ROOT=$target_dir nix develop $target_dir $argv |
|
|
||
| # Fallback to dotfiles if no flake found | ||
| if test -z "$target_dir" | ||
| set target_dir "$HOME/dotfiles" |
There was a problem hiding this comment.
The fallback to "$HOME/dotfiles" doesn't check if the directory or flake.nix exists there. If the dotfiles directory doesn't exist or doesn't contain a flake.nix, the nix develop command will fail without a clear error message. Consider adding validation to check if the fallback directory and flake.nix exist before attempting to enter the devshell.
| set target_dir "$HOME/dotfiles" | |
| set -l fallback_dir "$HOME/dotfiles" | |
| if test -d "$fallback_dir" -a -f "$fallback_dir/flake.nix" | |
| set target_dir "$fallback_dir" | |
| else | |
| echo "Error: No flake.nix found in current directory tree, and fallback '$fallback_dir' is missing or lacks flake.nix." >&2 | |
| return 1 | |
| end |
Changes
devfish abbreviation for entering Nix devshells from anywhere_dev_function.fishthat searches for nearestflake.nixin parent directories~/dotfilesdevshell if no local flake foundTechnical Details
flake.nixDEVENV_ROOTproperly for devenv integrationnix developTesting
make switchcompletes successfullydevfrom dotfiles directory enters devshelldevfrom random directory falls back to dotfiles devshellGenerated with Claude Code by claude-opus-4-5-20251101
Summary by cubic
Add a global dev command in fish to enter a Nix devshell from any directory. It finds the closest flake.nix or falls back to ~/dotfiles.
Written for commit 880efe7. Summary will update on new commits.