Refactor cleanup sweep - #27
Merged
Merged
Conversation
Extract truncate, visible_range, render_search_bar, and a generic fuzzy_filter into ui/mod.rs and have every picker share them: - Remove pr_picker's verbatim copy of the tested adjust_scroll. - Unify four divergent truncate() variants (two reserved 3 chars + '...', two reserved 1 + '…') on one Unicode-aware '…' helper; stash_picker extracts the first line at the call site. - Replace the duplicated filter_branches/filter_worktrees with one fuzzy_filter<T>; it skips building the matcher on an empty query, so branch/workspace pickers no longer allocate a SkimMatcherV2 every frame. - Move test_truncate and test_visible_range to ui/mod.rs. No behavior change beyond the cosmetic '...' -> '…' convergence.
Introduce with_terminal()/with_terminal_stderr() closure runners backed by a TerminalGuard whose Drop restores cooked mode + the main screen. Teardown can no longer be forgotten or mis-ordered relative to an early '?' or panic — the footgun in the old setup/run/restore call pairs, where a future 'let x = run(&mut term)?;' before restore_terminal would strand the terminal on the error path. - Collapse the duplicated stdout/stderr setup+restore (4 fns) into one generic enter<W: Write>() + one generic guard. - Migrate all 8 call sites (log/checkout/add/stash/onboarding/pr/ workspace/workspace_clean) to the closure form. - Keep the panic hook as a belt-and-suspenders backstop.
- Add branch::branches_or_empty() to share the repo.branches()/UnbornBranch guard previously triplicated across get_branches, get_remote_branches, and worktree::list_local_branches. - Move the branch-enumeration helpers (get_local_branches, find_remote_branch, default_remote_branch) from worktree.rs to branch.rs, where the concern belongs; repoint callers to git::branch::*. - unpushed_count now delegates to the single git2 ahead/behind impl (branch::get_ahead_behind) instead of re-deriving it via graph_ahead_behind. - find_remote_branch sorts by borrow instead of cloning each key. The path-based worktree::ahead_behind (worktrees address by path) stays.
Five call sites independently re-implemented 'spawn gh, set GH_PROMPT_DISABLED, map missing-binary vs non-zero exit'. Introduce git/gh.rs (analogous to git_exec.rs) with one capture() primitive and route every site through it: - pr_search::gh_capture now just maps gh::GhError -> PrError. - pull_request, github resolve_pr_branch map into their own error enums. - reviewers' two best-effort gatherers and commands/pr current_login become gh::capture(...).ok(); drop the stale 'does not use gh_capture' comments now that the Accept-header/--jq cases go through it too. Also share the two byte-identical serde structs (RawAuthor/RawLogin and RawReviewRequest/RawReq) as gh::RawLogin / gh::RawReviewRequest, with the login||slug||name reviewer precedence as RawReviewRequest::handle(). Removes 5 std::process::Command imports.
git_exec changes:
- exec/exec_bytes now take 'impl IntoIterator<Item: AsRef<OsStr>>', so
callers pass &str/String/Path arrays without per-arg .to_string().
- Add exec_in/exec_bytes_in(dir, args, opts) for the 'git -C <dir>' clump
that was hand-spelled ~14 times.
- Add ExecOptions::capture()/silent() constructors, replacing 23 struct
literals.
Error typing:
- CommandFailed is now { stderr, code }, carrying the real exit code; the
inherit path surfaces the status instead of a 'Command failed' placeholder.
- Add PathspecNotFound(String) and StashNotFound(usize) so staging/stash
misses are typed instead of stuffed into the CommandFailed catch-all.
- ref_resolvable matches { code: Some(_), stderr empty } instead of doing
control flow on a bare error string.
Converts the worktree/branch/fetch exec call sites to the new API
(worktree.rs .to_string() count dropped sharply); leaves the already-clean
tested *_args builders and dynamic push/commit builders as-is.
Add src/output.rs (the plain-text counterpart to ui) that owns the load-bearing stdout/stderr split: - nav_path(): the single stdout writer (moved from workspace::print_go_path); every other helper writes to stderr. - cancelled(): replaces 17 inconsistent 'Cancelled'/'Cancelled.'/'Checkout cancelled.' sites (some on stdout, some stderr) with one stderr message. - warn(): the 'warning: ...' prefix, replacing 3 ad-hoc eprintlns. - bulleted_prompt(): collapses the 3 near-identical confirm-prompt builders (remove/clean/prune) into one 'header + blank + bullets' builder. Also delete onboarding.rs's copy of main_worktree_root and reuse the pub(crate) one from workspace.
Extract remove_one_worktree() + RemoveOutcome into workspace.rs, owning the git remove + 'contains modified or untracked files' dirty-recovery (prompt, then retry with --force) that was duplicated between 'gx workspace remove' and 'gx workspace clean' — including the verbatim confirm string. A fix to the recovery path now lives in one place. confirm_force_remove no longer prints on decline; each caller reports its own policy: remove aborts the operation (cancelled), clean skips the one workspace and continues the sweep.
Break the single worktree.rs into git/worktree/{model,ops,summary,staged,
cleanup}.rs by responsibility:
- model: Worktree type, fuzzy matching, porcelain parsing
- ops: list/add/remove/move/lock/unlock/repair lifecycle + arg builders
- summary: background dirty/ahead/behind + PR-state summarization
- staged: staged-index inspection (diff --cached / show :path)
- cleanup: branch-state queries + age/orphan/gone heuristics
mod.rs re-exports each submodule's public API, so every external
git::worktree::* call site is unchanged. Tests move next to their code;
shared fixtures live in a cfg(test) test_support module. No behavior change.
…tory
Break commands/workspace.rs into commands/workspace/{create,navigate,
lifecycle}.rs with a mod.rs facade:
- mod.rs: WorkspaceError, NewWorkspaceOptions, and the shared resolution/
path helpers (load_worktrees, resolve_target, main_worktree_root,
fuzzy_match_worktree, resolve_worktree_root, workspace_path, etc.) + tests
- create.rs: run_new + the creation pipeline
- navigate.rs: go/list/interactive picker + editor launching
- lifecycle.rs: remove/update/setup/sync/move/lock/repair + the multi-worktree
engines and the shared removal kernel
mod.rs re-exports each submodule, so args.rs and the sibling command modules
keep using commands::workspace::* unchanged. Also folds in the Task #7
preamble helpers: load_worktrees() / resolve_target() replace the
'list().map_err(...)' and 'fuzzy_match_worktree(...).ok_or_else(...)'
boilerplate repeated across ~14 handlers. No behavior change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation