feat(fish): add clrc and clwrc abbreviations for remote-control - #1211
Conversation
Entire-Checkpoint: 766e55ae90ba
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: 7b75defaf526
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello, 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 developer experience by adding convenient Fish shell abbreviations for interacting with Claude remote-control, allowing for quick access to both stable and worktree versions. Additionally, it refines the repository update script to handle Git pulls more robustly, ensuring smoother integration of upstream changes by attempting a fast-forward merge before resorting to a rebase. 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. Footnotes
|
Mesa DescriptionTL;DRAdded What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request introduces two new fish shell abbreviations, clrc and clwrc, for interacting with claude remote-control. The implementation is straightforward, adding the abbreviations and their corresponding function definitions. I've suggested an improvement to the new fish function to make it more robust by adding error handling for cases where the claude command might not be found.
Additionally, there's an unrelated change to the update-local-binaries.sh script which modifies the git pull strategy. While the intention to use --ff-only and then --rebase is good, the rebase operation can hang the script if conflicts occur. I've provided a suggestion to make this process non-interactive and prevent the script from getting stuck.
Overall, the changes are good, but addressing these points will improve the robustness of the shell functions and scripts.
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
git pull --rebase can enter an interactive state if there are merge conflicts, which would cause this script to hang. To make the script more robust, you should handle this case by aborting the rebase if the pull fails. This ensures the script can continue or exit gracefully without requiring user intervention.
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi | |
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | |
| log_error " Git pull with rebase failed. Aborting rebase..." | |
| # Abort any rebase that might have been started to avoid leaving the repo in a bad state. | |
| (cd "$repo_dir" && git rebase --abort &>/dev/null) || true | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi |
| set -l claude_real (realpath (which claude)) | ||
| node $claude_real remote-control --worktree $argv |
There was a problem hiding this comment.
The current implementation lacks robust error handling. If the claude command is not found, which claude will fail, leading to a cascade of errors from realpath and node, which can be confusing for the user. It's better to explicitly check if the command exists and provide a single, clear error message.
set -l claude_path (command -v claude)
if test -z "$claude_path"
echo "Error: 'claude' command not found." >&2
return 1
end
set -l claude_real (realpath "$claude_path")
node "$claude_real" remote-control --worktree $argv
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdded two new fish abbreviations ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/update-local-binaries.sh`:
- Around line 240-246: The git pull --rebase failure handler leaves an
in-progress rebase; update the failure branch so that after a failed (cd
"$repo_dir" && git pull --rebase ...) you first run a cleanup (e.g., git rebase
--abort and/or git reset --hard to leave the repo in a clean state) before
calling log_error and appending to FAILURES, to ensure subsequent automation
runs are not left with a broken rebase; locate the block around the git pull
--rebase call that references repo_dir, log_error, and FAILURES and add the
abort/reset cleanup there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15b77e70-0dcf-40f3-8adc-77781b18c792
📒 Files selected for processing (3)
home-manager/programs/fish/default.nixhome-manager/programs/fish/functions/_clwrc_function.fishscripts/update-local-binaries.sh
| if ! (cd "$repo_dir" && git pull --ff-only 2>&1); then | ||
| log_warn " Fast-forward failed; rebasing onto remote..." | ||
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the rebase failure path includes cleanup.
rg -n -C3 'git pull --rebase|git rebase --abort' scripts/update-local-binaries.shRepository: shunkakinoki/dotfiles
Length of output: 388
Add rebase cleanup on failure to prevent automation breakage.
At line 242, when git pull --rebase fails, the function returns without aborting the in-progress rebase state. This leaves the repository in a broken state for subsequent automation runs, which can cause them to fail or hang until manual intervention.
Suggested fix
if ! (cd "$repo_dir" && git pull --ff-only 2>&1); then
log_warn " Fast-forward failed; rebasing onto remote..."
if ! (cd "$repo_dir" && git pull --rebase 2>&1); then
+ # Ensure repo is left in a clean state for future runs
+ (cd "$repo_dir" && git rebase --abort >/dev/null 2>&1 || true)
log_error " Git pull failed"
FAILURES+=("$repo_name (git pull failed)")
return 1
fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! (cd "$repo_dir" && git pull --ff-only 2>&1); then | |
| log_warn " Fast-forward failed; rebasing onto remote..." | |
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi | |
| if ! (cd "$repo_dir" && git pull --ff-only 2>&1); then | |
| log_warn " Fast-forward failed; rebasing onto remote..." | |
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | |
| # Ensure repo is left in a clean state for future runs | |
| (cd "$repo_dir" && git rebase --abort >/dev/null 2>&1 || true) | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/update-local-binaries.sh` around lines 240 - 246, The git pull
--rebase failure handler leaves an in-progress rebase; update the failure branch
so that after a failed (cd "$repo_dir" && git pull --rebase ...) you first run a
cleanup (e.g., git rebase --abort and/or git reset --hard to leave the repo in a
clean state) before calling log_error and appending to FAILURES, to ensure
subsequent automation runs are not left with a broken rebase; locate the block
around the git pull --rebase call that references repo_dir, log_error, and
FAILURES and add the abort/reset cleanup there.
There was a problem hiding this comment.
Pull request overview
Adds Fish abbreviations and function wiring for Claude Code “remote-control” commands, and adjusts a repo update script to prefer fast-forward pulls with a rebase fallback.
Changes:
- Update
update_repo()to trygit pull --ff-onlyfirst, then fall back togit pull --rebase. - Add Fish function
_clwrc_functionto runclaude remote-control --worktreevia a realpath-resolved binary. - Register
clrc/clwrcabbreviations and include the function names in the Home Manager Fish function list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/update-local-binaries.sh | Changes git pull strategy to ff-only with rebase fallback. |
| home-manager/programs/fish/functions/_clwrc_function.fish | Introduces a new Fish function to run remote-control in a worktree using a resolved binary path. |
| home-manager/programs/fish/default.nix | Registers new abbreviations and adds function names to the managed function list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| # Function-based abbreviations | ||
| cliproxyapi = "_cliproxyapi_function"; | ||
| clrc = "_clrc_function"; |
| }) | ||
| [ | ||
| "_cliproxyapi_function" | ||
| "_clrc_function" |
| set -l claude_real (realpath (which claude)) | ||
| node $claude_real remote-control --worktree $argv |
| if ! (cd "$repo_dir" && git pull --ff-only 2>&1); then | ||
| log_warn " Fast-forward failed; rebasing onto remote..." | ||
| if ! (cd "$repo_dir" && git pull --rebase 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 | ||
| fi |
Summary
clrcabbreviation →_clrc_function(stable claude remote-control)clwrcabbreviation →_clwrc_function(remote-control in workspace git worktree)Test plan
clrcrunsclaude remote-controlwith stable realpath binaryclwrcrunsclaude remote-control --worktreewith stable realpath binary🤖 Generated with Claude Code
Summary by cubic
Add
fishabbreviationsclrcandclwrcto runclaude remote-controlvia a stable binary path (usingrealpath), withclwrcsupporting--worktree. Also hardens the repo updater to fall back togit pull --rebasewhen a fast-forward isn’t possible.New Features
clrc→_clrc_function; runsclaude remote-controlvianodeusingrealpathofclaude.clwrc→_clwrc_function; adds--worktree.home-managerfish config.Bug Fixes
scripts/update-local-binaries.sh, trygit pull --ff-only, then fall back togit pull --rebaseon diverged branches.Written for commit e4b1d4d. Summary will update on new commits.