From acfdcaa8be8468fba390279b77a546c9afa7ea87 Mon Sep 17 00:00:00 2001 From: Daniels-Main Date: Thu, 11 Jun 2026 02:15:12 +0200 Subject: [PATCH 1/2] fable features --- ROADMAP.md | 2 + TASKS.md | 103 +++++++- crates/strand-core/src/branch.rs | 77 ++++++ crates/strand-core/src/commit.rs | 315 +++++++++++++++++++++-- crates/strand-core/src/file.rs | 146 +++++++++++ crates/strand-core/src/ignore.rs | 102 ++++++++ crates/strand-core/src/lib.rs | 3 + crates/strand-core/src/network.rs | 4 +- crates/strand-core/src/remote.rs | 200 +++++++++++++++ crates/strand-core/src/reset.rs | 238 ++++++++++++++++++ crates/strand-tauri/src/commands.rs | 69 ++++- crates/strand-tauri/src/main.rs | 8 + docs/improvements.md | 34 +++ docs/learnings.md | 46 ++++ ui/src/App.tsx | 141 ++++++++++- ui/src/components/DiffSearchBar.tsx | 153 +++++++++++ ui/src/components/ImageDiff.tsx | 118 +++++++++ ui/src/components/Sidebar.tsx | 114 ++++++++- ui/src/lib/db.ts | 8 +- ui/src/lib/diffSearch.test.ts | 87 +++++++ ui/src/lib/diffSearch.ts | 83 ++++++ ui/src/lib/ignore.test.ts | 53 ++++ ui/src/lib/ignore.ts | 27 ++ ui/src/lib/image.test.ts | 40 +++ ui/src/lib/image.ts | 41 +++ ui/src/lib/patchExport.test.ts | 57 +++++ ui/src/lib/patchExport.ts | 49 ++++ ui/src/lib/rebase.test.ts | 113 +++++++++ ui/src/lib/rebase.ts | 77 ++++++ ui/src/lib/reviewExport.test.ts | 140 +++++++++++ ui/src/lib/reviewExport.ts | 88 +++++++ ui/src/lib/tauri.ts | 23 ++ ui/src/lib/types.ts | 33 +++ ui/src/stores/repo.ts | 159 +++++++++++- ui/src/styles/features.css | 222 ++++++++++++++++ ui/src/views/CommitDetail.tsx | 18 +- ui/src/views/Commits.tsx | 31 ++- ui/src/views/LocalChanges.tsx | 148 ++++++++++- ui/src/views/RebaseEditor.tsx | 41 ++- ui/src/views/Reflog.tsx | 150 ++++++++--- ui/src/views/RemoteDialog.tsx | 203 +++++++++++++++ ui/src/views/RenameBranchDialog.tsx | 150 +++++++++++ ui/src/views/ResetDialog.tsx | 164 ++++++++++++ ui/src/views/Review.tsx | 378 +++++++++++++++++++++------- 44 files changed, 4252 insertions(+), 204 deletions(-) create mode 100644 crates/strand-core/src/ignore.rs create mode 100644 crates/strand-core/src/remote.rs create mode 100644 crates/strand-core/src/reset.rs create mode 100644 ui/src/components/DiffSearchBar.tsx create mode 100644 ui/src/components/ImageDiff.tsx create mode 100644 ui/src/lib/diffSearch.test.ts create mode 100644 ui/src/lib/diffSearch.ts create mode 100644 ui/src/lib/ignore.test.ts create mode 100644 ui/src/lib/ignore.ts create mode 100644 ui/src/lib/image.test.ts create mode 100644 ui/src/lib/image.ts create mode 100644 ui/src/lib/patchExport.test.ts create mode 100644 ui/src/lib/patchExport.ts create mode 100644 ui/src/lib/rebase.test.ts create mode 100644 ui/src/lib/rebase.ts create mode 100644 ui/src/lib/reviewExport.test.ts create mode 100644 ui/src/lib/reviewExport.ts create mode 100644 ui/src/views/RemoteDialog.tsx create mode 100644 ui/src/views/RenameBranchDialog.tsx create mode 100644 ui/src/views/ResetDialog.tsx diff --git a/ROADMAP.md b/ROADMAP.md index 2b63653..814f02e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -886,6 +886,8 @@ resolves (toasted), falls back to opening on Local Changes as before. Verified with `cargo test -p strand-core` (+1 `refs` test: fork-point, same-commit, and unknown-revspec cases), `clippy`, `tsc`, `vitest` (29 pass), and `vite build`. +**Second pass, ten features (2026-06-11):** git reset (soft/mixed/hard + safety snapshot) with reflog recovery + undo-last-commit, remote add/rename/set-url/remove + branch rename, signed commits via shell-out when `commit.gpgSign=true`, gitignore quick-add, fixup! creation + autosquash in the rebase editor, copy diff as patch/Markdown, in-diff ⌘F search, image diff previews, and review annotations with feedback export — see `docs/improvements.md` § "Second pass" and TASKS for the per-item detail. + --- ## 1.1+ — Post-1.0 diff --git a/TASKS.md b/TASKS.md index e8183e2..f75b5ea 100644 --- a/TASKS.md +++ b/TASKS.md @@ -59,6 +59,10 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked - ☐ Tree listing for a commit (powers file tree at a revision) - ☑ File content at a revision (`Repo::file_content` — working tree from disk via `safe_workdir_path`, or a blob at a revision; binary heuristic + 2 MB cap) +- ☑ Raw file blob at worktree / index / revision (`Repo::file_blob` in `file.rs` — + `FileBlob` + `BlobSource`, base64 over IPC via a std-only `base64_encode`, 8 MB + cap with a metadata pre-check on the worktree path, behind `safe_workdir_path`; + powers the image diff preview) - ◐ Commit search (message, author, hash) — in-graph highlight over the loaded log is done **client-side** (no backend; `Commits.tsx` `commitMatches`), so no `Repo` search command exists yet. Full-history search + `-G` / `-S` content @@ -84,11 +88,44 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked forward-applies the same slice back, surfaced as an Undo toast for 6s via `discardPatch` / `undoDiscard` + `lastDiscard` handle. Line-level discard still pending.) -- ☑ Commit (subject + body + amend; no GPG signing yet) +- ☑ Commit (subject + body + amend). **Signing works** (`commit.rs` rewrite): + when `commit.gpgSign=true` in the merged config (`signing_enabled`), the + commit shells out via `commit_via_git` — the user's real `git commit -F + --cleanup=whitespace [--amend]` — so gpg/ssh format config, key + lookup, and hooks come for free, and a signing failure surfaces as `Err` + instead of a silent unsigned commit. Default (unsigned) path stays git2, + byte-identical to before. (The GPG sign *status indicator* is still ☐ under + Commits view.) - ◐ Create / delete branch (`Repo::create_branch` from any revspec — HEAD, commit, remote-tracking branch; auto-sets upstream when starting from a remote branch. `Repo::delete_branch` refuses HEAD. Checkout from commit still pending.) +- ☑ Rename branch (`Repo::rename_branch` in `branch.rs` — git2 `find_branch` + + `rename`, no force; upstream config moves with the rename and HEAD follows a + current-branch rename for free. Sidebar branch menu "Rename branch…" + palette + "Rename current branch…" → `RenameBranchDialog`.) +- ☑ Remote add / remove / rename / set-url (`remote.rs` via git2 — blank-input + validation, URL/name safety gates (no `ext::`/`fd::`, no leading `-`), + duplicate name mapped to "remote X already exists", rename "problems" + returned for a warning toast (the rename has already happened by then). + Sidebar Remotes `+` + the remote folder row's + context menu — Edit URL… / Rename… / Copy URL / Remove remote (confirm) — + → `RemoteDialog` (add | rename | url modes); palette "Add remote…".) +- ☑ Reset soft / mixed / hard (`Repo::reset` in `reset.rs` — `ResetMode` / + `ResetOutcome`; refuses while a merge/rebase/cherry-pick/revert is paused; a + hard reset of a tracked-dirty tree first stashes a safety snapshot ("Safety: + before hard reset to ", tracked changes only — `reset --hard` never + touches untracked files), reported in the outcome + toast. + UI: graph context menu "Reset to here…" → `ResetDialog` + (radiogroup, mixed default, danger-styled hard) and the Reflog's "Reset HEAD + here…"; palette "Undo last commit (soft reset)" = soft reset to `HEAD~1`, + gated on a non-root HEAD.) +- ☑ Gitignore quick-add (`Repo::gitignore_add` in `ignore.rs` — validates + (non-empty, no `\n`/`\r`), no-ops on an exact duplicate line, newline-safe + append to the workdir-root `.gitignore`, creating it if absent. Context menus + on a *single untracked* file — Local Changes Unstaged tree + sidebar Files + tab — offer "Add to .gitignore" (root-anchored `/path`) and "Ignore all + *. files"; patterns built by `ignorePatterns` in `lib/ignore.ts`.) - ☑ Checkout branch / commit (`Repo::checkout_branch` — safe checkout, errors on dirty conflicts; `Repo::checkout_commit` — safe detached-HEAD checkout of any revspec via `set_head_detached`.) @@ -138,6 +175,14 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked sidebar menu, and ⌘K "Interactive rebase…". Conflicts route to Local Changes → resolve → Continue. (Std-only round-trip + conflict/continue tests in `history.rs`.) +- ☑ fixup! commits + autosquash (frontend-only). Graph context menu "Create + fixup! commit" commits the staged set as `fixup! ` via the existing + store `commit` (disabled with a "(stage changes first)" hint). Opening the + rebase editor then auto-arranges the plan like `git rebase --autosquash`: + `autosquashPlan` in `lib/rebase.ts` (pure; exact-subject → subject-prefix → + oid-prefix target resolution, stacked prefixes stripped, unmatched stay + `pick`) seeds `RebaseEditor.tsx`, which shows an "Autosquash: N fixup + commits moved…" notice; the seeded plan stays fully editable. - ☐ Interactive rebase: `edit` (pause-to-amend) action — needs an amend-during- rebase flow on top of the continue path - ☐ Interactive rebase: preserve merges (`--rebase-merges`) — v1 flattens; the @@ -175,7 +220,8 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked ### Hybrid concerns - ☑ Write-engine policy decided: `git2` for index/commit ops (stable Rust API, no spawn overhead); shell-out to user's `git` for network - ops (credentials, hooks, LFS, GPG come for free) + ops (credentials, hooks, LFS, GPG come for free) — and, since the signing + work, for commits when `commit.gpgSign=true` (see Writes → Commit) - ☐ Repo cache to avoid re-`discover` per command on hot paths - ☐ Tracing spans on every public fn for perf diagnostics @@ -186,11 +232,15 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked - ☑ Read commands: `repo_open`, `repo_meta`, `repo_status`, `repo_log`, `repo_refs`, `repo_diff_unstaged` / `_staged` / `_between`, `repo_tree`, `repo_submodules`, `repo_blame`, `repo_reflog`, `repo_file_content`, - `repo_file_history`, `repo_diff_commit_file`, `repo_merge_base` + `repo_file_blob`, `repo_file_history`, `repo_diff_commit_file`, + `repo_merge_base` - ☑ Write commands: `repo_stage`, `repo_unstage`, `repo_stage_many`, `repo_unstage_many`, `repo_discard_many`, `repo_discard`, `repo_commit`, `repo_checkout`, `repo_checkout_commit`, `repo_branch_create`, - `repo_branch_delete`, `repo_tag_create`, `repo_tag_delete`, + `repo_branch_delete`, `repo_branch_rename`, `repo_remote_add`, + `repo_remote_remove`, `repo_remote_rename`, `repo_remote_set_url`, + `repo_reset`, `repo_gitignore_add`, + `repo_tag_create`, `repo_tag_delete`, `repo_cherry_pick`, `repo_revert`, `repo_merge`, `repo_rebase`, `repo_rebase_todo`, `repo_interactive_rebase`, `repo_abort_operation`, `repo_continue_operation`, @@ -381,6 +431,27 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked inline Stage / Discard pair on each change block — Unstage on the staged side. `sliceChangeBlock` carves the synthetic single-hunk patch routed through `useRepo.applyPatch`.) +- ☑ Copy diff as patch / Markdown (`concatPatches` / `patchesToMarkdown` in + `lib/patchExport.ts` — raw multi-file patch with trailing-newline + normalization, or `### path` + ```` ```diff ```` fences with CommonMark + backtick-run lengthening and `_binary file changed_` notes. Context menu on + any file/folder/multi-selection in Local Changes (both sides) and the Review + queue; palette "Copy unstaged/staged/review diff" actions gated on + length-only selectors and reading the live arrays via `useRepo.getState()`.) +- ☑ In-diff text search (⌘F in Local Changes + Review, also palette "Search in + diff…" via the one-shot `diffSearchSignal`/`requestDiffSearch` store signal. + `searchDiffs` in `lib/diffSearch.ts` scans every patch in the pool — both + staging sides here, the whole review set there — tracking old/new line + numbers across hunks; `DiffSearchBar.tsx` floats over the diff pane with + Enter/⇧Enter wrap-stepping, i/N counter, and a path+line preview of the + current match. Stepping selects the matched *file*; scrolling to the exact + line inside Pierre's virtualized diff is a deliberate cut.) +- ☑ Image diff preview (binary images — png/jpg/gif/webp/bmp/ico/avif/svg — + render side-by-side Before/After panes (`components/ImageDiff.tsx`, blobs + via `repo_file_blob`) instead of "Binary file": token-based checkerboard, + dims + byte size, single pane for added/deleted. Wired in Local Changes + (unstaged HEAD→worktree, staged HEAD→index), Review (inbox + session), and + CommitDetail (`hash^`→`hash`); `isImagePath`/`imageMime` in `lib/image.ts`.) - ☐ Line-level (sub-change-block) stage / unstage — current smallest unit is the change block. Would require a line/char selection UI. @@ -431,6 +502,11 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked `aria-activedescendant`, ↑/↓ to move focus, Enter or click jumps to the entry's commit in the graph via `revealInGraph`. Recovery path for commits orphaned by reset/rebase/amend.) +- ☑ Reflog recovery actions (`Reflog.tsx` context menu — right-click or + ContextMenu key / Shift+F10 on the focused row: Jump to in graph / Checkout + (detached) / Create branch here… (`BranchDialog`) / Reset HEAD here… + (`ResetDialog` targeting `HEAD@{n}`) — so an orphaned commit is actually + recoverable, not just visible, keyboard included.) ### Worktrees - ☑ Worktree engine (`strand-core/src/worktree.rs` — `Repo::worktrees()` parses @@ -694,8 +770,23 @@ tree: watch the agent work, review fast, accept or reject safely. the Review view. `setBaseline(oid?)` in `stores/repo.ts` takes an optional target, defaulting to HEAD — the pin-at-HEAD palette/toolbar paths are unchanged.) -- ☐ Watcher: optional `.gitignore`-aware path filtering if build storms show - up in profiles. +- ☑ Review annotations (`m` key — or the file-head / per-hunk "Note" buttons — + opens an inline editor in `Review.tsx`; Enter saves, Esc cancels, editor + captures its target path at open so j/k scrubbing can't re-target. Notes + show as a compact list above the diff with `L` chips + × removal and + as `✎N` badges in the queue tree (count folded into `decorationKey`). + UI-only `ReviewNote` type; `reviewNotes` store slice with + `addReviewNote`/`removeReviewNote`/`clearReviewNotes`, persisted per-repo in + SQLite via `reviewSession.getNotes/setNotes` (`review-notes:`), + loaded in `loadReviewSession`. The per-hunk button rides the existing + `HunkAnnotatedDiff` via an optional `onNoteBlock` — Local Changes untouched.) +- ☑ Feedback export (`buildReviewFeedback` in `lib/reviewExport.ts` — one + markdown prompt: header + branch + baseline, per noted file `## path`, line + notes quote a ±4-line hunk-clipped excerpt in a fenced diff block (shared + `fencedDiff` from `patchExport.ts`), file notes as bullets, closing + instruction line — ready to paste back into the coding agent. Toolbar "Copy + feedback (N)" + palette "Review: copy feedback as prompt" / "Review: clear + notes"; only notes on files currently in the review pool export.) --- diff --git a/crates/strand-core/src/branch.rs b/crates/strand-core/src/branch.rs index 7151c1d..f4cbbb6 100644 --- a/crates/strand-core/src/branch.rs +++ b/crates/strand-core/src/branch.rs @@ -120,4 +120,81 @@ impl Repo { branch.delete()?; Ok(()) } + + /// Rename a local branch (`git branch -m `). git2 moves the + /// branch's config section (upstream) along, and HEAD follows when the + /// renamed branch is checked out. No force — errors if `new` exists. + pub fn rename_branch(&self, old: &str, new: &str) -> Result<()> { + let new = new.trim(); + if new.is_empty() { + return Err(crate::Error::Other("branch name is required".into())); + } + let repo = self.git2()?; + let mut branch = repo.find_branch(old, git2::BranchType::Local)?; + branch.rename(new, false)?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::{Path, PathBuf}; + use std::process::Command; + + /// Throwaway repo built with shell git (so `--set-upstream-to` is + /// available), std-only temp dir — same fixture as `history.rs`. + fn scratch_repo() -> (Repo, PathBuf) { + let dir = std::env::temp_dir().join(format!( + "strand-branch-test-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + git(&dir, &["init", "-q", "-b", "main"]); + git(&dir, &["config", "user.name", "Test"]); + git(&dir, &["config", "user.email", "test@example.com"]); + git(&dir, &["config", "commit.gpgsign", "false"]); + (Repo::discover(dir.to_str().unwrap()).unwrap(), dir) + } + + fn git(dir: &Path, args: &[&str]) -> String { + let out = Command::new("git").current_dir(dir).args(args).output().unwrap(); + assert!( + out.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + #[test] + fn rename_branch_moves_upstream_config_and_head_follows() { + let (repo, dir) = scratch_repo(); + std::fs::write(dir.join("a.txt"), "a\n").unwrap(); + git(&dir, &["add", "a.txt"]); + git(&dir, &["commit", "-q", "-m", "init"]); + + // Non-head branch with an upstream: the config moves with the rename. + git(&dir, &["branch", "feature"]); + git(&dir, &["branch", "--set-upstream-to=main", "feature"]); + repo.rename_branch("feature", "renamed").unwrap(); + assert_eq!(git(&dir, &["config", "branch.renamed.merge"]), "refs/heads/main"); + let branches = repo.refs().unwrap().branches; + assert!(branches.iter().any(|b| b.name == "renamed")); + assert!(!branches.iter().any(|b| b.name == "feature")); + + // Renaming onto an existing name errors (no force). + assert!(repo.rename_branch("renamed", "main").is_err()); + // Blank target is rejected. + assert!(repo.rename_branch("renamed", " ").is_err()); + + // Renaming the HEAD branch works and HEAD follows. + repo.rename_branch("main", "trunk").unwrap(); + assert_eq!(git(&dir, &["symbolic-ref", "HEAD"]), "refs/heads/trunk"); + + let _ = std::fs::remove_dir_all(dir); + } } diff --git a/crates/strand-core/src/commit.rs b/crates/strand-core/src/commit.rs index 1852bf1..ccac6e9 100644 --- a/crates/strand-core/src/commit.rs +++ b/crates/strand-core/src/commit.rs @@ -1,6 +1,12 @@ +use std::path::Path; +use std::process::Command; + use serde::{Deserialize, Serialize}; -use crate::{error::Result, repo::Repo}; +use crate::{ + error::{Error, Result}, + repo::Repo, +}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CommitOutcome { @@ -10,44 +16,65 @@ pub struct CommitOutcome { pub amended: bool, } +/// Whether the repo's effective config asks for signed commits +/// (`commit.gpgSign = true`). Read through a snapshot for a consistent merged +/// view (system + global + local), like [`gitconfig`](crate::gitconfig); git2 +/// config keys are case-insensitive, so the lowercase lookup matches any +/// spelling. +fn signing_enabled(repo: &git2::Repository) -> bool { + repo.config() + .and_then(|mut c| c.snapshot()) + .and_then(|s| s.get_bool("commit.gpgsign")) + .unwrap_or(false) +} + /// Write the current index as a new commit on HEAD. /// -/// GPG/SSH signing is out of scope for 0.1 — when the user has -/// `commit.gpgSign = true` set we still produce an unsigned commit. The -/// signing UX comes in 1.1+ (PRD §6.6 P2). +/// Two paths: when `commit.gpgSign` is off (the default) we commit in-process +/// via git2; when it's on we shell out to the user's `git` instead, because +/// git2 never signs — the shell-out picks up the user's gpg/ssh signing +/// config, `gpg.format`, and key lookup for free. impl Repo { pub fn commit(&self, subject: &str, body: Option<&str>, amend: bool) -> Result { let repo = self.git2()?; - let sig = repo.signature()?; - - let mut index = repo.index()?; - let tree_oid = index.write_tree()?; - let tree = repo.find_tree(tree_oid)?; let message = match body.map(str::trim).filter(|b| !b.is_empty()) { Some(b) => format!("{}\n\n{}\n", subject.trim(), b), None => format!("{}\n", subject.trim()), }; - let oid = if amend { - let head = repo.head()?; - let head_commit = head.peel_to_commit()?; - head_commit.amend( - Some("HEAD"), - Some(&sig), - Some(&sig), - None, - Some(&message), - Some(&tree), - )? + let oid = if signing_enabled(&repo) { + self.commit_via_git(&message, amend)?; + repo.head()?.peel_to_commit()?.id() } else { - // Parent list: HEAD if it exists; empty for the initial commit. - let parents: Vec = match repo.head() { - Ok(h) => vec![h.peel_to_commit()?], - Err(_) => Vec::new(), - }; - let parent_refs: Vec<&git2::Commit> = parents.iter().collect(); - repo.commit(Some("HEAD"), &sig, &sig, &message, &tree, &parent_refs)? + let sig = repo.signature()?; + let mut index = repo.index()?; + let tree_oid = index.write_tree()?; + let tree = repo.find_tree(tree_oid)?; + + if amend { + let head = repo.head()?; + let head_commit = head.peel_to_commit()?; + // Author `None` keeps the original author (git2 reuses the + // existing field), matching real `git commit --amend` and the + // shell-out path; only the committer is the current user. + head_commit.amend( + Some("HEAD"), + None, + Some(&sig), + None, + Some(&message), + Some(&tree), + )? + } else { + // Parent list: HEAD if it exists; empty for the initial commit. + let parents: Vec = match repo.head() { + Ok(h) => vec![h.peel_to_commit()?], + Err(_) => Vec::new(), + }; + let parent_refs: Vec<&git2::Commit> = parents.iter().collect(); + repo.commit(Some("HEAD"), &sig, &sig, &message, &tree, &parent_refs)? + } }; Ok(CommitOutcome { @@ -55,4 +82,238 @@ impl Repo { amended: amend, }) } + + /// Commit the staged index by shelling out to the user's `git` — the + /// signing path, since git2 cannot sign. The message goes through a temp + /// file (`-F`, with `--cleanup=verbatim`: the file is built by us and + /// already exact, so verbatim keeps `#` lines AND byte parity with the + /// git2 path, which never cleans) to dodge platform quoting. Unlike the + /// git2 path this runs the user's hooks (pre-commit / commit-msg) — that + /// matches plain `git commit` and is the same accepted trust boundary the + /// other shell-out ops have (PRD §10). + fn commit_via_git(&self, message: &str, amend: bool) -> Result<()> { + let file = temp_message_file(message)?; + let file_arg = file.to_string_lossy().into_owned(); + let mut args = vec!["commit", "-F", file_arg.as_str(), "--cleanup=verbatim"]; + if amend { + args.push("--amend"); + } + let res = run_git(&self.path, &args); + // Best effort, on the error path too — a leak here is only temp litter. + let _ = std::fs::remove_file(&file); + res.map(|_| ()) + } +} + +/// A unique temp file holding the commit message for `git commit -F`. Keyed by +/// pid + a process-wide counter so concurrent commits (different repos) don't +/// collide; the caller deletes it after the commit returns. `create_new` +/// refuses to open a path that already exists — including a pre-planted +/// symlink in the shared temp dir (local TOCTOU) — so a collision just bumps +/// the counter and retries (bounded). +fn temp_message_file(message: &str) -> Result { + use std::io::Write; + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + for _ in 0..16 { + let path = std::env::temp_dir().join(format!( + "strand-commit-msg-{}-{}", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + )); + match std::fs::OpenOptions::new().write(true).create_new(true).open(&path) { + Ok(mut file) => { + file.write_all(message.as_bytes()) + .map_err(|e| Error::Other(format!("write commit message: {e}")))?; + return Ok(path); + } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(e) => return Err(Error::Other(format!("write commit message: {e}"))), + } + } + Err(Error::Other( + "write commit message: could not create a fresh temp file".into(), + )) +} + +/// Run a blocking `git` subcommand in `cwd`, returning trimmed stdout and +/// mapping a non-zero exit to its combined stderr+stdout. Mirrors the helpers +/// in [`history`](crate::history) and `stash`; `GIT_TERMINAL_PROMPT=0` keeps a +/// stuck prompt (e.g. gpg pinentry fallback) from blocking. A free function +/// (not a `Repo` method) so it doesn't collide with `stash`'s same-named +/// helper on the same type. +fn run_git(cwd: &Path, args: &[&str]) -> Result { + let out = Command::new("git") + .current_dir(cwd) + .env("GIT_TERMINAL_PROMPT", "0") + // Detach stdin so git can never block reading from a TTY/pipe we don't + // have (the app isn't launched from a terminal) — it errors instead. + .stdin(std::process::Stdio::null()) + // Neutralize repo-local config that would run code as a side effect. + .args(crate::GIT_SAFE_CONFIG) + .args(args) + .output() + .map_err(|e| Error::Other(format!("spawn git failed: {e}")))?; + if !out.status.success() { + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + let combined = format!("{stdout}{stderr}").trim().to_string(); + return Err(Error::Other(if combined.is_empty() { + format!("git {} failed", args.join(" ")) + } else { + combined + })); + } + Ok(String::from_utf8_lossy(&out.stdout).trim().to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + use std::process::Command; + + /// Throwaway repo via shell git, std-only temp dir like `history.rs`. + /// Deliberately does NOT force `commit.gpgsign = false` — these tests set + /// it per case to exercise both commit paths. + fn scratch_repo() -> (Repo, PathBuf) { + let dir = std::env::temp_dir().join(format!( + "strand-commit-test-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + git(&dir, &["init", "-q", "-b", "main"]); + git(&dir, &["config", "user.name", "Test"]); + git(&dir, &["config", "user.email", "test@example.com"]); + (Repo::discover(dir.to_str().unwrap()).unwrap(), dir) + } + + fn git(dir: &Path, args: &[&str]) -> String { + let out = Command::new("git").current_dir(dir).args(args).output().unwrap(); + assert!( + out.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + fn stage(dir: &Path, file: &str, contents: &str) { + std::fs::write(dir.join(file), contents).unwrap(); + git(dir, &["add", file]); + } + + #[test] + fn signing_enabled_defaults_off_and_follows_config() { + let (repo, dir) = scratch_repo(); + let g2 = repo.git2().unwrap(); + assert!(!signing_enabled(&g2), "unset ⇒ off"); + + git(&dir, &["config", "commit.gpgsign", "true"]); + assert!(signing_enabled(&repo.git2().unwrap())); + + git(&dir, &["config", "commit.gpgsign", "false"]); + assert!(!signing_enabled(&repo.git2().unwrap())); + + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn unsigned_path_commits_subject_and_body() { + let (repo, dir) = scratch_repo(); + git(&dir, &["config", "commit.gpgsign", "false"]); + stage(&dir, "a.txt", "a\n"); + + let out = repo.commit("subject", Some("body line"), false).unwrap(); + assert!(!out.amended); + assert_eq!(git(&dir, &["rev-parse", "HEAD"]), out.oid); + assert_eq!(git(&dir, &["log", "-1", "--format=%B"]), "subject\n\nbody line"); + + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn commit_via_git_writes_message_and_amend_replaces() { + // gpgsign deliberately unset: commit_via_git itself doesn't read the + // config (its caller routes), so it just makes a normal commit here. + let (repo, dir) = scratch_repo(); + stage(&dir, "base.txt", "base\n"); + git(&dir, &["commit", "-q", "-m", "base"]); + + stage(&dir, "a.txt", "a\n"); + repo.commit_via_git("subject\n\nbody line\n", false).unwrap(); + assert_eq!(git(&dir, &["log", "-1", "--format=%B"]), "subject\n\nbody line"); + assert_eq!(git(&dir, &["rev-list", "--count", "HEAD"]), "2"); + + // Amend as a *different* configured user: real git keeps the original + // author and only updates the committer — assert that parity here. + git(&dir, &["config", "user.name", "Other"]); + git(&dir, &["config", "user.email", "other@example.com"]); + repo.commit_via_git("amended subject\n", true).unwrap(); + assert_eq!(git(&dir, &["log", "-1", "--format=%B"]), "amended subject"); + assert_eq!(git(&dir, &["rev-list", "--count", "HEAD"]), "2", "amend replaces, not adds"); + assert_eq!( + git(&dir, &["log", "-1", "--format=%an <%ae>"]), + "Test ", + "amend preserves the original author" + ); + assert_eq!( + git(&dir, &["log", "-1", "--format=%cn <%ce>"]), + "Other " + ); + + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn unsigned_amend_preserves_original_author() { + let (repo, dir) = scratch_repo(); + git(&dir, &["config", "commit.gpgsign", "false"]); + stage(&dir, "a.txt", "a\n"); + repo.commit("original", None, false).unwrap(); + + // Same parity check for the git2 path: a different configured user + // amends, the original author survives, the committer updates. + git(&dir, &["config", "user.name", "Other"]); + git(&dir, &["config", "user.email", "other@example.com"]); + stage(&dir, "a.txt", "a2\n"); + let out = repo.commit("amended", None, true).unwrap(); + assert!(out.amended); + assert_eq!(git(&dir, &["rev-list", "--count", "HEAD"]), "1", "amend replaces, not adds"); + assert_eq!( + git(&dir, &["log", "-1", "--format=%an <%ae>"]), + "Test ", + "amend preserves the original author" + ); + assert_eq!( + git(&dir, &["log", "-1", "--format=%cn <%ce>"]), + "Other " + ); + + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn gpgsign_true_routes_through_real_git_and_surfaces_failure() { + // No real key in CI, so prove the routing instead: with signing on and + // a key that can't exist, `Repo::commit` must fail (a silent unsigned + // commit would be the bug this module exists to prevent). + let (repo, dir) = scratch_repo(); + stage(&dir, "base.txt", "base\n"); + git(&dir, &["commit", "-q", "-m", "base"]); + + git(&dir, &["config", "commit.gpgsign", "true"]); + git(&dir, &["config", "gpg.format", "ssh"]); + let missing = dir.join("no-such-key").to_string_lossy().into_owned(); + git(&dir, &["config", "user.signingkey", &missing]); + + stage(&dir, "a.txt", "a\n"); + assert!(repo.commit("subject", None, false).is_err()); + assert_eq!(git(&dir, &["rev-list", "--count", "HEAD"]), "1", "failed commit left HEAD alone"); + + let _ = std::fs::remove_dir_all(dir); + } } diff --git a/crates/strand-core/src/file.rs b/crates/strand-core/src/file.rs index 1fb6096..be0b157 100644 --- a/crates/strand-core/src/file.rs +++ b/crates/strand-core/src/file.rs @@ -22,6 +22,10 @@ use crate::{ /// blob. const MAX_CONTENT_BYTES: usize = 2_000_000; +/// Cap on raw bytes returned by [`Repo::file_blob`] (image previews). Past +/// this the blob comes back empty with `too_large = true`. +const MAX_BLOB_BYTES: usize = 8_000_000; + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileContent { pub path: String, @@ -33,6 +37,25 @@ pub struct FileContent { pub truncated: bool, } +/// Raw file bytes (base64) for the image diff preview. +#[derive(Debug, Clone, Serialize)] +pub struct FileBlob { + /// Standard base64 (RFC 4648, padded). Empty when `too_large`. + pub base64: String, + /// Byte size of the file — reported even when `too_large`. + pub size: usize, + /// True when the file exceeded [`MAX_BLOB_BYTES`] and `base64` is empty. + pub too_large: bool, +} + +/// Where [`Repo::file_blob`] reads from. Internal — the IPC layer maps its +/// `(rev: Option, index: bool)` arguments onto this. +pub enum BlobSource<'a> { + Worktree, + Index, + Rev(&'a str), +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileHistoryEntry { pub hash: String, @@ -72,6 +95,45 @@ impl Repo { } } + /// Read a file's raw bytes for the image preview: the working-tree copy, + /// the staged (index) copy, or the blob at a revision. Oversized files + /// return `too_large` instead of bytes (the worktree branch checks the + /// fs metadata size before reading anything). + pub fn file_blob(&self, rel_path: &str, source: BlobSource) -> Result { + match source { + BlobSource::Worktree => { + let full = self.safe_workdir_path(rel_path)?; + let size = std::fs::metadata(&full)?.len(); + if size as usize > MAX_BLOB_BYTES { + return Ok(FileBlob { base64: String::new(), size: size as usize, too_large: true }); + } + Ok(build_blob(&std::fs::read(&full)?)) + } + BlobSource::Index => { + let repo = self.git2()?; + let entry = repo + .index()? + .get_path(Path::new(rel_path), 0) + .ok_or_else(|| Error::Other(format!("{rel_path} is not in the index")))?; + let blob = repo + .find_blob(entry.id) + .map_err(|_| Error::Other(format!("{rel_path} is not a file in the index")))?; + Ok(build_blob(blob.content())) + } + BlobSource::Rev(spec) => { + let repo = self.git2()?; + let tree = repo.revparse_single(spec)?.peel_to_commit()?.tree()?; + let entry = tree.get_path(Path::new(rel_path)).map_err(|_| { + Error::Other(format!("{rel_path} does not exist at {spec}")) + })?; + let blob = repo + .find_blob(entry.id()) + .map_err(|_| Error::Other(format!("{rel_path} is not a file at {spec}")))?; + Ok(build_blob(blob.content())) + } + } + } + /// Commits that touched `rel_path`, newest first, following renames /// (`git log --follow`). Returns up to `limit` entries with per-path /// add/delete counts. An untracked / never-committed path yields an empty @@ -142,6 +204,31 @@ fn looks_binary(bytes: &[u8]) -> bool { bytes.iter().take(8192).any(|&b| b == 0) } +fn build_blob(bytes: &[u8]) -> FileBlob { + if bytes.len() > MAX_BLOB_BYTES { + return FileBlob { base64: String::new(), size: bytes.len(), too_large: true }; + } + FileBlob { base64: base64_encode(bytes), size: bytes.len(), too_large: false } +} + +/// Standard base64 (RFC 4648 alphabet, `=`-padded). Std-only — the crate has +/// no base64 dependency and this is the lone encoder use. +fn base64_encode(bytes: &[u8]) -> String { + const ALPHABET: &[u8; 64] = + b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut out = String::with_capacity(bytes.len().div_ceil(3) * 4); + for chunk in bytes.chunks(3) { + let n = (u32::from(chunk[0]) << 16) + | (u32::from(chunk.get(1).copied().unwrap_or(0)) << 8) + | u32::from(chunk.get(2).copied().unwrap_or(0)); + out.push(ALPHABET[(n >> 18) as usize & 63] as char); + out.push(ALPHABET[(n >> 12) as usize & 63] as char); + out.push(if chunk.len() > 1 { ALPHABET[(n >> 6) as usize & 63] as char } else { '=' }); + out.push(if chunk.len() > 2 { ALPHABET[n as usize & 63] as char } else { '=' }); + } + out +} + /// Parse `git log --numstat --format=…` output into history entries. /// A line starting with [`MARKER`] opens a new commit; the `adds\tdels\tpath` /// rows that follow accumulate into it (binary changes report `-` and count 0). @@ -256,4 +343,63 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + + #[test] + fn base64_matches_known_vectors() { + assert_eq!(base64_encode(b""), ""); + assert_eq!(base64_encode(b"f"), "Zg=="); + assert_eq!(base64_encode(b"fo"), "Zm8="); + assert_eq!(base64_encode(b"foo"), "Zm9v"); + assert_eq!(base64_encode(b"foob"), "Zm9vYg=="); + assert_eq!(base64_encode(b"foobar"), "Zm9vYmFy"); + // Binary bytes (not valid UTF-8). + assert_eq!(base64_encode(&[0x00, 0xff, 0x10]), "AP8Q"); + } + + #[test] + fn blob_reads_worktree_index_and_rev() { + let (repo, dir) = scratch(); + // Bytes with a NUL so the file is unambiguously binary. + let v1: &[u8] = b"\x89PNG\x00\n"; + std::fs::write(dir.join("img.png"), v1).unwrap(); + git(&dir, &["add", "img.png"]); + git(&dir, &["commit", "-q", "-m", "v1"]); + + // Stage v2, then write v3 to the working tree — three distinct copies. + let v2: &[u8] = b"\x89PNG\x00\x01"; + std::fs::write(dir.join("img.png"), v2).unwrap(); + git(&dir, &["add", "img.png"]); + let v3: &[u8] = b"\x89PNG\x00\x02"; + std::fs::write(dir.join("img.png"), v3).unwrap(); + + let wt = repo.file_blob("img.png", BlobSource::Worktree).unwrap(); + assert_eq!(wt.base64, "iVBORwAC", "worktree bytes, precomputed base64"); + assert_eq!(wt.size, v3.len()); + assert!(!wt.too_large); + + let idx = repo.file_blob("img.png", BlobSource::Index).unwrap(); + assert_eq!(idx.base64, "iVBORwAB"); + + let head = repo.file_blob("img.png", BlobSource::Rev("HEAD")).unwrap(); + assert_eq!(head.base64, "iVBORwAK"); + + assert!(repo.file_blob("missing.png", BlobSource::Index).is_err()); + assert!(repo.file_blob("missing.png", BlobSource::Rev("HEAD")).is_err()); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn blob_caps_oversized_files() { + let (repo, dir) = scratch(); + let big = vec![0u8; MAX_BLOB_BYTES + 1]; + std::fs::write(dir.join("big.bin"), &big).unwrap(); + + let blob = repo.file_blob("big.bin", BlobSource::Worktree).unwrap(); + assert!(blob.too_large); + assert_eq!(blob.base64, ""); + assert_eq!(blob.size, MAX_BLOB_BYTES + 1); + + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/strand-core/src/ignore.rs b/crates/strand-core/src/ignore.rs new file mode 100644 index 0000000..dc6724b --- /dev/null +++ b/crates/strand-core/src/ignore.rs @@ -0,0 +1,102 @@ +//! `.gitignore` quick-edits — append a pattern to the workdir root file. +//! +//! Powers the "Add to .gitignore" context-menu action on untracked files. +//! Plain file I/O (no git involvement): git picks the change up on the next +//! status walk. + +use crate::{error::Result, repo::Repo, Error}; + +impl Repo { + /// Append `pattern` as its own line to the working-tree root `.gitignore`, + /// creating the file if missing. An exact-line duplicate is a no-op. + /// Rejects empty patterns and embedded newlines (a crafted "pattern" must + /// not smuggle extra ignore lines in). + pub fn gitignore_add(&self, pattern: &str) -> Result<()> { + let pattern = pattern.trim(); + if pattern.is_empty() || pattern.contains('\n') || pattern.contains('\r') { + return Err(Error::Other("invalid ignore pattern".into())); + } + + let file = self.path.join(".gitignore"); + let existing = match std::fs::read_to_string(&file) { + Ok(s) => s, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(e) => return Err(e.into()), + }; + // `lines()` strips a trailing `\r`, so this also matches CRLF files. + if existing.lines().any(|line| line == pattern) { + return Ok(()); + } + + let mut out = existing; + if !out.is_empty() && !out.ends_with('\n') { + out.push('\n'); + } + out.push_str(pattern); + out.push('\n'); + std::fs::write(&file, out)?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Build a throwaway repo and return its `Repo`. Uses a std-only unique + /// temp dir so the test pulls in no new dependency (strand-core + /// deliberately has no `tempfile` dev-dep). No commit needed — + /// `gitignore_add` only touches the working tree. + fn scratch_repo() -> (Repo, std::path::PathBuf) { + let dir = std::env::temp_dir().join(format!( + "strand-ignore-test-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + git2::Repository::init(&dir).unwrap(); + // Discover via our own API so we exercise the real code path. + (Repo::discover(dir.to_str().unwrap()).unwrap(), dir) + } + + #[test] + fn creates_file_when_missing() { + let (repo, dir) = scratch_repo(); + repo.gitignore_add("/target").unwrap(); + let content = std::fs::read_to_string(dir.join(".gitignore")).unwrap(); + assert_eq!(content, "/target\n"); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn appends_after_content_without_trailing_newline() { + let (repo, dir) = scratch_repo(); + std::fs::write(dir.join(".gitignore"), "node_modules").unwrap(); + repo.gitignore_add("*.log").unwrap(); + let content = std::fs::read_to_string(dir.join(".gitignore")).unwrap(); + assert_eq!(content, "node_modules\n*.log\n"); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn exact_duplicate_is_a_noop() { + let (repo, dir) = scratch_repo(); + repo.gitignore_add("*.log").unwrap(); + let before = std::fs::read(dir.join(".gitignore")).unwrap(); + repo.gitignore_add("*.log").unwrap(); + let after = std::fs::read(dir.join(".gitignore")).unwrap(); + assert_eq!(before, after, "duplicate add must leave the file byte-identical"); + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn rejects_empty_and_newline_injection() { + let (repo, dir) = scratch_repo(); + assert!(repo.gitignore_add(" ").is_err()); + assert!(repo.gitignore_add("a\nb").is_err()); + assert!(repo.gitignore_add("a\rb").is_err()); + assert!(!dir.join(".gitignore").exists(), "rejected patterns write nothing"); + let _ = std::fs::remove_dir_all(dir); + } +} diff --git a/crates/strand-core/src/lib.rs b/crates/strand-core/src/lib.rs index 362b681..cc7d84c 100644 --- a/crates/strand-core/src/lib.rs +++ b/crates/strand-core/src/lib.rs @@ -24,10 +24,12 @@ pub mod commit; pub mod network; pub mod refs; pub mod branch; +pub mod remote; pub mod conflict; pub mod external; pub mod gitconfig; pub mod history; +pub mod ignore; pub mod stash; pub mod tag; pub mod tree; @@ -36,6 +38,7 @@ pub mod worktree; pub mod blame; pub mod file; pub mod reflog; +pub mod reset; pub mod snapshot; pub mod watch; diff --git a/crates/strand-core/src/network.rs b/crates/strand-core/src/network.rs index d83121c..e77aa5f 100644 --- a/crates/strand-core/src/network.rs +++ b/crates/strand-core/src/network.rs @@ -237,7 +237,9 @@ pub fn clone( /// Reject a user-supplied remote/URL that git would mis-read as an option or /// a command-executing transport. Paired with an explicit `--` separator at /// the call site, this closes the "paste a malicious clone URL" vector. -fn validate_remote_arg(arg: &str, what: &str) -> Result<()> { +/// `pub(crate)` so `remote.rs` applies the same gate when a URL is *stored* +/// (add / set-url) — a saved `ext::` URL would execute on the next fetch. +pub(crate) fn validate_remote_arg(arg: &str, what: &str) -> Result<()> { if arg.starts_with('-') { return Err(Error::Other(format!("{what} may not start with '-': {arg}"))); } diff --git a/crates/strand-core/src/remote.rs b/crates/strand-core/src/remote.rs new file mode 100644 index 0000000..393ee3f --- /dev/null +++ b/crates/strand-core/src/remote.rs @@ -0,0 +1,200 @@ +//! Remote management — add, remove, rename, set-url. +//! +//! Remote *reads* live in `refs.rs` (`collect_remotes`); this is the mutating +//! side. All ops go through `git2`, which keeps the config section and the +//! remote-tracking refs in sync in one call (delete drops `refs/remotes//*`, +//! rename moves them). + +use crate::{error::Result, network::validate_remote_arg, repo::Repo, Error}; + +fn require(value: &str, what: &str) -> Result<()> { + if value.trim().is_empty() { + return Err(Error::Other(format!("{what} is required"))); + } + Ok(()) +} + +/// Reject an option-like remote name at creation time. The name later lands +/// in `git fetch` argv positions — `network.rs` validates again at fetch time, +/// but failing here gives the user a clear message instead of a stored remote +/// that can never be fetched. +fn require_plain_name(name: &str) -> Result<()> { + if name.starts_with('-') { + return Err(Error::Other(format!( + "remote name may not start with '-': {name}" + ))); + } + Ok(()) +} + +impl Repo { + /// Add a remote (`git remote add `). + pub fn add_remote(&self, name: &str, url: &str) -> Result<()> { + require(name, "remote name")?; + require(url, "remote URL")?; + require_plain_name(name)?; + // Same gate as the clone path: a stored `ext::`/`fd::` URL would run + // arbitrary commands on the next fetch. + validate_remote_arg(url, "remote URL")?; + match self.git2()?.remote(name, url) { + Ok(_) => Ok(()), + // git2's duplicate error reads "remote 'x' already exists" wrapped + // in config noise — surface the short, actionable form. + Err(e) if e.code() == git2::ErrorCode::Exists => { + Err(Error::Other(format!("remote {name} already exists"))) + } + Err(e) => Err(e.into()), + } + } + + /// Remove a remote (`git remote remove`) — also drops its remote-tracking + /// refs and config section. + pub fn remove_remote(&self, name: &str) -> Result<()> { + require(name, "remote name")?; + self.git2()?.remote_delete(name)?; + Ok(()) + } + + /// Rename a remote (`git remote rename`). git2 reports refspecs it could + /// not rewrite (non-default ones, e.g. a single-branch clone's) as + /// "problems" — by the time they are reported the rename HAS happened + /// (config section + remote-tracking refs moved), so they are returned + /// for the caller to surface as a warning, never as an error. Empty means + /// a clean rename. + pub fn rename_remote(&self, old: &str, new: &str) -> Result> { + require(old, "remote name")?; + require(new, "remote name")?; + require_plain_name(new)?; + let problems = self.git2()?.remote_rename(old, new)?; + Ok(problems.iter().flatten().map(str::to_string).collect()) + } + + /// Change a remote's fetch URL (`git remote set-url`). + pub fn set_remote_url(&self, name: &str, url: &str) -> Result<()> { + require(name, "remote name")?; + require(url, "remote URL")?; + validate_remote_arg(url, "remote URL")?; + self.git2()?.remote_set_url(name, url)?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Build a throwaway repo with a single commit and return its `Repo`. + /// Std-only unique temp dir, same fixture as `tag.rs`. + fn scratch_repo() -> (Repo, std::path::PathBuf) { + let dir = std::env::temp_dir().join(format!( + "strand-remote-test-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let repo = git2::Repository::init(&dir).unwrap(); + { + let sig = git2::Signature::now("Test", "test@example.com").unwrap(); + let tree_oid = { + let mut idx = repo.index().unwrap(); + let tree = idx.write_tree().unwrap(); + repo.find_tree(tree).unwrap().id() + }; + let tree = repo.find_tree(tree_oid).unwrap(); + repo.commit(Some("HEAD"), &sig, &sig, "init", &tree, &[]).unwrap(); + } + (Repo::discover(dir.to_str().unwrap()).unwrap(), dir) + } + + #[test] + fn add_set_url_rename_remove_round_trip() { + let (repo, dir) = scratch_repo(); + + repo.add_remote("origin", "https://example.com/a.git").unwrap(); + let remotes = repo.refs().unwrap().remotes; + assert_eq!(remotes.len(), 1); + assert_eq!(remotes[0].name, "origin"); + assert_eq!(remotes[0].url.as_deref(), Some("https://example.com/a.git")); + + // Duplicate name maps to the clear message, not raw config noise. + let err = repo.add_remote("origin", "https://example.com/b.git").unwrap_err(); + assert!(err.to_string().contains("already exists"), "{err}"); + + // Blank name / URL are rejected before touching git config. + assert!(repo.add_remote(" ", "https://example.com/c.git").is_err()); + assert!(repo.add_remote("upstream", "").is_err()); + assert!(repo.set_remote_url("origin", " ").is_err()); + + repo.set_remote_url("origin", "https://example.com/b.git").unwrap(); + let g = git2::Repository::open(&dir).unwrap(); + assert_eq!( + g.find_remote("origin").unwrap().url(), + Some("https://example.com/b.git") + ); + + let problems = repo.rename_remote("origin", "upstream").unwrap(); + assert!(problems.is_empty(), "default refspec renames cleanly: {problems:?}"); + let g = git2::Repository::open(&dir).unwrap(); + assert!(g.find_remote("origin").is_err(), "old name gone after rename"); + assert_eq!( + g.find_remote("upstream").unwrap().url(), + Some("https://example.com/b.git") + ); + + repo.remove_remote("upstream").unwrap(); + assert!(repo.refs().unwrap().remotes.is_empty()); + + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn rename_with_custom_refspec_succeeds_and_reports_problems() { + let (repo, dir) = scratch_repo(); + repo.add_remote("origin", "https://example.com/a.git").unwrap(); + // A non-default fetch refspec (single-branch clone style) — git2 can't + // rewrite it on rename and reports it as a "problem", but the rename + // itself has already landed by then. + { + let g = git2::Repository::open(&dir).unwrap(); + let mut cfg = g.config().unwrap(); + cfg.set_str( + "remote.origin.fetch", + "+refs/heads/main:refs/remotes/origin/main", + ) + .unwrap(); + } + + let problems = repo.rename_remote("origin", "upstream").unwrap(); + assert_eq!(problems.len(), 1, "the custom refspec is reported: {problems:?}"); + assert!(problems[0].contains("refs/heads/main"), "{problems:?}"); + let g = git2::Repository::open(&dir).unwrap(); + assert!(g.find_remote("origin").is_err(), "rename happened despite problems"); + assert!(g.find_remote("upstream").is_ok()); + + let _ = std::fs::remove_dir_all(dir); + } + + #[test] + fn rejects_dangerous_urls_and_option_like_names() { + let (repo, dir) = scratch_repo(); + + // `ext::` transports run arbitrary commands on the next fetch — the + // clone-path gate applies to stored remotes too, on add and set-url. + let err = repo.add_remote("evil", "ext::sh -c 'touch /tmp/x'").unwrap_err(); + assert!(err.to_string().contains("unsupported transport"), "{err}"); + repo.add_remote("origin", "https://example.com/a.git").unwrap(); + assert!(repo.set_remote_url("origin", "ext::sh -c 'touch /tmp/x'").is_err()); + + // Option-like names later land in `git fetch` argv positions — reject + // at creation, on add and rename. + let err = repo.add_remote("-evil", "https://example.com/a.git").unwrap_err(); + assert!(err.to_string().contains("may not start with '-'"), "{err}"); + assert!(repo.rename_remote("origin", "-evil").is_err()); + let g = git2::Repository::open(&dir).unwrap(); + assert!(g.find_remote("origin").is_ok(), "rejected rename left the remote alone"); + + let _ = std::fs::remove_dir_all(dir); + } +} diff --git a/crates/strand-core/src/reset.rs b/crates/strand-core/src/reset.rs new file mode 100644 index 0000000..3a56c94 --- /dev/null +++ b/crates/strand-core/src/reset.rs @@ -0,0 +1,238 @@ +//! `git reset` — move HEAD (and per mode the index / working tree) to a +//! target commit. +//! +//! Hard resets of a dirty tree take a safety snapshot first (the same +//! stash-based net `discard_paths` callers use), so "discard all changes" +//! is always recoverable from the stash stack. "Dirty" means tracked +//! changes only — `git reset --hard` never touches untracked files, so an +//! untracked-only tree needs no snapshot and the snapshot itself skips +//! untracked files (the lighter `stash create` path, no working-tree churn). + +use serde::{Deserialize, Serialize}; + +use crate::{ + error::{Error, Result}, + repo::Repo, +}; + +/// Reset flavour: what happens to the index + working tree. +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum ResetMode { + /// Move HEAD only — all changes stay staged. + Soft, + /// Move HEAD + reset the index — changes stay in the working tree, unstaged. + Mixed, + /// Move HEAD + reset index and working tree — changes are discarded. + Hard, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ResetOutcome { + /// Short hash of the commit HEAD now points at. + pub target_short: String, + /// OID of the safety snapshot stash taken before a hard reset of a dirty + /// tree; `None` for soft/mixed or a clean tree. + pub snapshot_oid: Option, +} + +impl Repo { + /// Reset HEAD (the current branch, or HEAD itself when detached) to + /// `target`. Refuses while a merge/rebase/cherry-pick/revert is paused — + /// resetting mid-operation strands the sequencer state. + pub fn reset(&self, target: &str, mode: ResetMode) -> Result { + if let Some(op) = self.meta()?.operation { + return Err(Error::Other(format!( + "finish or abort the in-progress {op} first" + ))); + } + + let repo = self.git2()?; + let obj = repo + .revparse_single(target)? + .peel(git2::ObjectType::Commit)?; + let target_short = obj + .short_id() + .ok() + .and_then(|b| b.as_str().map(str::to_string)) + .unwrap_or_else(|| target.to_string()); + + // A hard reset destroys uncommitted *tracked* work — snapshot it onto + // the stash stack first, mirroring discardMany's safety net. Untracked + // files survive a hard reset untouched, so a pure-WT_NEW entry doesn't + // count as dirty and the snapshot skips untracked files (avoiding the + // push+apply round-trip that can fail on Windows file locks). + let mut snapshot_oid = None; + if matches!(mode, ResetMode::Hard) { + let dirty = repo + .statuses(Some(&mut crate::status::status_options()))? + .iter() + .any(|e| { + !(e.status() & !(git2::Status::WT_NEW | git2::Status::IGNORED)).is_empty() + }); + if dirty { + let msg = format!("Safety: before hard reset to {target_short}"); + snapshot_oid = self.stash_snapshot(Some(&msg), false)?.oid; + } + } + + match mode { + ResetMode::Soft => repo.reset(&obj, git2::ResetType::Soft, None)?, + ResetMode::Mixed => repo.reset(&obj, git2::ResetType::Mixed, None)?, + ResetMode::Hard => { + let mut co = git2::build::CheckoutBuilder::new(); + co.force(); + repo.reset(&obj, git2::ResetType::Hard, Some(&mut co))?; + } + } + + Ok(ResetOutcome { + target_short, + snapshot_oid, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::{Path, PathBuf}; + use std::process::Command; + + /// Build a throwaway repo, configured enough to commit, and return its + /// `Repo` + working dir. Std-only (no `tempfile` dev-dep), like `tag.rs`. + fn scratch_repo() -> (Repo, PathBuf) { + let dir = std::env::temp_dir().join(format!( + "strand-reset-test-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + git(&dir, &["init", "-q", "-b", "main"]); + git(&dir, &["config", "user.name", "Test"]); + git(&dir, &["config", "user.email", "test@example.com"]); + git(&dir, &["config", "commit.gpgsign", "false"]); + (Repo::discover(dir.to_str().unwrap()).unwrap(), dir) + } + + fn git(dir: &Path, args: &[&str]) -> String { + let out = Command::new("git").current_dir(dir).args(args).output().unwrap(); + assert!( + out.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + fn write_commit(dir: &Path, file: &str, contents: &str, msg: &str) -> String { + std::fs::write(dir.join(file), contents).unwrap(); + git(dir, &["add", file]); + git(dir, &["commit", "-q", "-m", msg]); + git(dir, &["rev-parse", "HEAD"]) + } + + #[test] + fn soft_reset_moves_head_and_keeps_changes_staged() { + let (repo, dir) = scratch_repo(); + let first = write_commit(&dir, "a.txt", "one\n", "first"); + write_commit(&dir, "a.txt", "two\n", "second"); + + let outcome = repo.reset("HEAD~1", ResetMode::Soft).unwrap(); + assert!(outcome.snapshot_oid.is_none()); + assert_eq!(git(&dir, &["rev-parse", "HEAD"]), first); + // The second commit's content stays staged ("M " in porcelain; + // the git() helper trims, which only strips the unstaged leading space). + let status = git(&dir, &["status", "--porcelain"]); + assert_eq!(status, "M a.txt", "expected staged change"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn mixed_reset_leaves_changes_unstaged() { + let (repo, dir) = scratch_repo(); + let first = write_commit(&dir, "a.txt", "one\n", "first"); + write_commit(&dir, "a.txt", "two\n", "second"); + + repo.reset("HEAD~1", ResetMode::Mixed).unwrap(); + assert_eq!(git(&dir, &["rev-parse", "HEAD"]), first); + // Porcelain " M ", with the leading space lost to git()'s trim. + let status = git(&dir, &["status", "--porcelain"]); + assert_eq!(status, "M a.txt", "expected unstaged change"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn hard_reset_cleans_tree_and_snapshots_dirty_changes() { + let (repo, dir) = scratch_repo(); + let first = write_commit(&dir, "a.txt", "one\n", "first"); + write_commit(&dir, "a.txt", "two\n", "second"); + // Dirty the tree (tracked change) so the safety snapshot fires. + std::fs::write(dir.join("a.txt"), "wip\n").unwrap(); + + let outcome = repo.reset("HEAD~1", ResetMode::Hard).unwrap(); + assert!(outcome.snapshot_oid.is_some(), "tracked-dirty hard reset takes a snapshot"); + assert_eq!(git(&dir, &["rev-parse", "HEAD"]), first); + assert_eq!(git(&dir, &["status", "--porcelain"]), ""); + // The snapshot is on the stash stack, ready to recover from. + let stashes = repo.stash_list().unwrap(); + assert!(!stashes.is_empty(), "snapshot stash is on the stack"); + assert!(stashes[0].message.contains("Safety: before hard reset")); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn hard_reset_of_untracked_only_tree_takes_no_snapshot() { + let (repo, dir) = scratch_repo(); + let first = write_commit(&dir, "a.txt", "one\n", "first"); + write_commit(&dir, "a.txt", "two\n", "second"); + // Untracked-only "dirt": `git reset --hard` never touches untracked + // files, so no snapshot is needed (and none should be taken). + std::fs::write(dir.join("new.txt"), "untracked\n").unwrap(); + + let outcome = repo.reset("HEAD~1", ResetMode::Hard).unwrap(); + assert!(outcome.snapshot_oid.is_none(), "untracked-only tree needs no snapshot"); + assert!(repo.stash_list().unwrap().is_empty()); + assert_eq!(git(&dir, &["rev-parse", "HEAD"]), first); + assert!(dir.join("new.txt").exists(), "untracked file survives the hard reset"); + assert_eq!(git(&dir, &["status", "--porcelain"]), "?? new.txt"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn hard_reset_of_clean_tree_takes_no_snapshot() { + let (repo, dir) = scratch_repo(); + write_commit(&dir, "a.txt", "one\n", "first"); + write_commit(&dir, "a.txt", "two\n", "second"); + + let outcome = repo.reset("HEAD~1", ResetMode::Hard).unwrap(); + assert!(outcome.snapshot_oid.is_none()); + assert!(repo.stash_list().unwrap().is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn reset_during_merge_in_progress_errors() { + let (repo, dir) = scratch_repo(); + write_commit(&dir, "a.txt", "base\n", "base"); + git(&dir, &["checkout", "-q", "-b", "feature"]); + write_commit(&dir, "a.txt", "feature\n", "feat"); + git(&dir, &["checkout", "-q", "main"]); + write_commit(&dir, "a.txt", "main\n", "main change"); + // Conflicting merge: exits non-zero and leaves MERGE_HEAD behind. + let _ = Command::new("git") + .current_dir(&dir) + .args(["merge", "feature"]) + .output() + .unwrap(); + + let err = repo.reset("HEAD", ResetMode::Mixed).unwrap_err(); + assert!( + err.to_string().contains("in-progress merge"), + "unexpected error: {err}" + ); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/strand-tauri/src/commands.rs b/crates/strand-tauri/src/commands.rs index 90e14fd..b864baa 100644 --- a/crates/strand-tauri/src/commands.rs +++ b/crates/strand-tauri/src/commands.rs @@ -1,12 +1,13 @@ use serde::Serialize; use strand_core::{ apply::ApplyTarget, blame::BlameLine, branch::CheckoutOutcome, commit::CommitOutcome, - diff::FileDiff, file::{FileContent, FileHistoryEntry}, + diff::FileDiff, file::{BlobSource, FileBlob, FileContent, FileHistoryEntry}, gitconfig::{self, GlobalIdentity}, history::{MergeMode, RebaseEntry, RebaseStep}, log::Commit, network::{clone as core_clone, CancelHandle, CloneOutcome, NetworkOutcome, Progress}, reflog::ReflogEntry, - refs::Refs, repo::RepoMeta, snapshot::Snapshot, stash::{Stash, StashOutcome}, + refs::Refs, repo::RepoMeta, reset::{ResetMode, ResetOutcome}, + snapshot::Snapshot, stash::{Stash, StashOutcome}, status::FileStatus, submodule::Submodule, tree::WorkTreeEntry, worktree::Worktree, Repo, }; use tauri::ipc::Channel; @@ -187,6 +188,27 @@ pub fn repo_file_content(path: String, file: String, rev: Option) -> Cmd Ok(Repo::discover(&path)?.file_content(&file, rev.as_deref())?) } +/// Raw file bytes (base64) for the image diff preview. `index = true` reads +/// the staged copy; otherwise `rev = None` reads the working tree and +/// `rev = Some(spec)` the blob at that revision. +#[tauri::command] +pub fn repo_file_blob( + path: String, + file: String, + rev: Option, + index: bool, +) -> CmdResult { + let source = if index { + BlobSource::Index + } else { + match rev.as_deref() { + Some(spec) => BlobSource::Rev(spec), + None => BlobSource::Worktree, + } + }; + Ok(Repo::discover(&path)?.file_blob(&file, source)?) +} + #[tauri::command] pub fn repo_file_history( path: String, @@ -246,6 +268,12 @@ pub fn repo_discard(path: String, file: String) -> CmdResult<()> { Ok(()) } +#[tauri::command] +pub fn repo_gitignore_add(path: String, pattern: String) -> CmdResult<()> { + Repo::discover(&path)?.gitignore_add(&pattern)?; + Ok(()) +} + #[tauri::command] pub fn repo_apply_patch(path: String, patch: String, target: String) -> CmdResult<()> { let t = match target.as_str() { @@ -480,6 +508,38 @@ pub fn repo_branch_delete(path: String, name: String, force: bool) -> CmdResult< Ok(()) } +#[tauri::command] +pub fn repo_branch_rename(path: String, old_name: String, new_name: String) -> CmdResult<()> { + Repo::discover(&path)?.rename_branch(&old_name, &new_name)?; + Ok(()) +} + +#[tauri::command] +pub fn repo_remote_add(path: String, name: String, url: String) -> CmdResult<()> { + Repo::discover(&path)?.add_remote(&name, &url)?; + Ok(()) +} + +#[tauri::command] +pub fn repo_remote_remove(path: String, name: String) -> CmdResult<()> { + Repo::discover(&path)?.remove_remote(&name)?; + Ok(()) +} + +/// Returns the refspecs git2 could not rewrite ("problems") — the rename has +/// already happened by then, so the UI warns instead of erroring; empty means +/// a clean rename. +#[tauri::command] +pub fn repo_remote_rename(path: String, old_name: String, new_name: String) -> CmdResult> { + Ok(Repo::discover(&path)?.rename_remote(&old_name, &new_name)?) +} + +#[tauri::command] +pub fn repo_remote_set_url(path: String, name: String, url: String) -> CmdResult<()> { + Repo::discover(&path)?.set_remote_url(&name, &url)?; + Ok(()) +} + #[tauri::command] pub fn repo_tag_create( path: String, @@ -566,6 +626,11 @@ pub fn repo_rebase(path: String, onto: String) -> CmdResult { Ok(Repo::discover(&path)?.rebase(&onto)?) } +#[tauri::command] +pub fn repo_reset(path: String, target: String, mode: ResetMode) -> CmdResult { + Ok(Repo::discover(&path)?.reset(&target, mode)?) +} + #[tauri::command] pub fn repo_abort_operation(path: String) -> CmdResult<()> { Repo::discover(&path)?.abort_operation()?; diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index bf52eea..f837546 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -74,6 +74,7 @@ fn main() { commands::repo_diff_commit_file, commands::repo_diff_workdir_file, commands::repo_file_content, + commands::repo_file_blob, commands::repo_file_history, commands::repo_blame, commands::repo_reflog, @@ -83,6 +84,7 @@ fn main() { commands::repo_unstage_many, commands::repo_discard_many, commands::repo_discard, + commands::repo_gitignore_add, commands::repo_apply_patch, commands::repo_commit, commands::repo_fetch, @@ -100,6 +102,11 @@ fn main() { commands::repo_worktree_prune, commands::repo_branch_create, commands::repo_branch_delete, + commands::repo_branch_rename, + commands::repo_remote_add, + commands::repo_remote_remove, + commands::repo_remote_rename, + commands::repo_remote_set_url, commands::repo_tag_create, commands::repo_tag_delete, commands::repo_tag_push, @@ -109,6 +116,7 @@ fn main() { commands::repo_revert, commands::repo_merge, commands::repo_rebase, + commands::repo_reset, commands::repo_abort_operation, commands::repo_continue_operation, commands::repo_rebase_todo, diff --git a/docs/improvements.md b/docs/improvements.md index 1b1eed6..7e996a5 100644 --- a/docs/improvements.md +++ b/docs/improvements.md @@ -34,6 +34,40 @@ reliability hole · **P1** = strong improvement, schedule soon · **P2** = nice > > Details per item below are the original proposals, kept for rationale. +## Second pass (2026-06-11) + +A ten-item implementation pass over the next ring of gaps — mostly "real git +client" table stakes plus two review-loop features. Everything landed in the +working tree in one batch: + +- **git reset** (soft/mixed/hard, safety snapshot before a dirty hard reset) + + reflog recovery menu + "Undo last commit" — `crates/strand-core/src/reset.rs`, + `ui/src/views/ResetDialog.tsx`, `ui/src/views/Reflog.tsx` +- **Remote management** (add/rename/set-url/remove) + **branch rename** — + `crates/strand-core/src/remote.rs`, `branch.rs`, `ui/src/views/RemoteDialog.tsx`, + `RenameBranchDialog.tsx`, `ui/src/components/Sidebar.tsx` +- **Commit signing** via shell-out when `commit.gpgSign=true` — + `crates/strand-core/src/commit.rs` (default unsigned path stays git2) +- **Gitignore quick-add** for untracked files (root-anchored + `*.`) — + `crates/strand-core/src/ignore.rs`, `ui/src/lib/ignore.ts` +- **fixup! commit creation + autosquash** in the rebase editor — + `ui/src/lib/rebase.ts`, `ui/src/views/RebaseEditor.tsx`, `Commits.tsx` +- **Copy diff as patch / Markdown** (tree menus + palette) — + `ui/src/lib/patchExport.ts` +- **In-diff text search** (⌘F over the whole diff pool in Local Changes + + Review) — `ui/src/lib/diffSearch.ts`, `ui/src/components/DiffSearchBar.tsx` +- **Image diff preview** (side-by-side Before/After in Local Changes, Review, + CommitDetail) — `crates/strand-core/src/file.rs` (`file_blob`), + `ui/src/components/ImageDiff.tsx` +- **Review annotations + feedback export** (notes on files/hunks → one + markdown prompt for the agent) — `ui/src/lib/reviewExport.ts`, + `ui/src/views/Review.tsx`, `ui/src/stores/repo.ts` + +Still deliberately open (assessed as on-radar, pulled-forward priorities — not +part of this pass): line-level staging, the rebase `edit` action, the +multi-repo-tab architecture work, full-history / `-G` content search, and a +GitHub/GitLab PR review surface. + --- ## 1. AI-change review as a first-class surface diff --git a/docs/learnings.md b/docs/learnings.md index fff3824..07c62e5 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -815,3 +815,49 @@ non-virtualized `FileDiff` path re-reads the prop, which masks the bug until virtualization is enabled (Review's diff pane). Also reset the scroll container to the top on swap: the virtualizer keeps the previous file's offset, and a deep offset into a short file shows an empty window. + +--- + +## A palette-triggered surface that focuses an input must re-claim focus itself + +**Rule.** When a one-shot store signal (the `commitSearchFocus` / +`diffSearchSignal` pattern) is fired from a palette action and the consumer +mounts something with an `autoFocus` input, the consumer must *re-claim* focus +after the palette unmounts — e.g. via a `requestAnimationFrame` call to an +exported focus helper (`focusDiffSearchInput()` in +`components/DiffSearchBar.tsx`). `autoFocus` alone is not enough. + +**Why.** The palette restores focus to its opener on unmount (a deliberate +a11y behavior — see the palette learning above). The order is: action runs → +signal consumer mounts and autofocuses → palette unmounts → focus snaps back +to the opener, silently stealing it from the input you just focused. The bug +is invisible if you only test the direct-shortcut path (⌘F), which never goes +through the palette. + +**How to apply.** Export a `focusX()` helper from the component that owns the +input; in the signal-consuming effect, clear the signal and call the helper +inside `requestAnimationFrame` so it lands after the palette's unmount +refocus. Any new `requestX()` one-shot signal whose consumer wants focus needs +the same treatment. + +--- + +## Palette actions over big store slices: gate on counts, read content at run time + +**Rule.** A `PaletteAction` whose availability depends on a large store array +(diffs, review notes, the log) must subscribe `App.tsx` only to a cheap +derived value — a length/count selector like `s.unstagedDiffs.length` — and +have its `run` read the live data via `useRepo.getState()` at invocation time. +Never subscribe App to the array itself just to build a palette entry. + +**Why.** `App.tsx` is the app's root; subscribing it to diff *content* means a +whole-app re-render on every watcher-driven refresh (which, with the file +watcher, is every agent write burst). Length-only selectors keep the gating +reactive while content churn stays free, and `getState()` at run time +guarantees the action still operates on fresh data. The copy-diff and +review-feedback palette actions are the canonical sites. + +**How to apply.** New action: add a count-only selector for the gate +(`useRepo(s => s.xs.length)`), spread the action conditionally, and inside +`run` call `useRepo.getState().xs`. If the action needs multiple slices, +read them all at run time rather than widening the subscription. diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 288bc30..59dae0a 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -5,6 +5,7 @@ import { Panel, PanelGroup, PanelResizeHandle } from 'react-resizable-panels'; import { HistoryModeToggle } from './components/HistoryModeToggle'; import { Icon } from './components/Icon'; +import { copyToClipboard } from './components/PierreTree'; import { ProgressPopup, formatDuration } from './components/ProgressPopup'; import { Sidebar } from './components/Sidebar'; import { StatusBar } from './components/StatusBar'; @@ -14,6 +15,8 @@ import { useRepo } from './stores/repo'; import { useUpdates } from './stores/updates'; import { pickRepoDirectory } from './lib/dialog'; import { editorTemplate, osType, terminalTemplate } from './lib/integrations'; +import { concatPatches, patchesToMarkdown } from './lib/patchExport'; +import { buildReviewFeedback } from './lib/reviewExport'; import { appMenuInstalled, installAppMenu, type MenuHandlers } from './lib/menu'; import { errMessage, isCancelled, isTauri, tauri } from './lib/tauri'; import { useTheme } from './lib/theme'; @@ -24,6 +27,9 @@ import { BranchDialog } from './views/BranchDialog'; import { TagDialog } from './views/TagDialog'; import { MergeDialog } from './views/MergeDialog'; import { RebaseEditor } from './views/RebaseEditor'; +import { RemoteDialog, type RemoteDialogMode } from './views/RemoteDialog'; +import { RenameBranchDialog } from './views/RenameBranchDialog'; +import { ResetDialog } from './views/ResetDialog'; import { Commits } from './views/Commits'; import { FileView } from './views/FileView'; import { LocalChanges } from './views/LocalChanges'; @@ -32,7 +38,7 @@ import { Review } from './views/Review'; import { Worktrees } from './views/Worktrees'; import { WorktreeDialog } from './views/WorktreeDialog'; import { CommandPalette, type PaletteAction } from './views/Palette'; -import type { Progress, RepoMeta, StatusKind } from './lib/types'; +import type { FileDiff, Progress, RepoMeta, StatusKind } from './lib/types'; const waitForPaint = () => new Promise((r) => requestAnimationFrame(() => requestAnimationFrame(() => r()))); @@ -121,6 +127,7 @@ export function App() { const createBranch = useRepo((s) => s.createBranch); const revealInGraph = useRepo((s) => s.revealInGraph); const requestCommitSearch = useRepo((s) => s.requestCommitSearch); + const requestDiffSearch = useRepo((s) => s.requestDiffSearch); const requestSelectSinceBaseline = useRepo((s) => s.requestSelectSinceBaseline); const selectCommit = useRepo((s) => s.selectCommit); @@ -138,6 +145,18 @@ export function App() { const setBaseline = useRepo((s) => s.setBaseline); const clearBaseline = useRepo((s) => s.clearBaseline); const stageReviewed = useRepo((s) => s.stageReviewed); + const clearReviewNotes = useRepo((s) => s.clearReviewNotes); + // Count-only selector (like the diff counts below): gates the feedback + // actions without re-rendering App on unrelated store churn. + const reviewNoteCount = useRepo((s) => + Object.values(s.reviewNotes).reduce((n, list) => n + list.length, 0)); + const resetTo = useRepo((s) => s.reset); + // Length-only selectors: they gate the "Copy … diff" palette actions without + // re-rendering App on every diff-content refresh (the actions read the + // actual diffs from the store at run time). + const unstagedCount = useRepo((s) => s.unstagedDiffs.length); + const stagedCount = useRepo((s) => s.stagedDiffs.length); + const baselineDiffCount = useRepo((s) => s.baselineDiffs.length); const [paletteOpen, setPaletteOpen] = useState(false); const [settingsOpen, setSettingsOpen] = useState(false); @@ -148,11 +167,17 @@ export function App() { // null = closed; otherwise the tag target (revspec, null ⇒ HEAD) + its label. const [tagDialog, setTagDialog] = useState<{ target: string | null; label: string } | null>(null); const [branchDialog, setBranchDialog] = useState<{ start: string | null; label: string } | null>(null); + // null = closed; otherwise which remote-management flavour (add/rename/url). + const [remoteDialog, setRemoteDialog] = useState(null); + // null = closed; otherwise the branch to rename. + const [renameBranchDialog, setRenameBranchDialog] = useState<{ name: string } | null>(null); // null = closed; otherwise the branch to merge (`source`) into the current (`into`). const [mergeDialog, setMergeDialog] = useState<{ source: string; into: string } | null>(null); // null = closed; otherwise the interactive-rebase base (revspec before the // first editable commit, null ⇒ root) + a short label for the blurb. const [rebaseDialog, setRebaseDialog] = useState<{ base: string | null; label: string } | null>(null); + // null = closed; otherwise the commit-ish to reset to + its human label. + const [resetDialog, setResetDialog] = useState<{ target: string; label: string } | null>(null); const [worktreeOpen, setWorktreeOpen] = useState(false); const [syncing, setSyncing] = useState(false); const [pulling, setPulling] = useState(false); @@ -219,6 +244,17 @@ export function App() { .catch((e) => showToast(`Open editor failed: ${errMessage(e)}`)); }, [showToast, openSettingsAt]); + // Copy a diff list to the clipboard (raw patch or Markdown) and confirm + // with a file count — powers the palette's "Copy … diff" actions. + const copyDiffs = useCallback( + (diffs: FileDiff[], markdown: boolean, title?: string) => { + copyToClipboard(markdown ? patchesToMarkdown(diffs, { title }) : concatPatches(diffs)); + const n = diffs.filter((d) => d.patch.length > 0).length; + showToast(`Copied diff — ${n} file${n === 1 ? '' : 's'}`); + }, + [showToast], + ); + // Flash a button's check pulse for ~1.6s. The duration outlasts the // pop-in animation so the check lingers briefly before reverting. const flashDone = useCallback((set: (v: boolean) => void) => { @@ -738,6 +774,13 @@ export function App() { { id: 'worktrees', label: 'Show: Worktrees', group: 'Actions', shortcut: '⌘5', keywords: 'worktree agent feature checkout overview', run: () => { setView('worktrees'); selectFile(null); } }, { id: 'worktree-new', label: 'New worktree…', group: 'Actions', keywords: 'worktree add branch checkout agent', run: () => setWorktreeOpen(true) }, { id: 'search-commits', label: 'Search commits…', group: 'Actions', shortcut: '/', keywords: 'find filter grep message author hash', run: () => { requestCommitSearch(); } }, + // Opens the ⌘F bar in whichever diff view is showing; other views + // route to Local Changes first (the signal is consumed on mount). + { id: 'search-diff', label: 'Search in diff…', group: 'Actions', shortcut: '⌘F', keywords: 'find in diff grep text content search', run: () => { + const v = useRepo.getState().view; + if (v !== 'local' && v !== 'review') setView('local'); + requestDiffSearch(); + } }, { id: 'review-baseline', label: baseline ? `Review: move baseline to HEAD (now at ${baseline.short})` : 'Review: pin baseline at HEAD', group: 'Actions', keywords: 'ai agent session since diff review baseline', run: () => { void setBaseline().then(() => { setView('review'); selectFile(null); }) .catch((e) => showToast(`Set baseline failed: ${errMessage(e)}`)); @@ -747,11 +790,53 @@ export function App() { { id: 'review-stage', label: 'Review: stage reviewed files', group: 'Actions', keywords: 'accept reviewed stage bulk', run: () => { void stageReviewed().catch((e) => showToast(`Stage reviewed failed: ${errMessage(e)}`)); } }, + // Notes → one Markdown prompt to paste back into the coding agent. + ...(reviewNoteCount > 0 ? [ + { id: 'review-copy-feedback', label: 'Review: copy feedback as prompt', group: 'Actions', keywords: 'ai agent review notes feedback prompt export clipboard', run: () => { + const st = useRepo.getState(); + const pool = st.baseline ? st.baselineDiffs : st.reviewUnstagedDiffs; + const files = pool + .filter((d) => (st.reviewNotes[d.path]?.length ?? 0) > 0) + .map((d) => ({ path: d.path, patch: d.patch, notes: st.reviewNotes[d.path] })); + if (!st.activePath || files.length === 0) { + showToast('No notes on the current review files'); + return; + } + copyToClipboard(buildReviewFeedback({ + repoName: st.activePath.split(/[\\/]/).filter(Boolean).pop() ?? st.activePath, + branch: st.meta?.branch ?? null, + baselineShort: st.baseline?.short ?? null, + files, + })); + const n = files.reduce((a, f) => a + f.notes.length, 0); + showToast(`Copied feedback — ${n} note${n === 1 ? '' : 's'} across ${files.length} file${files.length === 1 ? '' : 's'}`); + } } satisfies PaletteAction, + { id: 'review-clear-notes', label: 'Review: clear notes', group: 'Actions', keywords: 'ai agent review notes clear remove reset', run: () => { + clearReviewNotes(); + showToast('Review notes cleared'); + } } satisfies PaletteAction, + ] : []), + // Diff export — paste a patch into an AI agent / PR comment. Markdown + // for staged/review stays reachable via the file-tree context menus. + ...(unstagedCount > 0 ? [ + { id: 'copy-unstaged-diff', label: 'Copy unstaged diff', group: 'Actions', keywords: 'patch clipboard export unstaged', run: () => copyDiffs(useRepo.getState().unstagedDiffs, false) } satisfies PaletteAction, + { id: 'copy-unstaged-diff-md', label: 'Copy unstaged diff as Markdown', group: 'Actions', keywords: 'patch clipboard export unstaged markdown', run: () => copyDiffs(useRepo.getState().unstagedDiffs, true, 'Unstaged changes') } satisfies PaletteAction, + ] : []), + ...(stagedCount > 0 ? [{ id: 'copy-staged-diff', label: 'Copy staged diff', group: 'Actions', keywords: 'patch clipboard export staged', run: () => copyDiffs(useRepo.getState().stagedDiffs, false) } satisfies PaletteAction] : []), + ...(baseline && baselineDiffCount > 0 + ? [{ id: 'copy-review-diff', label: `Copy review diff (since ${baseline.short})`, group: 'Actions', keywords: 'patch clipboard export review baseline session', run: () => copyDiffs(useRepo.getState().baselineDiffs, false) } satisfies PaletteAction] + : []), { id: 'open-editor', label: 'Open in editor', group: 'Actions', keywords: 'external code reveal vscode editor', run: openInEditor }, { id: 'open-terminal', label: 'Open in terminal', group: 'Actions', keywords: 'shell console cwd iterm terminal', run: openInTerminal }, { id: 'snapshot', label: 'Save snapshot…', group: 'Actions', run: () => setStashDialog({ snapshot: true }) }, { id: 'stash', label: 'Stash changes…', group: 'Actions', run: () => setStashDialog({ snapshot: false }) }, { id: 'branch-new', label: 'Create branch…', group: 'Actions', keywords: 'new branch from head', run: () => setBranchDialog({ start: null, label: 'HEAD' }) }, + // Renaming the short OID HEAD shows while detached is meaningless — + // only offer the rename on a real branch. + ...(meta.branch && !meta.detached + ? [{ id: 'branch-rename', label: 'Rename current branch…', group: 'Actions', keywords: 'branch rename move', run: () => setRenameBranchDialog({ name: meta.branch }) } satisfies PaletteAction] + : []), + { id: 'remote-add', label: 'Add remote…', group: 'Actions', keywords: 'remote origin upstream url add', run: () => setRemoteDialog({ kind: 'add' }) }, { id: 'tag', label: 'Create tag…', group: 'Actions', run: () => setTagDialog({ target: null, label: 'HEAD' }) }, { id: 'push-tags', label: 'Push all tags', group: 'Actions', run: () => { void (async () => { @@ -767,6 +852,21 @@ export function App() { })(); } }, { id: 'sync', label: 'Sync (Fetch + Pull + Push)', group: 'Actions', shortcut: '⌘⇧S', run: onSync }, + // Gated on a non-root HEAD commit — HEAD~1 must exist to reset to. + ...(meta.head_oid && commits.find((c) => c.hash === meta.head_oid)?.parents.length + ? [{ + id: 'undo-commit', + label: 'Undo last commit (soft reset)', + group: 'Actions', + keywords: 'uncommit rollback reset soft head', + icon: 'history', + run: () => { + void resetTo('HEAD~1', 'soft') + .then(() => showToast('Last commit undone — changes kept staged')) + .catch((e) => showToast(`Undo failed: ${errMessage(e)}`)); + }, + } satisfies PaletteAction] + : []), { id: 'rebase-i', label: 'Interactive rebase…', group: 'Actions', keywords: 'rebase reorder squash fixup reword drop history edit', run: () => { const st = useRepo.getState(); const c = st.selectedCommit ? st.commits.find((x) => x.hash === st.selectedCommit) : null; @@ -811,8 +911,11 @@ export function App() { return [...base, ...repoActions, ...recentActions]; }, [setView, selectFile, onSync, openViaDialog, openByPath, setTheme, recents, pushAllTags, onNetProgress, showToast, meta, abortOperation, requestCommitSearch, - requestSelectSinceBaseline, openInEditor, openInTerminal, openSettingsAt, - repoActions, setRebaseDialog, baseline, setBaseline, clearBaseline, stageReviewed]); + requestDiffSearch, requestSelectSinceBaseline, openInEditor, openInTerminal, openSettingsAt, + repoActions, setRebaseDialog, setRemoteDialog, setRenameBranchDialog, + baseline, setBaseline, clearBaseline, stageReviewed, commits, resetTo, + unstagedCount, stagedCount, baselineDiffCount, copyDiffs, + reviewNoteCount, clearReviewNotes]); const rootStyle = { '--font-ui': FONTS.ui[uiFont], @@ -853,6 +956,8 @@ export function App() { onCreateWorktree={() => setWorktreeOpen(true)} onMerge={(source, into) => setMergeDialog({ source, into })} onInteractiveRebase={(base, label) => setRebaseDialog({ base, label })} + onManageRemote={(mode) => setRemoteDialog(mode)} + onRenameBranch={(name) => setRenameBranchDialog({ name })} onToast={showToast} /> @@ -866,7 +971,13 @@ export function App() { {view === 'local' && } {view === 'review' && } - {view === 'reflog' && } + {view === 'reflog' && ( + setResetDialog({ target, label })} + onCreateBranch={(start, label) => setBranchDialog({ start, label })} + onToast={showToast} + /> + )} {view === 'worktrees' && ( setWorktreeOpen(true)} onToast={showToast} /> )} @@ -874,6 +985,7 @@ export function App() { setTagDialog({ target, label })} onInteractiveRebase={(base, label) => setRebaseDialog({ base, label })} + onResetTo={(target, label) => setResetDialog({ target, label })} onToast={showToast} /> )} @@ -969,6 +1081,18 @@ export function App() { /> )} + {remoteDialog && ( + setRemoteDialog(null)} onToast={showToast} /> + )} + + {renameBranchDialog && ( + setRenameBranchDialog(null)} + onToast={showToast} + /> + )} + {mergeDialog && ( )} + {resetDialog && ( + setResetDialog(null)} + onToast={showToast} + /> + )} + {worktreeOpen && setWorktreeOpen(false)} />} {!isTauri() && !meta && ( diff --git a/ui/src/components/DiffSearchBar.tsx b/ui/src/components/DiffSearchBar.tsx new file mode 100644 index 0000000..431aa89 --- /dev/null +++ b/ui/src/components/DiffSearchBar.tsx @@ -0,0 +1,153 @@ +import { useEffect, useMemo, useRef, useState } from 'react'; + +import { searchDiffs, type DiffMatch } from '../lib/diffSearch'; +import type { FileDiff } from '../lib/types'; +import { Icon } from './Icon'; + +/** + * Focus (and select) an open bar's input — for re-invoking ⌘F while the bar + * is already up, and for handing it focus after the command palette closes + * (the palette restores focus on unmount, which would otherwise steal it + * from the freshly mounted input). + */ +export function focusDiffSearchInput(): void { + requestAnimationFrame(() => { + const el = document.querySelector('.diff-search-bar input'); + el?.focus(); + el?.select(); + }); +} + +/** + * Floating ⌘F text-search bar over a diff pane (Local Changes / Review). + * Matches are computed with `searchDiffs` over the whole pool; Enter / + * Shift+Enter (or ↓/↑) step through them with wrapping, calling `onJump` + * so the owner selects the matched file. The preview line under the input + * carries the match's path + text, since jumping only lands on the file — + * not yet the exact line inside the virtualized diff. + */ +export function DiffSearchBar({ + diffs, + onJump, + onClose, + placeholder, +}: { + diffs: Pick[]; + onJump: (m: DiffMatch) => void; + onClose: () => void; + placeholder?: string; +}) { + const [query, setQuery] = useState(''); + // The matcher runs on a debounced copy so typing into a 40-file pool + // doesn't re-scan megabytes of patch text per keystroke. + const [debounced, setDebounced] = useState(''); + useEffect(() => { + const t = window.setTimeout(() => setDebounced(query), 120); + return () => window.clearTimeout(t); + }, [query]); + + const result = useMemo(() => searchDiffs(diffs, debounced), [diffs, debounced]); + const matches = result.matches; + + // -1 = not navigated yet: the first Enter lands on the first match instead + // of stepping past it. Reset whenever the match list recomputes. + const [pos, setPos] = useState(-1); + useEffect(() => setPos(-1), [matches]); + + const inputRef = useRef(null); + + const step = (dir: 1 | -1) => { + if (matches.length === 0) return; + const next = + pos === -1 + ? dir === 1 + ? 0 + : matches.length - 1 + : (pos + dir + matches.length) % matches.length; + setPos(next); + onJump(matches[next]); + // Stepping from the prev/next buttons must not strand focus there. + inputRef.current?.focus(); + }; + + const current = matches.length > 0 ? matches[Math.max(0, pos)] : null; + const total = `${matches.length}${result.truncated ? '+' : ''}`; + + return ( +
+
+ + setQuery(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault(); + step(e.shiftKey ? -1 : 1); + } else if (e.key === 'ArrowDown') { + e.preventDefault(); + step(1); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + step(-1); + } else if (e.key === 'Escape') { + e.preventDefault(); + onClose(); + } + }} + /> + + {!debounced.trim() + ? '' + : matches.length === 0 + ? 'No results' + : pos >= 0 + ? `${pos + 1}/${total}` + : `${total} found`} + + + + +
+ {current && ( +
+ + {current.path} + {current.lineText.trim()} +
+ )} +
+ ); +} diff --git a/ui/src/components/ImageDiff.tsx b/ui/src/components/ImageDiff.tsx new file mode 100644 index 0000000..850869c --- /dev/null +++ b/ui/src/components/ImageDiff.tsx @@ -0,0 +1,118 @@ +import { useEffect, useState } from 'react'; + +import { formatBytes, imageMime } from '../lib/image'; +import { errMessage, tauri } from '../lib/tauri'; +import { useRepo } from '../stores/repo'; +import type { FileBlob } from '../lib/types'; + +/** + * Where one side of an image diff reads from: `rev` null with no `index` = + * the working tree; `index: true` = the staged copy; `rev: 'HEAD'` etc. = the + * blob at that revision. A null side (the prop, not the ref) means the file + * doesn't exist there — added files have no Before, deleted files no After. + */ +export interface BlobRef { + rev: string | null; + index?: boolean; +} + +type SideState = + | { kind: 'loading' } + | { kind: 'error'; message: string } + | { kind: 'ok'; blob: FileBlob }; + +function useBlob(path: string, src: BlobRef | null): SideState { + const activePath = useRepo((s) => s.activePath); + const [state, setState] = useState({ kind: 'loading' }); + // Primitive deps so inline-object props don't refetch every render. + const absent = src == null; + const rev = src?.rev ?? null; + const index = src?.index ?? false; + useEffect(() => { + if (absent || !activePath) return; + let cancelled = false; + setState({ kind: 'loading' }); + tauri + .repoFileBlob(activePath, path, rev, index) + .then((b) => { if (!cancelled) setState({ kind: 'ok', blob: b }); }) + .catch((e) => { if (!cancelled) setState({ kind: 'error', message: errMessage(e) }); }); + return () => { cancelled = true; }; + }, [activePath, path, absent, rev, index]); + return state; +} + +/** + * Side-by-side before/after preview for a binary image diff. Each pane sits + * on a checkerboard so transparency reads; a null side (added/deleted file) + * collapses to a single pane. + */ +export function ImageDiff({ + path, + oldSrc, + newSrc, +}: { + path: string; + oldSrc: BlobRef | null; + newSrc: BlobRef | null; +}) { + const before = useBlob(path, oldSrc); + const after = useBlob(path, newSrc); + return ( +
+ {oldSrc != null && } + {newSrc != null && } +
+ ); +} + +function ImagePane({ + label, + tone, + path, + state, +}: { + label: string; + tone: 'del' | 'add'; + path: string; + state: SideState; +}) { + const [dims, setDims] = useState<{ w: number; h: number } | null>(null); + const src = + state.kind === 'ok' && !state.blob.too_large + ? `data:${imageMime(path)};base64,${state.blob.base64}` + : null; + // New image bytes → stale dimensions; remeasure from the next onLoad. + useEffect(() => setDims(null), [src]); + + return ( +
+
{label}
+
+ {state.kind === 'loading' ? ( + Loading… + ) : state.kind === 'error' ? ( + + Couldn’t load this image. + + ) : state.blob.too_large ? ( + Image too large to preview (>8 MB) + ) : ( + // A data-URL'd SVG in an never executes scripts — safe to preview. + {`${label}: + setDims({ w: e.currentTarget.naturalWidth, h: e.currentTarget.naturalHeight }) + } + /> + )} +
+ {state.kind === 'ok' && ( +
+ {dims ? `${dims.w}×${dims.h} · ` : ''} + {formatBytes(state.blob.size)} +
+ )} +
+ ); +} diff --git a/ui/src/components/Sidebar.tsx b/ui/src/components/Sidebar.tsx index 2bca5d6..d3e3278 100644 --- a/ui/src/components/Sidebar.tsx +++ b/ui/src/components/Sidebar.tsx @@ -4,9 +4,11 @@ import type { GitStatusEntry } from '@pierre/trees'; import { ContextMenu, type MenuItem } from './ContextMenu'; import { Icon, type IconName } from './Icon'; import { copyToClipboard, PierreTree, workStatusToGit, type TreeMenuItem } from './PierreTree'; +import { ignorePatterns } from '../lib/ignore'; import { errMessage } from '../lib/tauri'; import { defaultRemote, useRepo } from '../stores/repo'; import type { Branch, RemoteBranch, Stash, Submodule, SubmoduleState, Tag, Worktree } from '../lib/types'; +import type { RemoteDialogMode } from '../views/RemoteDialog'; type SideTab = 'git' | 'files'; @@ -70,6 +72,10 @@ interface SidebarProps { onMerge: (source: string, into: string) => void; /** Open the interactive-rebase editor over `base..HEAD` (base null = root). */ onInteractiveRebase: (base: string | null, label: string) => void; + /** Open the remote-management dialog in the given mode (add/rename/url). */ + onManageRemote: (mode: RemoteDialogMode) => void; + /** Open the Rename-branch dialog for the branch `name`. */ + onRenameBranch: (name: string) => void; /** Surface a transient message (tag push / remote-delete feedback). */ onToast: (msg: string) => void; } @@ -117,7 +123,7 @@ function sortTree(node: TreeNode, leafCmp: (a: T, b: T) => number): void { // ─── component ────────────────────────────────────────────────────────── -export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, onCreateBranch, onCreateWorktree, onMerge, onInteractiveRebase, onToast }: SidebarProps) { +export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, onCreateBranch, onCreateWorktree, onMerge, onInteractiveRebase, onManageRemote, onRenameBranch, onToast }: SidebarProps) { const view = useRepo((s) => s.view); const setView = useRepo((s) => s.setView); const selectFile = useRepo((s) => s.selectFile); @@ -132,6 +138,7 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, const revealInGraph = useRepo((s) => s.revealInGraph); const createBranch = useRepo((s) => s.createBranch); const deleteBranch = useRepo((s) => s.deleteBranch); + const removeRemote = useRepo((s) => s.removeRemote); const deleteTag = useRepo((s) => s.deleteTag); const pushTag = useRepo((s) => s.pushTag); const deleteRemoteTag = useRepo((s) => s.deleteRemoteTag); @@ -139,6 +146,7 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, const refreshRemoteTags = useRepo((s) => s.refreshRemoteTags); const workTree = useRepo((s) => s.workTree); const refreshTree = useRepo((s) => s.refreshTree); + const gitignoreAdd = useRepo((s) => s.gitignoreAdd); const stashes = useRepo((s) => s.stashes); const stashApply = useRepo((s) => s.stashApply); const stashPop = useRepo((s) => s.stashPop); @@ -297,15 +305,35 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, [workTree], ); const fileMenu = useCallback( - (targets: string[]): TreeMenuItem[] => [ - { label: 'Open', icon: 'content', onSelect: () => selectFile(targets[0]) }, - { + (targets: string[]): TreeMenuItem[] => { + const items: TreeMenuItem[] = [ + { label: 'Open', icon: 'content', onSelect: () => selectFile(targets[0]) }, + ]; + // .gitignore quick actions for a single untracked file. + if ( + targets.length === 1 && + workTree.some((e) => e.path === targets[0] && e.status === 'UNTRACKED') + ) { + const ignore = (pattern: string) => + void gitignoreAdd(pattern).catch((e) => onToast(`Ignore failed: ${errMessage(e)}`)); + const { exact, extension } = ignorePatterns(targets[0]); + items.push({ label: 'Add to .gitignore', icon: 'file', onSelect: () => ignore(exact) }); + if (extension) { + items.push({ + label: `Ignore all ${extension} files`, + icon: 'file', + onSelect: () => ignore(extension), + }); + } + } + items.push({ label: targets.length > 1 ? 'Copy paths' : 'Copy path', icon: 'file', onSelect: () => copyToClipboard(targets.join('\n')), - }, - ], - [selectFile], + }); + return items; + }, + [selectFile, workTree, gitignoreAdd, onToast], ); const runBranchOp = async (fn: () => Promise) => { @@ -362,6 +390,11 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, icon: 'plus', onSelect: () => onCreateBranch(b.name, b.name), }; + const renameItem: MenuItem = { + label: 'Rename branch…', + icon: 'edit', + onSelect: () => onRenameBranch(b.name), + }; if (b.is_head) { // Interactive rebase over the commits this branch is ahead of its // upstream (`upstream..HEAD`) — the unpushed work it's safe to edit. @@ -369,6 +402,7 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, return [ { label: 'Current branch', disabled: true, onSelect: () => {} }, newBranchItem, + renameItem, { label: 'Interactive rebase…', icon: 'rebase', @@ -382,6 +416,7 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, const items: MenuItem[] = [ { label: 'Checkout', icon: 'branch', onSelect: () => void runBranchOp(() => checkout(b.name)) }, newBranchItem, + renameItem, ]; if (currentBranch) { items.push({ label: `Merge into ${currentBranch}`, icon: 'branch', onSelect: () => onMerge(b.name, currentBranch) }); @@ -416,6 +451,27 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, return items; }; + // Menu for a remotes-tree top-level folder — the remote itself (`origin`), + // not one of its branches. + const remoteFolderMenu = (name: string): MenuItem[] => { + const url = refs.remotes.find((r) => r.name === name)?.url ?? null; + const items: MenuItem[] = [ + { label: 'Edit URL…', icon: 'edit', onSelect: () => onManageRemote({ kind: 'url', name, url: url ?? '' }) }, + { label: 'Rename…', icon: 'edit', onSelect: () => onManageRemote({ kind: 'rename', name }) }, + ]; + if (url) { + items.push({ label: 'Copy URL', icon: 'file', onSelect: () => { void copyToClipboard(url); onToast('Copied'); } }); + } + items.push({ + label: 'Remove remote', + icon: 'trash', + danger: true, + confirm: true, + onSelect: () => void removeRemote(name).catch((e) => onToast(`Remove failed: ${errMessage(e)}`)), + }); + return items; + }; + const tagMenu = (tg: Tag): MenuItem[] => { const items: MenuItem[] = [ { label: 'Checkout', icon: 'branch', onSelect: () => void runBranchOp(() => checkoutCommit(tg.target)) }, @@ -663,11 +719,14 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, collapsed={!sections.remotes} onToggle={() => toggle('remotes')} count={refs.remotes.length} + action={{ icon: 'plus', title: 'Add remote…', onClick: () => onManageRemote({ kind: 'add' }) }} /> {sections.remotes && renderTreeChildren(remoteTree, 0, collapsed, toggleCollapsed, renderRemoteLeaf, 'remotes', { folderIcon: 'remote', showFolderCount: false, + folderMenu: remoteFolderMenu, + openMenu, })} ( toggleCollapsed: (path: string) => void, renderLeaf: (item: T, depth: number) => React.ReactNode, keyPrefix: string, - folderOpts?: { folderIcon?: IconName; showFolderCount?: boolean }, + folderOpts?: { + folderIcon?: IconName; + showFolderCount?: boolean; + /** Context-menu items for a *top-level* (depth-0) folder — the remotes + * tree's remote-name rows. Opened via `openMenu` (the owner's menu state). */ + folderMenu?: (name: string) => MenuItem[]; + openMenu?: (x: number, y: number, items: MenuItem[]) => void; + }, ): React.ReactNode { + const open = folderOpts?.openMenu; + const menuFor = depth === 0 ? folderOpts?.folderMenu : undefined; return node.children.map((child) => { if (child.children.length === 0 && child.leaf != null) { return renderLeaf(child.leaf, depth); @@ -777,6 +845,7 @@ function renderTreeChildren( icon={folderOpts?.folderIcon ?? 'folder'} count={folderOpts?.showFolderCount === false ? undefined : leafCount(child)} onToggle={() => toggleCollapsed(collapseKey)} + onMenu={menuFor && open ? (x, y) => open(x, y, menuFor(child.name)) : undefined} /> {!isCollapsed && renderTreeChildren(child, depth + 1, collapsed, toggleCollapsed, renderLeaf, keyPrefix, folderOpts)} @@ -829,15 +898,42 @@ interface FolderRowProps { /** Child count shown on the right; omit to hide it (e.g. remote groups). */ count?: number; onToggle: () => void; + /** Open the folder's action menu at the given viewport coordinates — + * wired like SideLeaf's (right-click, or ContextMenu / Shift+F10). */ + onMenu?: (x: number, y: number) => void; } -function FolderRow({ name, depth, collapsed, icon = 'folder', count, onToggle }: FolderRowProps) { +function FolderRow({ name, depth, collapsed, icon = 'folder', count, onToggle, onMenu }: FolderRowProps) { + const rowRef = useRef(null); + const openKeyboardMenu = () => { + const r = rowRef.current?.getBoundingClientRect(); + if (r) onMenu?.(r.left + 12, r.bottom - 4); + }; return ( + {onNote && ( + + )} ); } diff --git a/ui/src/views/RebaseEditor.tsx b/ui/src/views/RebaseEditor.tsx index 150325b..590070f 100644 --- a/ui/src/views/RebaseEditor.tsx +++ b/ui/src/views/RebaseEditor.tsx @@ -1,6 +1,7 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { Icon } from '../components/Icon'; +import { autosquashPlan } from '../lib/rebase'; import { errMessage } from '../lib/tauri'; import { useRepo } from '../stores/repo'; import type { RebaseAction, RebaseStep } from '../lib/types'; @@ -64,6 +65,9 @@ export function RebaseEditor({ const interactiveRebase = useRepo((s) => s.interactiveRebase); const [rows, setRows] = useState(null); + // How many fixup!/squash! commits autosquash moved under their targets + // (0 = none, no notice). The seeded plan stays fully editable. + const [autosquashed, setAutosquashed] = useState(0); const [loadError, setLoadError] = useState(null); const [focused, setFocused] = useState(0); const [busy, setBusy] = useState(false); @@ -91,17 +95,25 @@ export function RebaseEditor({ .then((entries) => { if (!alive) return; origOrderRef.current = entries.map((e) => e.oid); - setRows( - entries.map((e) => ({ - oid: e.oid, - short: e.short, - subject: e.subject, - author: e.author, - isMerge: e.is_merge, - action: 'pick', - message: e.subject, - })), - ); + let next: Row[] = entries.map((e) => ({ + oid: e.oid, + short: e.short, + subject: e.subject, + author: e.author, + isMerge: e.is_merge, + action: 'pick', + message: e.subject, + })); + // Seed the plan like `git rebase --autosquash`: fixup!/squash! commits + // move under their targets with the matching verb. Still just a seed — + // every row stays editable, and isNoop correctly sees a non-noop plan. + const plan = autosquashPlan(entries.map((e) => ({ oid: e.oid, subject: e.subject }))); + if (plan) { + const byOid = new Map(next.map((r) => [r.oid, r])); + next = plan.map((s) => ({ ...byOid.get(s.oid)!, action: s.action })); + } + setAutosquashed(plan ? plan.filter((s) => s.action !== 'pick').length : 0); + setRows(next); setFocused(0); }) .catch((e) => { @@ -277,6 +289,13 @@ export function RebaseEditor({ p/r/s/f/d.

+ {autosquashed > 0 ? ( +
+ Autosquash: {autosquashed} fixup commit{autosquashed === 1 ? '' : 's'} moved under{' '} + {autosquashed === 1 ? 'its target' : 'their targets'} — review the plan before + running. +
+ ) : null} {hasMerges ? (
This range contains a merge commit — interactive rebase flattens merges into a diff --git a/ui/src/views/Reflog.tsx b/ui/src/views/Reflog.tsx index 3284371..d4b76fa 100644 --- a/ui/src/views/Reflog.tsx +++ b/ui/src/views/Reflog.tsx @@ -1,7 +1,10 @@ -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import type { KeyboardEvent } from 'react'; import { useRepo } from '../stores/repo'; +import { errMessage } from '../lib/tauri'; +import type { ReflogEntry } from '../lib/types'; +import { ContextMenu, type MenuItem } from '../components/ContextMenu'; import { Icon } from '../components/Icon'; /** @@ -10,19 +13,71 @@ import { Icon } from '../components/Icon'; * Unlike the commit graph (reachable history), the reflog includes commits * orphaned by a reset / rebase / amend, so it's the recovery path back to "lost" * work. Each row jumps to its target commit in the graph; if that commit is no - * longer reachable it won't appear there, but the short OID shown here is enough - * to recover it manually. Lazy-loaded: only this view triggers `refreshReflog`. + * longer reachable it won't appear there, but the right-click menu recovers it + * directly (checkout / branch / reset). Lazy-loaded: only this view triggers + * `refreshReflog`. */ -export function Reflog() { +export function Reflog({ + onResetTo, + onCreateBranch, + onToast, +}: { + /** Open the Reset dialog targeting a commit (revspec + label). */ + onResetTo: (target: string, label: string) => void; + /** Open the New-branch dialog from a start point (revspec + label). */ + onCreateBranch: (start: string, label: string) => void; + onToast: (msg: string) => void; +}) { const activePath = useRepo((s) => s.activePath); const reflog = useRepo((s) => s.reflog); const refreshReflog = useRepo((s) => s.refreshReflog); const revealInGraph = useRepo((s) => s.revealInGraph); + const checkoutCommit = useRepo((s) => s.checkoutCommit); const [focused, setFocused] = useState(0); const listRef = useRef(null); const focusedRowRef = useRef(null); + // Right-click (or Menu / Shift+F10) on a row — the recovery actions for the + // commit this entry points at, same pattern as the Commits graph. + const [menu, setMenu] = useState<{ x: number; y: number; items: MenuItem[] } | null>(null); + const openEntryMenu = useCallback( + (entry: ReflogEntry, x: number, y: number) => { + const selector = `HEAD@{${entry.index}}`; + const items: MenuItem[] = [ + { + label: 'Jump to in graph', + icon: 'graph', + onSelect: () => revealInGraph(entry.new_oid), + }, + { + label: 'Checkout (detached)', + icon: 'branch', + onSelect: () => void (async () => { + try { + await checkoutCommit(entry.new_oid); + onToast(`Checked out ${entry.new_short} (detached)`); + } catch (e) { + onToast(`Checkout failed: ${errMessage(e)}`); + } + })(), + }, + { + label: 'Create branch here…', + icon: 'plus', + onSelect: () => onCreateBranch(entry.new_oid, entry.new_short), + }, + { + label: 'Reset HEAD here…', + icon: 'history', + onSelect: () => onResetTo(entry.new_oid, selector), + }, + ]; + setMenu({ x, y, items }); + }, + [revealInGraph, checkoutCommit, onCreateBranch, onResetTo, onToast], + ); + // Load (and reload on tab switch) whenever this view is mounted. Re-reading on // focus is handled by the row click → graph jump, which leaves this view. useEffect(() => { @@ -50,6 +105,13 @@ export function Reflog() { e.preventDefault(); const entry = reflog[focused]; if (entry) revealInGraph(entry.new_oid); + } else if (e.key === 'ContextMenu' || (e.shiftKey && e.key === 'F10')) { + const entry = reflog[focused]; + const r = focusedRowRef.current?.getBoundingClientRect(); + if (entry && r) { + e.preventDefault(); + openEntryMenu(entry, r.left + 24, r.bottom - 6); + } } }; @@ -64,41 +126,51 @@ export function Reflog() { } return ( -
- {reflog.map((e, i) => { - const { op, rest } = splitMessage(e.message); - return ( - - ); - })} -
+ <> +
+ {reflog.map((e, i) => { + const { op, rest } = splitMessage(e.message); + return ( + + ); + })} +
+ {menu && ( + setMenu(null)} /> + )} + ); } diff --git a/ui/src/views/RemoteDialog.tsx b/ui/src/views/RemoteDialog.tsx new file mode 100644 index 0000000..a403cb5 --- /dev/null +++ b/ui/src/views/RemoteDialog.tsx @@ -0,0 +1,203 @@ +import { useEffect, useRef, useState } from 'react'; + +import { Icon } from '../components/Icon'; +import { errMessage } from '../lib/tauri'; +import { useRepo } from '../stores/repo'; + +/** Which remote-management flavour the dialog opens in. */ +export type RemoteDialogMode = + | { kind: 'add' } + | { kind: 'rename'; name: string } + | { kind: 'url'; name: string; url: string }; + +/** + * Modal for managing remotes — add a new one (Name + URL), rename an existing + * one, or edit its URL. Opened from the Remotes section `+`, a remote folder + * row's context menu, and the command palette ("Add remote…"). Submits to the + * matching store action; success surfaces via `onToast`. + */ +export function RemoteDialog({ + mode, + onClose, + onToast, +}: { + mode: RemoteDialogMode; + onClose: () => void; + onToast: (msg: string) => void; +}) { + const addRemote = useRepo((s) => s.addRemote); + const renameRemote = useRepo((s) => s.renameRemote); + const setRemoteUrl = useRepo((s) => s.setRemoteUrl); + + const [name, setName] = useState(mode.kind === 'rename' ? mode.name : ''); + const [url, setUrl] = useState(mode.kind === 'url' ? mode.url : ''); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const dialogRef = useRef(null); + const mountedRef = useRef(true); + useEffect(() => () => { mountedRef.current = false; }, []); + + // Restore focus to whatever opened the dialog when it closes, so keyboard + // flow returns to the sidebar/palette instead of falling to . + useEffect(() => { + const prev = document.activeElement as HTMLElement | null; + return () => prev?.focus?.(); + }, []); + + // Keep Tab focus inside the modal — same aria-modal contract as BranchDialog. + function onTrapKeyDown(e: React.KeyboardEvent) { + if (e.key !== 'Tab' || !dialogRef.current) return; + const focusables = Array.from( + dialogRef.current.querySelectorAll( + 'a[href], button:not([disabled]), input:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])', + ), + ); + if (focusables.length === 0) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (e.shiftKey && document.activeElement === first) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && document.activeElement === last) { + e.preventDefault(); + first.focus(); + } + } + + // Escape closes (unless an op is mid-flight). + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape' && !busy) onClose(); + }; + document.addEventListener('keydown', onKey); + return () => document.removeEventListener('keydown', onKey); + }, [busy, onClose]); + + const title = mode.kind === 'add' ? 'Add remote' + : mode.kind === 'rename' ? 'Rename remote' + : 'Edit remote URL'; + const submitLabel = mode.kind === 'add' ? 'Add remote' + : mode.kind === 'rename' ? 'Rename' + : 'Save URL'; + + async function submit() { + if (busy) return; + const remoteName = name.trim(); + const remoteUrl = url.trim(); + if (mode.kind !== 'url' && !remoteName) { + setError('Remote name is required.'); + return; + } + if (mode.kind !== 'rename' && !remoteUrl) { + setError('Remote URL is required.'); + return; + } + setBusy(true); + setError(null); + try { + if (mode.kind === 'add') { + await addRemote(remoteName, remoteUrl); + onToast(`Remote ${remoteName} added`); + } else if (mode.kind === 'rename') { + const problems = await renameRemote(mode.name, remoteName); + // Non-empty problems = the rename happened, but git could not rewrite + // these (non-default) refspecs — warn instead of claiming a clean run. + onToast( + problems.length > 0 + ? `Remote renamed — ${problems.length} refspec(s) need manual attention` + : `Remote ${mode.name} renamed to ${remoteName}`, + ); + } else { + await setRemoteUrl(mode.name, remoteUrl); + onToast(`Remote ${mode.name} URL updated`); + } + onClose(); + } catch (e) { + if (mountedRef.current) setError(errMessage(e)); + } finally { + if (mountedRef.current) setBusy(false); + } + } + + return ( +
{ + if (e.target === e.currentTarget && !busy) onClose(); + }} + > +
+
+ + {title} + +
+ +
+ {mode.kind !== 'add' && ( +

+ {mode.kind === 'rename' ? 'Rename remote ' : 'Change the URL of '} + {mode.name}. +

+ )} + + {mode.kind !== 'url' && ( + + )} + + {mode.kind !== 'rename' && ( + + )} + + {error ?
{error}
: null} +
+ +
+ + +
+
+
+ ); +} diff --git a/ui/src/views/RenameBranchDialog.tsx b/ui/src/views/RenameBranchDialog.tsx new file mode 100644 index 0000000..42310c7 --- /dev/null +++ b/ui/src/views/RenameBranchDialog.tsx @@ -0,0 +1,150 @@ +import { useEffect, useRef, useState } from 'react'; + +import { Icon } from '../components/Icon'; +import { errMessage } from '../lib/tauri'; +import { useRepo } from '../stores/repo'; + +/** + * Modal for renaming a local branch (`git branch -m`). Opened from a branch + * row's context menu and the palette's "Rename current branch…". The branch's + * upstream config moves with the rename, and HEAD follows when the renamed + * branch is checked out (core behavior). + */ +export function RenameBranchDialog({ + name, + onClose, + onToast, +}: { + name: string; + onClose: () => void; + onToast: (msg: string) => void; +}) { + const renameBranch = useRepo((s) => s.renameBranch); + + const [newName, setNewName] = useState(name); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const dialogRef = useRef(null); + const mountedRef = useRef(true); + useEffect(() => () => { mountedRef.current = false; }, []); + + // Restore focus to whatever opened the dialog when it closes, so keyboard + // flow returns to the sidebar/palette instead of falling to . + useEffect(() => { + const prev = document.activeElement as HTMLElement | null; + return () => prev?.focus?.(); + }, []); + + // Keep Tab focus inside the modal — same aria-modal contract as BranchDialog. + function onTrapKeyDown(e: React.KeyboardEvent) { + if (e.key !== 'Tab' || !dialogRef.current) return; + const focusables = Array.from( + dialogRef.current.querySelectorAll( + 'a[href], button:not([disabled]), input:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])', + ), + ); + if (focusables.length === 0) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (e.shiftKey && document.activeElement === first) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && document.activeElement === last) { + e.preventDefault(); + first.focus(); + } + } + + // Escape closes (unless an op is mid-flight). + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape' && !busy) onClose(); + }; + document.addEventListener('keydown', onKey); + return () => document.removeEventListener('keydown', onKey); + }, [busy, onClose]); + + async function submit() { + if (busy) return; + const branchName = newName.trim(); + if (!branchName) { + setError('Branch name is required.'); + return; + } + if (branchName === name) { + onClose(); + return; + } + setBusy(true); + setError(null); + try { + await renameBranch(name, branchName); + onToast(`Branch ${name} renamed to ${branchName}`); + onClose(); + } catch (e) { + if (mountedRef.current) setError(errMessage(e)); + } finally { + if (mountedRef.current) setBusy(false); + } + } + + return ( +
{ + if (e.target === e.currentTarget && !busy) onClose(); + }} + > +
+
+ + Rename branch + +
+ +
+

+ Rename {name} — its upstream moves along. +

+ + + + {error ?
{error}
: null} +
+ +
+ + +
+
+
+ ); +} diff --git a/ui/src/views/ResetDialog.tsx b/ui/src/views/ResetDialog.tsx new file mode 100644 index 0000000..bd47bf0 --- /dev/null +++ b/ui/src/views/ResetDialog.tsx @@ -0,0 +1,164 @@ +import { useEffect, useRef, useState } from 'react'; + +import { Icon } from '../components/Icon'; +import { errMessage } from '../lib/tauri'; +import { useRepo } from '../stores/repo'; +import type { ResetMode } from '../lib/types'; + +/** + * Modal for `git reset` to a chosen commit — opened from a commit row's + * "Reset … to here…" and the Reflog's "Reset HEAD here…". + * + * `target` is the revspec to reset to; `label` is the human label shown in + * the blurb (short hash or `HEAD@{n}`). Mode defaults to mixed; hard resets + * stash a safety snapshot first (the backend does this for a dirty tree), + * which the success toast points at. + */ +export function ResetDialog({ + target, + label, + onClose, + onToast, +}: { + target: string; + label: string; + onClose: () => void; + onToast: (msg: string) => void; +}) { + const reset = useRepo((s) => s.reset); + const meta = useRepo((s) => s.meta); + const headLabel = meta && !meta.detached ? meta.branch : 'HEAD'; + + const [mode, setMode] = useState('mixed'); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const dialogRef = useRef(null); + const mountedRef = useRef(true); + useEffect(() => () => { mountedRef.current = false; }, []); + + // Restore focus to whatever opened the dialog when it closes, so keyboard + // flow returns to the graph/reflog instead of falling to . + useEffect(() => { + const prev = document.activeElement as HTMLElement | null; + return () => prev?.focus?.(); + }, []); + + // Keep Tab focus inside the modal — same aria-modal contract as BranchDialog. + function onTrapKeyDown(e: React.KeyboardEvent) { + if (e.key !== 'Tab' || !dialogRef.current) return; + const focusables = Array.from( + dialogRef.current.querySelectorAll( + 'a[href], button:not([disabled]), input:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])', + ), + ); + if (focusables.length === 0) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (e.shiftKey && document.activeElement === first) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && document.activeElement === last) { + e.preventDefault(); + first.focus(); + } + } + + // Escape closes (unless an op is mid-flight). + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape' && !busy) onClose(); + }; + document.addEventListener('keydown', onKey); + return () => document.removeEventListener('keydown', onKey); + }, [busy, onClose]); + + async function submit() { + if (busy) return; + setBusy(true); + setError(null); + try { + const outcome = await reset(target, mode); + onToast( + `Reset to ${label} (${mode})` + + (outcome.snapshot_oid ? ' — snapshot saved to stashes' : ''), + ); + onClose(); + } catch (e) { + if (mountedRef.current) setError(errMessage(e)); + } finally { + if (mountedRef.current) setBusy(false); + } + } + + const option = (m: ResetMode, title: string, desc: string, danger?: boolean) => ( + + ); + + return ( +
{ + if (e.target === e.currentTarget && !busy) onClose(); + }} + > +
+
+ + Reset + +
+ +
+

