feat: make source optional with --source/-s and --target/-t flags#8
feat: make source optional with --source/-s and --target/-t flags#8
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughこのPRは、CLIの引数構造をリファクタリングして、SOURCE と TARGET を位置引数から明示的なオプション(-s/--source、-t/--target)に変更します。また、-s を省略した場合の自動検出として git worktree の主要ディレクトリを検出する機能を追加しています。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as ユーザー/CLI
participant Main as main.rs
participant Git as git.rs
participant GitCmd as git コマンド
User->>Main: cargo run (-t オプションのみ、-s は省略)
Main->>Main: target をカレントディレクトリで初期化・正規化
Main->>Main: source が None か確認
alt source が None
Main->>Git: detect_main_worktree()
Git->>Git: current_dir() を取得
Git->>Git: detect_main_worktree_in(dir)
Git->>GitCmd: git worktree list --porcelain
GitCmd-->>Git: porcelain 出力を返却
Git->>Git: parse_main_worktree() で<br/>最初のワークツリー抽出・正規化
Git-->>Main: 検出した PathBuf を返却
else source が Some
Main->>Main: 提供された source を正規化・検証
end
Main->>Main: source と target の妥当性チェック<br/>(同一ではない、ネストしていない)
Main-->>User: 処理実行
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI interface to use --source/-s and --target/-t flags (instead of positional args), and adds automatic main-worktree detection when --source is omitted.
Changes:
- Replace positional
<SOURCE> [TARGET]with optional--source/-sand--target/-tflags, with target defaulting to.. - Add
git worktree list --porcelainbased detection of the main worktree when--sourceis not provided. - Update README usage and examples to match the new flag-based CLI.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main.rs | Switches source/target resolution to optional CLI flags and invokes main-worktree auto-detection. |
| src/cli.rs | Updates clap argument definitions to introduce --source/-s and --target/-t as options. |
| src/git.rs | Introduces worktree detection logic plus unit tests for parsing/detection behavior. |
| README.md | Updates documentation, options table, and examples for the new CLI interface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update the test to call parse_main_worktree() instead of duplicating parsing logic, and verify git commit success. Also fix README formatting by replacing "Don't" with "Do not" for consistency. 🤖 Generated with AI
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4536b2b42e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/git.rs (1)
54-113: テスト用ディレクトリの後始末を追加すると安全です。
main_dir/dirの削除が一部漏れているので、最後にremove_dir_allを追加すると一時領域の残留を防げます。🧹 例
// Cleanup let _ = Command::new("git") .args(["worktree", "remove", "--force", wt_dir.to_str().unwrap()]) .current_dir(&main_dir) .status(); let _ = fs::remove_dir_all(&wt_dir); + let _ = fs::remove_dir_all(&main_dir); } #[test] fn detect_main_worktree_from_main_returns_self() { let main_dir = git_tempdir("detect_self"); let detected = detect_main_worktree_in(&main_dir).unwrap(); assert_eq!(detected, main_dir); + let _ = fs::remove_dir_all(&main_dir); } #[test] fn detect_main_worktree_outside_git_repo_fails() { let dir = tempdir("detect_nogit"); let result = detect_main_worktree_in(&dir); assert!(result.is_err()); let err_msg = result.unwrap_err().to_string(); assert!( err_msg.contains("--source"), "Error message should mention --source, got: {err_msg}" ); + let _ = fs::remove_dir_all(&dir); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/git.rs` around lines 54 - 113, Tests create temporary git dirs but don't always remove them: add cleanup to avoid leaving temp directories. In the tests detect_main_worktree_returns_main_path, detect_main_worktree_from_main_returns_self and detect_main_worktree_outside_git_repo_fails call git_tempdir/ tempdir to create main_dir/dir — after each test add a best-effort fs::remove_dir_all(...) (or equivalent) for those directories (and ensure the worktree removal in detect_main_worktree_returns_main_path remains), handling errors with ignore to avoid failing the test cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.rs`:
- Around line 38-49: When cli.source is omitted the code calls
git::detect_main_worktree() which checks the current working directory; change
the None branch so detection is based on the resolved --target path instead of
the process CWD: canonicalize cli.target (or the resolved target path variable),
ensure it's a directory, and pass that path into git detection (e.g., update or
call a git::detect_main_worktree_from/at function) so the detected main worktree
is derived from the target location rather than the current working directory;
update references to variables source and target accordingly and surface any
canonicalization errors with the existing with_context/bail pattern.
---
Nitpick comments:
In `@src/git.rs`:
- Around line 54-113: Tests create temporary git dirs but don't always remove
them: add cleanup to avoid leaving temp directories. In the tests
detect_main_worktree_returns_main_path,
detect_main_worktree_from_main_returns_self and
detect_main_worktree_outside_git_repo_fails call git_tempdir/ tempdir to create
main_dir/dir — after each test add a best-effort fs::remove_dir_all(...) (or
equivalent) for those directories (and ensure the worktree removal in
detect_main_worktree_returns_main_path remains), handling errors with ignore to
avoid failing the test cleanup.
- Revert target to non-optional PathBuf with default_value="." so clap shows the default in --help output - Include git stderr in error message when `git worktree list` fails - Remove unused PathBuf import from main.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When --source is omitted, auto-detection now runs from the resolved --target path rather than the process CWD. This prevents detecting the wrong repository when running from a different location. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
<SOURCE> [TARGET]with optional flags--source/-sand--target/-tgit worktree list --porcelainwhen--sourceis omitted--targetis omittedChanges
New module:
src/git.rsdetect_main_worktree()— auto-detects main worktree by parsinggit worktree list --porcelaindetect_main_worktree_in(dir)— testable variant that accepts a working directory--sourceas fallbackCLI changes:
src/cli.rssource: positional requiredPathBuf→ optional flagOption<PathBuf>with-s, --sourcetarget: positional with defaultPathBuf→ optional flagOption<PathBuf>with-t, --targetResolution logic:
src/main.rsDocumentation:
README.md--no-ignoreoption to the tableBreaking Changes
<SOURCE> [TARGET]removed--source/--targetflagsTest Plan
detect_main_worktree()(from worktree, from main, non-git error)cargo fmt --checkcleancargo clippy -- -D warningscleancargo run -- --helpshows updated flags🤖 Generated with AI