diff --git a/docs/perf-worktree-entry-plan.md b/docs/perf-worktree-entry-plan.md new file mode 100644 index 0000000..12541bb --- /dev/null +++ b/docs/perf-worktree-entry-plan.md @@ -0,0 +1,307 @@ +# Worktree Entry Latency Plan + +## Goal + +`create` と `batch delete` の初期レスポンスを改善する。 + +今回の主眼は「既存の挙動を壊さずに速くすること」であり、UI 文言や選択フローの変更は目的に含めない。 + +## Current Status + +以下は実装済み。 + +- Phase 1 + - `create` は `has_linked_worktrees()` を使う +- Phase 2 + - `batch delete` は `list_worktrees_basic()` を使う + - `is_branch_unique_to_worktree()` は軽量一覧ベース +- Phase 3 + - `rename / switch / search` も軽量一覧を使う + +以下は意図的に未変更。 + +- `list` + - 詳細表示のため、引き続き `list_worktrees()` を使う +- `WorktreeInfo` + - public API と既存 test を壊さないため維持する +- detailed status 取得 + - `has_changes / last_commit / ahead_behind` は `list` 系のために残す + +## Observed Symptoms + +- `create` は worktree 数が増えると、画面に入って最初の prompt が出るまで遅くなる +- `batch delete` は worktree 数が増えると、画面表示前と選択後の summary 生成で遅くなる + +## Root Cause Analysis + +### 1. `create` の入口が重い + +対象: +- [create_worktree_with_ui](/Users/a12622/git/git-workers/src/usecases/create_worktree.rs:79) + +現状: +- 冒頭で `manager.list_worktrees()?` を呼んでいる +- 使い道は `has_worktrees = !existing_worktrees.is_empty()` のみ + +問題: +- 件数確認しかしたくないのに、一覧表示向けの重い取得をしている + +### 2. `batch delete` の入口が重い + +対象: +- [batch_delete_worktrees_internal](/Users/a12622/git/git-workers/src/usecases/delete_worktree.rs:240) + +現状: +- 冒頭で `manager.list_worktrees()?` を呼んでいる +- 候補生成には `name / branch / current` 程度しか使っていない + +問題: +- 入口で full status を取りにいっている + +### 3. `batch delete` の後段も重い + +対象: +- [is_branch_unique_to_worktree](/Users/a12622/git/git-workers/src/infrastructure/git.rs:1156) + +現状: +- 内部で再び `self.list_worktrees()?` を呼んでいる +- 選択した worktree 数ぶん呼ばれる + +問題: +- 選択後の summary 生成が `O(selected_count * full_list_cost)` になる + +### 4. 重い本体 API + +対象: +- [GitWorktreeManager::list_worktrees](/Users/a12622/git/git-workers/src/infrastructure/git.rs:240) + +現状: +- 各 worktree ごとに以下を取得している + - `Repository::open(path)` + - branch 名 + - `repo.statuses(...)` + - last commit + +結論: +- `list_worktrees()` は一覧画面には妥当だが、軽量画面の入口には重すぎる + +## Design Principle + +重い API を最適化して全部を一度に変えるのではなく、用途別に API を分ける。 + +### Keep + +- `list_worktrees()` + - 一覧画面向けの詳細 API として残す + +### Add + +- `has_linked_worktrees()` + - linked worktree の有無だけを確認する軽量 API +- `list_worktrees_basic()` + - 軽量一覧 API + +### Precise Semantics + +- `has_linked_worktrees()` + - `main` worktree は数えない + - `git worktree list` 上の linked worktree が 1 件以上あるかだけを見る + - `create` の first-worktree 分岐と完全に同じ意味で使う +- `list_worktrees_basic()` + - 候補表示や選択解決に必要な最小情報だけ返す + - `list_worktrees()` の軽量版であり、表示順と identity 解決規則は現行と同じにする + +## Proposed Data Model + +新規に lightweight な struct を追加する。 + +例: + +```rust +pub struct BasicWorktreeInfo { + pub name: String, + pub git_name: String, + pub path: PathBuf, + pub branch: String, + pub is_current: bool, + pub is_locked: bool, +} +``` + +### Included + +- `name` +- `git_name` +- `path` +- `branch` +- `is_current` +- `is_locked` + +### Excluded + +- `has_changes` +- `last_commit` +- `ahead_behind` + +理由: +- `create / batch delete / rename / switch / search` の入口では不要 + +### Identity Contract + +- `name` + - UI 表示名として使う + - renamed worktree の表示も現行と同じくこの値を使う +- `git_name` + - destructive operation や内部解決に使う + - `remove`, `rename`, orphaned branch 判定の対象解決はこの値を基準にする +- renamed worktree が存在しても、現行の `name` と `git_name` の役割分担を変えない +- `list_worktrees_basic()` と `list_worktrees()` は、同じ worktree に対して同じ `name / git_name` の組を返す + +## Safe Refactoring Scope + +### Phase 1 + +`create` のみ軽量化する。 + +- `manager.list_worktrees()?` をやめる +- `manager.has_linked_worktrees()?` を導入して置き換える +- この phase では `BasicWorktreeInfo` と `list_worktrees_basic()` はまだ導入しない + +期待効果: +- 件数依存の初動遅延をかなり削減できる + +### Phase 2 + +`batch delete` を軽量化する。 + +- `manager.list_worktrees()?` を `manager.list_worktrees_basic()?` に置き換える +- `is_branch_unique_to_worktree()` を軽量一覧ベースに切り替える +- 候補順と選択 index の対応は現行のまま維持する + +期待効果: +- 入口の待ち時間を削減 +- summary 生成の二次的な遅延も削減 + +### Phase 3 + +同じ構造の他画面を軽量化する。 + +候補: +- `rename` +- `switch` +- `search` + +状態: +- 実装済み + +### Explicitly Out of Scope + +- `list` の軽量化 +- 文言変更 +- メニュー順変更 +- hook の仕様変更 +- path / branch の表示仕様変更 + +## Compatibility Rules + +以下は絶対に変えない。 + +- `create` + - worktree 0 件のときだけ location 選択を出す + - 1 件以上なら location 選択を出さない + - ここでいう件数は linked worktree 数だけを意味する +- `batch delete` + - current worktree を候補に出さない + - orphaned branch 判定の意味を変えない + - 候補順を変えない + - 選択 index と削除対象の対応を変えない + - renamed worktree がある場合も、表示は `name`、削除対象解決は現行と同じ identity を使う +- `rename / switch / search` + - current 判定を変えない + - 選択対象の path / branch を変えない +- `list` + - 表示内容は現状維持 + +## Test Plan + +### Existing Behavior Lock + +既存の以下を回帰として守る。 + +- `create` 系 test +- `batch delete` 系 test +- `switch / rename / search` 系 test +- `custom_path_behavior_test` +- `worktree lifecycle` 系 integration test + +### New Tests + +#### `create` + +- `has_linked_worktrees() == false` なら first-worktree location prompt が出る +- `has_linked_worktrees() == true` なら first-worktree location prompt が出ない +- `main` worktree しかない repo では location prompt が出る +- linked worktree が 1 件以上ある repo では location prompt が出ない + +#### `batch delete` + +- `list_worktrees_basic()` ベースでも候補一覧が現状と一致する +- `is_branch_unique_to_worktree()` の結果が現状と一致する +- 候補順が現状と一致する +- 選択 index と実際の削除対象の対応が現状と一致する +- renamed worktree がある場合も `name / git_name` の解決が現状と一致する + +#### `basic vs full` + +- 同じ repo で + - `name` + - `git_name` + - `path` + - `branch` + - `is_current` + - `is_locked` + が `list_worktrees_basic()` と `list_worktrees()` で一致する + +## Validation Commands + +最低限、各 phase ごとに以下を通す。 + +```bash +cargo fmt --check +cargo clippy --all-features -- -D warnings +cargo test --all-features -- --test-threads=1 +``` + +必要なら targeted 実行: + +```bash +cargo test --all-features unit::commands::create -- --test-threads=1 +cargo test --all-features unit::commands::delete -- --test-threads=1 +``` + +## Success Criteria + +- `create` の初期 prompt までの待ち時間が件数増加に対して改善される +- `batch delete` の画面表示前と summary 生成前の待ち時間が改善される +- `rename / switch / search` の一覧表示前の待ち時間も改善される +- full test が green +- 観測可能な挙動差分がない + +## Implementation Notes + +- まず `infrastructure::git` に軽量 API を追加する +- 既存の `WorktreeInfo` は残す +- `BasicWorktreeInfo` から `WorktreeInfo` への安易な変換は作らない + - 不要な status 取得を誘発しやすいため +- `batch delete` の branch uniqueness は、一覧取得結果の再利用を優先する +- `create` は `list_worktrees_basic()` を待たず、`has_linked_worktrees()` 単独で先に改善する + +## Recommended First Patch + +最初の 1 patch はここまでに限定する。 + +1. `GitWorktreeManager::has_linked_worktrees()` +2. `create` を `has_linked_worktrees()` に切り替え +3. first-worktree 分岐の回帰 test 追加 + +この単位なら、効果が大きく、差分も最小で済む。 diff --git a/src/adapters/git/git_worktree_repository.rs b/src/adapters/git/git_worktree_repository.rs index fb7d581..5546f8a 100644 --- a/src/adapters/git/git_worktree_repository.rs +++ b/src/adapters/git/git_worktree_repository.rs @@ -1 +1,3 @@ -pub use crate::infrastructure::git::{CommitInfo, GitWorktreeManager, WorktreeInfo}; +pub use crate::infrastructure::git::{ + BasicWorktreeInfo, CommitInfo, GitWorktreeManager, WorktreeInfo, +}; diff --git a/src/adapters/git/mod.rs b/src/adapters/git/mod.rs index 21316c4..d1835b8 100644 --- a/src/adapters/git/mod.rs +++ b/src/adapters/git/mod.rs @@ -2,6 +2,8 @@ pub mod git_worktree_repository; pub mod repo_discovery; pub mod worktree_lock; -pub use git_worktree_repository::{CommitInfo, GitWorktreeManager, WorktreeInfo}; +pub use git_worktree_repository::{ + BasicWorktreeInfo, CommitInfo, GitWorktreeManager, WorktreeInfo, +}; pub use repo_discovery::{open_repository_at_path, open_repository_from_env}; pub use worktree_lock::WorktreeLock; diff --git a/src/domain/worktree.rs b/src/domain/worktree.rs index f483149..228b933 100644 --- a/src/domain/worktree.rs +++ b/src/domain/worktree.rs @@ -1 +1 @@ -pub use crate::adapters::git::WorktreeInfo; +pub use crate::adapters::git::{BasicWorktreeInfo, WorktreeInfo}; diff --git a/src/infrastructure/git.rs b/src/infrastructure/git.rs index 7a9ff3b..2e09d3d 100644 --- a/src/infrastructure/git.rs +++ b/src/infrastructure/git.rs @@ -293,6 +293,65 @@ impl GitWorktreeManager { Ok(worktrees) } + /// Returns whether the repository has any linked worktrees. + /// + /// This intentionally does not count the main worktree. Its semantics must + /// stay aligned with `list_worktrees()` and the `create` first-worktree flow. + pub fn has_linked_worktrees(&self) -> Result { + let worktree_names = self.repo.worktrees()?; + Ok(worktree_names.iter().flatten().next().is_some()) + } + + /// Lists linked worktrees without loading status or commit metadata. + /// + /// This preserves the same ordering and identity fields as `list_worktrees()`, + /// but avoids the expensive per-worktree status scan used by detailed views. + pub fn list_worktrees_basic(&self) -> Result> { + let mut worktrees = Vec::new(); + let worktree_names = self.repo.worktrees()?; + + for name in worktree_names.iter().flatten() { + if let Ok(worktree) = self.repo.find_worktree(name) { + let path = worktree.path(); + let is_current = self.is_current_worktree(path); + let is_locked = worktree.is_locked().is_ok(); + + let branch = if let Ok(wt_repo) = Repository::open(path) { + if let Ok(head) = wt_repo.head() { + if let Some(shorthand) = head.shorthand() { + shorthand.to_string() + } else { + String::from(DEFAULT_BRANCH_DETACHED) + } + } else { + String::from(DEFAULT_BRANCH_DETACHED) + } + } else { + String::from(DEFAULT_BRANCH_UNKNOWN) + }; + + let display_name = path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(name) + .to_string(); + + worktrees.push(BasicWorktreeInfo { + name: display_name, + git_name: name.to_string(), + path: path.to_path_buf(), + branch, + is_locked, + is_current, + }); + } + } + + worktrees.sort_by(|a, b| a.name.cmp(&b.name)); + + Ok(worktrees) + } + /// Checks if the given path is the current worktree /// /// # Arguments @@ -1158,7 +1217,7 @@ impl GitWorktreeManager { branch_name: &str, worktree_name: &str, ) -> Result { - let worktrees = self.list_worktrees()?; + let worktrees = self.list_worktrees_basic()?; let mut count = 0; let mut found_in_target = false; @@ -1583,6 +1642,26 @@ pub struct WorktreeInfo { pub ahead_behind: Option<(usize, usize)>, // (ahead, behind) } +/// Lightweight information about a Git worktree. +/// +/// This keeps only the fields needed for selection UIs and identity resolution, +/// without opening each worktree to inspect status or commit metadata. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BasicWorktreeInfo { + /// The display name of the worktree (derived from the directory name) + pub name: String, + /// The internal Git name of the worktree (from .git/worktrees/) + pub git_name: String, + /// The absolute filesystem path to the worktree + pub path: PathBuf, + /// The current branch name or "detached" if in detached HEAD state + pub branch: String, + /// Whether the worktree is locked (prevents deletion) + pub is_locked: bool, + /// Whether this is the currently active worktree + pub is_current: bool, +} + /// Information about a Git commit /// /// Contains basic information about a commit for display purposes. diff --git a/src/usecases/create_worktree.rs b/src/usecases/create_worktree.rs index e49a47a..ef03b9e 100644 --- a/src/usecases/create_worktree.rs +++ b/src/usecases/create_worktree.rs @@ -85,8 +85,7 @@ pub fn create_worktree_with_ui( println!("{header}"); println!(); - let existing_worktrees = manager.list_worktrees()?; - let has_worktrees = !existing_worktrees.is_empty(); + let has_linked_worktrees = manager.has_linked_worktrees()?; let name = match ui.input(PROMPT_WORKTREE_NAME) { Ok(name) => name.trim().to_string(), @@ -106,7 +105,7 @@ pub fn create_worktree_with_ui( } }; - let final_name = if !has_worktrees { + let final_name = if !has_linked_worktrees { println!(); let msg = MSG_FIRST_WORKTREE_CHOOSE.bright_cyan(); println!("{msg}"); diff --git a/src/usecases/delete_worktree.rs b/src/usecases/delete_worktree.rs index 1fc4ca9..6258cfd 100644 --- a/src/usecases/delete_worktree.rs +++ b/src/usecases/delete_worktree.rs @@ -5,7 +5,7 @@ use dialoguer::{Confirm, MultiSelect}; use crate::adapters::git::GitWorktreeManager; use crate::adapters::hooks::{self, HookContext}; use crate::constants::{section_header, DEFAULT_MENU_SELECTION, HOOK_PRE_REMOVE}; -use crate::domain::worktree::WorktreeInfo; +use crate::domain::worktree::{BasicWorktreeInfo, WorktreeInfo}; use crate::ui::{DialoguerUI, UserInterface}; use crate::utils::{self, get_theme, press_any_key_to_continue}; @@ -71,6 +71,20 @@ pub fn prepare_batch_delete_items(worktrees: &[WorktreeInfo]) -> Vec { .collect() } +/// Pure business logic for filtering deletable worktrees for batch operations. +pub fn get_deletable_worktrees_basic(worktrees: &[BasicWorktreeInfo]) -> Vec<&BasicWorktreeInfo> { + worktrees.iter().filter(|w| !w.is_current).collect() +} + +/// Pure business logic for preparing batch delete labels from lightweight worktrees. +pub fn prepare_batch_delete_items_basic(worktrees: &[BasicWorktreeInfo]) -> Vec { + worktrees + .iter() + .filter(|w| !w.is_current) + .map(|w| format!("{} ({})", w.name, w.branch)) + .collect() +} + /// Pure business logic for analyzing deletion requirements pub fn analyze_deletion( worktree: &WorktreeInfo, @@ -238,7 +252,7 @@ pub fn batch_delete_worktrees() -> Result<()> { } fn batch_delete_worktrees_internal(manager: &GitWorktreeManager) -> Result<()> { - let worktrees = manager.list_worktrees()?; + let worktrees = manager.list_worktrees_basic()?; if worktrees.is_empty() { println!(); @@ -249,8 +263,7 @@ fn batch_delete_worktrees_internal(manager: &GitWorktreeManager) -> Result<()> { return Ok(()); } - let deletable_worktrees: Vec<&WorktreeInfo> = - worktrees.iter().filter(|w| !w.is_current).collect(); + let deletable_worktrees = get_deletable_worktrees_basic(&worktrees); if deletable_worktrees.is_empty() { println!(); @@ -270,7 +283,7 @@ fn batch_delete_worktrees_internal(manager: &GitWorktreeManager) -> Result<()> { println!("{header}"); println!(); - let items = prepare_batch_delete_items(&worktrees); + let items = prepare_batch_delete_items_basic(&worktrees); let selections = MultiSelect::with_theme(&get_theme()) .with_prompt( @@ -284,7 +297,7 @@ fn batch_delete_worktrees_internal(manager: &GitWorktreeManager) -> Result<()> { _ => return Ok(()), }; - let selected_worktrees: Vec<&WorktreeInfo> = + let selected_worktrees: Vec<&BasicWorktreeInfo> = selections.iter().map(|&i| deletable_worktrees[i]).collect(); let mut branches_to_delete = Vec::new(); @@ -573,4 +586,66 @@ mod tests { assert!(items[0].contains("feature/test")); assert!(!items[0].contains("main")); } + + #[test] + fn test_prepare_batch_delete_items_basic_preserves_order_and_filters_current() { + let worktrees = vec![ + BasicWorktreeInfo { + name: "z-current".to_string(), + git_name: "z-current".to_string(), + path: std::path::PathBuf::from("/test/z-current"), + branch: "main".to_string(), + is_current: true, + is_locked: false, + }, + BasicWorktreeInfo { + name: "alpha".to_string(), + git_name: "alpha".to_string(), + path: std::path::PathBuf::from("/test/alpha"), + branch: "alpha".to_string(), + is_current: false, + is_locked: false, + }, + BasicWorktreeInfo { + name: "beta-renamed".to_string(), + git_name: "beta-original".to_string(), + path: std::path::PathBuf::from("/test/beta-renamed"), + branch: "beta-branch".to_string(), + is_current: false, + is_locked: false, + }, + ]; + + let items = prepare_batch_delete_items_basic(&worktrees); + + assert_eq!(items, vec!["alpha (alpha)", "beta-renamed (beta-branch)"]); + } + + #[test] + fn test_get_deletable_worktrees_basic_preserves_identity_mapping() { + let worktrees = vec![ + BasicWorktreeInfo { + name: "main".to_string(), + git_name: "main".to_string(), + path: std::path::PathBuf::from("/test/main"), + branch: "main".to_string(), + is_current: true, + is_locked: false, + }, + BasicWorktreeInfo { + name: "renamed-feature".to_string(), + git_name: "feature-original".to_string(), + path: std::path::PathBuf::from("/test/renamed-feature"), + branch: "feature-branch".to_string(), + is_current: false, + is_locked: false, + }, + ]; + + let deletable = get_deletable_worktrees_basic(&worktrees); + + assert_eq!(deletable.len(), 1); + assert_eq!(deletable[0].name, "renamed-feature"); + assert_eq!(deletable[0].git_name, "feature-original"); + } } diff --git a/src/usecases/rename_worktree.rs b/src/usecases/rename_worktree.rs index 1b80e8f..6d6ba58 100644 --- a/src/usecases/rename_worktree.rs +++ b/src/usecases/rename_worktree.rs @@ -6,7 +6,7 @@ use crate::constants::{ section_header, DEFAULT_BRANCH_DETACHED, DEFAULT_BRANCH_UNKNOWN, DEFAULT_MENU_SELECTION, }; use crate::domain::validation::validate_worktree_name; -use crate::domain::worktree::WorktreeInfo; +use crate::domain::worktree::{BasicWorktreeInfo, WorktreeInfo}; use crate::ui::{DialoguerUI, UserInterface}; use crate::utils::{self, press_any_key_to_continue}; @@ -52,6 +52,23 @@ pub struct RenameAnalysis { pub is_feature_branch: bool, } +fn lightweight_worktrees_to_display(worktrees: Vec) -> Vec { + worktrees + .into_iter() + .map(|worktree| WorktreeInfo { + name: worktree.name, + git_name: worktree.git_name, + path: worktree.path, + branch: worktree.branch, + is_locked: worktree.is_locked, + is_current: worktree.is_current, + has_changes: false, + last_commit: None, + ahead_behind: None, + }) + .collect() +} + /// Pure business logic for filtering renameable worktrees pub fn get_renameable_worktrees(worktrees: &[WorktreeInfo]) -> Vec<&WorktreeInfo> { worktrees.iter().filter(|w| !w.is_current).collect() @@ -151,7 +168,7 @@ pub fn rename_worktree() -> Result<()> { /// Internal implementation of rename_worktree with dependency injection pub fn rename_worktree_with_ui(manager: &GitWorktreeManager, ui: &dyn UserInterface) -> Result<()> { - let worktrees = manager.list_worktrees()?; + let worktrees = lightweight_worktrees_to_display(manager.list_worktrees_basic()?); if worktrees.is_empty() { println!(); diff --git a/src/usecases/search_worktrees.rs b/src/usecases/search_worktrees.rs index 346a31e..8202aa6 100644 --- a/src/usecases/search_worktrees.rs +++ b/src/usecases/search_worktrees.rs @@ -9,7 +9,7 @@ use crate::constants::{ MSG_NO_WORKTREES_TO_SEARCH, MSG_SEARCH_FUZZY_ENABLED, PROMPT_SELECT_WORKTREE_SWITCH, SEARCH_CURRENT_INDICATOR, }; -use crate::domain::worktree::WorktreeInfo; +use crate::domain::worktree::{BasicWorktreeInfo, WorktreeInfo}; use crate::utils::{self, get_theme, press_any_key_to_continue}; #[derive(Debug, Clone)] @@ -25,6 +25,23 @@ pub struct SearchAnalysis { pub has_current: bool, } +fn lightweight_worktrees_to_display(worktrees: Vec) -> Vec { + worktrees + .into_iter() + .map(|worktree| WorktreeInfo { + name: worktree.name, + git_name: worktree.git_name, + path: worktree.path, + branch: worktree.branch, + is_locked: worktree.is_locked, + is_current: worktree.is_current, + has_changes: false, + last_commit: None, + ahead_behind: None, + }) + .collect() +} + pub fn create_search_items(worktrees: &[WorktreeInfo]) -> SearchAnalysis { let items: Vec = worktrees .iter() @@ -63,7 +80,7 @@ pub fn search_worktrees() -> Result { } fn search_worktrees_internal(manager: &GitWorktreeManager) -> Result { - let worktrees = manager.list_worktrees()?; + let worktrees = lightweight_worktrees_to_display(manager.list_worktrees_basic()?); if worktrees.is_empty() { println!(); diff --git a/src/usecases/switch_worktree.rs b/src/usecases/switch_worktree.rs index 70d3551..5b60355 100644 --- a/src/usecases/switch_worktree.rs +++ b/src/usecases/switch_worktree.rs @@ -7,7 +7,7 @@ use crate::adapters::shell::switch_file::write_switch_path; use crate::constants::{ section_header, DEFAULT_MENU_SELECTION, HOOK_POST_SWITCH, MSG_ALREADY_IN_WORKTREE, }; -use crate::domain::worktree::WorktreeInfo; +use crate::domain::worktree::{BasicWorktreeInfo, WorktreeInfo}; use crate::ui::{DialoguerUI, UserInterface}; use crate::utils::{self, press_any_key_to_continue}; @@ -60,6 +60,23 @@ pub struct SwitchAnalysis { pub is_already_current: bool, } +fn lightweight_worktrees_to_display(worktrees: Vec) -> Vec { + worktrees + .into_iter() + .map(|worktree| WorktreeInfo { + name: worktree.name, + git_name: worktree.git_name, + path: worktree.path, + branch: worktree.branch, + is_locked: worktree.is_locked, + is_current: worktree.is_current, + has_changes: false, + last_commit: None, + ahead_behind: None, + }) + .collect() +} + /// Pure business logic for sorting worktrees for display pub fn sort_worktrees_for_display(mut worktrees: Vec) -> Vec { worktrees.sort_by(|a, b| { @@ -122,7 +139,7 @@ pub fn switch_worktree_with_ui( manager: &GitWorktreeManager, ui: &dyn UserInterface, ) -> Result { - let worktrees = manager.list_worktrees()?; + let worktrees = lightweight_worktrees_to_display(manager.list_worktrees_basic()?); if worktrees.is_empty() { println!(); diff --git a/tests/custom_path_behavior_test.rs b/tests/custom_path_behavior_test.rs index bc7fa8a..4ca9329 100644 --- a/tests/custom_path_behavior_test.rs +++ b/tests/custom_path_behavior_test.rs @@ -42,6 +42,57 @@ fn test_custom_path_appends_worktree_name() -> Result<()> { Ok(()) } +/// Test that the first linked worktree still shows the location prompt +#[test] +fn test_first_worktree_still_prompts_for_location() -> Result<()> { + let temp_dir = TempDir::new()?; + let test_repo = TestRepo::new(&temp_dir)?; + let manager = test_repo.manager()?; + + let ui = TestUI::new() + .with_input("first-linked") + .with_selection(1) // custom path option from the first-worktree prompt + .with_input("work/") + .with_selection(0) // create from HEAD + .with_confirmation(false); + + let result = create_worktree_with_ui(&manager, &ui)?; + assert!(!result); + + let worktrees = manager.list_worktrees()?; + let worktree = worktrees + .iter() + .find(|w| w.name == "first-linked") + .expect("first-linked worktree should exist"); + assert!(worktree.path.ends_with("work/first-linked")); + + Ok(()) +} + +/// Test that once a linked worktree exists, create skips the location prompt +#[test] +fn test_subsequent_worktree_skips_location_prompt() -> Result<()> { + let temp_dir = TempDir::new()?; + let test_repo = TestRepo::new(&temp_dir)?; + let manager = test_repo.manager()?; + + manager.create_worktree_with_new_branch("seed-worktree", "seed-worktree", "main")?; + + let ui = TestUI::new() + .with_input("second-linked") + .with_selection(0) // branch option only; if location prompt appears this test fails + .with_confirmation(false); + + let result = create_worktree_with_ui(&manager, &ui)?; + assert!(!result); + + let worktrees = manager.list_worktrees()?; + assert!(worktrees.iter().any(|w| w.name == "seed-worktree")); + assert!(worktrees.iter().any(|w| w.name == "second-linked")); + + Ok(()) +} + /// Test that "./" creates worktree in project root #[test] fn test_dot_slash_creates_in_project_root() -> Result<()> { diff --git a/tests/unit/infrastructure/git.rs b/tests/unit/infrastructure/git.rs index c3b5ae9..8a2c8f7 100644 --- a/tests/unit/infrastructure/git.rs +++ b/tests/unit/infrastructure/git.rs @@ -133,6 +133,15 @@ fn test_list_worktrees_with_main() -> Result<()> { Ok(()) } +#[test] +fn test_has_linked_worktrees_ignores_main_worktree() -> Result<()> { + let (_temp_dir, manager) = setup_repo_with_commit()?; + + assert!(!manager.has_linked_worktrees()?); + + Ok(()) +} + #[test] fn test_list_worktrees_multiple() -> Result<()> { let (_temp_dir, manager) = setup_repo_with_commit()?; @@ -161,6 +170,78 @@ fn test_list_worktrees_multiple() -> Result<()> { Ok(()) } +#[test] +fn test_list_worktrees_basic_matches_full_fields_and_order() -> Result<()> { + let (_temp_dir, manager) = setup_repo_with_commit()?; + + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_millis(); + let alpha_name = format!("alpha-{timestamp}"); + let beta_name = format!("beta-{timestamp}"); + + manager.create_worktree_with_new_branch(&beta_name, &beta_name, "main")?; + manager.create_worktree_with_new_branch(&alpha_name, &alpha_name, "main")?; + + let basic = manager.list_worktrees_basic()?; + let full = manager.list_worktrees()?; + + assert_eq!(basic.len(), full.len()); + + for (basic_info, full_info) in basic.iter().zip(full.iter()) { + assert_eq!(basic_info.name, full_info.name); + assert_eq!(basic_info.git_name, full_info.git_name); + assert_eq!(basic_info.path, full_info.path); + assert_eq!(basic_info.branch, full_info.branch); + assert_eq!(basic_info.is_current, full_info.is_current); + assert_eq!(basic_info.is_locked, full_info.is_locked); + } + + Ok(()) +} + +#[test] +fn test_list_worktrees_basic_preserves_renamed_identity() -> Result<()> { + let (_temp_dir, manager) = setup_repo_with_commit()?; + + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_millis(); + let original_name = format!("original-{timestamp}"); + let branch_name = format!("branch-{timestamp}"); + let renamed_name = format!("renamed-{timestamp}"); + + manager.create_worktree_with_new_branch(&original_name, &branch_name, "main")?; + manager.rename_worktree(&original_name, &renamed_name)?; + + let basic = manager.list_worktrees_basic()?; + let renamed = basic + .iter() + .find(|w| w.name == renamed_name) + .expect("renamed worktree should exist in basic listing"); + + assert_eq!(renamed.git_name, original_name); + assert_eq!(renamed.branch, branch_name); + + Ok(()) +} + +#[test] +fn test_has_linked_worktrees_detects_linked_worktree() -> Result<()> { + let (_temp_dir, manager) = setup_repo_with_commit()?; + + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_millis(); + let worktree_name = format!("linked-{timestamp}"); + + manager.create_worktree_with_new_branch(&worktree_name, &worktree_name, "main")?; + + assert!(manager.has_linked_worktrees()?); + + Ok(()) +} + // ============================================================================ // Branch Management Tests // ============================================================================