+ Move {headLabel} to {label}. +

+ +
+ {option('soft', 'Soft', 'keep all changes staged')} + {option('mixed', 'Mixed', 'keep changes, unstaged')} + {option('hard', 'Hard', 'discard all changes (a safety snapshot stash is saved first)', true)} +
+ + {error ?
{error}
: null} +
+ +
+ + +
+
+
+ ); +} diff --git a/ui/src/views/Review.tsx b/ui/src/views/Review.tsx index 50cdd0d..d69b7df 100644 --- a/ui/src/views/Review.tsx +++ b/ui/src/views/Review.tsx @@ -4,7 +4,10 @@ import { Virtualizer, useWorkerPool } from '@pierre/diffs/react'; import type { GitStatusEntry } from '@pierre/trees'; import { Diff, parseCacheablePatch } from '../components/Diff'; +import { DiffSearchBar, focusDiffSearchInput } from '../components/DiffSearchBar'; import { Icon } from '../components/Icon'; +import { ImageDiff } from '../components/ImageDiff'; +import { isImagePath } from '../lib/image'; import { copyToClipboard, diffStatusToGit, @@ -13,6 +16,8 @@ import { type TreeRowDecoration, } from '../components/PierreTree'; import { hashPatch } from '../lib/patch'; +import { concatPatches, patchesToMarkdown } from '../lib/patchExport'; +import { buildReviewFeedback } from '../lib/reviewExport'; import { gitErrorHint } from '../lib/tauri'; import type { FileDiff } from '../lib/types'; import { useRepo } from '../stores/repo'; @@ -49,6 +54,11 @@ export function Review() { const unstagedDiffs = useRepo((s) => s.unstagedDiffs); const reviewed = useRepo((s) => s.reviewed); const toggleReviewed = useRepo((s) => s.toggleReviewed); + const reviewNotes = useRepo((s) => s.reviewNotes); + const addReviewNote = useRepo((s) => s.addReviewNote); + const removeReviewNote = useRepo((s) => s.removeReviewNote); + const activePath = useRepo((s) => s.activePath); + const meta = useRepo((s) => s.meta); const setBaseline = useRepo((s) => s.setBaseline); const clearBaseline = useRepo((s) => s.clearBaseline); const refreshReviewDiffs = useRepo((s) => s.refreshReviewDiffs); @@ -171,6 +181,19 @@ export function Review() { if (v) toggleReviewed(current.path, v.hash); }, [current, verdicts, toggleReviewed]); + // In-diff text search (⌘F): floats over the diff pane, searches the whole + // pool, and a jump selects the matched file in the queue. + const [searchOpen, setSearchOpen] = useState(false); + const diffSearchSignal = useRepo((s) => s.diffSearchSignal); + const clearDiffSearch = useRepo((s) => s.clearDiffSearch); + useEffect(() => { + if (!diffSearchSignal) return; + setSearchOpen(true); + // The palette restores focus on close — claim it back for the input. + focusDiffSearchInput(); + clearDiffSearch(); + }, [diffSearchSignal, clearDiffSearch]); + // Failed write ops surface here instead of vanishing into the console. const [opError, setOpError] = useState(null); useEffect(() => { @@ -183,6 +206,50 @@ export function Review() { [], ); + // ── Review notes (the agent feedback loop) ──────────────────────────── + // The m editor: which file the note attaches to and the optional new-file + // line it anchors at (pre-set by a per-hunk "Note" button). + const [noteEditor, setNoteEditor] = useState<{ path: string; line: number | null } | null>(null); + const closeNoteEditor = useCallback((el?: HTMLTextAreaElement) => { + // Blur before unmounting so focus falls back to the window and the + // j/k/space loop resumes immediately, no click needed. + el?.blur(); + setNoteEditor(null); + }, []); + + // Success notice ("Copied feedback …"); opError takes the toast slot first. + const [notice, setNotice] = useState(null); + useEffect(() => { + if (!notice) return; + const t = setTimeout(() => setNotice(null), 2600); + return () => clearTimeout(t); + }, [notice]); + + const displayedNotes = displayed ? (reviewNotes[displayed.path] ?? []) : []; + const notedFiles = useMemo( + () => pool.filter((d) => (reviewNotes[d.path]?.length ?? 0) > 0), + [pool, reviewNotes], + ); + const noteCount = useMemo( + () => notedFiles.reduce((n, d) => n + reviewNotes[d.path].length, 0), + [notedFiles, reviewNotes], + ); + const copyFeedback = useCallback(() => { + if (!activePath || notedFiles.length === 0) return; + copyToClipboard( + buildReviewFeedback({ + repoName: basename(activePath), + branch: meta?.branch ?? null, + baselineShort: baseline?.short ?? null, + files: notedFiles.map((d) => ({ path: d.path, patch: d.patch, notes: reviewNotes[d.path] })), + }), + ); + setNotice( + `Copied feedback — ${noteCount} note${noteCount === 1 ? '' : 's'} across ` + + `${notedFiles.length} file${notedFiles.length === 1 ? '' : 's'}`, + ); + }, [activePath, notedFiles, noteCount, reviewNotes, meta, baseline]); + // Two-step confirms for the destructive actions. const [armDiscardAll, setArmDiscardAll] = useState(false); const [confirmDiscard, setConfirmDiscard] = useState(null); @@ -224,20 +291,30 @@ export function Review() { const rowDecoration = useCallback( (path: string, kind: 'file' | 'directory'): TreeRowDecoration | null => { if (kind !== 'file') return null; + const notes = reviewNotes[path]?.length ?? 0; + const pen = notes > 0 ? ` ✎${notes}` : ''; + const penTitle = notes > 0 ? ` · ${notes} note${notes === 1 ? '' : 's'}` : ''; switch (verdicts.get(path)?.verdict) { case 'reviewed': - return { text: '✓', title: 'Reviewed' }; + return { text: '✓' + pen, title: 'Reviewed' + penTitle }; case 'stale': - return { text: 'changed', title: 'Changed since reviewed — review again' }; + return { text: 'changed' + pen, title: 'Changed since reviewed — review again' + penTitle }; default: - return null; + return notes > 0 + ? { text: `✎${notes}`, title: `${notes} note${notes === 1 ? '' : 's'}` } + : null; } }, - [verdicts], + [verdicts, reviewNotes], ); + // Note counts feed the decoration, so they're folded into the key — Pierre + // only repaints rows when this fingerprint moves (see docs/learnings.md). const decorationKey = useMemo( - () => pool.map((d) => `${d.path}:${verdicts.get(d.path)?.verdict}`).join('|'), - [pool, verdicts], + () => + pool + .map((d) => `${d.path}:${verdicts.get(d.path)?.verdict}:${reviewNotes[d.path]?.length ?? 0}`) + .join('|'), + [pool, verdicts, reviewNotes], ); // Activate (double-click / Enter): one file toggles its reviewed mark; a @@ -299,14 +376,33 @@ export function Review() { icon: 'file', onSelect: () => copyToClipboard(known.join('\n')), }); + const diffs = known + .map((p) => pool.find((d) => d.path === p)) + .filter((d): d is FileDiff => d != null); + if (diffs.some((d) => d.patch.length > 0)) { + items.push( + { label: 'Copy diff', icon: 'file', onSelect: () => copyToClipboard(concatPatches(diffs)) }, + { label: 'Copy diff as Markdown', icon: 'file', onSelect: () => copyToClipboard(patchesToMarkdown(diffs)) }, + ); + } return items; }, - [verdicts, unstagedSet, toggleReviewed, stageMany, discardMany, fail], + [verdicts, unstagedSet, toggleReviewed, stageMany, discardMany, fail, pool], ); // ── Keyboard loop ───────────────────────────────────────────────────── useEffect(() => { const onKey = (e: KeyboardEvent) => { + // ⌘F / Ctrl+F opens the in-diff search — checked before the mod-combo + // guard below. Inert while a dialog or the palette is up. + if ((e.metaKey || e.ctrlKey) && !e.altKey && e.key.toLowerCase() === 'f') { + const ft = e.target as HTMLElement | null; + if (ft?.closest('[role="dialog"], [role="combobox"], .palette-backdrop')) return; + e.preventDefault(); + setSearchOpen(true); + focusDiffSearchInput(); // already open → refocus + select + return; + } if (e.metaKey || e.ctrlKey || e.altKey || e.defaultPrevented) return; const t = e.target as HTMLElement | null; if (t?.closest('input, textarea, select, [contenteditable="true"], [role="dialog"], [role="combobox"]')) { @@ -332,6 +428,14 @@ export function Review() { e.preventDefault(); markReviewed(); break; + case 'm': + // The input/textarea guard above keeps this inert while the note + // editor itself (or any other field) has focus. + if (current) { + e.preventDefault(); + setNoteEditor({ path: current.path, line: null }); + } + break; case 's': if (current && unstagedSet.has(current.path)) { e.preventDefault(); @@ -412,6 +516,16 @@ export function Review() { onClear={() => void clearBaseline()} extra={ <> + {noteCount > 0 && ( + + )} {stageableReviewed > 0 && ( + {unstagedSet.has(displayed.path) && ( + <> + + + + )} + {sessionMode && !unstagedSet.has(displayed.path) && stagedSet.has(displayed.path) && ( - - - )} - {sessionMode && !unstagedSet.has(displayed.path) && stagedSet.has(displayed.path) && ( + )} - )} - - -
- {/* Pierre's Virtualizer makes it the scroll container and - window-renders the diff rows — whole-file patches of any - size (lockfiles…) mount only what's on screen. */} - - {displayed.binary || displayed.patch.length === 0 ? ( -
- {displayed.binary ? 'Binary file — no diff shown.' : 'No textual diff.'} + +
+ {noteEditor && ( +
+