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..8a08733 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,6 +770,27 @@ 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.) +- ☑ 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". Exports the *union* via `collectFeedbackFiles`: pool files with + notes plus noted paths that left the pool (those skip the excerpt), so a + stored note never silently drops. Notes on deletion-only blocks anchor + old-side (`ReviewNote.side`) and the excerpt locator counts the matching + side.) - ☐ Watcher: optional `.gitignore`-aware path filtering if build storms show up in profiles. 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/diff.rs b/crates/strand-core/src/diff.rs index f08c496..8d78756 100644 --- a/crates/strand-core/src/diff.rs +++ b/crates/strand-core/src/diff.rs @@ -226,6 +226,15 @@ fn collect(mut diff: git2::Diff<'_>) -> Result> { let Some(i) = delta_index(&delta) else { return true; }; let f = &mut files[i]; let origin = line.origin(); + // `DiffFile::is_binary()` is only populated once content is examined — + // which happens *here*, during print — so the pre-pass that builds + // `files` reads false for every delta. libgit2 emits one 'B' line + // ("Binary files … differ") per binary delta; flag it now so the UI + // routes images to the preview and other binaries to the note instead + // of rendering a header-only patch as an empty diff. + if origin == 'B' { + f.binary = true; + } if matches!(origin, 'F' | 'H' | ' ' | '+' | '-' | '=' | '<' | '>') { match origin { '+' => f.adds += 1, @@ -309,6 +318,24 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + #[test] + fn untracked_binary_file_is_flagged_binary() { + let (repo, dir) = scratch_repo(); + // A PNG-ish blob: a NUL byte in the first 8 KiB is what git (and our + // heuristic) treat as binary. is_binary() on the DiffFile is false + // until print examines the content, so this regressed to a header-only + // text patch (blank image preview) before the print-time flag. + std::fs::write(dir.join("1.png"), [0x89, b'P', b'N', b'G', 0x00, 0x01, 0x02]).unwrap(); + + let diffs = repo.diff_unstaged().unwrap(); + let img = diffs.iter().find(|d| d.path == "1.png").expect("untracked png listed"); + assert_eq!(img.status, DiffStatus::Added); + assert!(img.binary, "untracked binary must be flagged binary"); + assert_eq!((img.adds, img.dels), (0, 0)); + + let _ = std::fs::remove_dir_all(dir); + } + #[test] fn full_context_diff_carries_the_whole_file() { let (repo, dir) = scratch_repo(); 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..0ea2e46 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, collectFeedbackFiles } 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,10 @@ 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 { IgnoreDialog } from './views/IgnoreDialog'; import { Commits } from './views/Commits'; import { FileView } from './views/FileView'; import { LocalChanges } from './views/LocalChanges'; @@ -32,7 +39,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 +128,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 +146,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 +168,21 @@ 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); + // The "Add ignore pattern…" dialog is opened from two surfaces (Local Changes + // + the Files tab), so its draft lives in the store; App renders the one modal. + const ignoreDraft = useRepo((s) => s.ignoreDraft); + const closeIgnoreDialog = useRepo((s) => s.closeIgnoreDialog); const [worktreeOpen, setWorktreeOpen] = useState(false); const [syncing, setSyncing] = useState(false); const [pulling, setPulling] = useState(false); @@ -219,6 +249,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 +779,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 +795,54 @@ 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; + // Union: pool files with notes + noted paths outside the pool + // (staged away, or Review never populated the pool this session) — + // a stored note always exports, just without an excerpt. + const files = collectFeedbackFiles(pool, st.reviewNotes); + if (!st.activePath || files.length === 0) { + showToast('No review notes to copy'); + 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 +858,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 +917,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 +962,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 +977,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 +991,7 @@ export function App() { setTagDialog({ target, label })} onInteractiveRebase={(base, label) => setRebaseDialog({ base, label })} + onResetTo={(target, label) => setResetDialog({ target, label })} onToast={showToast} /> )} @@ -969,6 +1087,18 @@ export function App() { /> )} + {remoteDialog && ( + setRemoteDialog(null)} onToast={showToast} /> + )} + + {renameBranchDialog && ( + setRenameBranchDialog(null)} + onToast={showToast} + /> + )} + {mergeDialog && ( )} + {resetDialog && ( + setResetDialog(null)} + onToast={showToast} + /> + )} + + {ignoreDraft != null && ( + + )} + {worktreeOpen && setWorktreeOpen(false)} />} {!isTauri() && !meta && ( diff --git a/ui/src/components/ContextMenu.tsx b/ui/src/components/ContextMenu.tsx index d0ff5d6..585bc05 100644 --- a/ui/src/components/ContextMenu.tsx +++ b/ui/src/components/ContextMenu.tsx @@ -14,33 +14,47 @@ export interface MenuItem { * right-click equivalent of the old inline-confirm on destructive rows. */ confirm?: boolean; - onSelect: () => void; + /** + * Nested items. The row renders a "›" and opens a child menu to its right + * on hover, → , or Enter; the parent item is a disclosure, not an action. + */ + submenu?: MenuItem[]; + /** Run on activation. Optional only when `submenu` is set (a parent row). */ + onSelect?: () => void; } /** * A right-click menu for sidebar rows, rendered in a portal at the cursor (or, * when opened from the keyboard, at the row's corner). Keyboard-operable: - * ↑/↓ move, Enter/Space select, Esc closes (or cancels a pending confirm). - * Destructive items with `confirm` swap to a "Confirm: …" step before running, - * so a delete still takes two deliberate actions. Closes on outside click, - * right-click elsewhere, or selection; restores focus to the opener on close. + * ↑/↓ move, Enter/Space select, → opens a submenu, ← / Esc closes it (or the + * whole menu). Destructive items with `confirm` swap to a "Confirm: …" step + * before running. Closes on outside click, right-click elsewhere, or + * selection; restores focus to the opener on close. + * + * `onBack` is set only on a nested submenu: it returns to the parent (← / Esc) + * without tearing down the whole chain, and tells the component to skip its own + * backdrop (the root menu's backdrop already catches outside clicks). */ export function ContextMenu({ x, y, items, onClose, + onBack, }: { x: number; y: number; items: MenuItem[]; onClose: () => void; + onBack?: () => void; }) { const ref = useRef(null); const [pos, setPos] = useState({ x, y }); const firstEnabled = items.findIndex((it) => !it.disabled); const [active, setActive] = useState(firstEnabled); const [confirming, setConfirming] = useState(null); + // The open submenu: which parent index, and where to anchor the child. + const [sub, setSub] = useState<{ index: number; x: number; y: number } | null>(null); // Clamp into the viewport once the menu has a measured size. useLayoutEffect(() => { @@ -70,18 +84,124 @@ export function ContextMenu({ setActive(enabled[(base + dir + enabled.length) % enabled.length]); }; + // Anchor a child menu to the right edge of item `i`'s row. + const openSub = (i: number) => { + const el = ref.current?.querySelector(`[data-idx="${i}"]`); + if (!el) return; + const r = el.getBoundingClientRect(); + setActive(i); + setSub({ index: i, x: r.right - 4, y: r.top - 4 }); + }; + const choose = (i: number) => { const it = items[i]; if (!it || it.disabled) return; + if (it.submenu && it.submenu.length > 0) { + openSub(i); + return; + } if (it.confirm && confirming !== i) { setConfirming(i); setActive(i); return; } onClose(); - it.onSelect(); + it.onSelect?.(); }; + const menu = ( +
{ + if (e.key === 'ArrowDown') { + e.preventDefault(); + move(1); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + move(-1); + } else if (e.key === 'ArrowRight') { + e.preventDefault(); + if (items[active]?.submenu?.length) openSub(active); + } else if (e.key === 'ArrowLeft') { + e.preventDefault(); + if (sub) setSub(null); + else onBack?.(); + } else if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + choose(active); + } else if (e.key === 'Escape') { + e.preventDefault(); + if (confirming != null) setConfirming(null); + else if (sub) setSub(null); + else if (onBack) onBack(); + else onClose(); + } + }} + > + {items.map((it, i) => { + const isConfirm = confirming === i; + const hasSub = !!it.submenu?.length; + return ( + + ); + })} + + {sub && items[sub.index]?.submenu && ( + { + setSub(null); + ref.current?.focus(); + }} + /> + )} +
+ ); + + // A submenu skips its own backdrop and rides at a higher z-index above the + // parent (whose backdrop still catches outside clicks for the whole chain). + if (onBack) return createPortal(menu, document.body); + return createPortal(
-
{ - if (e.key === 'ArrowDown') { - e.preventDefault(); - move(1); - } else if (e.key === 'ArrowUp') { - e.preventDefault(); - move(-1); - } else if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - choose(active); - } else if (e.key === 'Escape') { - e.preventDefault(); - if (confirming != null) setConfirming(null); - else onClose(); - } - }} - > - {items.map((it, i) => { - const isConfirm = confirming === i; - return ( - - ); - })} -
+ {menu}
, document.body, ); diff --git a/ui/src/components/DiffSearchBar.tsx b/ui/src/components/DiffSearchBar.tsx new file mode 100644 index 0000000..f7cc4f2 --- /dev/null +++ b/ui/src/components/DiffSearchBar.tsx @@ -0,0 +1,164 @@ +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, +}: { + /** `tag` rides through to each match (see {@link DiffMatch.tag}). */ + diffs: (Pick & { tag?: unknown })[]; + 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 ( +
{ + if (e.key === 'Escape') { + e.preventDefault(); + onClose(); + } + }} + > +
+ + 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); + } + // Escape is handled by the container (works from the buttons too). + }} + /> + + {!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..0ff17b1 --- /dev/null +++ b/ui/src/components/ImageDiff.tsx @@ -0,0 +1,138 @@ +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 diffsTick = useRepo((s) => s.diffsTick); + 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; + // Worktree/index sources are mutable — refetch when the diffs refresh (the + // agent-watch loop edits files under us). Rev-pinned blobs are immutable. + const refetchKey = rev == null ? diffsTick : 0; + useEffect(() => { + if (absent || !activePath) return; + let cancelled = false; + // Keep the previous image while revalidating so watcher refreshes don't + // flash "Loading…" when the bytes are unchanged. + setState((s) => (s.kind === 'ok' ? s : { 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, refetchKey]); + 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 && } +
+ ); +} + +/** + * Single-image preview (no before/after) — the File view's Content tab for a + * binary image, where there's just the one version to show. + */ +export function ImagePreview({ path, src }: { path: string; src: BlobRef }) { + const state = useBlob(path, src); + return ( +
+ +
+ ); +} + +function ImagePane({ + label, + tone = 'add', + path, + state, +}: { + /** Omit for a single-image preview (no before/after caption). */ + 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 &&
{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..61e873c 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,8 @@ 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 fetchRemote = useRepo((s) => s.fetchRemote); const deleteTag = useRepo((s) => s.deleteTag); const pushTag = useRepo((s) => s.pushTag); const deleteRemoteTag = useRepo((s) => s.deleteRemoteTag); @@ -139,6 +147,8 @@ 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 openIgnoreDialog = useRepo((s) => s.openIgnoreDialog); const stashes = useRepo((s) => s.stashes); const stashApply = useRepo((s) => s.stashApply); const stashPop = useRepo((s) => s.stashPop); @@ -216,9 +226,20 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, const remoteTree = useMemo(() => { const t = buildTree(filtered.remotes, (rb) => [rb.remote, rb.branch]); + // Every *configured* remote gets a top-level row, even with zero + // remote-tracking refs (just added, or never fetched) — otherwise the + // remote exists in the section count but renders nowhere, and its + // management menu (Fetch / Edit URL / Rename / Remove) is unreachable. + const q = filter.trim().toLowerCase(); + for (const r of refs.remotes) { + if (q && !r.name.toLowerCase().includes(q)) continue; + if (!t.children.some((c) => c.name === r.name)) { + t.children.push({ name: r.name, fullPath: r.name, children: [] }); + } + } sortTree(t, (a, b) => a.name.localeCompare(b.name)); return t; - }, [filtered.remotes]); + }, [filtered.remotes, refs.remotes, filter]); const tagTree = useMemo(() => { const t = buildTree(filtered.tags, (tg) => tg.name.split('/')); @@ -297,15 +318,37 @@ 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 target = targets[0]; + const base = target.slice(target.lastIndexOf('/') + 1); + const { exact, extension } = ignorePatterns(target); + const submenu: MenuItem[] = [ + { label: `Ignore “${base}”`, onSelect: () => ignore(exact) }, + ]; + if (extension) { + submenu.push({ label: `Ignore all ${extension} files`, onSelect: () => ignore(extension) }); + } + submenu.push({ label: 'Custom pattern…', onSelect: () => openIgnoreDialog(target) }); + items.push({ label: 'Ignore', icon: 'file', submenu }); + } + items.push({ label: targets.length > 1 ? 'Copy paths' : 'Copy path', icon: 'file', onSelect: () => copyToClipboard(targets.join('\n')), - }, - ], - [selectFile], + }); + return items; + }, + [selectFile, workTree, gitignoreAdd, openIgnoreDialog, onToast], ); const runBranchOp = async (fn: () => Promise) => { @@ -362,6 +405,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 +417,7 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, return [ { label: 'Current branch', disabled: true, onSelect: () => {} }, newBranchItem, + renameItem, { label: 'Interactive rebase…', icon: 'rebase', @@ -382,6 +431,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 +466,36 @@ 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: 'Fetch', + icon: 'arrow-down', + onSelect: () => + void fetchRemote(name).then( + () => onToast(`Fetched ${name}`), + (e) => onToast(`Fetch failed: ${errMessage(e)}`), + ), + }, + { 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 +743,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 +869,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 +922,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 ( + + +
+

+ Append a pattern to the repo’s .gitignore. +

+ +

+ Glob syntax: *.log · build/ ·{' '} + src/**​/*.tmp · !keep.txt (un-ignore) +

+ {error ?
{error}
: null} +
+ +
+ + +
+ + + ); +} diff --git a/ui/src/views/LocalChanges.tsx b/ui/src/views/LocalChanges.tsx index 6075eea..2760332 100644 --- a/ui/src/views/LocalChanges.tsx +++ b/ui/src/views/LocalChanges.tsx @@ -8,8 +8,13 @@ import { FileDiff as PierreFileDiff } from '@pierre/diffs/react'; import type { GitStatusEntry } from '@pierre/trees'; import { Diff, diffAppearanceOptions, 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, PierreTree, type TreeMenuItem } from '../components/PierreTree'; +import { ignorePatterns } from '../lib/ignore'; +import { concatPatches, patchesToMarkdown } from '../lib/patchExport'; import { gitErrorHint } from '../lib/tauri'; import { sliceChangeBlock, type SliceDirection } from '../lib/patch'; import type { LocalSelection } from '../stores/repo'; @@ -36,6 +41,8 @@ export function LocalChanges() { const stageMany = useRepo((s) => s.stageMany); const unstageMany = useRepo((s) => s.unstageMany); const discardMany = useRepo((s) => s.discardMany); + const gitignoreAdd = useRepo((s) => s.gitignoreAdd); + const openIgnoreDialog = useRepo((s) => s.openIgnoreDialog); const stageAll = useRepo((s) => s.stageAll); const unstageAll = useRepo((s) => s.unstageAll); const selectLocalFile = useRepo((s) => s.selectLocalFile); @@ -49,6 +56,14 @@ export function LocalChanges() { }, [status]); const conflictSet = useMemo(() => new Set(conflicts), [conflicts]); + // Untracked paths, also from status — FileSection's diffs don't carry + // StatusKind, so the .gitignore quick actions key off this set instead. + const untracked = useMemo(() => { + const set = new Set(); + for (const s of status) if (s.kind === 'UNTRACKED') set.add(s.path); + return set; + }, [status]); + // Conflicted files are handled by the Conflicts bar + landing, so keep them // out of the normal Unstaged/Staged lists (git lists them on both sides, // which read as confusing duplicates). @@ -89,6 +104,37 @@ export function LocalChanges() { else if (stagedView.length) selectLocalFile({ file: '', staged: true, all: true }); }, [selection, unstagedView, stagedView, selectLocalFile]); + // ── In-diff text search (⌘F) ────────────────────────────────────────── + // The bar floats over the diff pane and searches both sides at once; each + // pool entry is tagged with its side, so a jump lands on the copy that + // actually contains the match (a partially staged path sits on both sides + // with different hunks in each). + 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]); + // Tag each entry with its side so a jump lands on the diff that actually + // contains the match — a path can appear on BOTH sides (partially staged) + // with different hunks in each copy. + const searchPool = useMemo( + () => [ + ...unstagedView.map((d) => ({ ...d, tag: false })), + ...stagedView.map((d) => ({ ...d, tag: true })), + ], + [unstagedView, stagedView], + ); + const closeSearch = useCallback(() => { + setSearchOpen(false); + // Hand focus back to the diff pane host so the j/k loop reads naturally. + document.querySelector('.lc-diff-scroll')?.focus(); + }, []); + // The diff(s) the selection drives. "Show all" → the whole side. A file // row → just that file. A folder row (Pierre reports directory paths with // a trailing slash, no exact file match) → every changed file beneath it. @@ -126,6 +172,17 @@ export function LocalChanges() { const confirmTimer = useRef(null); useEffect(() => { const onKey = (e: KeyboardEvent) => { + // ⌘F / Ctrl+F opens the in-diff search — checked before the mod-combo + // guard below. Inert while a dialog/palette or the resolver 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; + if (resolverOpen.current) 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 ( @@ -248,6 +305,9 @@ export function LocalChanges() { onAction={(files) => void stageMany(files).catch(fail('Stage'))} actionLabel="Stage" onDiscard={(files) => void discardMany(files).catch(fail('Discard'))} + isUntracked={(p) => untracked.has(p)} + onIgnore={(pattern) => void gitignoreAdd(pattern).catch(fail('Ignore'))} + onIgnoreCustom={openIgnoreDialog} onBulk={() => void stageAll().catch(fail('Stage all'))} bulkLabel="Stage all" /> @@ -271,15 +331,25 @@ export function LocalChanges() { - {activeConflict ? ( - setResolverFile(activeConflict)} - /> - ) : ( - - )} +
+ {activeConflict ? ( + setResolverFile(activeConflict)} + /> + ) : ( + + )} + {searchOpen && ( + selectFileRow({ file: m.path, staged: m.tag === true })} + onClose={closeSearch} + placeholder="Search changes…" + /> + )} +
@@ -375,6 +445,13 @@ interface SectionProps { actionLabel: string; /** Discard the given files' working-tree changes — unstaged section only. */ onDiscard?: (files: string[]) => void; + /** Whether a path is untracked — gates the .gitignore quick actions + * (FileDiff rows don't carry StatusKind). Unstaged section only. */ + isUntracked?: (path: string) => boolean; + /** Append a pattern to the repo's root .gitignore. */ + onIgnore?: (pattern: string) => void; + /** Open the custom ignore-pattern dialog, prefilled with the given path. */ + onIgnoreCustom?: (initial: string) => void; onBulk(): void; bulkLabel: string; } @@ -396,6 +473,9 @@ function FileSection({ onAction, actionLabel, onDiscard, + isUntracked, + onIgnore, + onIgnoreCustom, onBulk, bulkLabel, }: SectionProps) { @@ -426,14 +506,38 @@ function FileSection({ onSelect: () => onDiscard(targets), }); } + if (n === 1 && onIgnore && isUntracked?.(targets[0])) { + const target = targets[0]; + const base = target.slice(target.lastIndexOf('/') + 1); + const { exact, extension } = ignorePatterns(target); + const submenu: TreeMenuItem[] = [ + { label: `Ignore “${base}”`, onSelect: () => onIgnore(exact) }, + ]; + if (extension) { + submenu.push({ label: `Ignore all ${extension} files`, onSelect: () => onIgnore(extension) }); + } + if (onIgnoreCustom) { + submenu.push({ label: 'Custom pattern…', onSelect: () => onIgnoreCustom(target) }); + } + items.push({ label: 'Ignore', icon: 'file', submenu }); + } items.push({ label: n > 1 ? 'Copy paths' : 'Copy path', icon: 'file', onSelect: () => copyToClipboard(targets.join('\n')), }); + const diffs = targets + .map((p) => files.find((f) => f.path === p)) + .filter((f): f is FileDiff => f != null); + if (diffs.some((f) => f.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; }, - [actionLabel, staged, onAction, onDiscard], + [actionLabel, staged, onAction, onDiscard, isUntracked, onIgnore, onIgnoreCustom, files], ); return ( @@ -503,7 +607,9 @@ function DiffPane({ diffs, staged }: { diffs: FileDiff[]; staged: boolean }) { return (
-
+ {/* tabIndex so closing the ⌘F bar can park focus here (not an input, + so the staging keyboard loop stays live). */} +
{diffs.length === 0 ? (
Select a file @@ -545,6 +651,7 @@ function FileDiffSection({ onToggle: () => void; }) { const empty = diff.binary || diff.patch.length === 0; + const image = diff.binary && isImagePath(diff.path); // Viewport-lazy mount: the "show all" view can stack hundreds of files, and // mounting every Pierre diff at once froze the app on open / after a big @@ -552,11 +659,13 @@ function FileDiffSection({ // the viewport, and keep it mounted thereafter (mounting is the cost, not // staying mounted). Until then a placeholder reserves the file's estimated // height so the scrollbar stays honest and far-off diffs aren't counted as - // near. Empty/binary bodies are trivial, so they skip the gating. + // near. Empty/binary bodies are trivial, so they skip the gating — except + // images, whose preview costs an IPC blob fetch per side: they gate like + // text diffs so a "show all" stack doesn't fire N fetches on open. const blockRef = useRef(null); const [seen, setSeen] = useState(false); useEffect(() => { - if (seen || collapsed || empty) return; + if (seen || collapsed || (empty && !image)) return; const el = blockRef.current; if (!el) return; const io = new IntersectionObserver( @@ -571,7 +680,7 @@ function FileDiffSection({ ); io.observe(el); return () => io.disconnect(); - }, [seen, collapsed, empty]); + }, [seen, collapsed, empty, image]); // Rough body-height estimate (changed lines, no context) so the placeholder // reserves space close to the real diff height. @@ -584,7 +693,27 @@ function FileDiffSection({
{!collapsed && - (empty ? ( + (image && !seen ? ( +
+ ) : image ? ( + // Old side: the unstaged diff's base is the *index* (a partially + // staged image's HEAD copy would lie); the staged diff's base is + // HEAD. An added file has no old side. + // New side: the working tree (unstaged) or the index (staged). + + ) : empty ? (
{diff.binary ? 'Binary file — no diff shown.' : 'No textual diff.'}
@@ -643,10 +772,14 @@ export function HunkAnnotatedDiff({ diff, layout, side, + onNoteBlock, }: { diff: FileDiff; layout: 'unified' | 'split'; side: 'unstaged' | 'staged'; + /** When provided (the Review view), each change block grows a "Note" + * action that hands its meta back for a line-anchored review note. */ + onNoteBlock?: (meta: BlockMeta) => void; }) { const applyPatch = useRepo((s) => s.applyPatch); const discardPatch = useRepo((s) => s.discardPatch); @@ -931,6 +1064,7 @@ export function HunkAnnotatedDiff({ side={side} pending={pending} onRun={(d, t) => void run(a.metadata, d, t)} + onNote={onNoteBlock && (() => onNoteBlock(a.metadata))} />
))} @@ -980,11 +1114,13 @@ function BlockActions({ side, pending, onRun, + onNote, }: { meta: BlockMeta; side: 'unstaged' | 'staged'; pending: string | null; onRun(direction: SliceDirection, target: ApplyTarget): void; + onNote?: () => void; }) { const busy = pending != null; const myKey = (target: ApplyTarget) => `${blockKey(meta)}:${target}`; @@ -1026,6 +1162,17 @@ function BlockActions({ > {discardingMe ? 'Discarding…' : 'Discard'} + {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..27b6a2f --- /dev/null +++ b/ui/src/views/ResetDialog.tsx @@ -0,0 +1,174 @@ +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?.(); + }, []); + + // Move focus INTO the modal on open (after the opener is captured above — + // effect order matters). The other dialogs get this from an input's + // autoFocus; radios have none, and without it the keyboard keeps driving + // the view behind the backdrop. + useEffect(() => { + dialogRef.current + ?.querySelector('input[type="radio"]:checked') + ?.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..4e704e4 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, collectFeedbackFiles } 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,58 @@ 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; + /** Diff side `line` counts on — 'old' for deletion-only blocks. */ + side: 'new' | 'old'; + } | 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] ?? []) : []; + // The export is the UNION of pool files with notes and noted paths that + // left the pool (staged away in inbox mode, …) — a stored note must never + // silently drop from the feedback. Counts follow the same union. + const feedbackFiles = useMemo( + () => collectFeedbackFiles(pool, reviewNotes), + [pool, reviewNotes], + ); + const noteCount = useMemo( + () => feedbackFiles.reduce((n, f) => n + f.notes.length, 0), + [feedbackFiles], + ); + const copyFeedback = useCallback(() => { + if (!activePath || feedbackFiles.length === 0) return; + copyToClipboard( + buildReviewFeedback({ + repoName: basename(activePath), + branch: meta?.branch ?? null, + baselineShort: baseline?.short ?? null, + files: feedbackFiles, + }), + ); + setNotice( + `Copied feedback — ${noteCount} note${noteCount === 1 ? '' : 's'} across ` + + `${feedbackFiles.length} file${feedbackFiles.length === 1 ? '' : 's'}`, + ); + }, [activePath, feedbackFiles, noteCount, meta, baseline]); + // Two-step confirms for the destructive actions. const [armDiscardAll, setArmDiscardAll] = useState(false); const [confirmDiscard, setConfirmDiscard] = useState(null); @@ -224,20 +299,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 +384,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 +436,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, side: 'new' }); + } + break; case 's': if (current && unstagedSet.has(current.path)) { e.preventDefault(); @@ -412,6 +524,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 && ( +
+