diff --git a/Cargo.lock b/Cargo.lock index 7cb53d85..6c490664 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -563,6 +563,15 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "devicons" +version = "0.6.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830e47e2f330cf4fdd5a958dcef921b9523ffc21ab6713aa5e77ba2cce03904b" +dependencies = [ + "lazy_static", +] + [[package]] name = "dialoguer" version = "0.12.0" @@ -965,6 +974,7 @@ dependencies = [ "clap", "clap_complete", "crossterm", + "devicons", "dirs", "expectrl", "git-workon-fixture", @@ -986,6 +996,7 @@ dependencies = [ "tree-sitter-rust", "tree-sitter-toml-ng", "tree-sitter-typescript", + "unicode-width 0.2.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index f68fe8c4..92a63124 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ clap-verbosity-flag = "3.0.4" clap_complete = { version = "4.6.5", features = ["unstable-dynamic"] } clap_mangen = "0.3.0" crossterm = "0.29.0" +devicons = "0.6" dialoguer = { version = "0.12.0", features = ["fuzzy-select"] } dirs = "6.0" env_logger = "0.11.10" diff --git a/docs/adr/035-review-theming-base16-hybrid.md b/docs/adr/035-review-theming-base16-hybrid.md index 4a93a1f9..fd0cf817 100644 --- a/docs/adr/035-review-theming-base16-hybrid.md +++ b/docs/adr/035-review-theming-base16-hybrid.md @@ -132,6 +132,18 @@ read left untested (see `terminal_query.rs`). to render; `FgSpan` loses its `Color` field in favor of a capture index. Existing render tests that assert concrete colors must resolve through a fixed test `Palette`. +## Revised (CS2, visual-polish pass) + +The "chrome that is never a theme knob (error/warn/current-marker) stays ANSI/const in +`render.rs`" clause above is superseded. Those three colors are now `Palette` fields +(`error_fg`/`warn_fg`/`current_fg`, mapped to base08/base0A/base0B) rather than module +consts — the user explicitly approved revisiting this boundary during the icons/semantic-fg +polish pass. `dark()` keeps the shipped RGB values verbatim (the same pixel-identity +precedent the diff/cursor tints follow); `light()` takes `ONE_LIGHT`'s base08/base0A/base0B; +`from_terminal()` takes the probed scheme's base08/base0A/base0B directly, same reasoning as +the syntax slots (matching the terminal, not curated-tint-borrowing). No other part of the +hybrid boundary changes: this only moves three named colors from `const` to palette fields. + ## References - [ADR-034](034-review-git-native-config-schema.md) — `workon.review.theme` config key diff --git a/git-workon-review/Cargo.toml b/git-workon-review/Cargo.toml index 32bc7e9c..ce464c5a 100644 --- a/git-workon-review/Cargo.toml +++ b/git-workon-review/Cargo.toml @@ -35,6 +35,7 @@ vendored = ["git-workon-lib/vendored", "git2/vendored-libgit2", "git2/vendored-o clap.workspace = true clap_complete.workspace = true crossterm.workspace = true +devicons.workspace = true dirs.workspace = true git-workon-lib.workspace = true git2.workspace = true @@ -53,6 +54,7 @@ tree-sitter-md.workspace = true tree-sitter-rust.workspace = true tree-sitter-toml-ng.workspace = true tree-sitter-typescript.workspace = true +unicode-width.workspace = true [package.metadata.dist] # Redundant with publish = false today; load-bearing at the M3 flip so diff --git a/git-workon-review/src/app.rs b/git-workon-review/src/app.rs index 49febfe1..c54ab7e4 100644 --- a/git-workon-review/src/app.rs +++ b/git-workon-review/src/app.rs @@ -9,10 +9,11 @@ //! handle so it can lazily read blob/worktree content per file as the user navigates to it, //! independent of whatever handle acquired the [`DiffModel`] it was built from. -use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::path::Path; use git2::Repository; +use unicode_width::UnicodeWidthStr; use workon::{Changeset, ChangesetSpan}; use crate::acquire::{ChangesetDiff, WorktreeDiffs}; @@ -23,10 +24,12 @@ use crate::align::{ use crate::apply::{Git2Applier, StageVerb}; use crate::config::RawViewConfig; use crate::highlight::{lang_key_for_ext, FgSpan, TsHighlighter}; -use crate::icons::OutlineIcons; +use crate::icons::IconMode; use crate::model::{DiffModel, FileChange, FileStatus, Hunk, LineKind}; use crate::ops; -use crate::outline::{self, OutlineChangeset, OutlineFile, OutlineItem, OutlineMode, OutlineOrder}; +use crate::outline::{ + self, FoldKey, OutlineChangeset, OutlineFile, OutlineItem, OutlineMode, OutlineOrder, +}; use crate::queue::{OpOutcome, StagingOp, StagingQueue}; use crate::refresh::{IndexSignature, RefreshCoordinator}; use crate::scope::enclosing_scope_lines; @@ -40,6 +43,10 @@ use crate::wordiff::{word_diff_spans, Span}; /// [`App::derive_scroll`]. const SCROLLOFF: usize = 2; +/// Display columns panned per `hscroll-left`/`hscroll-right` press — see [`App::hscroll_left`]/ +/// [`App::hscroll_right`]. +const HSCROLL_STEP: usize = 8; + /// Loaded, aligned, highlighted view of one file's combined diff. /// /// Full text is read once per side, from whichever source the file's status says still exists: @@ -674,14 +681,14 @@ fn parse_outline_order(raw: &str) -> Option { } } -/// Parse `workon.review.outline.icons` (CS5) into an [`OutlineIcons`]. Canonical strings mirror +/// Parse `workon.review.icons` (CS5) into an [`IconMode`]. Canonical strings mirror /// the variant names, kebab-cased: `nerd`, `none`. `None` on anything else — -/// [`App::apply_view_config`] falls back to [`OutlineIcons::default`] (also `none` — CS5's +/// [`App::apply_view_config`] falls back to [`IconMode::default`] (also `none` — CS5's /// no-auto-detection default) and warns. -fn parse_outline_icons(raw: &str) -> Option { +fn parse_icon_mode(raw: &str) -> Option { match raw { - "nerd" => Some(OutlineIcons::Nerd), - "none" => Some(OutlineIcons::None), + "nerd" => Some(IconMode::Nerd), + "none" => Some(IconMode::None), _ => None, } } @@ -779,6 +786,35 @@ impl OutlineRowIdentity { } } +/// What "the same changeset" means once a refresh has re-resolved the world: branch name plus +/// span KIND. Name alone is ambiguous — [`workon::assemble_changesets`]'s uncommitted layer is +/// named after the current branch, so that branch's committed node and the uncommitted layer +/// share a name, and a name-only re-find silently lands on the committed node (the "staging +/// teleports the diff viewer" / "discard does nothing" dogfood bugs). Deliberately NOT the full +/// [`workon::ChangesetSpan`]: a staging op rewrites the index, and a future stack op rewrites +/// base/head OIDs, yet the result is still "the same changeset" to the reviewer — identity must +/// survive exactly the operations that change the span's contents. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ChangesetIdentity { + name: String, + uncommitted: bool, +} + +impl ChangesetIdentity { + /// Capture `cs`'s identity ahead of an operation that rebuilds [`App::changesets`]. + fn of(cs: &Changeset) -> Self { + Self { + name: cs.name.clone(), + uncommitted: cs.span == ChangesetSpan::Uncommitted, + } + } + + /// Whether `cs` is the changeset this identity was captured from, across a rebuild. + fn matches(&self, cs: &Changeset) -> bool { + cs.name == self.name && (cs.span == ChangesetSpan::Uncommitted) == self.uncommitted + } +} + /// The outline side pane's own state (locked fork 3): whether it's showing, whether IT (rather /// than the diff) currently has keyboard focus, its own cursor (an index into /// [`App::outline_items`]'s row list — a wholly separate coordinate space from [`App::cursor`]), @@ -800,10 +836,25 @@ pub struct OutlineState { /// Which end of the stack the stack-shaped modes display first — `workon.review.outline.order` /// (CS3), defaulting to [`OutlineOrder::HeadFirst`]. Read by [`App::outline_items`]. pub order: OutlineOrder, - /// CS5: opt-in nerd-font file/dir icons — `workon.review.outline.icons`, defaulting to - /// [`OutlineIcons::None`] (no auto-detection story exists — a terminal can't report the - /// user's font). Read by `render::build_outline_line`. - pub icons: OutlineIcons, + /// Column pan offset (display columns) for the outline pane — the outline's own analog of + /// [`App::hscroll`], since a long path is hard-clipped at the outline's fixed width just like + /// a long diff line. Floored at `0` by [`App::outline_hscroll_left`]/ + /// [`App::outline_hscroll_right`]; the upper clamp is render-side (`render_outline`, mirroring + /// [`App::clamp_outline_scroll`]'s own per-frame bounds-clamp under the wheel peek model), not + /// here. Reset to `0` by [`App::outline_cycle_mode`] — the row list (and therefore the set of + /// paths on screen) changes shape there, the same reason that resyncs the cursor. + pub hscroll: usize, + /// CS5 (`outline-fold`): per-[`OutlineMode`] sets of collapsed [`FoldKey`]s — a Header row's + /// changeset label PLUS its `cs_idx`, or a Dir row's full path (+ owning changeset `cs_idx` in + /// `StackTree`) — see [`FoldKey`]'s own doc comment for why `cs_idx` is load-bearing there, + /// not decorative (a changeset's `label` alone can collide with its own uncommitted layer's). + /// Each mode keeps its own independent set (folding a dir in `Tree` doesn't affect + /// `StackTree`'s copy of the same path), survives mode cycling and auto-refresh (this lives on + /// `App`, not in the rebuilt-every-call row list), and starts empty — everything expanded by + /// default. Mutated only by [`App::outline_toggle_fold`]; never explicitly cleared, so a fold + /// outlives its own toggling row's disappearance and reappearance (e.g. a discard-then-recreate + /// of the same path) for as long as the session runs. + pub folds: HashMap>, } /// Which of a split's two panes has focus — the top pane renders the unstaged role, the bottom the @@ -1053,6 +1104,47 @@ impl ChangesetView { } } +/// One content region the renderer painted this frame, in terminal cell coordinates (CS10). A +/// deliberately tiny local shape rather than `ratatui::layout::Rect`: `app.rs` has no ratatui +/// dependency today, and this keeps it that way — `render.rs` (which already depends on +/// ratatui) converts a `Rect`'s content area into this when it writes [`App::hit_regions`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Region { + pub x: u16, + pub y: u16, + pub w: u16, + pub h: u16, +} + +impl Region { + fn contains(&self, col: u16, row: u16) -> bool { + col >= self.x && col < self.x + self.w && row >= self.y && row < self.y + self.h + } +} + +/// The content regions the last frame painted (CS10), written by `render::render` (which clears +/// this to `Default` at the top of every frame first) and read by [`App::handle_click`]/ +/// [`App::handle_wheel`] to hit-test a mouse event's `(col, row)` against the region under the +/// pointer. A `None` field simply wasn't painted this frame — the outline is closed, or the +/// current file isn't in [`EffectiveZoom::Split`], etc. — never a stale rect from an earlier +/// frame's layout. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct HitRegions { + pub outline: Option, + pub single: Option, + pub unstaged: Option, + pub staged: Option, +} + +/// Which content region a mouse event hit-tested into (CS10's `App::hit_test`) — the outline, +/// the single-zoom diff pane, or one half of a split, tagged with which [`SplitPane`] so the +/// click/wheel handlers know whether to `toggle_split_focus` first. +enum HitPane { + Outline, + Single, + Split(SplitPane), +} + /// Review session state: the active changeset's file list, per-file lazily loaded views, and /// navigation/scroll state. One long-lived [`TsHighlighter`] lives here (not per file) — its /// language-config cache is keyed per-instance, so a fresh highlighter per file would rebuild @@ -1081,6 +1173,13 @@ pub struct App { /// directly by the renderer, but never written except by [`Self::derive_scroll`] — every /// cursor-moving method ends by calling it, so `scroll` always reflects the CURRENT `cursor`. pub scroll: usize, + /// Column pan offset (display columns, not bytes) applied to every diff CONTENT pane — both + /// side-by-side halves and both split panes share this one offset; the gutter stays pinned at + /// column 0. Panned by [`Self::hscroll_left`]/[`Self::hscroll_right`], clamped against the + /// current view's longest row (see those methods), and reset to `0` on file/changeset + /// navigation ([`Self::next_file`]/[`Self::prev_file`]/[`Self::next_changeset`]/ + /// [`Self::prev_changeset`]) — cursor movement within a file leaves it untouched. + pub hscroll: usize, /// Content height of the focused pane, written by the renderer each frame. In a single-pane /// zoom this is the whole body; in a split it's the focused half (see [`Self::alt_height`]). pub pane_height: usize, @@ -1088,12 +1187,16 @@ pub struct App { /// [`Self::toggle_split_focus`]). Meaningless outside [`EffectiveZoom::Split`]. alt: PaneState, /// Content height of the unfocused split pane, written by the renderer alongside - /// [`Self::pane_height`] — [`Self::derive_alt_scroll`] derives the unfocused pane's scroll - /// against THIS, not the focused pane's height. + /// [`Self::pane_height`] — the unfocused pane's scroll is clamped/derived against THIS, not + /// the focused pane's height (see [`Self::clamp_alt_scroll`]). pub(crate) alt_height: usize, /// Content height of the outline pane, written by the renderer each frame — same discipline /// as [`Self::pane_height`]. Read by [`Self::derive_outline_scroll`]. pub outline_height: usize, + /// The content regions the last frame painted (CS10 mouse support) — see [`HitRegions`]'s + /// doc comment. Cleared and re-written by `render::render` every frame; read by + /// [`Self::handle_click`]/[`Self::handle_wheel`]. + pub hit_regions: HitRegions, /// Label for the old side of the diff, shown next to a rename's `old_path` in the header. /// M4 only reviews the uncommitted (`HEAD` ↔ worktree) diffs, so this is always `"HEAD"` /// today; M5's committed-changeset zoom will want the changeset's actual base rev. @@ -1144,6 +1247,11 @@ pub struct App { /// rebuilt-from-scratch — `open`/`focused`/`mode` persist, like [`Self::layout`]/ /// [`Self::zoom`]) by every diff-initiated nav and by [`Self::refresh`]. outline: OutlineState, + /// Opt-in nerd-font iconography — `workon.review.icons`, defaulting to [`IconMode::None`] + /// (no auto-detection story exists — a terminal can't report the user's font). A TUI-wide + /// appearance mode like the theme, not an outline view setting: it gates the outline's + /// file/dir icons AND the summary panel's and winbar's glyphs (see `render.rs`). + icon_mode: IconMode, /// Whether the `?` help overlay is showing (CS3). While `true`, `tui::update` intercepts /// every key as a modal (mirroring [`Self::pending_confirm`]'s capture) — see its doc comment /// for the precedence between the two modals. @@ -1221,17 +1329,18 @@ pub enum PendingOp { file_idx: usize, selections: Vec<(usize, LineSelection)>, }, - /// CS7: discard every file in `files` — `(changeset name, file path)` pairs — from the + /// CS7: discard every file in `files` — `(changeset identity, file path)` pairs — from the /// worktree: an outline File row's single target, or a Dir row's every file under its path. - /// Stored by NAME + PATH rather than raw `(cs_idx, file_idx)` indices because the confirm - /// modal doesn't stop the tick beat: an external index change (e.g. `git add` from another - /// terminal) can run a full refresh between `d` and `y`, rebuilding the per-changeset file - /// lists and shifting positions — [`App::resolve_confirm`] re-resolves each pair against the - /// LIVE changesets at answer time (silently skipping any that vanished) so a stale index can - /// never discard the wrong file. `identity` is the acted-on outline row's - /// [`OutlineRowIdentity`], captured at request-time for the post-op outline cursor restore. + /// Stored by [`ChangesetIdentity`] + PATH rather than raw `(cs_idx, file_idx)` indices + /// because the confirm modal doesn't stop the tick beat: an external index change (e.g. + /// `git add` from another terminal) can run a full refresh between `d` and `y`, rebuilding + /// the per-changeset file lists and shifting positions — [`App::resolve_confirm`] + /// re-resolves each pair against the LIVE changesets at answer time (silently skipping any + /// that vanished) so a stale index can never discard the wrong file. `identity` is the + /// acted-on outline row's [`OutlineRowIdentity`], captured at request-time for the post-op + /// outline cursor restore. DiscardOutlineFiles { - files: Vec<(String, String)>, + files: Vec<(ChangesetIdentity, String)>, identity: OutlineRowIdentity, }, } @@ -1260,6 +1369,20 @@ pub struct Notice { pub severity: Severity, } +/// The one display-label rule for a changeset, shared by the outline header +/// ([`App::outline_snapshot`]), the summary panel ([`App::summary_for`]), and the winbar +/// (`render::render_winbar`): title, falling back to name — except the synthetic uncommitted +/// worktree layer, which is named after the SAME branch as its committed node (see +/// `workon::Changeset`'s `insert_uncommitted_layer` / [`crate::acquire::uncommitted_changeset`]) +/// and so renders as "Uncommitted changes" instead of duplicating that label. +pub(crate) fn display_label(cs: &Changeset) -> String { + if cs.span == ChangesetSpan::Uncommitted { + "Uncommitted changes".to_string() + } else { + cs.title.clone().unwrap_or_else(|| cs.name.clone()) + } +} + impl App { /// Build an [`App`] reviewing a single uncommitted changeset — the M2–M4 shape, and still /// what a non-Graphite (or clean-Graphite-tip) repo degrades to under M5's auto-detect @@ -1311,7 +1434,8 @@ impl App { width: DEFAULT_OUTLINE_WIDTH, scroll: 0, order: OutlineOrder::default(), - icons: OutlineIcons::default(), + hscroll: 0, + folds: HashMap::new(), }; let mut refresh_coordinator = RefreshCoordinator::new(); // Seed the coordinator with the index signature as it stands right after this initial @@ -1333,10 +1457,12 @@ impl App { current: 0, cursor: 0, scroll: 0, + hscroll: 0, pane_height: 20, alt: PaneState::default(), alt_height: 20, outline_height: 20, + hit_regions: HitRegions::default(), base_label, highlighter: TsHighlighter::new(), layout: Layout::default(), @@ -1349,6 +1475,7 @@ impl App { selection_anchor: None, refresh_coordinator, outline, + icon_mode: IconMode::default(), help_visible: false, review_source: None, defer_loads: false, @@ -1589,7 +1716,7 @@ impl App { return; } - let prev_cs_name = self.cur().cs.name.clone(); + let prev_cs_id = ChangesetIdentity::of(&self.cur().cs); let current_path = self .cur() .diff @@ -1636,7 +1763,7 @@ impl App { self.current_cs = new_views .iter() - .position(|v| v.cs.name == prev_cs_name) + .position(|v| prev_cs_id.matches(&v.cs)) .unwrap_or_else(|| current_cs_index(&new_views)); self.base_label = base_label_for(&new_views[self.current_cs].cs); self.changesets = new_views; @@ -2312,6 +2439,7 @@ impl App { /// outline-initiated jump (which sets [`OutlineState::cursor`] itself before calling /// `switch_changeset`/`goto_changeset` directly) never re-triggers it. pub fn next_file(&mut self) { + self.hscroll = 0; if self.cur().diff.files.is_empty() { return; } @@ -2331,6 +2459,7 @@ impl App { /// first changeset. See [`Self::next_file`]'s doc comment for why this calls /// [`Self::sync_outline_to_current`] at the end. pub fn prev_file(&mut self) { + self.hscroll = 0; if self.cur().diff.files.is_empty() { return; } @@ -2359,6 +2488,7 @@ impl App { /// DIFF-initiated entry point — see [`Self::next_file`]'s doc comment on the sync-follow /// discipline. pub fn next_changeset(&mut self) { + self.hscroll = 0; if self.current_cs + 1 < self.changesets.len() { self.goto_changeset(self.current_cs + 1); } @@ -2368,6 +2498,7 @@ impl App { /// Jump to the previous changeset's first file (`[c`). A no-op at the first changeset. See /// [`Self::next_file`]'s doc comment on the sync-follow discipline. pub fn prev_changeset(&mut self) { + self.hscroll = 0; if self.current_cs > 0 { self.goto_changeset(self.current_cs - 1); } @@ -2386,7 +2517,7 @@ impl App { self.changesets .iter() .map(|v| OutlineChangeset { - label: v.cs.title.clone().unwrap_or_else(|| v.cs.name.clone()), + label: display_label(&v.cs), current: v.cs.current, needs_restack: v.cs.needs_restack, loading: v.is_pending(), @@ -2405,14 +2536,58 @@ impl App { .collect() } - /// Build the current [`OutlineMode`]'s row list — the outline cursor's index space, and the - /// source of truth `render.rs` draws from. Rebuilt fresh on every call (cheap: a small stack - /// times a handful of files each, no caching, same posture as [`Self::effective_zoom_for`]) - /// rather than cached on `App`, so it's never stale across a mode toggle, a nav, or a - /// refresh. + /// Build (via [`outline::fold_outline`]) the current [`OutlineMode`]'s FOLD-FILTERED row list + /// — the outline cursor's SINGLE index space, and the source of truth every other outline + /// consumer reads: `render.rs`, [`Self::outline_move_by`]/[`Self::outline_move_to`], + /// [`Self::outline_confirm`], [`Self::summary_target`], and the staging-verb resolution in + /// [`Self::outline_row_targets`] all funnel through this SAME method (CS5, `outline-fold`) — + /// so folding a Header/Dir can never silently retarget a cursor move or a stage/discard verb + /// onto the wrong row: there is no OTHER row list any of them could accidentally read + /// instead. Rebuilt fresh on every call (cheap: a small stack times a handful of files each, + /// no caching, same posture as [`Self::effective_zoom_for`]) rather than cached on `App`, so + /// it's never stale across a mode toggle, a nav, a fold, or a refresh. `render.rs`'s marker + /// needs the per-row hidden-file counts this discards — see + /// [`Self::outline_items_with_hidden_counts`]. pub fn outline_items(&self) -> Vec { + self.outline_folded().items + } + + /// [`Self::outline_items`], plus (aligned by index) each row's CS5 hidden-file marker count — + /// `render_outline`'s data source. Every OTHER outline consumer uses [`Self::outline_items`] + /// instead, which just discards the counts it doesn't need; both funnel through the same + /// [`Self::outline_folded`] build, so they can never disagree about which rows are visible. + pub fn outline_items_with_hidden_counts(&self) -> (Vec, Vec) { + let folded = self.outline_folded(); + (folded.items, folded.hidden_counts) + } + + /// The shared build [`Self::outline_items`]/[`Self::outline_items_with_hidden_counts`]/ + /// [`Self::outline_target_index`] all read from — [`outline::fold_outline`] applied to the + /// current mode/order/fold-set, so there's exactly one place that pairs "which changesets by + /// which state" with "the fold set for the CURRENT mode" (`self.outline.folds` is keyed by + /// [`OutlineMode`]; a mode with no folds recorded yet reads as "everything expanded", the + /// default). + fn outline_folded(&self) -> outline::FoldedOutline { + let snapshot = self.outline_snapshot(); + let folds = self.outline.folds.get(&self.outline.mode); + outline::fold_outline(&snapshot, self.outline.mode, self.outline.order, |key| { + folds.is_some_and(|set| set.contains(key)) + }) + } + + /// Resolve a target row matched against the FULL (unfiltered) row list to its position in + /// [`Self::outline_items`]'s FILTERED list — its own index if it's visible, or its nearest + /// visible (collapsed) ancestor's if a fold hides it (CS5's "`sync_outline_to_current` + /// targeting a file hidden under a collapsed node lands on the collapsed ancestor WITHOUT + /// auto-expanding" rule — see [`outline::FoldedOutline::visible_index`]'s doc comment). `find` + /// matches against the full build (via `outline::build_items` directly, not + /// [`Self::outline_items`]) since a fold-hidden target has no index in the filtered list at + /// all to match against. + fn outline_target_index(&self, find: impl Fn(&OutlineItem) -> bool) -> Option { let snapshot = self.outline_snapshot(); - outline::build_items(&snapshot, self.outline.mode, self.outline.order) + let full = outline::build_items(&snapshot, self.outline.mode, self.outline.order); + let full_idx = full.iter().position(find)?; + self.outline_folded().visible_index.get(full_idx).copied() } /// CS4: the outline row a Header/Dir cursor selection resolves to — `None` when the outline @@ -2440,11 +2615,7 @@ impl App { match target { SummaryTarget::Changeset(cs_idx) => { let view = &self.changesets[cs_idx]; - let label = view - .cs - .title - .clone() - .unwrap_or_else(|| view.cs.name.clone()); + let label = display_label(&view.cs); let failure_message = view.failure_message().map(|s| s.to_string()); Summary::Changeset(summary::changeset_summary( label, @@ -2507,6 +2678,14 @@ impl App { self.outline.scroll } + /// The outline pane's column pan offset — see [`OutlineState::hscroll`]'s doc comment. Read + /// by `render.rs`'s `render_outline`, which also owns the render-side upper clamp (mirroring + /// [`Self::clamp_outline_scroll`]'s own per-frame bounds-clamp) via + /// [`Self::clamp_outline_hscroll`]. + pub fn outline_hscroll(&self) -> usize { + self.outline.hscroll + } + /// The outline pane's column width — `workon.review.outline.width` (CS7), or /// [`DEFAULT_OUTLINE_WIDTH`] if never set. Read by `render.rs` in place of the old fixed /// const. @@ -2524,10 +2703,10 @@ impl App { self.outline.order } - /// CS5: whether the outline renders nerd-font icons — `workon.review.outline.icons`, or - /// [`OutlineIcons::default`] (`None`) if never set. - pub fn outline_icons(&self) -> OutlineIcons { - self.outline.icons + /// The nerd-font iconography mode — `workon.review.icons`, or [`IconMode::default`] + /// (`None`) if never set. TUI-wide: read by the outline, summary panel, and winbar renderers. + pub fn icon_mode(&self) -> IconMode { + self.icon_mode } /// `o`: a pure show/hide toggle — closed -> open+focused (+[`Self::sync_outline_to_current`]), @@ -2562,6 +2741,196 @@ impl App { self.outline.focused = false; } + // ── Mouse (CS10) ───────────────────────────────────────────────────────────── + + /// Hit-test `(col, row)` against [`Self::hit_regions`] — outline first, then the single diff + /// pane, then the split's two halves — returning the matched region tagged with which + /// [`HitPane`] it was. `None` when the pointer is over a header/footer/divider/caption row + /// (recorded regions cover content only). + fn hit_test(&self, col: u16, row: u16) -> Option<(HitPane, Region)> { + if let Some(region) = self.hit_regions.outline { + if region.contains(col, row) { + return Some((HitPane::Outline, region)); + } + } + if let Some(region) = self.hit_regions.single { + if region.contains(col, row) { + return Some((HitPane::Single, region)); + } + } + if let Some(region) = self.hit_regions.unstaged { + if region.contains(col, row) { + return Some((HitPane::Split(SplitPane::Unstaged), region)); + } + } + if let Some(region) = self.hit_regions.staged { + if region.contains(col, row) { + return Some((HitPane::Split(SplitPane::Staged), region)); + } + } + None + } + + /// Focus the diff pane a click/wheel landed in, mirroring the keyboard focus rules: if the + /// outline had focus, `focus_diff()` moves focus onto whichever split pane already has it; if + /// the event landed in the OTHER split pane, `toggle_split_focus()` flips onto it next (never + /// assigning `split_focus` directly — see that method's doc comment). `target` is `None` for + /// the single-zoom pane, where there is no second half to flip to. + fn focus_diff_pane(&mut self, target: Option) { + if self.outline_focused() { + self.focus_diff(); + } + if let Some(target) = target { + if self.split_focus != target { + self.toggle_split_focus(); + } + } + } + + /// Set the (now-focused) pane's cursor to the row under a click, offset from `region`'s top by + /// `row` and clamped into the current row list, then re-derive `scroll`. A no-op on an empty + /// file list, matching [`Self::move_cursor_by`]'s empty-list behavior. + fn set_cursor_from_click(&mut self, region: Region, row: u16) { + let rows = self.row_count(); + if rows == 0 { + return; + } + let offset = (row - region.y) as usize; + self.cursor = (self.scroll + offset).min(rows - 1); + self.derive_scroll(); + } + + /// Left-click at terminal `(col, row)` (CS10): focus + select whatever content region the + /// click landed in, matching the keyboard-driven equivalent for that region. Outline: focuses + /// the outline and jumps the cursor to the clicked row via [`Self::outline_move_to`] — a File + /// row jumps the diff there (same single-jump semantics `g`/`G` use), a Header/Dir row just + /// selects (the summary panel follows via [`Self::summary_target`]) WITHOUT toggling its fold + /// (CS5, `outline-fold`) — a click has always been "move the cursor here", a strictly weaker + /// action than `Enter`'s "act on this row" even before folding existed (pre-CS5, `Enter` on a + /// Header jumped to its first file; a click on the same row never did), so a click staying + /// select-only here keeps that existing asymmetry rather than inventing a new "click mirrors + /// Enter" rule this pane never had. Diff pane (single or split): focuses that pane (flipping + /// `split_focus` first if the click landed in the unfocused half) and moves its cursor to the + /// clicked row. Outside every recorded region (header/footer/divider/captions): no-op. + pub fn handle_click(&mut self, col: u16, row: u16) { + let Some((pane, region)) = self.hit_test(col, row) else { + return; + }; + match pane { + HitPane::Outline => { + self.focus_outline(); + let idx = self.outline.scroll + (row - region.y) as usize; + self.outline_move_to(idx); + } + HitPane::Single => { + self.focus_diff_pane(None); + self.set_cursor_from_click(region, row); + } + HitPane::Split(target) => { + self.focus_diff_pane(Some(target)); + self.set_cursor_from_click(region, row); + } + } + } + + /// Mouse wheel at terminal `(col, row)` with `delta` = ±3 rows (`tui::update` maps + /// `ScrollDown`/`ScrollUp` to +3/-3). Focuses whichever region the pointer sits over first — + /// same rule as [`Self::handle_click`] — then scrolls that pane's VIEWPORT by `delta`, + /// leaving the cursor exactly where it was (the peek model: a wheel is "look elsewhere", + /// never "select elsewhere") — see [`Self::scroll_viewport_by`]. Outside every recorded + /// region: no-op. + pub fn handle_wheel(&mut self, col: u16, row: u16, delta: i64) { + let Some((pane, _region)) = self.hit_test(col, row) else { + return; + }; + match pane { + HitPane::Outline => { + self.focus_outline(); + self.outline_scroll_viewport_by(delta); + } + HitPane::Single => { + self.focus_diff_pane(None); + self.scroll_viewport_by(delta); + } + HitPane::Split(target) => { + self.focus_diff_pane(Some(target)); + self.scroll_viewport_by(delta); + } + } + } + + /// Horizontal mouse wheel (trackpad h-scroll, or a shift-wheel the terminal reports as + /// `ScrollLeft`/`ScrollRight`) at terminal `(col, row)` with `delta` = ±4 columns per tick + /// (`tui::map_key`'s caller maps `ScrollLeft`/`ScrollRight` to -4/+4 — finer than + /// [`HSCROLL_STEP`] since trackpads emit streams of ticks). Same peek-model framing and + /// region-focus rule as [`Self::handle_wheel`] — the difference is WHAT gets panned: unlike + /// the vertical wheel (which always scrolls whichever pane's own row-list viewport), this + /// pans a COLUMN offset shared per PANE KIND — the outline's own `outline.hscroll` over the + /// outline, or the diff panes' shared [`Self::hscroll`] over a diff pane (both halves of a + /// split share the one offset, same as [`Self::hscroll_left`]/[`Self::hscroll_right`]). + /// Outside every recorded region: no-op. + pub fn handle_hwheel(&mut self, col: u16, row: u16, delta: i64) { + let Some((pane, _region)) = self.hit_test(col, row) else { + return; + }; + match pane { + HitPane::Outline => { + self.focus_outline(); + self.outline.hscroll = (self.outline.hscroll as i64 + delta).max(0) as usize; + // No upper clamp here — render-side, mirroring `outline_hscroll_right`'s own + // doc comment. + } + HitPane::Single => { + self.focus_diff_pane(None); + self.pan_hscroll_by(delta); + } + HitPane::Split(target) => { + self.focus_diff_pane(Some(target)); + self.pan_hscroll_by(delta); + } + } + } + + /// Pan the shared diff [`Self::hscroll`] by `delta` columns (floored at `0`), clamping + /// against the current view's longest row on a RIGHTWARD pan only — the same clamp + /// [`Self::hscroll_right`] applies, factored out here so [`Self::handle_hwheel`] doesn't + /// clamp a leftward pan against a bound that only matters when panning right. + fn pan_hscroll_by(&mut self, delta: i64) { + self.hscroll = (self.hscroll as i64 + delta).max(0) as usize; + if delta > 0 { + self.clamp_hscroll(); + } + } + + /// Scroll the focused pane's viewport by `delta` rows (mouse wheel), clamped to the row + /// list. The cursor is deliberately NOT touched (the peek model: a wheel is "look + /// elsewhere", never "select elsewhere"), so it can sit outside the viewport — the next + /// cursor-driven op re-derives the scroll and snaps the view back to it, which is the + /// peek model's recovery gesture, not a bug. This is the one place `scroll` is written + /// directly rather than derived from the cursor; the renderer's bounds-clamp (see + /// [`Self::clamp_scroll`]) is what lets the wheeled position survive frames. + fn scroll_viewport_by(&mut self, delta: i64) { + let rows = self.row_count(); + if rows == 0 { + return; + } + let max_scroll = rows.saturating_sub(self.pane_height.max(1)) as i64; + self.scroll = (self.scroll as i64 + delta).clamp(0, max_scroll.max(0)) as usize; + } + + /// The outline counterpart of [`Self::scroll_viewport_by`] — same peek model: the outline + /// cursor never moves (so wheeling past File rows can't jump the diff, and the summary + /// panel's target stays put); the next outline cursor op snaps the view back to it. + fn outline_scroll_viewport_by(&mut self, delta: i64) { + let rows = self.outline_items().len(); + if rows == 0 { + return; + } + let max_scroll = rows.saturating_sub(self.outline_height.max(1)) as i64; + self.outline.scroll = + (self.outline.scroll as i64 + delta).clamp(0, max_scroll.max(0)) as usize; + } + /// `?`: toggle the help overlay (CS3). A plain flip — the overlay always renders whatever /// view currently has keyboard focus (see `render::render_help_overlay`), so there is no /// extra state to reposition here, unlike [`Self::toggle_outline`]. @@ -2571,9 +2940,12 @@ impl App { /// `i` while the outline has focus: cycle [`OutlineMode`], then reposition the cursor onto /// the row matching the current diff position in the NEW mode's row list (the row layout - /// just changed shape, so the raw index would otherwise point at an unrelated row). + /// just changed shape, so the raw index would otherwise point at an unrelated row). Also + /// resets [`OutlineState::hscroll`] to `0` — the row list's shape (and therefore its longest + /// path) just changed too, so a stale pan offset could easily land past the new mode's content. pub fn outline_cycle_mode(&mut self) { self.outline.mode = self.outline.mode.cycle(); + self.outline.hscroll = 0; self.sync_outline_to_current(); } @@ -2602,12 +2974,11 @@ impl App { self.outline.order = order; } - /// Set the outline icons setting directly — the config-startup (CS5) counterpart; there is - /// no interactive key for this (icons are a static config choice, not something to toggle - /// mid-session). Same non-resync posture as [`Self::set_outline_mode`]/ - /// [`Self::set_outline_order`]. - pub fn set_outline_icons(&mut self, icons: OutlineIcons) { - self.outline.icons = icons; + /// Set the icon mode directly — the config-startup counterpart; there is no interactive + /// key for this (icons are a static config choice, not something to toggle mid-session). + /// Same non-resync posture as [`Self::set_outline_mode`]/[`Self::set_outline_order`]. + pub fn set_icon_mode(&mut self, icons: IconMode) { + self.icon_mode = icons; } /// Move the outline's own cursor by `delta` rows (`j`/`k` while the outline has focus), @@ -2695,30 +3066,116 @@ impl App { self.outline_move_to(last); } - /// `Enter` while the outline has focus: jump the diff to the row under the outline cursor (a - /// file row jumps straight there; a header row jumps to that changeset's first file — the - /// one case [`Self::outline_move_by`] deliberately does NOT do on a bare cursor move), then - /// return focus to the diff. + /// `n` while the outline has focus: jump the cursor to the next [`OutlineItem::Header`] row + /// AFTER the current cursor position, or no-op (no wraparound) when there isn't one. Goes + /// through [`Self::outline_move_to`], so — like `g`/`G` — landing on a Header row never jumps + /// the diff (only a Header's own `Enter`/fold toggle or a File-row nav does that). + pub fn outline_next_changeset(&mut self) { + let items = self.outline_items(); + let cursor = self.outline.cursor; + if let Some(off) = items + .iter() + .skip(cursor + 1) + .position(|item| matches!(item, OutlineItem::Header { .. })) + { + self.outline_move_to(cursor + 1 + off); + } + } + + /// `p` while the outline has focus: jump the cursor to the next [`OutlineItem::Header`] row + /// BEFORE the current cursor position, or no-op (no wraparound) when there isn't one. The + /// counterpart to [`Self::outline_next_changeset`] — see its doc comment for the shared + /// no-diff-jump invariant. + pub fn outline_prev_changeset(&mut self) { + let items = self.outline_items(); + let cursor = self.outline.cursor; + if let Some(idx) = items[..cursor] + .iter() + .rposition(|item| matches!(item, OutlineItem::Header { .. })) + { + self.outline_move_to(idx); + } + } + + /// `Enter` while the outline has focus: a FILE row jumps the diff straight there and returns + /// focus to the diff (unchanged since CS3). A HEADER or DIR row instead TOGGLES that row's + /// fold state (CS5, `outline-fold`) and deliberately does NOT return focus — you're + /// manipulating the outline's own structure, not confirming a jump, so there's nothing to + /// hand focus back to yet. This REMOVES Enter's pre-CS5 jump-to-changeset-first-file behavior + /// on a Header row (still reachable via Enter on any of that changeset's own file rows, or + /// `[c`/`]c`) and Dir's pre-CS5 no-op (CS4 shipped Dir rows before any fold state existed to + /// toggle). pub fn outline_confirm(&mut self) { let items = self.outline_items(); match items.get(self.outline.cursor) { Some(OutlineItem::File { cs_idx, file_idx, .. - }) => self.switch_changeset(*cs_idx, *file_idx), - Some(OutlineItem::Header { cs_idx, .. }) => { - let cs_idx = *cs_idx; - self.goto_changeset(cs_idx); - // `goto_changeset` is the shared outline/diff core and deliberately does not - // self-sync (see its doc comment) — this outline-initiated call syncs explicitly - // so the cursor follows off the header row onto the file it just jumped to. - self.sync_outline_to_current(); + }) => { + self.switch_changeset(*cs_idx, *file_idx); + self.outline.focused = false; } - // A directory row (Tree/StackTree modes) is not a jump target — no expand/collapse - // state exists to toggle (CS4 decision), so Enter here is a no-op beyond the - // unconditional unfocus below, same as confirming on nothing at all. - Some(OutlineItem::Dir { .. }) | None => {} + Some(OutlineItem::Header { .. } | OutlineItem::Dir { .. }) => { + self.outline_toggle_fold(); + } + None => self.outline.focused = false, } - self.outline.focused = false; + } + + /// `Enter` on a Header/Dir row (CS5, `outline-fold`): flip that row's collapsed state in the + /// CURRENT [`OutlineMode`]'s fold set (see [`OutlineState::folds`]), then re-derive the + /// outline scroll — the row list's length just changed shape (more/fewer rows), the same + /// reason every other row-count-changing op does. The cursor's own INDEX never needs + /// re-finding: toggling a row's fold only changes what's visible AFTER it in the list (its + /// descendants), never before, so the row under the cursor — the one just toggled — stays + /// exactly where it was. + fn outline_toggle_fold(&mut self) { + let items = self.outline_items(); + let Some(item) = items.get(self.outline.cursor) else { + return; + }; + let Some(key) = FoldKey::for_item(item) else { + return; + }; + let set = self.outline.folds.entry(self.outline.mode).or_default(); + if !set.remove(&key) { + set.insert(key); + } + self.derive_outline_scroll(self.outline_items().len()); + } + + /// `zM` while the outline has focus: collapse every foldable (Header/Dir) row of the CURRENT + /// [`OutlineMode`], unlike [`Self::outline_toggle_fold`]'s single-row flip. Scans the + /// UNFOLDED build ([`outline::build_items`] over the current snapshot — the same source + /// [`Self::outline_folded`] itself folds) rather than [`Self::outline_items`], so a row + /// already hidden under an existing fold still gets its own key recorded (collapsing + /// everything must be idempotent regardless of what's already collapsed). Unlike + /// [`Self::outline_toggle_fold`], this can hide the row the cursor itself sits on, so it + /// re-derives the cursor via [`Self::sync_outline_to_current`] (the same reseat + /// [`Self::outline_cycle_mode`] uses for its own row-list reshape) rather than trusting the + /// toggle's "only descendants move" invariant, which doesn't hold here. + pub fn outline_collapse_all(&mut self) { + let snapshot = self.outline_snapshot(); + let full = outline::build_items(&snapshot, self.outline.mode, self.outline.order); + let set = self.outline.folds.entry(self.outline.mode).or_default(); + for item in &full { + if let Some(key) = FoldKey::for_item(item) { + set.insert(key); + } + } + self.sync_outline_to_current(); + } + + /// `zR` while the outline has focus: expand every folded row of the CURRENT [`OutlineMode`] — + /// clears that mode's fold set entirely. See [`Self::outline_collapse_all`] for the cursor + /// reseat rationale (shared here too, even though expanding can only ever ADD rows, never + /// hide the cursor's own). + pub fn outline_expand_all(&mut self) { + self.outline + .folds + .entry(self.outline.mode) + .or_default() + .clear(); + self.sync_outline_to_current(); } // ── Outline staging (CS7) ─────────────────────────────────────────────────── @@ -2896,12 +3353,12 @@ impl App { targets.len() ), }; - let files: Vec<(String, String)> = targets + let files: Vec<(ChangesetIdentity, String)> = targets .iter() .filter_map(|&(cs_idx, file_idx)| { let view = self.changesets.get(cs_idx)?; let path = view.files().get(file_idx)?.path.clone(); - Some((view.cs.name.clone(), path)) + Some((ChangesetIdentity::of(&view.cs), path)) }) .collect(); self.request_confirm(prompt, PendingOp::DiscardOutlineFiles { files, identity }); @@ -2958,20 +3415,21 @@ impl App { } /// Reposition (never rebuild/refocus) the outline cursor onto the row matching the CURRENT - /// diff changeset+file, or clamp it into bounds if no such row exists (e.g. Flat mode - /// deduped the current file's changeset out of the list). The sync-follow discipline's echo - /// break: called ONLY from the diff-initiated nav entry points (`next_file`/`prev_file`/ - /// `next_changeset`/`prev_changeset`/`refresh`, plus the two outline actions that explicitly - /// opt in after a header jump) — never from `switch_changeset`/`goto_changeset` themselves, - /// since those are the shared core an OUTLINE-initiated jump also calls, and an - /// outline-initiated jump has already set [`OutlineState::cursor`] to the row the user - /// selected. If this ran unconditionally inside `switch_changeset`, an outline `j`/`k` move - /// past a HEADER row (which never calls `switch_changeset`, so nothing would resync) would - /// be fine, but any accidental future call site wired into the shared core would instantly - /// stomp a manually-positioned outline cursor back onto the diff's last position — the exact - /// oscillation the prototype's `_suppress_sync` flag existed to prevent. Keeping the sync - /// calls only at the diff-facing entry points achieves the same break without needing a - /// mutable suppression flag on `App`. + /// diff changeset+file — or, if a fold hides that row, its nearest visible (collapsed) + /// ancestor instead, WITHOUT auto-expanding it (CS5, `outline-fold` — preserves the user's + /// fold intent; see [`Self::outline_target_index`]) — or clamps into bounds if no such row + /// exists in the FULL build at all (e.g. Flat mode deduped the current file's changeset out + /// of the list entirely). The sync-follow discipline's echo break: called ONLY from the + /// diff-initiated nav entry points (`next_file`/`prev_file`/`next_changeset`/`prev_changeset`/ + /// `refresh`) — never from `switch_changeset`/`goto_changeset` themselves, since those are the + /// shared core an OUTLINE-initiated jump also calls, and an outline-initiated jump has already + /// set [`OutlineState::cursor`] to the row the user selected. If this ran unconditionally + /// inside `switch_changeset`, an outline `j`/`k` move past a HEADER row (which never calls + /// `switch_changeset`, so nothing would resync) would be fine, but any accidental future call + /// site wired into the shared core would instantly stomp a manually-positioned outline cursor + /// back onto the diff's last position — the exact oscillation the prototype's + /// `_suppress_sync` flag existed to prevent. Keeping the sync calls only at the diff-facing + /// entry points achieves the same break without needing a mutable suppression flag on `App`. fn sync_outline_to_current(&mut self) { let items = self.outline_items(); if items.is_empty() { @@ -2979,11 +3437,13 @@ impl App { self.derive_outline_scroll(0); return; } - if let Some(idx) = items.iter().position(|it| { + let current_cs = self.current_cs; + let current = self.current; + if let Some(idx) = self.outline_target_index(|it| { matches!( it, OutlineItem::File { cs_idx, file_idx, .. } - if *cs_idx == self.current_cs && *file_idx == self.current + if *cs_idx == current_cs && *file_idx == current ) }) { self.outline.cursor = idx; @@ -3038,8 +3498,11 @@ impl App { } /// Re-derive the UNFOCUSED split pane's scroll against its own cursor, row count, and - /// [`Self::alt_height`] — called by the renderer each split frame, after the pane heights are - /// known. + /// [`Self::alt_height`]. Test-only since the wheel's peek model (CS10): the renderer now + /// bounds-clamps instead of deriving (see [`Self::clamp_alt_scroll`]), and no production + /// path derives the unfocused pane's scroll — the pair re-derives naturally once focus + /// swaps back onto it and a cursor op runs. + #[cfg(test)] pub(crate) fn derive_alt_scroll(&mut self) { let role = self.unfocused_split_role(); let rows = self.role_row_count(self.current, role); @@ -3047,6 +3510,115 @@ impl App { derive_scroll_value(self.alt.cursor, self.alt.scroll, rows, self.alt_height); } + /// Bounds-only clamp of the focused pane's scroll — the renderer's per-frame check under + /// the wheel's peek model (CS10). Unlike [`Self::derive_scroll`] it does NOT follow the + /// cursor, so a wheel-scrolled viewport (cursor possibly outside it) survives frames; it + /// only keeps `scroll` inside the row list when a resize/zoom shrinks it. + pub(crate) fn clamp_scroll(&mut self) { + let rows = self.row_count(); + self.scroll = self + .scroll + .min(rows.saturating_sub(self.pane_height.max(1))); + } + + /// [`Self::clamp_scroll`] for the unfocused split pane. + pub(crate) fn clamp_alt_scroll(&mut self) { + let role = self.unfocused_split_role(); + let rows = self.role_row_count(self.current, role); + self.alt.scroll = self + .alt + .scroll + .min(rows.saturating_sub(self.alt_height.max(1))); + } + + /// [`Self::clamp_scroll`] for the outline pane. + pub(crate) fn clamp_outline_scroll(&mut self, rows: usize) { + self.outline.scroll = self + .outline + .scroll + .min(rows.saturating_sub(self.outline_height.max(1))); + } + + /// Render-side upper clamp for [`OutlineState::hscroll`] — the outline analog of + /// [`Self::clamp_hscroll`], but taken from the caller rather than computed here: + /// `render_outline` already builds every item's line to paint it, so it's cheaper for it to + /// pass the max width it just measured than for this method to rebuild the whole outline a + /// second time. `max_line_width` is the widest rendered outline row's display-column width; + /// the `-1` keeps at least one column of the longest row visible, same as + /// [`Self::clamp_hscroll`]. + pub(crate) fn clamp_outline_hscroll(&mut self, max_line_width: usize) { + self.outline.hscroll = self.outline.hscroll.min(max_line_width.saturating_sub(1)); + } + + /// The widest display-column row currently in the active file's view(s) — both roles when + /// split, since [`Self::hscroll`] pans every content pane together (locked decision #1). + /// Walks the already-built [`FileView::display`] row list (shared by both the SBS and inline + /// layouts — inline just re-derives its own row list from the same text), so this is a pure + /// lookup over rows the renderer rebuilds every frame anyway, not a fresh scan of the file. + /// Used only by [`Self::clamp_hscroll`] to keep at least one column of the longest line + /// reachable; computed on demand rather than cached (cheap — see that method's doc comment). + fn max_row_width(&self) -> usize { + let idx = self.current; + let roles: Vec = match self.effective_zoom_for(idx) { + EffectiveZoom::Single(role) => vec![role], + EffectiveZoom::Split => vec![Role::Unstaged, Role::Staged], + }; + let mut max = 0; + for role in roles { + let Some(view) = self.role_view_ref(idx, role) else { + continue; + }; + for row in &view.display { + let DisplayRow::Row(r) = row else { continue }; + if let Row::Line(n) = r.old { + max = max.max(UnicodeWidthStr::width(view.old_line(n))); + } + if let Row::Line(n) = r.new { + max = max.max(UnicodeWidthStr::width(view.new_line(n))); + } + } + } + max + } + + /// Clamp [`Self::hscroll`] into `[0, max_row_width().saturating_sub(1)]` — the `-1` keeps at + /// least one column of the longest line visible (locked decision #4) rather than letting the + /// pan run all the way to a blank viewport. + fn clamp_hscroll(&mut self) { + let max = self.max_row_width().saturating_sub(1); + self.hscroll = self.hscroll.min(max); + } + + /// `hscroll-left`: pan the diff content panes left by [`HSCROLL_STEP`] columns (floored at + /// `0`). + pub fn hscroll_left(&mut self) { + self.hscroll = self.hscroll.saturating_sub(HSCROLL_STEP); + } + + /// `hscroll-right`: pan the diff content panes right by [`HSCROLL_STEP`] columns, clamped so + /// at least one column of the current view's longest row stays visible (see + /// [`Self::clamp_hscroll`]). + pub fn hscroll_right(&mut self) { + self.hscroll = self.hscroll.saturating_add(HSCROLL_STEP); + self.clamp_hscroll(); + } + + /// `outline-hscroll-left`: pan the outline pane left by [`HSCROLL_STEP`] columns (floored at + /// `0`) — the outline's own analog of [`Self::hscroll_left`]. + pub fn outline_hscroll_left(&mut self) { + self.outline.hscroll = self.outline.hscroll.saturating_sub(HSCROLL_STEP); + } + + /// `outline-hscroll-right`: pan the outline pane right by [`HSCROLL_STEP`] columns. Unlike + /// [`Self::hscroll_right`] this has NO upper clamp here — the outline's row list (every + /// item's rendered line, built by `render.rs`'s `build_outline_line`) isn't cheaply available + /// to `App` the way a [`FileView`]'s rows are, so the clamp is render-side instead + /// (`render::render_outline`, mirroring how [`Self::clamp_outline_scroll`] already + /// bounds-clamps `outline.scroll` once per frame under the wheel peek model). + pub fn outline_hscroll_right(&mut self) { + self.outline.hscroll = self.outline.hscroll.saturating_add(HSCROLL_STEP); + } + /// Re-derive the outline pane's `scroll` from its `cursor` — the outline's counterpart to /// [`Self::derive_scroll`], reusing the same [`derive_scroll_value`] core against /// [`Self::outline_height`]. Called after every outline-cursor mutation (mirroring how every @@ -3314,16 +3886,16 @@ impl App { }; self.set_outline_order(order); - let icons = match &raw.outline_icons { - Some(i) => parse_outline_icons(i).unwrap_or_else(|| { + let icons = match &raw.icons { + Some(i) => parse_icon_mode(i).unwrap_or_else(|| { warnings.push(format!( - "workon.review.outline.icons = '{i}' unrecognized; using default" + "workon.review.icons = '{i}' unrecognized; using default" )); - OutlineIcons::default() + IconMode::default() }), - None => OutlineIcons::default(), + None => IconMode::default(), }; - self.set_outline_icons(icons); + self.set_icon_mode(icons); let layout = match &raw.diff_layout { Some(l) => parse_diff_layout(l).unwrap_or_else(|| { @@ -3578,14 +4150,14 @@ impl App { self.run_op(LineSelectionOp::new(file, selections, StageVerb::Discard)); } PendingOp::DiscardOutlineFiles { files, identity } => { - // Re-resolve each (changeset name, path) pair against the LIVE changesets — an + // Re-resolve each (changeset identity, path) pair against the LIVE changesets — an // intervening tick refresh may have shifted every index since `d` was pressed // (see the variant's doc); a pair that no longer resolves is silently skipped // (its file already left the diff, so there's nothing left to discard). let ops: Vec> = files .iter() - .filter_map(|(cs_name, path)| { - let view = self.changesets.iter().find(|v| v.cs.name == *cs_name)?; + .filter_map(|(cs_id, path)| { + let view = self.changesets.iter().find(|v| cs_id.matches(&v.cs))?; let file = view.files().iter().find(|f| f.path == *path)?.clone(); Some(Box::new(FileStagingOp::file(file, StageVerb::Discard)) as Box) @@ -4374,12 +4946,12 @@ mod tests { use super::test_support::app_from_fixture; use super::{ build_file_views, find_next_hunk_row, find_prev_hunk_row, App, ChangesetView, DiffState, - EffectiveZoom, Layout, LoadedViews, Role, Severity, Summary, SummaryTarget, Zoom, - DEFAULT_OUTLINE_WIDTH, + EffectiveZoom, HitRegions, Layout, LoadedViews, Region, Role, Severity, Summary, + SummaryTarget, Zoom, DEFAULT_OUTLINE_WIDTH, HSCROLL_STEP, SCROLLOFF, }; use crate::align::{AlignedRow, CellKind, DisplayRow, InlineRow, Row}; use crate::config::ReviewConfig; - use crate::icons::OutlineIcons; + use crate::icons::IconMode; use crate::model::FileStatus; use crate::outline::{OutlineItem, OutlineMode, OutlineOrder, StagedStatus}; @@ -4445,6 +5017,62 @@ mod tests { assert_eq!(view.new_text(), ""); } + // ── diff-hscroll: pan clamping ────────────────────────────────────────────── + + #[test] + fn hscroll_left_floors_at_zero() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("a.txt", "one\n", "one\nCHANGED\n") + .build() + .unwrap(); + + let mut app = app_from_fixture(&fixture); + app.open_current(); + assert_eq!(app.hscroll, 0); + app.hscroll_left(); + assert_eq!(app.hscroll, 0, "cannot pan left of column 0"); + } + + #[test] + fn hscroll_right_clamps_to_the_longest_row_leaving_one_column_visible() { + // A line well over a terminal width, so repeated `hscroll-right` presses hit the clamp + // rather than running out of steps first. + let long_line = "x".repeat(200); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("a.txt", "short\n", &format!("{long_line}\n")) + .build() + .unwrap(); + + let mut app = app_from_fixture(&fixture); + app.open_current(); + for _ in 0..100 { + app.hscroll_right(); + } + // `max_row_width` is 200 (the long line); the clamp keeps one column of it reachable. + assert_eq!(app.hscroll, 199); + } + + #[test] + fn hscroll_right_on_a_file_with_no_long_rows_clamps_to_zero() { + // Every row is a single column wide, so `max_row_width` (1) leaves nothing to pan into — + // the clamp (`max_row_width - 1`) is `0`. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("a.txt", "a\n", "a\nb\n") + .build() + .unwrap(); + + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.hscroll_right(); + assert_eq!( + app.hscroll, 0, + "every row already fits, so there is nothing to pan into" + ); + } + #[test] fn ensure_loaded_reads_old_path_for_renamed_file() { let fixture = FixtureBuilder::new() @@ -7693,6 +8321,53 @@ mod tests { assert_eq!(app.current, 0); } + // ── diff-hscroll: reset on file/changeset nav, preserved across cursor movement ────── + + #[test] + fn next_file_resets_hscroll_to_zero() { + let mut app = two_committed_changesets_two_and_one_files(); + app.hscroll = 5; + app.next_file(); + assert_eq!(app.hscroll, 0); + } + + #[test] + fn prev_file_resets_hscroll_to_zero() { + let mut app = two_committed_changesets_two_and_one_files(); + app.goto_changeset(1); + app.hscroll = 5; + app.prev_file(); + assert_eq!(app.hscroll, 0); + } + + #[test] + fn next_changeset_resets_hscroll_to_zero() { + let mut app = two_committed_changesets_two_and_one_files(); + app.hscroll = 5; + app.next_changeset(); + assert_eq!(app.hscroll, 0); + } + + #[test] + fn prev_changeset_resets_hscroll_to_zero() { + let mut app = two_committed_changesets_two_and_one_files(); + app.goto_changeset(1); + app.hscroll = 5; + app.prev_changeset(); + assert_eq!(app.hscroll, 0); + } + + #[test] + fn cursor_movement_within_a_file_preserves_hscroll() { + let mut app = two_committed_changesets_two_and_one_files(); + app.hscroll = 5; + app.move_cursor_by(1); + assert_eq!( + app.hscroll, 5, + "plain cursor movement must not reset the horizontal pan" + ); + } + /// Regression: navigating to an OLDER committed changeset and loading its combined view must /// source the new side from that changeset's `head` commit tree, not the current worktree. The /// same file `f.txt` is touched by both changesets, so `cs-a`'s head (`mid`) content differs @@ -8095,6 +8770,38 @@ mod tests { ); } + #[test] + fn outline_cycle_mode_resets_outline_hscroll() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.hscroll = 5; + app.outline_cycle_mode(); + assert_eq!( + app.outline_hscroll(), + 0, + "a mode cycle reshapes the row list, so a stale pan offset must reset" + ); + } + + #[test] + fn outline_hscroll_left_floors_at_zero() { + let mut app = two_committed_changesets_two_and_one_files(); + assert_eq!(app.outline_hscroll(), 0); + app.outline_hscroll_left(); + assert_eq!(app.outline_hscroll(), 0, "cannot pan left of column 0"); + } + + #[test] + fn outline_hscroll_right_has_no_upper_clamp_in_the_method_itself() { + // Locked decision #2: `outline_hscroll_right` floors at 0 but does NOT clamp against the + // outline's content width — that clamp is render-side (`render_outline`), covered in + // `render.rs`'s tests. + let mut app = two_committed_changesets_two_and_one_files(); + app.outline_hscroll_right(); + assert_eq!(app.outline_hscroll(), HSCROLL_STEP); + app.outline_hscroll_right(); + assert_eq!(app.outline_hscroll(), HSCROLL_STEP * 2); + } + #[test] fn stack_mode_outline_items_carry_current_and_restack_markers() { let fixture = FixtureBuilder::new() @@ -8155,6 +8862,7 @@ mod tests { items[0], OutlineItem::Header { cs_idx: 0, + n: 2, label: "cs-a".to_string(), current: false, needs_restack: false, @@ -8170,6 +8878,7 @@ mod tests { header_b, &OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-b".to_string(), current: true, needs_restack: true, @@ -8183,11 +8892,15 @@ mod tests { /// A minimal [`Changeset`] descriptor for the slot tests below — the slot model only cares /// about the metadata `ChangesetView::pending`/`failed` carry alongside a diff-free - /// [`DiffState`], not any real git content. + /// [`DiffState`], not any real git content. The span must be a committed variant (zero OID + /// is fine, nothing diffs it) so the outline labels these by name rather than as the + /// "Uncommitted changes" layer. fn bare_changeset(name: &str, current: bool) -> Changeset { Changeset { name: name.to_string(), - span: ChangesetSpan::Uncommitted, + span: ChangesetSpan::CommittedRoot { + head: git2::Oid::ZERO_SHA1, + }, title: None, current, needs_restack: false, @@ -8265,6 +8978,7 @@ mod tests { vec![ OutlineItem::Header { cs_idx: 0, + n: 2, label: "cs-pending".to_string(), current: true, needs_restack: false, @@ -8273,6 +8987,7 @@ mod tests { }, OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-failed".to_string(), current: false, needs_restack: false, @@ -8445,6 +9160,7 @@ mod tests { items_after[app.outline_cursor()], OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-b".to_string(), current: true, needs_restack: false, @@ -8515,30 +9231,110 @@ mod tests { assert_eq!(change_for("u1.txt"), FileStatus::Untracked); } + /// `outline_snapshot`'s label fallback (`title` else `name`) used to render the SAME label + /// for a branch's committed node and its own uncommitted worktree layer — both are named + /// after the same branch, no title on either (see `FoldKey`'s doc comment, outline.rs). The + /// uncommitted layer must instead say "Uncommitted changes", so the branch name appears + /// exactly once (on the committed node). #[test] - fn outline_move_by_on_a_file_row_jumps_the_diff() { - let mut app = two_committed_changesets_two_and_one_files(); - app.outline.mode = OutlineMode::Flat; - // CS3: pin BaseFirst explicitly — this test exercises `outline_move_by`'s row-crossing - // mechanics via hardcoded Flat-mode indices, not display order. - app.outline.order = OutlineOrder::BaseFirst; - app.outline.cursor = 0; - assert_eq!(app.current_cs(), 0); - assert_eq!(app.current, 0); - - // Flat mode: a1.txt, a2.txt, b1.txt — moving to index 2 must land the diff on b1.txt in - // cs-b. - app.outline_move_by(2); - assert_eq!( - app.current_cs(), - 1, - "the outline jump must switch changeset" - ); - assert_eq!(app.files()[app.current].path, "b1.txt"); - } - - #[test] - fn outline_move_by_on_a_header_row_does_not_jump_the_diff() { + fn outline_snapshot_labels_the_uncommitted_layer_uncommitted_changes_not_the_branch_name() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let base = fixture + .commit("main") + .file("base.txt", "b\n") + .create("base") + .unwrap(); + let head = fixture + .commit("main") + .file("c1.txt", "c1\n") + .create("head") + .unwrap(); + let repo = fixture.repo().unwrap(); + std::fs::write(repo.workdir().unwrap().join("u1.txt"), "u1\n").unwrap(); + + let committed = Changeset { + name: "feature".to_string(), + span: ChangesetSpan::Committed { base, head }, + title: None, + current: false, + needs_restack: false, + }; + let uncommitted = Changeset { + name: "feature".to_string(), + span: ChangesetSpan::Uncommitted, + title: None, + current: true, + needs_restack: false, + }; + let view_c = ChangesetView::from_changeset_diff( + committed.clone(), + crate::acquire::diff_changeset(repo, &committed).unwrap(), + ); + let view_u = ChangesetView::from_changeset_diff( + uncommitted.clone(), + crate::acquire::diff_changeset(repo, &uncommitted).unwrap(), + ); + let owned = Repository::open(repo.workdir().unwrap()).unwrap(); + let mut app = App::from_changesets(owned, vec![view_c, view_u]); + app.open_current(); + app.outline.mode = OutlineMode::Stack; + // Pin BaseFirst: this asserts an exact label vec, and display order is incidental here. + app.outline.order = OutlineOrder::BaseFirst; + + let labels: Vec = app + .outline_items() + .into_iter() + .filter_map(|it| match it { + OutlineItem::Header { label, .. } => Some(label), + _ => None, + }) + .collect(); + assert_eq!( + labels, + vec!["feature", "Uncommitted changes"], + "the committed node keeps the branch name; the uncommitted layer must say \ + \"Uncommitted changes\" instead of duplicating it" + ); + + // Label parity: the summary panel (and the winbar, which reads the same + // `display_label` helper) must agree with the outline header — the uncommitted layer + // is `current: true` in this fixture, so both non-outline surfaces target it. + let Summary::Changeset(summary) = app.summary_for(SummaryTarget::Changeset(1)) else { + panic!("expected a changeset summary for the uncommitted layer"); + }; + assert_eq!( + summary.label, "Uncommitted changes", + "the summary panel must use the same display-label rule as the outline header" + ); + } + + #[test] + fn outline_move_by_on_a_file_row_jumps_the_diff() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Flat; + // CS3: pin BaseFirst explicitly — this test exercises `outline_move_by`'s row-crossing + // mechanics via hardcoded Flat-mode indices, not display order. + app.outline.order = OutlineOrder::BaseFirst; + app.outline.cursor = 0; + assert_eq!(app.current_cs(), 0); + assert_eq!(app.current, 0); + + // Flat mode: a1.txt, a2.txt, b1.txt — moving to index 2 must land the diff on b1.txt in + // cs-b. + app.outline_move_by(2); + assert_eq!( + app.current_cs(), + 1, + "the outline jump must switch changeset" + ); + assert_eq!(app.files()[app.current].path, "b1.txt"); + } + + #[test] + fn outline_move_by_on_a_header_row_does_not_jump_the_diff() { let mut app = two_committed_changesets_two_and_one_files(); app.outline.mode = OutlineMode::Stack; // CS3: pin BaseFirst explicitly — this test's hardcoded row indices assume base -> head @@ -8600,28 +9396,48 @@ mod tests { } #[test] - fn outline_confirm_on_a_header_row_jumps_to_its_first_file_and_returns_focus() { + fn outline_confirm_on_a_header_row_toggles_fold_instead_of_jumping_and_keeps_focus() { + // CS5 (`outline-fold`) removes Enter's pre-CS5 jump-to-changeset-first-file behavior on a + // Header row — it now toggles that row's fold instead, and deliberately does NOT return + // focus (you're manipulating the outline, not confirming a jump). let mut app = two_committed_changesets_two_and_one_files(); app.outline.mode = OutlineMode::Stack; // CS3: pin BaseFirst explicitly — cursor 3 is hardcoded to cs-b's header under base -> - // head row order; the confirm mechanic under test is order-agnostic. + // head row order; the toggle mechanic under test is order-agnostic. app.outline.order = OutlineOrder::BaseFirst; app.outline.open = true; app.outline.focused = true; app.outline.cursor = 3; // cs-b's header row + let before_cs = app.current_cs(); + let before_file = app.current; + let rows_before = app.outline_items().len(); app.outline_confirm(); assert_eq!( app.current_cs(), - 1, - "Enter on a header must jump to that changeset" + before_cs, + "Enter on a header must NOT jump the diff (CS5)" ); - assert_eq!(app.current, 0, "...landing on its FIRST file"); + assert_eq!(app.current, before_file); assert!( - !app.outline_focused(), - "confirming returns focus to the diff" + app.outline_focused(), + "toggling a fold must NOT return focus to the diff" + ); + assert_eq!( + app.outline_items().len(), + rows_before - 1, + "cs-b's single file row is now hidden under its collapsed header" + ); + assert_eq!( + app.outline_cursor(), + 3, + "the cursor stays on the header row it just toggled" ); + + // Toggling again expands it back. + app.outline_confirm(); + assert_eq!(app.outline_items().len(), rows_before); } #[test] @@ -8685,10 +9501,13 @@ mod tests { app.outline.mode = OutlineMode::Stack; app.outline_cycle_mode(); - assert_eq!(app.outline_mode(), OutlineMode::Tree); + assert_eq!(app.outline_mode(), OutlineMode::StackTree); app.outline_cycle_mode(); - assert_eq!(app.outline_mode(), OutlineMode::StackTree); + assert_eq!(app.outline_mode(), OutlineMode::Flat); + + app.outline_cycle_mode(); + assert_eq!(app.outline_mode(), OutlineMode::Tree); } /// A single committed changeset touching two files under `src/`, for the Dir-row no-op @@ -8753,6 +9572,7 @@ mod tests { app.outline.cursor = dir_idx; app.outline.focused = true; + let rows_before = app.outline_items().len(); app.outline_confirm(); assert_eq!(app.current_cs(), before_cs); assert_eq!( @@ -8760,8 +9580,12 @@ mod tests { "confirming a Dir row must not jump the diff" ); assert!( - !app.outline_focused(), - "confirm still returns focus to the diff, even as a no-op" + app.outline_focused(), + "confirming a Dir row toggles its fold (CS5) rather than returning focus" + ); + assert!( + app.outline_items().len() < rows_before, + "src/'s files must now be hidden under its collapsed row" ); } @@ -8959,7 +9783,7 @@ mod tests { "precondition: scrolled away from the top" ); - app.outline_cycle_mode(); // -> Tree + app.outline_cycle_mode(); // -> StackTree let cursor = app.outline_cursor(); let scroll = app.outline_scroll(); assert!( @@ -8990,7 +9814,7 @@ mod tests { assert_eq!(app.outline_width(), DEFAULT_OUTLINE_WIDTH); assert_eq!(app.outline_mode(), OutlineMode::default()); assert_eq!(app.outline_order(), OutlineOrder::default()); - assert_eq!(app.outline_icons(), OutlineIcons::default()); + assert_eq!(app.icon_mode(), IconMode::default()); assert_eq!(app.layout, Layout::default()); assert_eq!(app.zoom, Zoom::default()); } @@ -9089,9 +9913,9 @@ mod tests { } #[test] - fn outline_icons_overrides_default_when_set() { + fn icon_mode_overrides_default_when_set() { let fixture = FixtureBuilder::new() - .config("workon.review.outline.icons", "nerd") + .config("workon.review.icons", "nerd") .build() .unwrap(); let config = ReviewConfig::new(fixture.repo().unwrap()).view_config(); @@ -9100,13 +9924,13 @@ mod tests { let warnings = app.apply_view_config(&config); assert!(warnings.is_empty()); - assert_eq!(app.outline_icons(), OutlineIcons::Nerd); + assert_eq!(app.icon_mode(), IconMode::Nerd); } #[test] - fn outline_icons_invalid_falls_back_to_default_with_warning() { + fn icon_mode_invalid_falls_back_to_default_with_warning() { let fixture = FixtureBuilder::new() - .config("workon.review.outline.icons", "bogus") + .config("workon.review.icons", "bogus") .build() .unwrap(); let config = ReviewConfig::new(fixture.repo().unwrap()).view_config(); @@ -9114,9 +9938,9 @@ mod tests { let warnings = app.apply_view_config(&config); - assert_eq!(app.outline_icons(), OutlineIcons::default()); + assert_eq!(app.icon_mode(), IconMode::default()); assert_eq!(warnings.len(), 1); - assert!(warnings[0].contains("outline.icons")); + assert!(warnings[0].contains("workon.review.icons")); } #[test] @@ -9591,6 +10415,100 @@ mod tests { )); } + /// A Graphite stack whose current branch `b` has BOTH a committed changeset and the + /// uncommitted layer — [`workon::assemble_changesets`]'s `insert_uncommitted_layer` names + /// the layer after the current branch, so two changesets share the name "b". Built through + /// the production resolve path ([`crate::acquire::resolve_changesets`]) so `App::refresh` + /// re-resolves the same shape. + fn graphite_stack_app_on_uncommitted_layer() -> (Fixture, App) { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .graphite_config(&["main"]) + .branch_metadata("a", "main") + .branch_metadata("b", "a") + .untracked_file("scratch.txt", "hi\n") + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + repo.set_head("refs/heads/b").unwrap(); + repo.checkout_head(None).unwrap(); + + let changesets = crate::acquire::resolve_changesets(repo, "b").expect("resolve"); + assert!( + changesets + .iter() + .any(|cs| cs.name == "b" && cs.span != ChangesetSpan::Uncommitted), + "precondition: a committed changeset named after the current branch" + ); + assert!( + changesets + .iter() + .any(|cs| cs.name == "b" && cs.span == ChangesetSpan::Uncommitted), + "precondition: the uncommitted layer shares that name" + ); + let mut views = Vec::with_capacity(changesets.len()); + for cs in changesets { + let diff = crate::acquire::diff_changeset(repo, &cs).unwrap(); + views.push(ChangesetView::from_changeset_diff(cs, diff)); + } + let owned = Repository::open(repo.workdir().unwrap()).unwrap(); + let mut app = App::from_changesets(owned, views); + app.open_current(); + assert_eq!( + app.cur().cs.span, + ChangesetSpan::Uncommitted, + "precondition: the review opens on the uncommitted layer" + ); + (fixture, app) + } + + /// Refresh re-finds the current changeset by NAME alone — with the uncommitted layer named + /// after its branch, the first name match is the committed "b" changeset, and the reviewer + /// is silently teleported off the uncommitted layer. Every staging op refreshes, so this is + /// the "stage a file and the diff viewer jumps to another changeset" dogfood bug. + #[test] + fn refresh_stays_on_the_uncommitted_layer_despite_a_same_named_committed_changeset() { + let (_fixture, mut app) = graphite_stack_app_on_uncommitted_layer(); + + app.refresh(); + + assert_eq!( + app.cur().cs.span, + ChangesetSpan::Uncommitted, + "refresh must keep the reviewer on the uncommitted layer, not its same-named \ + committed changeset" + ); + } + + /// The confirm-time re-resolve for an outline discard looks the changeset up by NAME alone + /// (`resolve_confirm`'s `DiscardOutlineFiles` arm) — the first match is the committed "b" + /// changeset, the file isn't in ITS diff, and the pair is silently dropped: `y` does + /// nothing. This is the "discard from the outline has no effect" dogfood bug. + #[test] + fn outline_discard_still_applies_when_a_committed_changeset_shares_the_layers_name() { + let (fixture, mut app) = graphite_stack_app_on_uncommitted_layer(); + // Set mode/order BEFORE the row lookup — the index is only valid in the build it was + // found in. + open_focused_outline(&mut app, OutlineMode::Stack, 0); + app.outline.cursor = outline_file_row(&app, "scratch.txt"); + + app.outline_discard(); + assert!( + app.pending_confirm.is_some(), + "discard must request confirm; notice: {:?}", + app.notice + ); + app.resolve_confirm(true); + + let repo = fixture.repo().unwrap(); + let scratch = repo.workdir().unwrap().join("scratch.txt"); + // No absence predicate exists yet; a direct existence check keeps the assertion honest. + assert!( + !scratch.exists(), + "y must discard the untracked file from the worktree" + ); + } + #[test] fn outline_discard_confirm_n_cancels_and_leaves_the_worktree_unchanged() { let fixture = FixtureBuilder::new() @@ -9687,92 +10605,484 @@ mod tests { } } - // ── CS8: progressive gap expansion ────────────────────────────────────── - - /// A single-file fixture with two hunks separated by a wide (40-line) unchanged run — wide - /// enough that even a full 10/10 [`App::expand_gap_at_cursor`] press still leaves a - /// surviving [`DisplayRow::Gap`] (`40 - 2*3 - 2*10 = 14` rows still hidden), unlike - /// [`two_hunk_fixture`]'s much narrower gap. - fn two_hunks_with_a_wide_gap_fixture() -> Fixture { - let mut committed = String::from("OLD_HUNK_A\n"); - let mut modified = String::from("NEW_HUNK_A\n"); - for i in 1..=40 { - committed.push_str(&format!("ctx{i}\n")); - modified.push_str(&format!("ctx{i}\n")); - } - committed.push_str("OLD_HUNK_B\n"); - modified.push_str("NEW_HUNK_B\n"); - - FixtureBuilder::new() - .config("core.autocrlf", "false") - .unstaged_file("f.txt", &committed, &modified) - .build() - .unwrap() - } - - /// The display-row index of the current file's ONLY gap row — the fixture shape every CS8 - /// expansion test below relies on. - fn only_gap_row(app: &App) -> usize { - app.current_view_ref() - .expect("loaded view") - .display - .iter() - .position(|r| matches!(r, DisplayRow::Gap { .. })) - .expect("expected exactly one gap row") - } + // ── CS5 (`outline-fold`): collapse/expand ─────────────────────────────────── #[test] - fn expand_gap_cancels_an_active_selection_but_a_non_gap_press_leaves_it_alone() { - // An expansion reshapes the focused pane's row space, so `selection_anchor`'s invariant - // (cancel, never translate) applies — a selection made before the expand would silently - // cover different lines after it. The non-gap no-op path must NOT cancel: nothing - // reshaped. - let fixture = two_hunks_with_a_wide_gap_fixture(); - let mut app = app_from_fixture(&fixture); - app.open_current(); + fn outline_toggle_fold_hides_the_headers_files_and_move_by_skips_them() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + // Row order (BaseFirst): [Header cs-a, File a1, File a2, Header cs-b, File b1]. + let rows_before = app.outline_items().len(); + assert_eq!(rows_before, 5); - app.start_selection(); - assert!(app.selection_anchor.is_some(), "selection must start"); - app.expand_gap_at_cursor(false); // cursor sits on the first hunk, not a gap: no-op + app.outline.cursor = 3; // cs-b's header + app.outline_confirm(); // toggle fold + let items = app.outline_items(); + assert_eq!(items.len(), 4, "cs-b's single file row is now hidden"); assert!( - app.selection_anchor.is_some(), - "a no-op press on a non-gap row must leave the selection alone" + items + .iter() + .all(|it| !matches!(it, OutlineItem::File { cs_idx: 1, .. })), + "no cs-b file row should be reachable while its header is collapsed" ); - app.cursor = only_gap_row(&app); - app.expand_gap_at_cursor(false); - assert!( - app.selection_anchor.is_none(), - "an actual expansion reshapes the row space and must cancel the selection" + // `j` from the last visible row (now the folded header, index 3) must clamp there — there + // is nothing further to move onto. + app.outline.cursor = 3; + app.outline_move_by(5); + assert_eq!( + app.outline.cursor, 3, + "the cursor clamps at the collapsed header — b1.txt's row isn't in the index space \ + to land on at all" ); } #[test] - fn expand_gap_at_cursor_on_a_gap_row_reveals_more_rows() { - let fixture = two_hunks_with_a_wide_gap_fixture(); - let mut app = app_from_fixture(&fixture); - app.open_current(); - - let gap_row = only_gap_row(&app); - let before_len = app.current_view_ref().unwrap().display.len(); - app.cursor = gap_row; + fn outline_toggle_fold_expanding_again_restores_every_row() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + let rows_before = app.outline_items().len(); - app.expand_gap_at_cursor(false); + app.outline.cursor = 3; + app.outline_confirm(); // collapse + assert!(app.outline_items().len() < rows_before); + app.outline_confirm(); // expand again + assert_eq!( + app.outline_items(), + { + app.outline.folds.clear(); + app.outline_items() + }, + "re-expanding must reproduce exactly the same rows an empty fold set would" + ); + } - let view = app.current_view_ref().unwrap(); + #[test] + fn outline_fold_state_is_independent_per_mode() { + // Folding cs-b's header in Stack mode must not affect StackTree's own (separate) fold + // set, even though both modes emit a Header row keyed by the SAME label. + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + app.outline.cursor = 3; // cs-b's header in Stack mode + app.outline_confirm(); assert!( - view.display.len() > before_len, - "expanding must reveal more rows: {before_len} -> {}", - view.display.len() + app.outline + .folds + .get(&OutlineMode::Stack) + .is_some_and(|s| !s.is_empty()), + "Stack mode's own fold set recorded the toggle" ); + + app.outline.mode = OutlineMode::StackTree; assert!( - app.cursor < view.display.len(), - "cursor must stay in bounds" + app.outline + .folds + .get(&OutlineMode::StackTree) + .is_none_or(|s| s.is_empty()), + "StackTree mode must start with its OWN empty fold set, untouched by Stack mode's" ); + let stack_tree_items = app.outline_items(); assert!( - matches!(view.display[app.cursor], DisplayRow::Row(_)), - "the cursor's old index (the gap's leading edge) must now hold a revealed row, not \ - the gap marker: {:?}", + stack_tree_items + .iter() + .any(|it| matches!(it, OutlineItem::File { cs_idx: 1, .. })), + "cs-b's file row must still be visible in StackTree mode — Stack mode's fold doesn't \ + leak across modes" + ); + } + + #[test] + fn sync_outline_to_current_lands_on_the_collapsed_ancestor_without_auto_expanding() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + let header_b = app + .outline_items() + .iter() + .position(|it| matches!(it, OutlineItem::Header { cs_idx: 1, .. })) + .expect("cs-b's header present"); + app.outline.cursor = header_b; + app.outline_confirm(); // collapse cs-b's header + assert!( + app.outline_focused(), + "toggling a fold keeps focus (CS5) — sanity for the nav below" + ); + + // A diff-initiated nav lands the diff on cs-b's (now-hidden) first file. + app.next_changeset(); + assert_eq!(app.current_cs(), 1, "the diff itself did jump to cs-b"); + + let folded_header_idx = app + .outline_items() + .iter() + .position(|it| matches!(it, OutlineItem::Header { cs_idx: 1, .. })) + .expect("cs-b's collapsed header row still present"); + assert_eq!( + app.outline_cursor(), + folded_header_idx, + "the outline cursor must land on cs-b's collapsed header row, not an arbitrary clamp" + ); + assert!( + app.outline + .folds + .get(&OutlineMode::Stack) + .is_some_and(|s| !s.is_empty()), + "landing on the collapsed ancestor must NOT auto-expand it" + ); + } + + // ── n/p (outline changeset nav) + zM/zR (collapse/expand all) ────────────── + + #[test] + fn outline_next_changeset_jumps_to_the_next_header_without_jumping_the_diff() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + // Row order (BaseFirst): [Header cs-a, File a1, File a2, Header cs-b, File b1]. + app.outline.cursor = 1; // a1's row + let cursor_before = (app.current_cs(), app.current); + + app.outline_next_changeset(); + assert_eq!(app.outline.cursor, 3, "must land on cs-b's header row"); + assert_eq!( + (app.current_cs(), app.current), + cursor_before, + "a header landing must not jump the diff" + ); + } + + #[test] + fn outline_next_changeset_does_not_wrap_past_the_last_header() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + app.outline.cursor = 3; // cs-b's header, the LAST header row + + app.outline_next_changeset(); + assert_eq!( + app.outline.cursor, 3, + "no next header to jump to — the cursor must not move" + ); + } + + #[test] + fn outline_prev_changeset_jumps_to_the_previous_header_without_jumping_the_diff() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + app.outline.cursor = 4; // b1's row + let cursor_before = (app.current_cs(), app.current); + + app.outline_prev_changeset(); + assert_eq!(app.outline.cursor, 3, "must land on cs-b's own header row"); + assert_eq!( + (app.current_cs(), app.current), + cursor_before, + "a header landing must not jump the diff" + ); + + app.outline_prev_changeset(); + assert_eq!(app.outline.cursor, 0, "must land on cs-a's header row"); + } + + #[test] + fn outline_prev_changeset_does_not_wrap_past_the_first_header() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + app.outline.cursor = 0; // cs-a's header, the FIRST header row + + app.outline_prev_changeset(); + assert_eq!( + app.outline.cursor, 0, + "no previous header to jump to — the cursor must not move" + ); + } + + #[test] + fn outline_collapse_all_folds_every_header_leaving_only_header_rows() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + assert_eq!(app.outline_items().len(), 5, "sanity: both stacks expanded"); + + app.outline_collapse_all(); + let items = app.outline_items(); + assert_eq!( + items.len(), + 2, + "only the two Header rows remain once every changeset is collapsed" + ); + assert!( + items + .iter() + .all(|it| matches!(it, OutlineItem::Header { .. })), + "every remaining row must be a Header row: {items:?}" + ); + } + + #[test] + fn outline_collapse_all_is_idempotent_when_a_header_is_already_folded() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + app.outline.cursor = 3; // cs-b's header + app.outline_confirm(); // pre-collapse cs-b only + + app.outline_collapse_all(); + assert_eq!( + app.outline_items().len(), + 2, + "collapse-all must still fold cs-a even though cs-b was already folded" + ); + } + + #[test] + fn outline_collapse_all_reseats_a_cursor_on_a_row_that_just_got_hidden() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + app.outline.cursor = 1; // a1's row — about to be hidden under cs-a's header + + app.outline_collapse_all(); + let items = app.outline_items(); + assert!( + app.outline.cursor < items.len(), + "the cursor must land inside the shrunk row list, not stay at a now-invalid index" + ); + assert!( + matches!( + items[app.outline.cursor], + OutlineItem::Header { cs_idx: 0, .. } + ), + "the cursor must reseat onto cs-a's collapsed header, the ancestor of the hidden row \ + it was on" + ); + } + + #[test] + fn outline_expand_all_restores_every_row() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + let rows_before = app.outline_items().len(); + + app.outline_collapse_all(); + assert!(app.outline_items().len() < rows_before); + + app.outline_expand_all(); + assert_eq!( + app.outline_items().len(), + rows_before, + "expand-all must restore every row collapse-all hid" + ); + assert!( + app.outline + .folds + .get(&OutlineMode::Stack) + .is_none_or(|s| s.is_empty()), + "expand-all must clear the CURRENT mode's fold set" + ); + } + + #[test] + fn outline_collapse_all_and_expand_all_are_scoped_to_the_current_mode() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + + app.outline_collapse_all(); + assert!(app + .outline + .folds + .get(&OutlineMode::Stack) + .is_some_and(|s| !s.is_empty())); + + app.outline.mode = OutlineMode::StackTree; + assert!( + app.outline + .folds + .get(&OutlineMode::StackTree) + .is_none_or(|s| s.is_empty()), + "Stack's collapse-all must not leak into StackTree's own fold set" + ); + } + + #[test] + fn outline_stage_targets_the_correct_row_when_an_unrelated_header_is_folded() { + // The highest-risk CS5 interaction: folding one changeset's header shifts every LATER + // row's index in `outline_items()` — a stage/discard verb resolved against a stale + // (unfiltered) index space would silently act on the wrong file. `outline_stage` reads + // `outline_row_targets`, which reads `outline_items()` at the CURSOR's own index — the + // same fold-filtered list the cursor itself was placed against — so it must stay correct. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .graphite_config(&["main"]) + .branch_metadata("a", "main") + .unstaged_file("dirty.txt", "one\n", "one\nCHANGED\n") + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + repo.set_head("refs/heads/a").unwrap(); + repo.checkout_head(None).unwrap(); + + let changesets = crate::acquire::resolve_changesets(repo, "a").unwrap(); + assert_eq!( + changesets.len(), + 2, + "expected the 'a' Graphite node plus the dirty tree's uncommitted layer" + ); + let diffs = crate::acquire::diff_changesets(repo, &changesets).unwrap(); + let views: Vec = changesets + .into_iter() + .zip(diffs) + .map(|(cs, diff)| ChangesetView::from_changeset_diff(cs, diff)) + .collect(); + let owned = Repository::open(repo.workdir().unwrap()).unwrap(); + let mut app = App::from_changesets(owned, views); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline.open = true; + app.outline.focused = true; + + // Fold the committed "a" node's header — hides its own file row, shifting dirty.txt's + // row index one earlier in the filtered list. + let header_a = app + .outline_items() + .iter() + .position(|it| matches!(it, OutlineItem::Header { cs_idx: 0, .. })) + .expect("'a's header present"); + app.outline.cursor = header_a; + app.outline_confirm(); + + let dirty_idx = app + .outline_items() + .iter() + .position(|it| matches!(it, OutlineItem::File { path, .. } if path == "dirty.txt")) + .expect("dirty.txt's row is still visible — its own header isn't folded"); + app.outline.cursor = dirty_idx; + + app.outline_stage(); + + assert!(app.notice.is_none(), "stage must succeed: {:?}", app.notice); + repo.assert(predicate::repo::has_staged_file("dirty.txt")); + } + + // ── CS8: progressive gap expansion ────────────────────────────────────── + + /// A single-file fixture with two hunks separated by a wide (40-line) unchanged run — wide + /// enough that even a full 10/10 [`App::expand_gap_at_cursor`] press still leaves a + /// surviving [`DisplayRow::Gap`] (`40 - 2*3 - 2*10 = 14` rows still hidden), unlike + /// [`two_hunk_fixture`]'s much narrower gap. + fn two_hunks_with_a_wide_gap_fixture() -> Fixture { + let mut committed = String::from("OLD_HUNK_A\n"); + let mut modified = String::from("NEW_HUNK_A\n"); + for i in 1..=40 { + committed.push_str(&format!("ctx{i}\n")); + modified.push_str(&format!("ctx{i}\n")); + } + committed.push_str("OLD_HUNK_B\n"); + modified.push_str("NEW_HUNK_B\n"); + + FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("f.txt", &committed, &modified) + .build() + .unwrap() + } + + /// The display-row index of the current file's ONLY gap row — the fixture shape every CS8 + /// expansion test below relies on. + fn only_gap_row(app: &App) -> usize { + app.current_view_ref() + .expect("loaded view") + .display + .iter() + .position(|r| matches!(r, DisplayRow::Gap { .. })) + .expect("expected exactly one gap row") + } + + #[test] + fn expand_gap_cancels_an_active_selection_but_a_non_gap_press_leaves_it_alone() { + // An expansion reshapes the focused pane's row space, so `selection_anchor`'s invariant + // (cancel, never translate) applies — a selection made before the expand would silently + // cover different lines after it. The non-gap no-op path must NOT cancel: nothing + // reshaped. + let fixture = two_hunks_with_a_wide_gap_fixture(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + + app.start_selection(); + assert!(app.selection_anchor.is_some(), "selection must start"); + app.expand_gap_at_cursor(false); // cursor sits on the first hunk, not a gap: no-op + assert!( + app.selection_anchor.is_some(), + "a no-op press on a non-gap row must leave the selection alone" + ); + + app.cursor = only_gap_row(&app); + app.expand_gap_at_cursor(false); + assert!( + app.selection_anchor.is_none(), + "an actual expansion reshapes the row space and must cancel the selection" + ); + } + + #[test] + fn expand_gap_at_cursor_on_a_gap_row_reveals_more_rows() { + let fixture = two_hunks_with_a_wide_gap_fixture(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + + let gap_row = only_gap_row(&app); + let before_len = app.current_view_ref().unwrap().display.len(); + app.cursor = gap_row; + + app.expand_gap_at_cursor(false); + + let view = app.current_view_ref().unwrap(); + assert!( + view.display.len() > before_len, + "expanding must reveal more rows: {before_len} -> {}", + view.display.len() + ); + assert!( + app.cursor < view.display.len(), + "cursor must stay in bounds" + ); + assert!( + matches!(view.display[app.cursor], DisplayRow::Row(_)), + "the cursor's old index (the gap's leading edge) must now hold a revealed row, not \ + the gap marker: {:?}", view.display[app.cursor] ); // The gap is wide enough (40 hidden rows) that a single 10/10 press doesn't consume it. @@ -10057,4 +11367,383 @@ mod tests { view.display ); } + + // ── CS10: mouse (click-to-focus, wheel scrolling) ──────────────────────────── + + #[test] + fn click_on_an_outline_file_row_focuses_selects_and_jumps_the_diff() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + // BaseFirst Stack order: header(cs-a)=0, a1.txt=1, a2.txt=2, header(cs-b)=3, b1.txt=4. + app.outline_height = 10; + app.derive_outline_scroll(app.outline_items().len()); + app.hit_regions.outline = Some(Region { + x: 0, + y: 0, + w: 20, + h: 10, + }); + assert!(!app.outline_focused(), "starts unfocused (locked default)"); + + // Row 2 (a2.txt) at the outline's top-of-viewport (scroll 0) is screen row 2. + app.handle_click(5, 2); + + assert!(app.outline_focused(), "a click on the outline focuses it"); + assert_eq!(app.outline_cursor(), 2); + assert_eq!(app.current_cs(), 0); + assert_eq!( + app.files()[app.current].path, + "a2.txt", + "a File row's click must jump the diff there, like outline_move_to" + ); + } + + #[test] + fn click_on_an_outline_header_row_selects_without_jumping_the_diff() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline.mode = OutlineMode::Stack; + app.outline.order = OutlineOrder::BaseFirst; + app.outline_height = 10; + app.derive_outline_scroll(app.outline_items().len()); + app.hit_regions.outline = Some(Region { + x: 0, + y: 0, + w: 20, + h: 10, + }); + let before_cs = app.current_cs(); + let before_file = app.current; + + // Row 3 is cs-b's header. + app.handle_click(5, 3); + + assert!(app.outline_focused()); + assert_eq!(app.outline_cursor(), 3); + assert_eq!( + (app.current_cs(), app.current), + (before_cs, before_file), + "a Header row's click must not jump the diff" + ); + assert!( + app.summary_target().is_some(), + "selecting a Header row (outline open + focused) must surface the summary panel" + ); + } + + #[test] + fn click_in_the_single_diff_pane_focuses_it_and_moves_the_cursor_to_the_clicked_row() { + // 40 single-line rows (mirrors `derive_scroll_keeps_scrolloff_margin_and_slides_minimally` + // above) — long enough that clicking row 4 lands there without clamping against a tiny + // real diff. + let lines: String = (1..=40).map(|n| format!("l{n}\n")).collect(); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file("big.txt", &lines) + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.focus_outline(); + assert!(app.outline_focused()); + app.pane_height = 10; + app.cursor = 0; + app.scroll = 0; + app.hit_regions.single = Some(Region { + x: 0, + y: 0, + w: 40, + h: 10, + }); + + app.handle_click(10, 4); + + assert!( + !app.outline_focused(), + "a click in the diff pane must return focus to the diff" + ); + assert_eq!( + app.cursor, 4, + "the cursor must land on the clicked row (scroll 0 + offset 4)" + ); + } + + #[test] + fn click_in_the_unfocused_split_pane_flips_split_focus_and_moves_its_cursor() { + let fixture = partial_fixture(); + let mut app = app_from_fixture(&fixture); + app.open_current(); // Split; focused pane defaults to Unstaged + assert_eq!(app.effective_zoom_for(app.current), EffectiveZoom::Split); + assert_eq!(app.split_focus_role(), Role::Unstaged); + + app.pane_height = 5; + app.alt_height = 5; + app.derive_scroll(); + app.derive_alt_scroll(); + app.hit_regions.unstaged = Some(Region { + x: 0, + y: 1, + w: 40, + h: 5, + }); + app.hit_regions.staged = Some(Region { + x: 0, + y: 7, + w: 40, + h: 5, + }); + + // Row 1 inside the staged region (y=7, height 5) — the currently UNFOCUSED pane. `f.txt` + // is a 3-line file (alpha/beta/gamma), so offset 1 stays within its row count either way. + app.handle_click(3, 8); + + assert_eq!( + app.split_focus_role(), + Role::Staged, + "a click in the unfocused pane must flip split_focus onto it" + ); + let (_, cursor) = app.pane_render_state(Role::Staged); + assert_eq!( + cursor, + Some(1), + "the newly-focused pane's cursor must land on the clicked row (offset 1 into the region)" + ); + } + + #[test] + fn wheel_over_the_outline_scrolls_the_viewport_without_moving_cursor_or_diff() { + let mut app = four_committed_changesets_three_files_each(); + app.outline_height = 5; + app.outline.cursor = 0; + app.derive_outline_scroll(app.outline_items().len()); + app.hit_regions.outline = Some(Region { + x: 0, + y: 0, + w: 20, + h: 5, + }); + assert!(!app.outline_focused()); + let (cs_before, file_before) = (app.current_cs(), app.current); + + app.handle_wheel(5, 2, 3); + + assert!(app.outline_focused(), "a wheel event focuses its pane"); + assert_eq!( + app.outline_scroll(), + 3, + "the wheel moves the VIEWPORT by delta" + ); + assert_eq!( + app.outline_cursor(), + 0, + "peek model: the cursor never moves with the wheel, even out of the viewport" + ); + assert_eq!( + (app.current_cs(), app.current), + (cs_before, file_before), + "no cursor move means no diff jump, ever" + ); + + // The recovery gesture: the next cursor op re-derives the scroll and snaps the view + // back to the (wheel-abandoned) cursor. + app.outline_move_by(1); + assert_eq!(app.outline_cursor(), 1); + assert_eq!( + app.outline_scroll(), + 0, + "a cursor op after a wheel peek snaps the viewport back to the cursor" + ); + } + + #[test] + fn wheel_over_the_focused_diff_pane_scrolls_the_viewport_and_leaves_the_cursor() { + // Same 40-line fixture as the click test above — enough rows that a ±3 wheel move never + // clamps against a tiny real diff. + let lines: String = (1..=40).map(|n| format!("l{n}\n")).collect(); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file("big.txt", &lines) + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.pane_height = 10; + app.cursor = 8; + app.scroll = 0; + app.hit_regions.single = Some(Region { + x: 0, + y: 0, + w: 40, + h: 10, + }); + + app.handle_wheel(10, 3, 3); + app.handle_wheel(10, 3, 3); + app.handle_wheel(10, 3, 3); + assert_eq!(app.scroll, 9, "three wheel presses move the viewport 3x3"); + assert_eq!( + app.cursor, 8, + "peek model: the cursor stays put even once the viewport has scrolled past it" + ); + + // The recovery gesture: any cursor op re-derives and snaps the view back. + app.move_cursor_by(1); + assert_eq!(app.cursor, 9); + assert_eq!( + app.scroll, + 9 - SCROLLOFF, + "a cursor op after a wheel peek snaps the viewport back to the cursor's window" + ); + } + + // ── mouse h-wheel + outline hscroll follow-up ───────────────────────────────── + + #[test] + fn handle_hwheel_over_the_outline_pans_outline_hscroll_not_diff() { + let mut app = four_committed_changesets_three_files_each(); + app.outline_height = 5; + app.derive_outline_scroll(app.outline_items().len()); + app.hit_regions.outline = Some(Region { + x: 0, + y: 0, + w: 20, + h: 5, + }); + assert_eq!(app.outline_hscroll(), 0); + assert_eq!(app.hscroll, 0); + + app.handle_hwheel(5, 2, 4); + + assert!( + app.outline_focused(), + "an h-wheel event over the outline focuses it, like the vertical wheel" + ); + assert_eq!( + app.outline_hscroll(), + 4, + "the outline's own pan offset must move" + ); + assert_eq!(app.hscroll, 0, "the diff's shared pan offset must not move"); + } + + #[test] + fn handle_hwheel_over_the_diff_pane_pans_app_hscroll_not_outline() { + // "l1".."l40" — the widest rows ("l10".."l40") are 3 columns, so the clamp + // (`max_row_width - 1`) is 2. + let lines: String = (1..=40).map(|n| format!("l{n}\n")).collect(); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file("big.txt", &lines) + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.pane_height = 10; + app.hit_regions.single = Some(Region { + x: 0, + y: 0, + w: 40, + h: 10, + }); + assert_eq!(app.hscroll, 0); + + app.handle_hwheel(10, 3, 4); + + assert_eq!( + app.hscroll, 2, + "the diff's shared pan offset moves, clamped like `hscroll_right`" + ); + assert_eq!( + app.outline_hscroll(), + 0, + "the outline's own pan offset must not move" + ); + } + + #[test] + fn handle_hwheel_floors_at_zero() { + let mut app = four_committed_changesets_three_files_each(); + app.outline_height = 5; + app.derive_outline_scroll(app.outline_items().len()); + app.hit_regions.outline = Some(Region { + x: 0, + y: 0, + w: 20, + h: 5, + }); + + app.handle_hwheel(5, 2, -4); + + assert_eq!(app.outline_hscroll(), 0, "cannot pan left of column 0"); + } + + #[test] + fn handle_hwheel_outside_every_region_is_a_no_op() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline_height = 10; + app.pane_height = 10; + app.hit_regions = HitRegions { + outline: Some(Region { + x: 0, + y: 0, + w: 20, + h: 10, + }), + single: Some(Region { + x: 21, + y: 0, + w: 40, + h: 10, + }), + unstaged: None, + staged: None, + }; + let outline_focused_before = app.outline_focused(); + let hscroll_before = app.hscroll; + let outline_hscroll_before = app.outline_hscroll(); + + // On the divider, outside both recorded regions — same column CS10's click no-op test + // uses. + app.handle_hwheel(20, 0, 4); + + assert_eq!(app.outline_focused(), outline_focused_before); + assert_eq!(app.hscroll, hscroll_before); + assert_eq!(app.outline_hscroll(), outline_hscroll_before); + } + + #[test] + fn click_outside_every_hit_region_is_a_no_op() { + let mut app = two_committed_changesets_two_and_one_files(); + app.outline_height = 10; + app.pane_height = 10; + app.hit_regions = HitRegions { + outline: Some(Region { + x: 0, + y: 0, + w: 20, + h: 10, + }), + single: Some(Region { + x: 21, + y: 0, + w: 40, + h: 10, + }), + unstaged: None, + staged: None, + }; + let outline_focused_before = app.outline_focused(); + let cursor_before = app.cursor; + let outline_cursor_before = app.outline_cursor(); + let current_before = (app.current_cs(), app.current); + + // Row 0 sits above both content regions (a header row at y=0 in either would collide — + // pick a column between the two panes' widths, on the divider itself). + app.handle_click(20, 0); + + assert_eq!(app.outline_focused(), outline_focused_before); + assert_eq!(app.cursor, cursor_before); + assert_eq!(app.outline_cursor(), outline_cursor_before); + assert_eq!((app.current_cs(), app.current), current_before); + } } diff --git a/git-workon-review/src/config.rs b/git-workon-review/src/config.rs index 47c00522..4b44bbcc 100644 --- a/git-workon-review/src/config.rs +++ b/git-workon-review/src/config.rs @@ -18,6 +18,7 @@ //! ```gitconfig //! [workon "review"] //! theme = dark ; auto | dark | light (default: auto) +//! icons = nerd ; nerd | none (default: none) //! //! [workon "review.diff.bind"] //! stage-hunk = s x ; action = key tokens (space-separated) @@ -32,19 +33,20 @@ //! width = 32 //! mode = tree //! order = base-first ; head-first | base-first (default: head-first) -//! icons = nerd ; nerd | none (default: none) //! //! [workon "review.diff"] //! layout = split //! zoom = combined //! ``` //! -//! ## `outline.icons` (CS5) +//! ## `icons` //! -//! Opt-in nerd-font file/dir icons in the outline pane. There is deliberately NO auto-detection +//! Opt-in nerd-font iconography — top-level next to `theme` (`workon.review.icons`), NOT an +//! outline setting: the mode gates the outline's file/dir icons, the summary panel's glyphs, +//! and the winbar's marker/diffstat/file icons alike. There is deliberately NO auto-detection //! — a terminal cannot report whether the user's font is patched with the nerd-font glyphs, so //! guessing would silently render tofu/mojibake for anyone without one. Default is `none` -//! (today's plain text); set `icons = nerd` explicitly once your terminal font supports it. See +//! (plain text); set `icons = nerd` explicitly once your terminal font supports it. See //! [`crate::icons`] for the glyph table. use git2::Repository; @@ -112,7 +114,7 @@ pub struct RawViewConfig { pub outline_width: Option, pub outline_mode: Option, pub outline_order: Option, - pub outline_icons: Option, + pub icons: Option, pub diff_layout: Option, pub diff_zoom: Option, } @@ -220,10 +222,15 @@ impl<'repo> ReviewConfig<'repo> { self.get_view_string(View::Outline, "order") } - /// Get `workon.review.outline.icons`, raw. `None` if unset — callers apply the current - /// default ([`crate::icons::OutlineIcons::None`], CS5: no auto-detection story exists). - pub fn outline_icons(&self) -> Result, git2::Error> { - self.get_view_string(View::Outline, "icons") + /// Get `workon.review.icons`, raw. `None` if unset — callers apply the current default + /// ([`crate::icons::IconMode::None`]; no auto-detection story exists). Top-level like + /// `theme`, not a view setting: icon mode gates the outline, summary panel, AND winbar. + pub fn icons(&self) -> Result, git2::Error> { + let config = self.repo.config()?; + match config.get_string("workon.review.icons") { + Ok(val) => Ok(Some(val)), + Err(_) => Ok(None), + } } /// Get `workon.review.diff.layout`, raw. `None` if unset. @@ -248,7 +255,7 @@ impl<'repo> ReviewConfig<'repo> { outline_width: self.outline_width().ok().flatten(), outline_mode: self.outline_mode().ok().flatten(), outline_order: self.outline_order().ok().flatten(), - outline_icons: self.outline_icons().ok().flatten(), + icons: self.icons().ok().flatten(), diff_layout: self.diff_layout().ok().flatten(), diff_zoom: self.diff_zoom().ok().flatten(), } @@ -433,7 +440,7 @@ mod tests { .config("workon.review.outline.width", "40") .config("workon.review.outline.mode", "tree") .config("workon.review.outline.order", "base-first") - .config("workon.review.outline.icons", "nerd") + .config("workon.review.icons", "nerd") .config("workon.review.diff.layout", "split") .config("workon.review.diff.zoom", "staged") .build() @@ -450,10 +457,7 @@ mod tests { config.outline_order().expect("order"), Some("base-first".to_string()) ); - assert_eq!( - config.outline_icons().expect("icons"), - Some("nerd".to_string()) - ); + assert_eq!(config.icons().expect("icons"), Some("nerd".to_string())); assert_eq!( config.diff_layout().expect("layout"), Some("split".to_string()) @@ -473,7 +477,7 @@ mod tests { assert_eq!(config.outline_width().expect("width"), None); assert_eq!(config.outline_mode().expect("mode"), None); assert_eq!(config.outline_order().expect("order"), None); - assert_eq!(config.outline_icons().expect("icons"), None); + assert_eq!(config.icons().expect("icons"), None); assert_eq!(config.diff_layout().expect("layout"), None); assert_eq!(config.diff_zoom().expect("zoom"), None); } diff --git a/git-workon-review/src/icons.rs b/git-workon-review/src/icons.rs index cd3ba7df..aa68c6c6 100644 --- a/git-workon-review/src/icons.rs +++ b/git-workon-review/src/icons.rs @@ -3,16 +3,32 @@ //! //! A terminal cannot report which font (patched with the nerd-font private-use glyphs or not) //! the user has configured, so there is NO auto-detection here or anywhere else in the crate — -//! icons are strictly opt-in via `workon.review.outline.icons = nerd` (see `config.rs`'s schema +//! icons are strictly opt-in via `workon.review.icons = nerd` (see `config.rs`'s schema //! doc block and `App::apply_view_config`). With the config left at its default (`none`), //! nothing in this module is ever called from `render.rs`. +//! +//! **Icon table (CS1 polish pass):** per-file glyphs and brand colors are looked up via the +//! [`devicons`] crate (Apache-2.0, `alexpasmantier/devicons`) rather than a hand-rolled table — +//! 597 filename+extension entries with the same filename-before-extension precedence +//! [`icon_for_path`] already followed. devicons ships separate Dark/Light color maps; the caller +//! picks one from the active [`crate::theme::Palette`] (see [`icon_for_path`]'s doc comment). +//! **Nerd-font v3 requirement:** devicons' glyphs are drawn from nerd-font v3's private-use +//! codepoints, roughly a fifth of which sit in a Unicode supplementary plane (outside the BMP). +//! The crate's own `IconMode::Nerd` glyphs picked in CS3 (status/header markers) stay +//! BMP-only for wider font compatibility, but a per-file icon from devicons may require a v3 +//! nerd-font — this is the same "no auto-detection" opt-in tradeoff as the rest of this module. +//! devicons does not cover directories (it is a per-file mapper), so [`DIR_ICON`] is still ours. + +use devicons::{icon_for_file, FileIcon, Theme as DeviconsTheme}; +use ratatui::style::Color; -/// Which of the outline's icon strategies is active — `workon.review.outline.icons` -/// (`nerd`/`none`), read once at startup by `App::apply_view_config` (CS5 mirrors CS3's -/// `OutlineOrder` plumbing exactly: `RawViewConfig` field -> `ReviewConfig` getter -> -/// `parse_outline_icons` -> warn-and-fallback in `apply_view_config` -> `OutlineState` field). +/// Which iconography strategy is active TUI-wide — `workon.review.icons` (`nerd`/`none`), +/// read once at startup by `App::apply_view_config` (`RawViewConfig` field -> `ReviewConfig` +/// getter -> `parse_icon_mode` -> warn-and-fallback in `apply_view_config` -> `App` field). +/// Top-level like the theme, not an outline setting: it gates the outline's file/dir icons, +/// the summary panel's glyphs, and the winbar's marker/diffstat/file icons alike. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -pub enum OutlineIcons { +pub enum IconMode { /// No icon glyph — today's plain `[glyph][letter] path` row (CS5's unconditional part only). #[default] None, @@ -22,40 +38,59 @@ pub enum OutlineIcons { } /// The directory-row icon (nerd-font `nf-fa-folder`, U+F07B) — used for every -/// [`crate::outline::OutlineItem::Dir`] row when [`OutlineIcons::Nerd`] is active. +/// [`crate::outline::OutlineItem::Dir`] row when [`IconMode::Nerd`] is active. devicons is a +/// per-file mapper (it has no directory entries), so this stays our own constant. pub const DIR_ICON: char = '\u{f07b}'; // nf-fa-folder -/// The fallback file icon (nerd-font `nf-fa-file`, U+F15B) for any extension not in -/// [`icon_for_path`]'s table (including extensionless files). +/// The fallback file icon (nerd-font `nf-fa-file`, U+F15B) — devicons itself falls back to a +/// generic glyph for unrecognized extensions, but [`icon_for_path`] surfaces this constant +/// instead so callers (and this module's own tests) have a stable, documented "unknown" glyph. pub const DEFAULT_ICON: char = '\u{f15b}'; // nf-fa-file -/// Look up the nerd-font glyph for `path`'s extension — small, deliberately-curated table -/// covering the languages this crate's own `highlight.rs` already bundles grammars for -/// (`lang_key_for_ext`), plus a couple of common project files. Every codepoint below is in the -/// nerd-font private-use area (`seti`/`devicons`/`fa` icon sets); unrecognized extensions and -/// extensionless files fall back to [`DEFAULT_ICON`]. -pub fn icon_for_path(path: &str) -> char { - // `Cargo.lock`/other `*.lock` files: match on the file NAME first, since "lock" isn't a - // meaningful extension-based language distinction the way the rest of the table is. - let name = path.rsplit('/').next().unwrap_or(path); - if name.ends_with(".lock") { - return '\u{f023}'; // nf-fa-lock +/// Look up the nerd-font glyph and brand color for `path`, via [`devicons::icon_for_file`]. +/// `light_background` selects devicons' Light vs Dark color map — pass +/// `crate::theme::is_light_background(palette.background)` so the icon color suits the active +/// [`crate::theme::Palette`], not necessarily the terminal's real background. +/// +/// Returns `(glyph, color)`, where `color` is `None` if devicons' hex string didn't parse (never +/// observed in practice, but handled without panicking rather than trusting an external crate's +/// string format unconditionally) — callers should fall back to their own plain foreground. +pub fn icon_for_path(path: &str, light_background: bool) -> (char, Option) { + let devicons_theme = Some(if light_background { + DeviconsTheme::Light + } else { + DeviconsTheme::Dark + }); + let mut resolved = icon_for_file(path, &devicons_theme); + if resolved.icon == FileIcon::default().icon { + // devicons' filename match is exact-case (only its EXTENSION fallback lowercases), so + // "Makefile" misses its lowercase-only "makefile" key. Mirror its extension strategy: + // exact first (some keys, e.g. "PKGBUILD"/".Xresources", exist ONLY in exact case), then + // retry with the lowercased basename. `FileIcon::default().icon` is devicons' unknown-file + // sentinel — no real table entry uses it (verified against 0.6.12's tables). + let name = path.rsplit('/').next().unwrap_or(path).to_lowercase(); + resolved = icon_for_file(name.as_str(), &devicons_theme); + } + if resolved.icon == FileIcon::default().icon { + // Still unknown: surface OUR stable fallback glyph (see `DEFAULT_ICON`) instead of + // devicons' bare `*`, which reads as a typo next to real icon glyphs. + return (DEFAULT_ICON, None); } - let ext = match name.rsplit_once('.') { - Some((_, ext)) => ext, - None => return DEFAULT_ICON, - }; - match ext { - "rs" => '\u{e7a8}', // seti-rust - "lua" => '\u{e620}', // seti-lua - "js" | "mjs" | "cjs" => '\u{e74e}', // seti-javascript - "jsx" | "tsx" => '\u{e7ba}', // seti-react - "ts" | "mts" | "cts" => '\u{e628}', // seti-typescript - "json" => '\u{e60b}', // seti-json - "toml" => '\u{e6b2}', // seti-config (toml has no dedicated seti glyph) - "md" | "markdown" => '\u{e73e}', // seti-markdown - _ => DEFAULT_ICON, + (resolved.icon, parse_hex_color(resolved.color)) +} + +/// Parse a `"#rrggbb"` hex string (devicons' [`FileIcon::color`] format) into a +/// [`ratatui::style::Color::Rgb`]. Returns `None` on anything malformed rather than panicking — +/// this is data from an external crate, not a value this codebase controls. +fn parse_hex_color(hex: &str) -> Option { + let hex = hex.strip_prefix('#')?; + if hex.len() != 6 { + return None; } + let r = u8::from_str_radix(&hex[0..2], 16).ok()?; + let g = u8::from_str_radix(&hex[2..4], 16).ok()?; + let b = u8::from_str_radix(&hex[4..6], 16).ok()?; + Some(Color::Rgb(r, g, b)) } #[cfg(test)] @@ -63,29 +98,80 @@ mod tests { use super::*; #[test] - fn known_extensions_map_to_their_glyphs() { - assert_eq!(icon_for_path("src/main.rs"), '\u{e7a8}'); - assert_eq!(icon_for_path("scripts/init.lua"), '\u{e620}'); - assert_eq!(icon_for_path("index.js"), '\u{e74e}'); - assert_eq!(icon_for_path("app.mjs"), '\u{e74e}'); - assert_eq!(icon_for_path("component.tsx"), '\u{e7ba}'); - assert_eq!(icon_for_path("component.jsx"), '\u{e7ba}'); - assert_eq!(icon_for_path("types.ts"), '\u{e628}'); - assert_eq!(icon_for_path("package.json"), '\u{e60b}'); - assert_eq!(icon_for_path("Cargo.toml"), '\u{e6b2}'); - assert_eq!(icon_for_path("README.md"), '\u{e73e}'); + fn known_extensions_map_to_devicons_glyphs() { + // Pinned against devicons 0.6.12's actual table — these WILL need updating if the crate + // revises its glyph picks; that's an intentional pin, not a bug. + assert_eq!(icon_for_path("src/main.rs", false).0, '\u{e68b}'); + assert_eq!(icon_for_path("index.js", false).0, '\u{e60c}'); + assert_eq!(icon_for_path("component.tsx", false).0, '\u{e7ba}'); + assert_eq!(icon_for_path("types.ts", false).0, '\u{e628}'); + assert_eq!(icon_for_path("package.json", false).0, '\u{e71e}'); + assert_eq!(icon_for_path("Cargo.toml", false).0, '\u{e6b2}'); + assert_eq!(icon_for_path("README.md", false).0, '\u{f48a}'); + } + + #[test] + fn filename_precedence_matches_our_previous_lock_file_handling() { + // devicons matches on filename before extension, same shape as the old hand-rolled table. + assert_eq!(icon_for_path("Cargo.lock", false).0, '\u{e672}'); + assert_eq!(icon_for_path("nested/dir/yarn.lock", false).0, '\u{e672}'); + } + + #[test] + fn newly_covered_project_files_resolve_to_devicons_glyphs() { + // Names our old 8-entry table didn't cover — devicons' broader table does. + assert_eq!(icon_for_path("Makefile", false).0, '\u{e779}'); + assert_eq!(icon_for_path("Dockerfile", false).0, '\u{f0868}'); + assert_eq!(icon_for_path(".gitignore", false).0, '\u{e702}'); + } + + #[test] + fn filename_match_retries_lowercased_when_the_exact_case_misses() { + // devicons' own filename lookup is exact-case; its table has "makefile" but no + // "Makefile", so without our lowercased retry the standard spelling would fall through + // to the unknown-file fallback. + assert_eq!( + icon_for_path("Makefile", false), + icon_for_path("makefile", false) + ); + assert_eq!(icon_for_path("nested/dir/LICENSE", false).0, '\u{e60a}'); + } + + #[test] + fn unknown_files_fall_back_to_our_default_icon_not_devicons_asterisk() { + // devicons returns a literal '*' for unknown files; `icon_for_path` surfaces our stable + // DEFAULT_ICON (with no color) instead — see `DEFAULT_ICON`'s doc comment. + assert_eq!( + icon_for_path("file.zzznotreal", false), + (DEFAULT_ICON, None) + ); + assert_eq!(icon_for_path("noextension", false), (DEFAULT_ICON, None)); + } + + #[test] + fn colors_differ_between_dark_and_light_themes_for_a_branded_extension() { + let (_, dark_color) = icon_for_path("src/main.rs", false); + let (_, light_color) = icon_for_path("src/main.rs", true); + assert!(dark_color.is_some()); + assert!(light_color.is_some()); } #[test] - fn lock_files_match_on_name_not_extension() { - assert_eq!(icon_for_path("Cargo.lock"), '\u{f023}'); - assert_eq!(icon_for_path("nested/dir/yarn.lock"), '\u{f023}'); + fn parse_hex_color_accepts_well_formed_rrggbb() { + assert_eq!( + parse_hex_color("#a074c4"), + Some(Color::Rgb(0xa0, 0x74, 0xc4)) + ); + assert_eq!(parse_hex_color("#000000"), Some(Color::Rgb(0, 0, 0))); + assert_eq!(parse_hex_color("#ffffff"), Some(Color::Rgb(255, 255, 255))); } #[test] - fn unknown_and_extensionless_paths_fall_back_to_the_default_icon() { - assert_eq!(icon_for_path("Makefile"), DEFAULT_ICON); - assert_eq!(icon_for_path("script.sh"), DEFAULT_ICON); - assert_eq!(icon_for_path("noextension"), DEFAULT_ICON); + fn parse_hex_color_rejects_malformed_input_without_panicking() { + assert_eq!(parse_hex_color("a074c4"), None); // missing '#' + assert_eq!(parse_hex_color("#a074c"), None); // too short + assert_eq!(parse_hex_color("#a074c400"), None); // too long + assert_eq!(parse_hex_color("#zzzzzz"), None); // not hex digits + assert_eq!(parse_hex_color(""), None); } } diff --git a/git-workon-review/src/keymap.rs b/git-workon-review/src/keymap.rs index 10d1dcd3..d42b803f 100644 --- a/git-workon-review/src/keymap.rs +++ b/git-workon-review/src/keymap.rs @@ -25,6 +25,7 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use crate::config::{RawBinding, View}; +use crate::outline::OutlineMode; /// One rebindable action. The action *identity* — distinct from `tui.rs`'s `Action`, which is the /// concrete effect applied to the `App` (and carries runtime data like a half-page scroll delta @@ -64,6 +65,8 @@ pub enum Command { PrevChangeset, ExpandGap, ExpandGapAll, + HscrollLeft, + HscrollRight, // Diff view. FocusOutline, // Outline view. @@ -76,6 +79,12 @@ pub enum Command { OutlineBottom, OutlineStage, OutlineDiscard, + OutlineHscrollLeft, + OutlineHscrollRight, + OutlineNextChangeset, + OutlinePrevChangeset, + OutlineCollapseAll, + OutlineExpandAll, } /// One row of the action registry: a [`Command`] with its stable config identity (`view` + @@ -273,7 +282,21 @@ pub static REGISTRY: &[Registered] = &[ view: View::Diff, name: "focus-outline", default_keys: "h left", - description: "Focus the outline", + description: "Focus the outline (pans the diff back to column 0 first, if panned)", + }, + Registered { + command: Command::HscrollLeft, + view: View::Diff, + name: "hscroll-left", + default_keys: "<", + description: "Pan the diff content left", + }, + Registered { + command: Command::HscrollRight, + view: View::Diff, + name: "hscroll-right", + default_keys: "> l right", + description: "Pan the diff content right", }, Registered { command: Command::ExpandGap, @@ -309,14 +332,15 @@ pub static REGISTRY: &[Registered] = &[ view: View::Outline, name: "open", default_keys: "enter", - description: "Jump to the selected outline entry", + description: "Jump to a file, or fold/unfold a header or directory", }, Registered { command: Command::OutlineCycleMode, view: View::Outline, name: "cycle-mode", default_keys: "i", - description: "Cycle the outline mode", + description: + "Cycle the outline mode (stack \u{25b8} stack-tree \u{25b8} flat \u{25b8} tree)", }, Registered { command: Command::FocusDiff, @@ -353,6 +377,48 @@ pub static REGISTRY: &[Registered] = &[ default_keys: "d", description: "Discard the file/directory under the cursor", }, + Registered { + command: Command::OutlineHscrollLeft, + view: View::Outline, + name: "outline-hscroll-left", + default_keys: "<", + description: "Pan the outline left", + }, + Registered { + command: Command::OutlineHscrollRight, + view: View::Outline, + name: "outline-hscroll-right", + default_keys: ">", + description: "Pan the outline right", + }, + Registered { + command: Command::OutlineNextChangeset, + view: View::Outline, + name: "outline-next-changeset", + default_keys: "n", + description: "Jump to the next changeset", + }, + Registered { + command: Command::OutlinePrevChangeset, + view: View::Outline, + name: "outline-prev-changeset", + default_keys: "p", + description: "Jump to the previous changeset", + }, + Registered { + command: Command::OutlineCollapseAll, + view: View::Outline, + name: "outline-collapse-all", + default_keys: "zM", + description: "Collapse every changeset/directory in the outline", + }, + Registered { + command: Command::OutlineExpandAll, + view: View::Outline, + name: "outline-expand-all", + default_keys: "zR", + description: "Expand every changeset/directory in the outline", + }, ]; /// One matchable key press: a [`KeyCode`] plus whether Ctrl/Alt are required. **Shift is @@ -774,9 +840,17 @@ enum HintItem { Pair(Command, Command, &'static str), } -fn render_hint_item(keymap: &Keymap, item: &HintItem) -> Option { +/// CS4 (`outline-mode-cycle`): most hint labels are the static string baked into the `HintItem`, +/// but `OutlineCycleMode`'s label shows the mode `i` would switch TO instead — computed from +/// `outline_mode` (the outline's CURRENT mode, so this is `outline_mode.cycle()`'s label). +fn render_hint_item(keymap: &Keymap, item: &HintItem, outline_mode: OutlineMode) -> Option { match item { HintItem::One(command, label) => { + let label = if *command == Command::OutlineCycleMode { + format!("\u{2192}{}", outline_mode.cycle().label()) + } else { + (*label).to_string() + }; primary_key(keymap, *command).map(|k| format!("{k} {label}")) } HintItem::Pair(down, up, label) => { @@ -805,10 +879,15 @@ const DIFF_HINTS: &[HintItem] = &[ const OUTLINE_HINTS: &[HintItem] = &[ HintItem::Pair(Command::OutlineDown, Command::OutlineUp, "move"), HintItem::One(Command::OutlineConfirm, "open"), + HintItem::Pair( + Command::OutlineNextChangeset, + Command::OutlinePrevChangeset, + "changeset", + ), HintItem::One(Command::OutlineCycleMode, "mode"), - HintItem::One(Command::ToggleOutline, "outline"), + // No `o outline` / `q quit` here: with the changeset pair the full set no longer fits 80 + // cols, and both stay discoverable in the diff footer and the help overlay. HintItem::One(Command::ToggleHelp, "help"), - HintItem::One(Command::Quit, "quit"), ]; /// Build the persistent, always-visible footer hint string for `focused` ([`View::Diff`] or @@ -816,7 +895,7 @@ const OUTLINE_HINTS: &[HintItem] = &[ /// string, so a rebind shows here too. A notice temporarily replaces this in the footer (the /// caller's job, see `render::render_footer`); an unbound curated action is simply dropped from /// the string rather than leaving a stale/wrong key visible. -pub fn footer_hint(keymap: &Keymap, focused: View) -> String { +pub fn footer_hint(keymap: &Keymap, focused: View, outline_mode: OutlineMode) -> String { let items: &[HintItem] = match focused { View::Diff => DIFF_HINTS, View::Outline => OUTLINE_HINTS, @@ -824,7 +903,7 @@ pub fn footer_hint(keymap: &Keymap, focused: View) -> String { }; items .iter() - .filter_map(|item| render_hint_item(keymap, item)) + .filter_map(|item| render_hint_item(keymap, item, outline_mode)) .collect::>() .join(" \u{b7} ") } @@ -998,6 +1077,99 @@ mod tests { ); } + // ── diff-hscroll: `hscroll-left`/`hscroll-right` registry rows ───────────── + + #[test] + fn hscroll_commands_are_registered_with_no_collision_warnings() { + let km = Keymap::defaults(); + assert!( + km.warnings().is_empty(), + "the new commands' defaults must not collide with anything: {:?}", + km.warnings() + ); + assert!( + !km.keys_for(Command::HscrollLeft).is_empty(), + "hscroll-left must resolve to at least one bound key" + ); + assert!( + !km.keys_for(Command::HscrollRight).is_empty(), + "hscroll-right must resolve to at least one bound key" + ); + } + + #[test] + fn less_than_and_greater_than_dispatch_hscroll_in_the_diff_view() { + let km = Keymap::defaults(); + assert_eq!( + feed(&km, false, &[key(KeyCode::Char('<'))]), + Dispatch::Command(Command::HscrollLeft) + ); + assert_eq!( + feed(&km, false, &[key(KeyCode::Char('>'))]), + Dispatch::Command(Command::HscrollRight) + ); + } + + /// `l`/`right` are free in the Diff view (they're only bound in the Outline view, to + /// `focus-diff`) — the handoff's locked decision #2 reuses them for `hscroll-right` there, + /// mirroring the Outline view's `l`/`right` = focus-diff. + #[test] + fn l_and_right_dispatch_hscroll_right_in_the_diff_view() { + let km = Keymap::defaults(); + assert_eq!( + feed(&km, false, &[key(KeyCode::Char('l'))]), + Dispatch::Command(Command::HscrollRight) + ); + assert_eq!( + feed(&km, false, &[key(KeyCode::Right)]), + Dispatch::Command(Command::HscrollRight) + ); + } + + #[test] + fn n_and_p_dispatch_outline_changeset_nav_with_no_collisions() { + let km = Keymap::defaults(); + assert!( + km.warnings().is_empty(), + "n/p defaults must not collide with anything: {:?}", + km.warnings() + ); + assert_eq!( + feed(&km, true, &[key(KeyCode::Char('n'))]), + Dispatch::Command(Command::OutlineNextChangeset) + ); + assert_eq!( + feed(&km, true, &[key(KeyCode::Char('p'))]), + Dispatch::Command(Command::OutlinePrevChangeset) + ); + } + + #[test] + fn z_m_and_z_r_dispatch_outline_fold_all_with_no_collisions() { + let km = Keymap::defaults(); + assert!( + km.warnings().is_empty(), + "zM/zR defaults must not collide with anything: {:?}", + km.warnings() + ); + assert_eq!( + feed( + &km, + true, + &[key(KeyCode::Char('z')), key(KeyCode::Char('M'))] + ), + Dispatch::Command(Command::OutlineCollapseAll) + ); + assert_eq!( + feed( + &km, + true, + &[key(KeyCode::Char('z')), key(KeyCode::Char('R'))] + ), + Dispatch::Command(Command::OutlineExpandAll) + ); + } + #[test] fn a_config_rebind_overrides_the_default() { let km = Keymap::from_bindings(&[RawBinding { @@ -1147,6 +1319,29 @@ mod tests { assert_eq!(outline_sections[1].title, "Outline"); } + #[test] + fn help_sections_cycle_mode_entry_spells_out_the_full_order() { + // CS4: descriptions are static `&'static str`s baked into `REGISTRY`, so the help + // overlay can't mark the CURRENT mode dynamically without a broader refactor — the + // locked fallback is a static full-order description, with the dynamic `→next` shown + // only in the footer hint (see `footer_hint_outline_cycle_label_tracks_the_current_mode`). + let km = Keymap::defaults(); + let sections = help_sections(&km, View::Outline); + let outline = §ions[1]; + let entry = outline + .entries + .iter() + .find(|e| e.description.contains("Cycle the outline mode")) + .expect("cycle-mode row present"); + assert!( + entry + .description + .contains("stack \u{25b8} stack-tree \u{25b8} flat \u{25b8} tree"), + "got: {:?}", + entry.description + ); + } + #[test] fn help_sections_skip_an_unbound_action() { let km = Keymap::from_bindings(&[RawBinding { @@ -1185,7 +1380,7 @@ mod tests { #[test] fn footer_hint_renders_the_curated_diff_entries() { let km = Keymap::defaults(); - let hint = footer_hint(&km, View::Diff); + let hint = footer_hint(&km, View::Diff, OutlineMode::default()); assert!(hint.contains("j/k move"), "got: {hint:?}"); assert!(hint.contains("s stage"), "got: {hint:?}"); assert!(hint.contains("d discard"), "got: {hint:?}"); @@ -1197,10 +1392,39 @@ mod tests { #[test] fn footer_hint_renders_the_curated_outline_entries() { let km = Keymap::defaults(); - let hint = footer_hint(&km, View::Outline); + let hint = footer_hint(&km, View::Outline, OutlineMode::Stack); assert!(hint.contains("j/k move"), "got: {hint:?}"); assert!(hint.contains("enter open"), "got: {hint:?}"); - assert!(hint.contains("i mode"), "got: {hint:?}"); + assert!(hint.contains("n/p changeset"), "got: {hint:?}"); + assert!( + hint.contains("i \u{2192}stack-tree"), + "cycling from Stack must show the next mode, StackTree; got: {hint:?}" + ); + assert!(hint.contains("? help"), "got: {hint:?}"); + assert!( + hint.chars().count() <= 80, + "the curated outline hint must fit an 80-col footer even with the longest \ + next-mode label (stack-tree); got {} chars: {hint:?}", + hint.chars().count() + ); + } + + #[test] + fn footer_hint_outline_cycle_label_tracks_the_current_mode() { + let km = Keymap::defaults(); + for (mode, next) in [ + (OutlineMode::Stack, "stack-tree"), + (OutlineMode::StackTree, "flat"), + (OutlineMode::Flat, "tree"), + (OutlineMode::Tree, "stack"), + ] { + let hint = footer_hint(&km, View::Outline, mode); + let want = format!("i \u{2192}{next}"); + assert!( + hint.contains(&want), + "mode {mode:?} should hint the NEXT mode {next:?}; got: {hint:?}" + ); + } } #[test] @@ -1210,7 +1434,7 @@ mod tests { action: "stage-hunk".to_string(), keys: "x".to_string(), }]); - let hint = footer_hint(&km, View::Diff); + let hint = footer_hint(&km, View::Diff, OutlineMode::default()); assert!(hint.contains("x stage"), "got: {hint:?}"); assert!(!hint.contains("s stage"), "got: {hint:?}"); } @@ -1222,7 +1446,7 @@ mod tests { action: "stage-hunk".to_string(), keys: String::new(), }]); - let hint = footer_hint(&km, View::Diff); + let hint = footer_hint(&km, View::Diff, OutlineMode::default()); assert!(!hint.contains("stage"), "got: {hint:?}"); // The rest of the curated set is unaffected. assert!(hint.contains("d discard"), "got: {hint:?}"); diff --git a/git-workon-review/src/outline.rs b/git-workon-review/src/outline.rs index 4f7206ef..3a34f5de 100644 --- a/git-workon-review/src/outline.rs +++ b/git-workon-review/src/outline.rs @@ -12,6 +12,15 @@ //! `crate::model::FileStatus` keeps this module's pure-data posture intact: `model.rs` is itself //! a pure data module (no `App`/`ChangesetView` dependency), so importing its plain enum doesn't //! reintroduce the `App` coupling this module was factored out to avoid. +//! +//! CS5 (`outline-fold`) also adds a second stage layered on top of [`build_items`]: collapse/ +//! expand. [`build_items`] itself stays wholly unaware of fold state (its extensive mode/dedup/ +//! guide tests below are untouched by CS5) — [`apply_fold`] takes its output and a per-row +//! collapsed predicate and returns the filtered row list plus the two extra pieces of data render/ +//! cursor logic needs (a collapsed row's hidden-file count, and a full-list -> filtered-list index +//! map for re-finding a fold-hidden target). [`fold_outline`] is the two steps composed — +//! `App::outline_items`'s single entry point (see that method's doc comment for why every +//! cursor/staging/render consumer funnels through the SAME filtered list). use std::collections::HashMap; @@ -19,7 +28,7 @@ use crate::model::FileStatus; /// Which of the outline's row-building strategies is active — cycled by `i` (only while the /// outline pane has focus; see `App::outline_cycle_mode`). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] pub enum OutlineMode { /// Every changed path across the whole stack, once each, no changeset headers. Flat, @@ -37,16 +46,27 @@ pub enum OutlineMode { } impl OutlineMode { - /// `i`'s cycle order: `Flat -> Stack -> Tree -> StackTree -> Flat`. Flat/Stack (the - /// non-trie modes) come first since they're the CS3 default pair; the trie modes follow in - /// the same flat/grouped pairing (Tree mirrors Flat's cross-stack dedup, StackTree mirrors - /// Stack's per-changeset grouping). + /// `i`'s cycle order: `Stack -> StackTree -> Flat -> Tree -> Stack` (CS4) — the default + /// [`Self::Stack`] leads, its trie sibling [`Self::StackTree`] follows immediately, then the + /// non-grouped pair [`Self::Flat`]/[`Self::Tree`] closes the loop. pub fn cycle(self) -> Self { match self { - OutlineMode::Flat => OutlineMode::Stack, - OutlineMode::Stack => OutlineMode::Tree, - OutlineMode::Tree => OutlineMode::StackTree, + OutlineMode::Stack => OutlineMode::StackTree, OutlineMode::StackTree => OutlineMode::Flat, + OutlineMode::Flat => OutlineMode::Tree, + OutlineMode::Tree => OutlineMode::Stack, + } + } + + /// The kebab-cased display name (CS4, `outline-mode-cycle`) — used by the footer's `i + /// →` hint and mirrors `App::parse_outline_mode`'s config strings (`app.rs`), so the + /// two never drift apart. + pub fn label(self) -> &'static str { + match self { + OutlineMode::Stack => "stack", + OutlineMode::StackTree => "stack-tree", + OutlineMode::Flat => "flat", + OutlineMode::Tree => "tree", } } } @@ -80,12 +100,15 @@ fn scan_order( entries } -/// A file's staged-ness for the outline's status column — a minimal indicator (locked CS3 -/// scope: NOT the prototype's X/Y two-column git-status matrix). Only meaningful for the -/// uncommitted changeset's files; a committed changeset's files always resolve to `None` -/// because their `unstaged_idx`/`staged_idx` maps are always-empty (see +/// A file's staged-ness for the outline's status column — the data model `render.rs` derives its +/// git-porcelain-style X/Y two-column status matrix from (CS3, `outline-status-xy`). Only +/// meaningful for the uncommitted changeset's files; a committed changeset's files always +/// resolve to `None` because their `unstaged_idx`/`staged_idx` maps are always-empty (see /// `DiffState::from_committed`) — the same "derive, don't special-case" collapse /// `effective_zoom` already relies on, so no committed-specific branch is needed here either. +/// `render::build_outline_line`'s File arm reads `None` as "render a committed single letter + +/// pad column" and `Unstaged`/`Staged`/`Partial` as "render the X/Y matrix" — see that fn's doc +/// comment. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum StagedStatus { /// No staged/unstaged sub-diff info for this file (a committed changeset's file, or an @@ -112,18 +135,6 @@ impl StagedStatus { (false, false) => StagedStatus::None, } } - - /// The single-character glyph the outline renders in the status column, or a blank space - /// for [`StagedStatus::None`] (keeps every file row's path starting at the same column - /// regardless of whether it carries a status). - pub fn glyph(self) -> char { - match self { - StagedStatus::None => ' ', - StagedStatus::Unstaged => '+', - StagedStatus::Staged => '\u{2713}', // ✓ - StagedStatus::Partial => '\u{25D0}', // ◐ - } - } } /// One file's outline-relevant data, as extracted from its owning changeset by @@ -146,8 +157,9 @@ pub struct OutlineFile { /// to know about [`crate::app::ChangesetView`] or `workon::Changeset` at all. #[derive(Debug, Clone)] pub struct OutlineChangeset { - /// The changeset's title, falling back to its name — same rule the winbar (render.rs) - /// already uses. + /// The changeset's display label (`crate::app::display_label` — title falling back to name, + /// with the uncommitted layer rendered as "Uncommitted changes"), the same rule the winbar + /// and summary panel use. pub label: String, /// Mirrors `workon::Changeset::current` — drives the outline's green current marker. pub current: bool, @@ -171,7 +183,7 @@ pub struct OutlineChangeset { /// nesting level from the shallowest ancestor down to the row itself, `true` meaning "this /// level is its parent's last child". Rendering uses every-element-but-the-last to decide /// whether to draw a continuing `│` or blank space at that column, and the last element to draw -/// `└─`/`├─` for the row's own connector. [`OutlineMode::Flat`]/[`OutlineMode::Stack`] rows carry +/// `╰─`/`├─` for the row's own connector (CS4 rounds the last-child corner). [`OutlineMode::Flat`]/[`OutlineMode::Stack`] rows carry /// an EMPTY `guides` — that's the signal to `render::build_outline_line` to fall back to the /// flat two-space indent instead of drawing tree connectors; a non-empty `guides` of length 1 /// means "top-level tree row" (depth 0), so emptiness and depth-0 are deliberately distinguishable. @@ -180,6 +192,10 @@ pub enum OutlineItem { /// A changeset header — emitted in [`OutlineMode::Stack`]/[`OutlineMode::StackTree`]. Header { cs_idx: usize, + /// Changeset count (CS1, `outline-header-polish`) — paired with `cs_idx` at render time + /// to draw the `[i/n]` counter (`i` = `cs_idx + 1`, base=1). Always `changesets.len()` at + /// build time, so it's the same for every `Header` row a given `build_items` call emits. + n: usize, label: String, current: bool, needs_restack: bool, @@ -190,9 +206,10 @@ pub enum OutlineItem { }, /// A directory row — only emitted in [`OutlineMode::Tree`]/[`OutlineMode::StackTree`]. Not a /// jump target: it carries no `file_idx`, so `App::outline_move_by` no-ops on it (same as - /// [`Self::Header`]) and `App::outline_confirm` also no-ops on it (CS4 decision — there's no - /// expand/collapse state to toggle, so Enter on a directory row does nothing but still - /// returns focus to the diff, matching every other confirm outcome). + /// [`Self::Header`]); `App::outline_confirm` toggles this row's fold state instead of jumping + /// (CS5, `outline-fold`) and deliberately does NOT return focus to the diff — see that + /// method's doc comment. Fold state itself lives on `App` (per-[`OutlineMode`] sets keyed by + /// [`FoldKey`]), not here — this row stays a plain data snapshot either way. Dir { name: String, /// The FULL path from the trie root (e.g. `"src/cmd"`), unlike `name` which is just the @@ -242,7 +259,11 @@ impl OutlineItem { /// (that array's own base -> head storage order never changes) regardless of `order` — only the /// ROW SEQUENCE the outline paints flips. [`build_tree`]'s de-dupe is order-independent (see its /// own doc comment), so `order` is accepted but unused there. -pub fn build_items( +/// +/// `pub(crate)` (CS5): this is the "unfiltered build" [`fold_outline`]'s doc comment refers to — +/// every outside-the-module consumer (i.e. `App`) goes through `fold_outline`/`apply_fold` +/// instead, so a fold is never accidentally bypassed by calling this directly. +pub(crate) fn build_items( changesets: &[OutlineChangeset], mode: OutlineMode, order: OutlineOrder, @@ -255,16 +276,195 @@ pub fn build_items( } } +// ── Fold (collapse/expand), CS5 `outline-fold` ────────────────────────────────── + +/// A foldable outline row's identity — the key `App`'s per-[`OutlineMode`] fold sets store. +/// [`OutlineItem::Header`] is keyed by its changeset's label PLUS its `cs_idx`; [`OutlineItem::Dir`] +/// by its full path plus, in [`OutlineMode::StackTree`], its owning changeset's `cs_idx` (`None` +/// in [`OutlineMode::Tree`], mirroring [`OutlineItem::Dir::cs_idx`]'s own `Option` — that mode's +/// single trie has no one owning changeset to key by). +/// +/// `cs_idx` is load-bearing here, not just belt-and-suspenders: a changeset's own `label` is NOT +/// guaranteed unique across a single snapshot in general (e.g. two changesets could otherwise +/// share a title), so keying by label alone would risk folding unrelated rows together the +/// moment either was toggled. `cs_idx` is still the true index into `App::changesets` (stable +/// across an ordinary refresh — only a structural stack change, e.g. a changeset added/removed, +/// shifts it), matching the same "identity survives refresh via `cs_idx`" precedent +/// [`crate::app::OutlineRowIdentity`] already relies on for staging-verb restore. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum FoldKey { + Header { label: String, cs_idx: usize }, + Dir { path: String, owner: Option }, +} + +impl FoldKey { + /// `item`'s [`FoldKey`], or `None` for a [`OutlineItem::File`] row (never foldable — it + /// carries no fold state of its own). Reads only fields the item already carries on itself + /// (`cs_idx`, `label`/`path`) — no external lookup needed. `pub(crate)`: also + /// `App::outline_toggle_fold`'s way of turning "the row under the cursor" into the key its + /// fold set is keyed by, without duplicating this match. + pub(crate) fn for_item(item: &OutlineItem) -> Option { + match item { + OutlineItem::Header { cs_idx, label, .. } => Some(FoldKey::Header { + label: label.clone(), + cs_idx: *cs_idx, + }), + OutlineItem::Dir { path, cs_idx, .. } => Some(FoldKey::Dir { + path: path.clone(), + owner: *cs_idx, + }), + OutlineItem::File { .. } => None, + } + } +} + +/// The outline's row list after CS5's fold filtering is layered on top of [`build_items`]'s raw +/// build — see [`apply_fold`]/[`fold_outline`]'s doc comments for how it's derived, and +/// `App::outline_items`'s doc comment for why this is the SINGLE choke point every cursor/ +/// staging/render consumer reads through. +#[derive(Debug, Clone)] +pub(crate) struct FoldedOutline { + /// The visible rows, in order — a subsequence of [`build_items`]'s full (unfiltered) output. + pub items: Vec, + /// Parallel to `items`: the count of hidden FILE rows (not dirs — CS5's locked "N = hidden + /// FILE rows only" rule) under a collapsed Header/Dir row. `0` for every other row, including + /// an EXPANDED Header/Dir — render reads `0` as "no marker", so an expanded row never draws + /// the trailing ` ▸ N` chevron. + pub hidden_counts: Vec, + /// Parallel to the FULL (unfiltered) [`build_items`] output, NOT to `items`: for original row + /// `i`, the index into `items`/`hidden_counts` a cursor targeting that row should land on — + /// its own filtered position if it survived filtering, or its nearest VISIBLE ancestor's if a + /// fold hides it (CS5's "lands on the collapsed ancestor without auto-expanding" rule). Used + /// by `App::sync_outline_to_current` to re-target a diff-initiated jump onto a folded row's + /// row instead of leaving the outline cursor on an arbitrary clamp. + pub visible_index: Vec, +} + +/// Filter `items` (a fresh [`build_items`] call's output) down to the rows `is_folded`'s per-mode +/// fold set leaves visible, computing each collapsed row's hidden-file marker and the full-list -> +/// filtered-list index map described on [`FoldedOutline::visible_index`]. Needs no `changesets` +/// snapshot of its own — [`FoldKey::for_item`] reads only what each item already carries on +/// itself (see that fn's doc comment on why `cs_idx`, not a label lookup, is what disambiguates). +/// +/// One linear pass with an explicit stack of "open ancestor" frames, mirroring [`emit`]'s own +/// depth-first row order: a [`OutlineItem::Header`] frame's scope is "everything up to the next +/// Header" (depth `-1`, a sentinel shallower than every real tree depth); a [`OutlineItem::Dir`] +/// frame's scope is "everything with a deeper tree `guides` prefix than its own" (its +/// [`OutlineItem::depth`]). Both close the same way: popping frames whose recorded depth is `>=` +/// the current row's depth, since a shallower-or-equal row can't be that frame's descendant. A row +/// hidden by ANY currently-open ancestor being folded is dropped from the output entirely, but a +/// hidden File row still bumps every open ancestor's running hidden-file count (even an unfolded +/// one — that count is simply never read unless the frame turns out to be folded when it's +/// popped), so a doubly-nested fold's OUTER marker still counts files hidden two levels down. +pub(crate) fn apply_fold( + items: &[OutlineItem], + is_folded: impl Fn(&FoldKey) -> bool, +) -> FoldedOutline { + struct Frame { + depth: isize, + folded: bool, + hidden: usize, + /// Index into the output `items`/`hidden_counts` this frame's OWN row landed at — `None` + /// if the frame's own row was itself hidden by a still-further-out fold (a doubly-nested + /// collapse), in which case it never got a marker to write into. + out_idx: Option, + } + + /// Write a popped frame's final hidden-file count into its own row's marker slot — only if + /// the frame is folded (an expanded frame's count is dead data, never read) and was itself + /// visible (`out_idx: Some`; a hidden frame has no marker slot to write into at all). + fn finalize(frame: Frame, hidden_counts: &mut [usize]) { + if frame.folded { + if let Some(idx) = frame.out_idx { + hidden_counts[idx] = frame.hidden; + } + } + } + + let mut stack: Vec = Vec::new(); + let mut out_items: Vec = Vec::new(); + let mut hidden_counts: Vec = Vec::new(); + let mut visible_index: Vec = Vec::with_capacity(items.len()); + + for item in items { + let depth: isize = match item { + OutlineItem::Header { .. } => -1, + OutlineItem::Dir { .. } | OutlineItem::File { .. } => item.depth() as isize, + }; + while stack.last().is_some_and(|f| f.depth >= depth) { + finalize( + stack.pop().expect("just checked the stack is non-empty"), + &mut hidden_counts, + ); + } + + let hidden = stack.iter().any(|f| f.folded); + if hidden && matches!(item, OutlineItem::File { .. }) { + for f in &mut stack { + f.hidden += 1; + } + } + + let out_idx = + if hidden { + stack.iter().rev().find_map(|f| f.out_idx).expect( + "row 0 of any build is always visible, so some open ancestor must be too", + ) + } else { + let idx = out_items.len(); + out_items.push(item.clone()); + hidden_counts.push(0); + idx + }; + visible_index.push(out_idx); + + if let OutlineItem::Header { .. } | OutlineItem::Dir { .. } = item { + let key = FoldKey::for_item(item) + .expect("just matched Header/Dir, both of which always resolve a FoldKey"); + stack.push(Frame { + depth, + folded: is_folded(&key), + hidden: 0, + out_idx: if hidden { None } else { Some(out_idx) }, + }); + } + } + while let Some(frame) = stack.pop() { + finalize(frame, &mut hidden_counts); + } + + FoldedOutline { + items: out_items, + hidden_counts, + visible_index, + } +} + +/// [`build_items`] + [`apply_fold`] composed — `App::outline_items`'s (and its private +/// `App::outline_folded` helper's) single entry point, so `app.rs` never has to import both +/// functions and remember to always pair them. +pub(crate) fn fold_outline( + changesets: &[OutlineChangeset], + mode: OutlineMode, + order: OutlineOrder, + is_folded: impl Fn(&FoldKey) -> bool, +) -> FoldedOutline { + let items = build_items(changesets, mode, order); + apply_fold(&items, is_folded) +} + /// [`OutlineMode::Stack`]: a header per changeset, then its files in order — no de-duplication, /// every changeset's own copy of a path (if touched more than once across the stack) gets its /// own row under its own header. `order` picks which end of the stack paints first; `cs_idx`/ /// `file_idx` are computed from the ORIGINAL (base -> head) enumeration before any reversal, so /// they stay true indices into `App::changesets` either way. fn build_stack(changesets: &[OutlineChangeset], order: OutlineOrder) -> Vec { + let n = changesets.len(); let mut items = Vec::new(); for (cs_idx, cs) in scan_order(changesets, order) { items.push(OutlineItem::Header { cs_idx, + n, label: cs.label.clone(), current: cs.current, needs_restack: cs.needs_restack, @@ -474,10 +674,12 @@ fn build_tree(changesets: &[OutlineChangeset]) -> Vec { /// changeset's own copy gets its own row" rule). `order` picks which end of the stack paints /// first, same as [`build_stack`]; `cs_idx`/`file_idx` stay true indices regardless. fn build_stack_tree(changesets: &[OutlineChangeset], order: OutlineOrder) -> Vec { + let n = changesets.len(); let mut items = Vec::new(); for (cs_idx, cs) in scan_order(changesets, order) { items.push(OutlineItem::Header { cs_idx, + n, label: cs.label.clone(), current: cs.current, needs_restack: cs.needs_restack, @@ -578,6 +780,7 @@ mod tests { vec![ OutlineItem::Header { cs_idx: 0, + n: 2, label: "cs-a".to_string(), current: false, needs_restack: false, @@ -594,6 +797,7 @@ mod tests { }, OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-b".to_string(), current: true, needs_restack: true, @@ -626,6 +830,7 @@ mod tests { vec![ OutlineItem::Header { cs_idx: 0, + n: 2, label: "cs-pending".to_string(), current: false, needs_restack: false, @@ -634,6 +839,7 @@ mod tests { }, OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-failed".to_string(), current: false, needs_restack: false, @@ -784,10 +990,10 @@ mod tests { #[test] fn mode_cycle_round_trips_all_four_modes() { - assert_eq!(OutlineMode::Flat.cycle(), OutlineMode::Stack); - assert_eq!(OutlineMode::Stack.cycle(), OutlineMode::Tree); - assert_eq!(OutlineMode::Tree.cycle(), OutlineMode::StackTree); + assert_eq!(OutlineMode::Stack.cycle(), OutlineMode::StackTree); assert_eq!(OutlineMode::StackTree.cycle(), OutlineMode::Flat); + assert_eq!(OutlineMode::Flat.cycle(), OutlineMode::Tree); + assert_eq!(OutlineMode::Tree.cycle(), OutlineMode::Stack); } /// Deep-path fixture used by the tree-mode tests: a top-level file, a top-level directory @@ -901,6 +1107,7 @@ mod tests { vec![ OutlineItem::Header { cs_idx: 0, + n: 2, label: "cs-a".to_string(), current: false, needs_restack: false, @@ -923,6 +1130,7 @@ mod tests { }, OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-b".to_string(), current: true, needs_restack: true, @@ -958,6 +1166,7 @@ mod tests { items[0], OutlineItem::Header { cs_idx: 2, + n: 3, label: "cs-c".to_string(), current: true, needs_restack: false, @@ -1012,6 +1221,7 @@ mod tests { items[0], OutlineItem::Header { cs_idx: 1, + n: 2, label: "cs-b".to_string(), current: true, needs_restack: false, @@ -1033,4 +1243,246 @@ mod tests { "cs-b's own file follows immediately under its head-first header" ); } + + // ── Fold (collapse/expand), CS5 `outline-fold` ────────────────────────────── + + #[test] + fn apply_fold_with_nothing_folded_leaves_every_row_visible_with_zero_markers() { + let changesets = vec![ + cs("cs-a", false, false, &[("a1.txt", StagedStatus::None)]), + cs("cs-b", true, true, &[("b1.txt", StagedStatus::None)]), + ]; + let items = build_items(&changesets, OutlineMode::Stack, OutlineOrder::BaseFirst); + let folded = apply_fold(&items, |_| false); + assert_eq!( + folded.items, items, + "nothing folded, so nothing is filtered" + ); + assert!( + folded.hidden_counts.iter().all(|&n| n == 0), + "no collapsed row, so no marker anywhere" + ); + assert_eq!( + folded.visible_index, + (0..items.len()).collect::>(), + "every row maps onto its own (only) position" + ); + } + + #[test] + fn apply_fold_hides_a_collapsed_headers_files_and_marks_the_hidden_count() { + let changesets = vec![ + cs( + "cs-a", + false, + false, + &[ + ("a1.txt", StagedStatus::None), + ("a2.txt", StagedStatus::None), + ], + ), + cs("cs-b", true, false, &[("b1.txt", StagedStatus::None)]), + ]; + let items = build_items(&changesets, OutlineMode::Stack, OutlineOrder::BaseFirst); + let folded = apply_fold(&items, |key| { + *key == FoldKey::Header { + label: "cs-a".to_string(), + cs_idx: 0, + } + }); + assert_eq!( + folded.items.len(), + 3, + "cs-a's header survives (its 2 files hidden); cs-b's header AND its own file both \ + survive (cs-b isn't folded)" + ); + assert!(matches!( + folded.items[0], + OutlineItem::Header { ref label, .. } if label == "cs-a" + )); + assert_eq!( + folded.hidden_counts[0], 2, + "cs-a's collapsed header marks its 2 hidden files" + ); + assert!(matches!( + folded.items[1], + OutlineItem::Header { ref label, .. } if label == "cs-b" + )); + assert_eq!(folded.hidden_counts[1], 0, "cs-b is not collapsed"); + assert!(matches!( + folded.items[2], + OutlineItem::File { ref path, .. } if path == "b1.txt" + )); + } + + #[test] + fn apply_fold_leaves_a_sibling_headers_files_untouched() { + let changesets = vec![ + cs("cs-a", false, false, &[("a1.txt", StagedStatus::None)]), + cs("cs-b", true, false, &[("b1.txt", StagedStatus::None)]), + ]; + let items = build_items(&changesets, OutlineMode::Stack, OutlineOrder::BaseFirst); + let folded = apply_fold(&items, |key| { + *key == FoldKey::Header { + label: "cs-a".to_string(), + cs_idx: 0, + } + }); + let paths: Vec<&str> = folded + .items + .iter() + .filter_map(|it| match it { + OutlineItem::File { path, .. } => Some(path.as_str()), + _ => None, + }) + .collect(); + assert_eq!( + paths, + vec!["b1.txt"], + "cs-b's own file stays visible; only cs-a's collapsed section is hidden" + ); + } + + #[test] + fn apply_fold_collapsing_a_dir_hides_its_nested_files_and_subdirs_but_counts_only_files() { + let changesets = vec![deep_path_changeset("cs-a", true, false)]; + let items = build_items(&changesets, OutlineMode::Tree, OutlineOrder::HeadFirst); + // `src` (depth 0) contains `src/a` (a nested dir, depth 1) and `src/d.rs`, and `src/a` + // itself contains `src/a/b.rs` + `src/a/c.rs` — collapsing `src` should hide all 3 files + // (b.rs, c.rs, d.rs) it contains at any depth, but the marker counts files only, not the + // nested `src/a` dir row itself. + let folded = apply_fold(&items, |key| { + *key == FoldKey::Dir { + path: "src".to_string(), + owner: None, + } + }); + assert_eq!( + folded.items.len(), + 2, + "src/ (collapsed) and top.rs (an unrelated sibling) survive" + ); + let src_idx = folded + .items + .iter() + .position(|it| matches!(it, OutlineItem::Dir { name, .. } if name == "src")) + .expect("src/ row survives collapsed"); + assert_eq!( + folded.hidden_counts[src_idx], 3, + "b.rs, c.rs, and d.rs are all hidden under collapsed src/ — src/a/ itself doesn't count" + ); + } + + #[test] + fn apply_fold_doubly_nested_collapse_still_counts_toward_the_outer_markers_hidden_files() { + let changesets = vec![deep_path_changeset("cs-a", true, false)]; + let items = build_items(&changesets, OutlineMode::Tree, OutlineOrder::HeadFirst); + // Collapse BOTH `src` and its nested `src/a` — `src/a`'s own row is hidden (nested inside + // the already-collapsed `src`), but its 2 files must still count toward `src`'s own + // marker, even though `src/a`'s marker is never written (it has no visible row to write + // into). + let folded = apply_fold(&items, |key| { + matches!( + key, + FoldKey::Dir { path, owner: None } if path == "src" || path == "src/a" + ) + }); + assert_eq!( + folded.items.len(), + 2, + "only src/ (collapsed) and top.rs survive; src/a/ is hidden under src/'s own fold" + ); + let src_idx = folded + .items + .iter() + .position(|it| matches!(it, OutlineItem::Dir { name, .. } if name == "src")) + .expect("src/ row survives collapsed"); + assert_eq!( + folded.hidden_counts[src_idx], 3, + "src/'s marker still counts all 3 descendant files, even the 2 nested two levels down \ + under the also-collapsed (and therefore invisible) src/a/" + ); + } + + #[test] + fn apply_fold_visible_index_maps_a_hidden_files_full_list_position_to_its_visible_ancestor() { + // `deep_path_changeset`'s full (unfolded) Tree-mode row order is exactly: + // [src/ (0), src/a/ (1), src/a/b.rs (2), src/a/c.rs (3), src/d.rs (4), top.rs (5)] — see + // `tree_mode_builds_dirs_before_files_alpha_within_group_with_correct_depth_and_guides` + // above, which pins this same order. Collapsing `src/` hides everything at indices 1..=4 + // (all nested under it, regardless of their own depth); index 5 (`top.rs`) is a sibling, + // untouched. + let changesets = vec![deep_path_changeset("cs-a", true, false)]; + let items = build_items(&changesets, OutlineMode::Tree, OutlineOrder::HeadFirst); + let folded = apply_fold(&items, |key| { + *key == FoldKey::Dir { + path: "src".to_string(), + owner: None, + } + }); + let src_visible_idx = folded + .items + .iter() + .position(|it| matches!(it, OutlineItem::Dir { name, .. } if name == "src")) + .expect("src/ survives collapsed"); + + assert_eq!( + folded.visible_index[0], src_visible_idx, + "src/'s own row maps onto itself" + ); + for (full_idx, item) in items.iter().enumerate().take(5).skip(1) { + assert_eq!( + folded.visible_index[full_idx], src_visible_idx, + "row {full_idx} ({item:?}) is hidden under collapsed src/, so it must map onto \ + src/'s own visible row" + ); + } + let top_rs_visible_idx = folded + .items + .iter() + .position(|it| matches!(it, OutlineItem::File { path, .. } if path == "top.rs")) + .expect("top.rs survives, unaffected by src/'s fold"); + assert_eq!( + folded.visible_index[5], top_rs_visible_idx, + "top.rs (a sibling of src/, not nested under it) maps onto its own visible row" + ); + } + + #[test] + fn fold_outline_composes_build_items_and_apply_fold() { + let changesets = vec![cs( + "cs-a", + true, + false, + &[ + ("a1.txt", StagedStatus::None), + ("a2.txt", StagedStatus::None), + ], + )]; + let folded = fold_outline( + &changesets, + OutlineMode::Stack, + OutlineOrder::HeadFirst, + |key| { + *key == FoldKey::Header { + label: "cs-a".to_string(), + cs_idx: 0, + } + }, + ); + assert_eq!( + folded.items, + vec![OutlineItem::Header { + cs_idx: 0, + n: 1, + label: "cs-a".to_string(), + current: true, + needs_restack: false, + loading: false, + failed: false, + }], + "the header survives collapsed; both files are hidden" + ); + assert_eq!(folded.hidden_counts, vec![2]); + } } diff --git a/git-workon-review/src/render.rs b/git-workon-review/src/render.rs index d536cd7e..bb24d2cd 100644 --- a/git-workon-review/src/render.rs +++ b/git-workon-review/src/render.rs @@ -11,15 +11,16 @@ use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span as TSpan}; use ratatui::widgets::{Block, Borders, Clear, Paragraph}; use ratatui::Frame; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use crate::align::{CellKind, DisplayRow, InlineRow, Row}; use crate::app::{ - App, EffectiveZoom, FileView, Layout as AppLayout, Notice, Role, Severity, Summary, + App, EffectiveZoom, FileView, Layout as AppLayout, Notice, Region, Role, Severity, Summary, }; use crate::attribute::Attribution; use crate::config::View; use crate::highlight::FgSpan; -use crate::icons::OutlineIcons; +use crate::icons::IconMode; use crate::keymap::{footer_hint, help_sections, Keymap}; use crate::model::FileStatus; use crate::outline::OutlineItem; @@ -30,20 +31,136 @@ use crate::wordiff::Span as WordSpan; // The on-tint colors (diff add/del gradient + staged variants, cursor/selection washes, and syntax // foreground) come from a [`Palette`] threaded through render (ADR-035). The canvas background and // default/dim/gutter chrome foreground ALSO now come from the palette (`theme.background`/ -// `theme.foreground`/`theme.dim`/`theme.gutter`) — see the theme module's revised hybrid-boundary -// doc comment — so a curated theme fully controls the look. Only semantic chrome that is never a -// theme knob (error/warn/current-marker) stays ANSI-named / const below. - -/// Footer text color for an [`Severity::Error`] [`Notice`] — a clearly-red tone that reads on -/// both light and dark terminal themes. -const FG_ERROR: Color = Color::Rgb(220, 60, 60); -/// Warning tone for the winbar's needs-restack marker (locked decision #9) — an amber, distinct -/// from [`FG_ERROR`]'s red: a stale-parent changeset is a heads-up to `gt restack`, not a failure. -const FG_WARN: Color = Color::Rgb(214, 158, 46); -/// Tone for the outline's "this is the lib-marked `current` changeset" marker (locked decision -/// #9's outline half) — a green, distinct from every other marker color in this module so -/// "current" reads unambiguously at a glance. -const FG_CURRENT: Color = Color::Rgb(96, 200, 128); +// `theme.foreground`/`theme.dim`/`theme.gutter`), as does the semantic chrome that used to be +// const here — error/warn/current-marker are now `theme.error_fg`/`theme.warn_fg`/ +// `theme.current_fg` (CS2, revising ADR-035's hybrid boundary) — see the theme module's revised +// hybrid-boundary doc comment. A curated theme now fully controls the look; nothing in this +// module hardcodes a semantic color anymore. + +// CS3's nerd-mode status/header/summary glyphs (gated on `IconMode::Nerd`; the plain unicode +// defaults below stay byte-identical when `icons = none` — see icons.rs's module doc for why no +// auto-detection ever picks Nerd for the user). Picked from the classic BMP nerd-font sets +// (`fa`/`oct`) rather than devicons' broader (partly supplementary-plane) table, for wider +// font compatibility — see `icons.rs`'s v3 doc note. +/// Nerd-mode "this is the current changeset" marker, replacing the plain `•` (U+2022). +const NERD_CURRENT_MARKER: char = '\u{f444}'; // nf-oct-dot-fill +/// Nerd-mode needs-restack marker, replacing the plain `⚠` (U+26A0). +const NERD_WARN_MARKER: char = '\u{f071}'; // nf-fa-warning +/// Nerd-mode failed-changeset marker, replacing the plain `✗` (U+2717). +const NERD_ERROR_MARKER: char = '\u{f00d}'; // nf-fa-times +/// Nerd-mode loading marker, replacing the plain `…` (U+2026). +const NERD_LOADING_MARKER: char = '\u{f141}'; // nf-fa-ellipsis-h +/// Nerd-mode branch glyph prepended to a changeset header row's title (both the outline's Header +/// row and the summary panel's changeset title) — purely decorative (dim-colored), so it carries +/// no semantic color of its own. +const NERD_BRANCH_ICON: char = '\u{f418}'; // nf-oct-git-branch +/// Nerd-mode diffstat glyph for the summary panel's added-lines count, replacing the plain `+`. +const NERD_DIFF_ADDED: char = '\u{f457}'; // nf-oct-diff-added +/// Nerd-mode diffstat glyph for the summary panel's deleted-lines count, replacing the plain `-`. +const NERD_DIFF_REMOVED: char = '\u{f458}'; // nf-oct-diff-removed + +/// The current-changeset marker for the active icon strategy. These four one-switch helpers are +/// the single source of each semantic marker's glyph pair — the outline's Header arm and the +/// summary panel (and, upstack, the winbar) deliberately draw the SAME markers, so the selection +/// lives in one place instead of a hand-synced `match` per call site. +fn current_marker(icons: IconMode) -> char { + match icons { + IconMode::Nerd => NERD_CURRENT_MARKER, + IconMode::None => '\u{2022}', + } +} + +/// The needs-restack marker for the active icon strategy (see [`current_marker`]). +fn warn_marker(icons: IconMode) -> char { + match icons { + IconMode::Nerd => NERD_WARN_MARKER, + IconMode::None => '\u{26A0}', + } +} + +/// The failed-changeset marker for the active icon strategy (see [`current_marker`]). +fn error_marker(icons: IconMode) -> char { + match icons { + IconMode::Nerd => NERD_ERROR_MARKER, + IconMode::None => '\u{2717}', + } +} + +/// The loading marker for the active icon strategy (see [`current_marker`]). +fn loading_marker(icons: IconMode) -> char { + match icons { + IconMode::Nerd => NERD_LOADING_MARKER, + IconMode::None => '\u{2026}', + } +} + +/// The diffstat `+`/`-` prefixes for the active icon strategy (nerd: the oct diff glyphs) — +/// shared by the summary panel's totals line and any other diffstat surface. +fn diffstat_prefixes(icons: IconMode) -> (String, String) { + match icons { + IconMode::Nerd => ( + format!("{NERD_DIFF_ADDED} "), + format!("{NERD_DIFF_REMOVED} "), + ), + IconMode::None => ("+".to_string(), "-".to_string()), + } +} + +/// The shared changeset-title span run — `[current-marker] [branch-icon] ([i/n] )label +/// [warn-marker]` — drawn by both `build_outline_line`'s Header arm and +/// [`changeset_summary_lines`]. **The two call sites no longer render identically** (CS1, +/// `outline-header-polish`): `counter` is `Some((cs_idx + 1, n))` for the outline's Header row +/// only, and its presence ALSO switches the label from the plain [`Palette::foreground`] look to +/// [`Palette::heading_fg`] + bold — the summary panel passes `None` and keeps the original +/// foreground-bold label with no counter, matching its pre-CS1 appearance exactly. Failed/loading +/// markers are still NOT included: the two call sites place them differently (trailing spans on +/// the header row vs. a line of their own in the summary). +fn changeset_title_spans( + label: &str, + current: bool, + needs_restack: bool, + theme: &Palette, + icons: IconMode, + counter: Option<(usize, usize)>, +) -> Vec> { + let mut spans = Vec::new(); + if current { + spans.push(TSpan::styled( + format!("{} ", current_marker(icons)), + Style::default().fg(theme.current_fg), + )); + } + if icons == IconMode::Nerd { + spans.push(TSpan::styled( + format!("{NERD_BRANCH_ICON} "), + Style::default().fg(theme.dim), + )); + } + // The `[i/n]` counter and the accented label are outline-only (`counter.is_some()`) — see + // this fn's doc comment for why the summary panel's `None` call site is unaffected. + let label_fg = if counter.is_some() { + theme.heading_fg + } else { + theme.foreground + }; + if let Some((i, n)) = counter { + spans.push(TSpan::styled( + format!("[{i}/{n}] "), + Style::default().fg(theme.dim), + )); + } + spans.push(TSpan::styled( + label.to_string(), + Style::default().fg(label_fg).add_modifier(Modifier::BOLD), + )); + if needs_restack { + spans.push(TSpan::styled( + format!(" {}", warn_marker(icons)), + Style::default().fg(theme.warn_fg), + )); + } + spans +} /// Blend the cursor row's tint into an existing background, so the cursor highlight composites /// with (rather than replaces) del/add/word-diff emphasis on the same row — the row highlight is @@ -94,6 +211,30 @@ fn apply_selection_row(line: Line<'static>, width: u16, theme: &Palette) -> Line apply_row_tint(line, width, theme.selection_bg) } +/// Horizontal-scroll right-edge marker (decision #7): if `line` (as already blitted into `area` +/// by the caller's `set_line`) is wider than `area`'s content width, overwrite the pane's last +/// cell with a dim `…` so a panned-right line still signals there's more to the right. Applied +/// AFTER `set_line` (and after any cursor/selection wash, which paints its own background first) +/// so the marker survives on a cursor row — `Buffer::set_string`'s `Cell::set_style` only +/// overwrites `fg` when the given style sets it (leaves `bg` untouched when it doesn't, per +/// ratatui's `Style::patch` semantics), so this only ever changes the glyph + foreground, never +/// erasing the wash underneath. +fn apply_right_edge_marker( + buf: &mut Buffer, + area: Rect, + y: u16, + line: &Line<'static>, + theme: &Palette, +) { + if area.width == 0 { + return; + } + if line.width() > area.width as usize { + let x = area.x + area.width - 1; + buf.set_string(x, y, HSCROLL_MARKER, Style::default().fg(theme.dim)); + } +} + /// One resolved (bg, fg) pair for a byte range of a line. struct Segment { start: usize, @@ -245,6 +386,104 @@ enum Side { New, } +/// Horizontal-scroll left-edge marker (decision #7): replaces the first visible content column +/// whenever a line actually had content panned off to the left. Dim-styled like the gap-row/ +/// filler markers — no new color, just `theme.dim` on the existing `…` glyph. +const HSCROLL_MARKER: &str = "…"; + +/// Find the byte offset that cuts `text` at display column `col` (0 for `col == 0`), for +/// [`content_spans`]'s horizontal-scroll slicing. Column, not byte, is the unit `App::hscroll` +/// counts in, so this walks chars accumulating [`UnicodeWidthChar`] widths rather than indexing +/// `text` directly — indexing by column count would panic on a non-char-boundary byte offset for +/// any multibyte UTF-8 line. +/// +/// Returns `(byte_offset, pad)`: `pad` is `true` when a wide (2-column) char straddles the cut — +/// e.g. `col` lands mid-CJK-glyph — in which case that char is dropped entirely (skipping it +/// half-visible would misalign every column after it) and the caller should prepend a one-column +/// space to keep alignment. `col` at or beyond the line's total width returns `(text.len(), false)` +/// (nothing left to show). +fn hscroll_cut(text: &str, col: usize) -> (usize, bool) { + if col == 0 { + return (0, false); + } + let mut acc = 0usize; + for (i, c) in text.char_indices() { + if acc >= col { + return (i, false); + } + let w = UnicodeWidthChar::width(c).unwrap_or(0); + if acc + w > col { + return (i + c.len_utf8(), true); + } + acc += w; + } + (text.len(), false) +} + +/// Pan an already-built line of styled spans (diff content, or — as of the mouse/outline +/// follow-up — an outline row) `cols` display columns to the left. The shared core +/// [`content_spans`]/`render::render_outline` both build their spans at FULL width first, then +/// apply this — never the other way around — so every existing style/segment computation +/// (word-diff spans, syntax highlight, outline icon/label coloring) stays untouched by hscroll; +/// this function only ever drops or re-slices spans, never recolors one. +/// +/// Walks `spans` in order with a running column budget (`cols`, plus one extra reserved for the +/// left-edge marker below): a per-span [`hscroll_cut`] call consumes as much of that budget as +/// the span's own display width allows, carrying any remainder into the next span — exactly as +/// if `hscroll_cut` had been called once over the whole line's concatenated text, since spans +/// partition that text contiguously and in original order. A wide char straddling the cut is +/// dropped whole (never half-rendered) and compensated with a one-column space pad, same as +/// [`hscroll_cut`]'s own doc comment describes for a single string. Once the budget reaches `0`, +/// every remaining span is pushed through unchanged. +/// +/// When `cols == 0`, or the line has no content at all to cut, `spans` passes through unchanged +/// (no marker, no pad) — matching [`hscroll_cut`]'s own "nothing to show" cases. +fn pan_spans(spans: Vec>, cols: usize, theme: &Palette) -> Vec> { + if cols == 0 { + return spans; + } + if spans.iter().all(|s| s.content.is_empty()) { + return spans; + } + + // Reserve one extra column for the left-edge marker (decision #7's affordance) — mirrors the + // pre-refactor `content_spans`' own "cut at `hscroll`, then one column further for the + // marker" two-step. + let mut skip = cols + 1; + let mut out = Vec::with_capacity(spans.len() + 2); + out.push(TSpan::styled( + HSCROLL_MARKER.to_string(), + Style::default().fg(theme.dim), + )); + + for span in spans { + if skip == 0 { + out.push(span); + continue; + } + let text = span.content.as_ref(); + let (cut, straddled) = hscroll_cut(text, skip); + if straddled || cut < text.len() { + // The remaining budget was fully spent inside this span — everything from `cut` + // onward (possibly nothing) survives, unchanged in style. + skip = 0; + if straddled { + out.push(TSpan::styled( + " ".to_string(), + Style::default().fg(theme.foreground), + )); + } + if cut < text.len() { + out.push(TSpan::styled(text[cut..].to_string(), span.style)); + } + } else { + // The whole span fit inside the remaining budget — drop it and keep consuming. + skip = skip.saturating_sub(UnicodeWidthStr::width(text)); + } + } + out +} + /// Build the styled content spans (everything after the gutter) for one line of text, shared by /// [`build_pane_line`] (SBS) and [`build_inline_line`] (inline) — the two differ only in how they /// resolve `text`/`hl`/`emphasis` from a [`Row`] vs an [`InlineRow`] and in their gutter, not in @@ -253,6 +492,11 @@ enum Side { /// `emphasis` is `Some((subtle, strong))` for a `Del`/`Add` line (whole-line subtle background, /// plus per-`word_spans` strong background when `is_word_pair`; whole-line strong when not paired /// — an unpaired excess line) and `None` for `Context`/`Filler` (no background emphasis at all). +/// +/// `hscroll` (display columns, [`App::hscroll`]) pans the returned spans via [`pan_spans`] — the +/// segments below are always composed over the FULL, unsliced `text` first (byte-identical to the +/// pre-hscroll behavior), and [`pan_spans`] applies the cut/pad/marker afterward. +#[allow(clippy::too_many_arguments)] fn content_spans( text: &str, hl: Option<&Vec>, @@ -260,6 +504,7 @@ fn content_spans( word_spans: &[WordSpan], is_word_pair: bool, theme: &Palette, + hscroll: usize, ) -> Vec> { let mut bg_spans: Vec<(usize, usize, Color)> = Vec::new(); if let Some((subtle_bg, strong_bg)) = emphasis { @@ -289,7 +534,7 @@ fn content_spans( } spans.push(TSpan::styled(text[seg.start..seg.end].to_string(), style)); } - spans + pan_spans(spans, hscroll, theme) } /// Build a single rendered line for one pane at a display row's resolved [`Row`]/[`CellKind`]. @@ -305,6 +550,7 @@ fn build_pane_line( gutter_w: usize, content_w: usize, theme: &Palette, + hscroll: usize, ) -> Line<'static> { match row { Row::Filler => { @@ -337,6 +583,7 @@ fn build_pane_line( word_spans, is_word_pair, theme, + hscroll, )); Line::from(spans) } @@ -353,6 +600,11 @@ fn build_pane_line( pub fn render(frame: &mut Frame, app: &mut App, keymap: &Keymap, theme: &Palette) { let area = frame.area(); + // CS10: reset every recorded hit region at the start of the frame — a region only survives + // this frame if one of the panes below actually painted it again. Prevents a stale rect from + // an earlier frame's layout (e.g. the outline just closed) from staying hit-testable. + app.hit_regions = Default::default(); + // Paint the whole screen with the theme's background FIRST — a curated theme (light/dark) // controls the canvas outright; `auto` leaves `paint_canvas` false so the terminal's own // background (and any transparency) shows through instead. Everything drawn below only sets @@ -410,6 +662,17 @@ pub fn render(frame: &mut Frame, app: &mut App, keymap: &Keymap, theme: &Palette } } +/// Convert a ratatui [`Rect`] into the [`Region`] shape [`App::hit_regions`] stores (CS10) — +/// `app.rs` has no ratatui dependency, so every write into `hit_regions` goes through this. +fn region_from(area: Rect) -> Region { + Region { + x: area.x, + y: area.y, + w: area.width, + h: area.height, + } +} + /// Compute a centered `percent_x` × `percent_y` sub-rect of `area` — the standard ratatui popup /// pattern (two nested percentage splits). fn centered_rect(percent_x: u16, percent_y: u16, area: Rect) -> Rect { @@ -469,11 +732,18 @@ fn render_help_overlay(frame: &mut Frame, app: &App, keymap: &Keymap, area: Rect } /// Render the outline side pane's rows into `area`: [`OutlineItem::Header`]s (Stack mode only) -/// carry the changeset's position marker (green ● for `cs.current`) and needs-restack glyph -/// (amber ⚠, [`FG_WARN`] — locked decision #9's outline half); [`OutlineItem::File`]s carry an -/// indent, a one-character staged-ness glyph (blank for a committed changeset's files — see -/// [`crate::outline::StagedStatus`]'s doc comment for why no special-casing is needed here), and -/// the path. The cursor row (the outline's OWN cursor — a separate coordinate space from the +/// carry the changeset's position marker (green • for `cs.current`), a `[i/n]` TRUE-stack-position +/// counter, an accented ([`Palette::heading_fg`]) bold label (CS1, `outline-header-polish` — see +/// [`changeset_title_spans`]'s doc comment), and needs-restack glyph (amber ⚠, +/// [`crate::theme::Palette::warn_fg`] — locked decision #9's outline half); [`OutlineItem::File`]s carry an +/// indent, a two-column git-porcelain-style status matrix (CS3, `outline-status-xy` — see +/// [`outline_status_spans`]'s doc comment for the X/Y-vs-single-letter split), and +/// the path — Flat/Stack rows (CS2) split it into `basename dim/dirname` (no suffix for a +/// root-level file); Tree/StackTree rows already carry the directory via ancestor Dir rows, so +/// `path` there is just the bare basename. A COLLAPSED [`OutlineItem::Header`]/[`OutlineItem::Dir`] +/// row (CS5, `outline-fold`) additionally carries a trailing dim ` ▸ N` (`N` = hidden FILE rows +/// only), from [`App::outline_items_with_hidden_counts`]'s per-row marker count — an expanded row +/// gets no chevron at all. The cursor row (the outline's OWN cursor — a separate coordinate space from the /// diff's [`App::cursor`]) gets the theme's cursor tint while the outline has focus, or the dimmer /// [`Palette::outline_cursor_unfocused_bg`] while it's merely open (so the remembered position stays /// legible even after focus returns to the diff). `&mut App` (CS2, precedent: [`render_body`] @@ -483,13 +753,28 @@ fn render_help_overlay(frame: &mut Frame, app: &App, keymap: &Keymap, area: Rect /// transient bottom-anchor scroll computed fresh each frame. fn render_outline(frame: &mut Frame, app: &mut App, area: Rect, theme: &Palette) { app.outline_height = area.height as usize; - let items = app.outline_items(); - app.derive_outline_scroll(items.len()); + app.hit_regions.outline = Some(region_from(area)); + let (items, hidden_counts) = app.outline_items_with_hidden_counts(); + // Bounds-clamp only — NOT a cursor-following derive: under the wheel's peek model a + // scrolled-away viewport must survive the frame; cursor ops re-derive on their own. + app.clamp_outline_scroll(items.len()); let cursor = app.outline_cursor(); let focused = app.outline_focused(); let scroll = app.outline_scroll(); - let icons = app.outline_icons(); + let icons = app.icon_mode(); + + // Render-side upper clamp of the outline's own pan offset (mirroring `clamp_outline_scroll` + // just above) — from EVERY item's built line width, not just the visible rows: outlines are + // small (file trees, not file contents), so re-measuring the whole thing here is cheap. + let max_line_width = items + .iter() + .zip(&hidden_counts) + .map(|(item, &hidden)| build_outline_line(item, theme, icons, hidden).width()) + .max() + .unwrap_or(0); + app.clamp_outline_hscroll(max_line_width); + let hscroll = app.outline_hscroll(); let buf = frame.buffer_mut(); for row in 0..area.height { @@ -498,8 +783,10 @@ fn render_outline(frame: &mut Frame, app: &mut App, area: Rect, theme: &Palette) let Some(item) = items.get(item_idx) else { continue; }; + let hidden = hidden_counts.get(item_idx).copied().unwrap_or(0); let is_cursor = item_idx == cursor; - let line = build_outline_line(item, theme, icons); + let line = build_outline_line(item, theme, icons, hidden); + let line = Line::from(pan_spans(line.spans, hscroll, theme)); let line = if is_cursor && focused { apply_cursor_row(line, area.width, theme) } else if is_cursor { @@ -508,93 +795,188 @@ fn render_outline(frame: &mut Frame, app: &mut App, area: Rect, theme: &Palette) line }; buf.set_line(area.x, y, &line, area.width); + apply_right_edge_marker(buf, area, y, &line, theme); } } /// Render a tree-guide prefix from an [`OutlineItem::Dir`]/[`OutlineItem::File`] `guides` /// vector: every element but the last draws a continuing `│` (if that ancestor level was NOT /// its parent's last child) or blank space (if it was), and the last element draws the row's own -/// `└─`/`├─` connector. +/// `╰─`/`├─` connector — CS4 rounds the last-child corner (`╰`, U+2570) from the square `└` +/// (U+2514); there's no widely-supported rounded "tee" glyph, so the non-last `├─` connector is +/// unchanged. CS2 tightens indent to 2 cols/level: continuation is `│ ` (bar + space, no third +/// column), and connectors (`├─`/`╰─`) carry no trailing space — the glyph that follows hugs the +/// connector directly. fn tree_prefix(guides: &[bool]) -> String { let mut s = String::new(); let Some((&is_last, ancestors)) = guides.split_last() else { return s; }; for &last in ancestors { - s.push_str(if last { " " } else { "\u{2502} " }); + s.push_str(if last { " " } else { "\u{2502} " }); } s.push_str(if is_last { - "\u{2514}\u{2500} " + "\u{2570}\u{2500}" } else { - "\u{251C}\u{2500} " + "\u{251C}\u{2500}" }); s } -/// The [`FileStatus`] change-letter's foreground color (CS5): a create-like status (Added/ -/// Untracked) reuses the theme's `add_strong` tint, a destroy-like status (Deleted) reuses -/// `del_strong`, and everything else (Modified/Renamed/Copied/Unmerged — a change to EXISTING -/// content, not a create/destroy) gets the theme's neutral `foreground`. No new [`Palette`] -/// fields — this is deliberately just a remap of tints CS4's summary rows already use. -fn change_letter_color(change: FileStatus, theme: &Palette) -> Color { +/// Placeholder glyph for an empty XY status column (CS3, `outline-status-xy`) — U+00B7 middle +/// dot, always `theme.dim`, standing in for "nothing to report on this axis." Deliberately not a +/// space: the two-column matrix should read as a grid even when one side is empty, not look like +/// a ragged single-letter row. +const STATUS_PLACEHOLDER: char = '\u{b7}'; + +/// A committed changeset's single-letter status color (CS3): A green (`add_strong`), D red +/// (`del_strong`), M/R/C (a change to EXISTING content, not a create/destroy) the dedicated amber +/// [`Palette::modified_fg`], and `?`/`U` dim (Untracked never reaches here — see +/// [`outline_status_spans`]'s doc comment — and Unmerged is a worktree-only conflict state a +/// committed changeset can't carry; both fold to `dim` only so this match stays exhaustive). +fn committed_letter_color(change: FileStatus, theme: &Palette) -> Color { match change { - FileStatus::Added | FileStatus::Untracked => theme.add_strong, + FileStatus::Added => theme.add_strong, FileStatus::Deleted => theme.del_strong, - FileStatus::Modified | FileStatus::Renamed | FileStatus::Copied | FileStatus::Unmerged => { - theme.foreground + FileStatus::Modified | FileStatus::Renamed | FileStatus::Copied => theme.modified_fg, + FileStatus::Untracked | FileStatus::Unmerged => theme.dim, + } +} + +/// Build a file row's two-column status matrix (CS3, `outline-status-xy`) — always exactly 2 +/// [`TSpan`]s' worth of display columns, in every mode, so committed and uncommitted rows stay +/// aligned (the changeset's Gotcha). +/// +/// - `change == FileStatus::Untracked` wins over everything else and renders a dim `??` — noise, +/// not danger, regardless of `status` (see [`crate::outline::StagedStatus`]'s doc comment: an +/// untracked worktree file is always `Unstaged`, but git's own convention for untracked is `??`, +/// not a staged-ness-derived letter). +/// - `StagedStatus::None` is the committed-changeset case (see that type's doc comment for why no +/// special-casing is needed to detect it): a single [`FileStatus::letter`] colored by +/// [`committed_letter_color`], plus a blank pad column. +/// - `Unstaged`/`Staged`/`Partial` render the git-porcelain X/Y matrix: `letter` (from the SAME +/// underlying [`FileStatus`] — there's only one change kind per file, not separate staged/ +/// unstaged kinds) in whichever column(s) that axis has a change, [`STATUS_PLACEHOLDER`] in the +/// other; X (staged/index) is `add_strong` green, Y (worktree) is `del_strong` red, matching +/// git's own status convention. +fn outline_status_spans( + status: crate::outline::StagedStatus, + change: FileStatus, + theme: &Palette, +) -> Vec> { + use crate::outline::StagedStatus; + + if change == FileStatus::Untracked { + return vec![TSpan::styled( + "??".to_string(), + Style::default().fg(theme.dim), + )]; + } + match status { + StagedStatus::None => { + let letter = change.letter(); + vec![ + TSpan::styled( + letter.to_string(), + Style::default().fg(committed_letter_color(change, theme)), + ), + TSpan::styled(" ".to_string(), Style::default().fg(theme.foreground)), + ] + } + StagedStatus::Unstaged | StagedStatus::Staged | StagedStatus::Partial => { + let letter = change.letter(); + let staged = matches!(status, StagedStatus::Staged | StagedStatus::Partial); + let unstaged = matches!(status, StagedStatus::Unstaged | StagedStatus::Partial); + let x_char = if staged { letter } else { STATUS_PLACEHOLDER }; + let y_char = if unstaged { letter } else { STATUS_PLACEHOLDER }; + let x_color = if staged { theme.add_strong } else { theme.dim }; + let y_color = if unstaged { + theme.del_strong + } else { + theme.dim + }; + vec![ + TSpan::styled(x_char.to_string(), Style::default().fg(x_color)), + TSpan::styled(y_char.to_string(), Style::default().fg(y_color)), + ] } } } +/// CS5 (`outline-fold`): a collapsed Header/Dir row's trailing marker — dim ` ▸ N`, `N` being the +/// count of hidden FILE rows (not dirs) [`App::outline_items_with_hidden_counts`] attached to that +/// row. `None` for `hidden == 0` (an EXPANDED Header/Dir — or a File row, which never carries a +/// hidden count at all) — the locked "no chevron when expanded" rule reads a zero count as "don't +/// draw a marker" rather than "draw ` ▸ 0`". +fn fold_marker(hidden: usize, theme: &Palette) -> Option> { + (hidden > 0).then(|| { + TSpan::styled( + format!(" \u{25b8} {hidden}"), + Style::default().fg(theme.dim), + ) + }) +} + /// Build one outline row's rendered [`Line`] — see [`render_outline`]'s doc comment for the -/// marker rules. `icons` (CS5, `workon.review.outline.icons`) is [`OutlineIcons::None`] by +/// marker rules. `icons` (CS5, `workon.review.icons`) is [`IconMode::None`] by /// default, which reproduces the pre-CS5 row text exactly (no icon glyph, no extra space); only -/// [`OutlineIcons::Nerd`] inserts an icon before the name/path. -fn build_outline_line(item: &OutlineItem, theme: &Palette, icons: OutlineIcons) -> Line<'static> { +/// [`IconMode::Nerd`] inserts an icon before the name/path. `hidden` (CS5, `outline-fold`) is the +/// row's collapsed hidden-file count from [`App::outline_items_with_hidden_counts`] — `0` for +/// every row that isn't a collapsed Header/Dir; see [`fold_marker`]. +fn build_outline_line( + item: &OutlineItem, + theme: &Palette, + icons: IconMode, + hidden: usize, +) -> Line<'static> { match item { OutlineItem::Header { + cs_idx, + n, label, current, needs_restack, loading, failed, - .. } => { - let marker = if *current { "\u{25CF} " } else { " " }; - let mut spans = vec![TSpan::styled( - marker.to_string(), - Style::default().fg(FG_CURRENT), - )]; - spans.push(TSpan::styled( - label.clone(), - Style::default() - .fg(theme.foreground) - .add_modifier(Modifier::BOLD), - )); - if *needs_restack { - spans.push(TSpan::styled(" \u{26A0}", Style::default().fg(FG_WARN))); - } + let mut spans = changeset_title_spans( + label, + *current, + *needs_restack, + theme, + icons, + Some((cs_idx + 1, *n)), + ); // ADR-037: a Failed changeset's marker wins over Pending's (a slot is never both, // but Failed is the more actionable state to surface if it somehow were). if *failed { - spans.push(TSpan::styled(" \u{2717}", Style::default().fg(FG_ERROR))); + spans.push(TSpan::styled( + format!(" {}", error_marker(icons)), + Style::default().fg(theme.error_fg), + )); } else if *loading { - spans.push(TSpan::styled(" \u{2026}", Style::default().fg(theme.dim))); + spans.push(TSpan::styled( + format!(" {}", loading_marker(icons)), + Style::default().fg(theme.dim), + )); } + spans.extend(fold_marker(hidden, theme)); Line::from(spans) } OutlineItem::Dir { name, guides, .. } => { let icon = match icons { - OutlineIcons::Nerd => format!("{} ", crate::icons::DIR_ICON), - OutlineIcons::None => String::new(), + IconMode::Nerd => format!("{} ", crate::icons::DIR_ICON), + IconMode::None => String::new(), }; let text = format!("{}{icon}{name}/", tree_prefix(guides)); - Line::from(TSpan::styled( + let mut spans = vec![TSpan::styled( text, Style::default() .fg(theme.dim) .add_modifier(Modifier::ITALIC), - )) + )]; + spans.extend(fold_marker(hidden, theme)); + Line::from(spans) } OutlineItem::File { path, @@ -603,34 +985,70 @@ fn build_outline_line(item: &OutlineItem, theme: &Palette, icons: OutlineIcons) guides, .. } => { - let glyph = status.glyph(); - let letter = change.letter(); // Empty `guides` (Flat/Stack modes) keeps the original two-space indent; a // non-empty `guides` (Tree/StackTree modes) draws tree connectors instead — see - // `OutlineItem`'s doc comment for why emptiness is the mode signal. - let prefix = if guides.is_empty() { - " ".to_string() - } else { - tree_prefix(guides) - }; - let icon = match icons { - OutlineIcons::Nerd => format!("{} ", crate::icons::icon_for_path(path)), - OutlineIcons::None => String::new(), - }; - Line::from(vec![ - TSpan::styled( - format!("{prefix}{glyph}"), + // `OutlineItem`'s doc comment for why emptiness is the mode signal. CS4: a non-empty + // prefix (real tree connectors) gets its own `theme.dim`-styled span — matching the + // Dir row's already-dim guides — so the guide lines read as quiet structure, not part + // of the file's own status column. The status matrix itself (CS3, + // `outline_status_spans`) is always exactly 2 display columns, same width the old + // glyph+letter pair occupied, so this swap doesn't shift anything after it. + let mut spans = Vec::new(); + if guides.is_empty() { + spans.push(TSpan::styled( + " ".to_string(), Style::default().fg(theme.foreground), - ), - TSpan::styled( - letter.to_string(), - Style::default().fg(change_letter_color(*change, theme)), - ), - TSpan::styled( - format!(" {icon}{path}"), + )); + } else { + spans.push(TSpan::styled( + tree_prefix(guides), + Style::default().fg(theme.dim), + )); + } + spans.extend(outline_status_spans(*status, *change, theme)); + spans.push(TSpan::styled( + " ".to_string(), + Style::default().fg(theme.foreground), + )); + if icons == IconMode::Nerd { + let (icon, color) = crate::icons::icon_for_path( + path, + crate::theme::is_light_background(theme.background), + ); + spans.push(TSpan::styled( + format!("{icon} "), + Style::default().fg(color.unwrap_or(theme.foreground)), + )); + } + // Flat/Stack rows (empty `guides`) split `path` at render time into `basename dim/ + // dirname` — basename first (bright, matching the tree modes' bare-name leaves) so + // truncation eats the dim dirname before the name a user is scanning for (CS2 + // gotcha). Tree/StackTree rows (non-empty `guides`) already carry the path via + // ancestor Dir rows, so `path` there is already just the basename — render it as-is. + if guides.is_empty() { + match path.rsplit_once('/') { + Some((dir, base)) => { + spans.push(TSpan::styled( + base.to_string(), + Style::default().fg(theme.foreground), + )); + spans.push(TSpan::styled( + format!(" {dir}"), + Style::default().fg(theme.dim), + )); + } + None => spans.push(TSpan::styled( + path.clone(), + Style::default().fg(theme.foreground), + )), + } + } else { + spans.push(TSpan::styled( + path.clone(), Style::default().fg(theme.foreground), - ), - ]) + )); + } + Line::from(spans) } } } @@ -665,26 +1083,48 @@ fn render_header(frame: &mut Frame, app: &App, area: Rect, theme: &Palette) { let idx = app.current + 1; let n = app.files().len(); let text = format!("[{idx}/{n}] {}", current_file_label(app)); - frame.render_widget( - Paragraph::new(text).style( - Style::default() - .fg(theme.foreground) - .add_modifier(Modifier::BOLD), - ), - area, - ); + let mut spans = vec![TSpan::styled( + text, + Style::default() + .fg(theme.foreground) + .add_modifier(Modifier::BOLD), + )]; + if let Some(span) = hscroll_indicator_span(app, theme) { + spans.push(span); + } + frame.render_widget(Paragraph::new(Line::from(spans)), area); +} + +/// While [`App::hscroll`] is panned, a small dim `»42` (the column offset) appended to the header/ +/// winbar (locked decision #8) — `None` at column `0`, matching the diffstat span's own +/// present-or-absent pattern above/below. +fn hscroll_indicator_span(app: &App, theme: &Palette) -> Option> { + if app.hscroll == 0 { + return None; + } + Some(TSpan::styled( + format!(" »{}", app.hscroll), + Style::default().fg(theme.dim), + )) } /// The multi-changeset winbar (locked decisions #8 + #9): `[i/n] -/// — (fidx/nfiles)`, where `i/n` is the changeset's position in the -/// stack and `fidx/nfiles` the active file's position within it. Only reached when +/// — (fidx/nfiles)`, where `i/n` is the changeset's position +/// in the stack and `fidx/nfiles` the active file's position within it. Only reached when /// [`App::changeset_count`] > 1 (see [`render_header`]) — a lone uncommitted changeset never /// shows this, keeping the M4 full-width look. +/// +/// CS4 polish: a tight `+A -D` diffstat for the ACTIVE changeset (there wasn't one before), +/// tinted with the same [`Palette::add_strong`]/[`Palette::del_strong`] the summary panel's own +/// totals line uses; in [`IconMode::Nerd`] mode the restack marker and diffstat prefixes swap +/// to their nerd glyphs (same consts `build_outline_line`/`push_summary_body` use), and the +/// active file's path gets its devicons file icon. fn render_winbar(frame: &mut Frame, app: &App, area: Rect, theme: &Palette) { let cs = app.current_changeset(); let i = app.current_cs() + 1; let n = app.changeset_count(); - let title = cs.title.as_deref().unwrap_or(cs.name.as_str()); + let title = crate::app::display_label(cs); + let icons = app.icon_mode(); let mut spans = vec![TSpan::styled( format!("[{i}/{n}] {title}"), @@ -696,18 +1136,67 @@ fn render_winbar(frame: &mut Frame, app: &App, area: Rect, theme: &Palette) { // from the plain title so a stale-parent changeset reads as a heads-up at a glance. if cs.needs_restack { spans.push(TSpan::styled( - " ⚠ needs restack", - Style::default().fg(FG_WARN).add_modifier(Modifier::BOLD), + format!(" {} needs restack", warn_marker(icons)), + Style::default() + .fg(theme.warn_fg) + .add_modifier(Modifier::BOLD), + )); + } + // A pending/failed changeset's `files()` is always empty (ADR-037) — skip the diffstat + // segment entirely rather than show a misleading "+0 -0". + if !app.files().is_empty() { + let (adds, dels) = app + .files() + .iter() + .map(crate::summary::file_diffstat) + .fold((0, 0), |(a, d), (fa, fd)| (a + fa, d + fd)); + let (added_prefix, removed_prefix) = diffstat_prefixes(icons); + spans.push(TSpan::raw(" ")); + spans.push(TSpan::styled( + format!("{added_prefix}{adds}"), + Style::default() + .fg(theme.add_strong) + .add_modifier(Modifier::BOLD), + )); + spans.push(TSpan::raw(" ")); + spans.push(TSpan::styled( + format!("{removed_prefix}{dels}"), + Style::default() + .fg(theme.del_strong) + .add_modifier(Modifier::BOLD), )); } let fidx = app.current + 1; let nfiles = app.files().len(); spans.push(TSpan::styled( - format!(" — {} ({fidx}/{nfiles})", current_file_label(app)), + " — ".to_string(), + Style::default() + .fg(theme.foreground) + .add_modifier(Modifier::BOLD), + )); + if icons == IconMode::Nerd { + if let Some(f) = app.files().get(app.current) { + let (icon, color) = crate::icons::icon_for_path( + &f.path, + crate::theme::is_light_background(theme.background), + ); + spans.push(TSpan::styled( + format!("{icon} "), + Style::default() + .fg(color.unwrap_or(theme.foreground)) + .add_modifier(Modifier::BOLD), + )); + } + } + spans.push(TSpan::styled( + format!("{} ({fidx}/{nfiles})", current_file_label(app)), Style::default() .fg(theme.foreground) .add_modifier(Modifier::BOLD), )); + if let Some(span) = hscroll_indicator_span(app, theme) { + spans.push(span); + } frame.render_widget(Paragraph::new(Line::from(spans)), area); } @@ -718,7 +1207,7 @@ fn render_winbar(frame: &mut Frame, app: &App, area: Rect, theme: &Palette) { fn render_footer(frame: &mut Frame, app: &App, area: Rect, keymap: &Keymap, theme: &Palette) { if let Some(confirm) = &app.pending_confirm { frame.render_widget( - Paragraph::new(confirm.prompt.as_str()).style(Style::default().fg(FG_ERROR)), + Paragraph::new(confirm.prompt.as_str()).style(Style::default().fg(theme.error_fg)), area, ); return; @@ -726,7 +1215,7 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect, keymap: &Keymap, them match &app.notice { Some(Notice { text, severity }) => { let fg = match severity { - Severity::Error => FG_ERROR, + Severity::Error => theme.error_fg, Severity::Info => theme.foreground, }; frame.render_widget( @@ -744,7 +1233,7 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect, keymap: &Keymap, them } else { View::Diff }; - let text = footer_hint(keymap, focused); + let text = footer_hint(keymap, focused, app.outline_mode()); frame.render_widget( Paragraph::new(text).style(Style::default().fg(theme.dim)), area, @@ -875,11 +1364,13 @@ fn push_summary_body( total_dels: usize, height: usize, theme: &Palette, + icons: IconMode, ) { lines.push(Line::from("")); let footer_budget = 1; // the totals line always shows let file_budget = height.saturating_sub(lines.len() + footer_budget); push_summary_file_rows(lines, files, file_budget, theme); + let (added_prefix, removed_prefix) = diffstat_prefixes(icons); lines.push(Line::from(vec![ TSpan::styled( format!("{} files", files.len()), @@ -887,41 +1378,38 @@ fn push_summary_body( ), TSpan::raw(" "), TSpan::styled( - format!("+{total_adds}"), + format!("{added_prefix}{total_adds}"), Style::default().fg(theme.add_strong), ), TSpan::raw(" "), TSpan::styled( - format!("-{total_dels}"), + format!("{removed_prefix}{total_dels}"), Style::default().fg(theme.del_strong), ), ])); } -/// Build a [`ChangesetSummary`]'s lines: title line (carrying the same current/needs-restack/ -/// failed markers `build_outline_line`'s Header arm draws), a loading/failed line OR the per-file -/// list + totals line. +/// Build a [`ChangesetSummary`]'s lines: title line (the same current/needs-restack markers +/// `build_outline_line`'s Header arm draws, structurally shared via [`changeset_title_spans`] — +/// but passing `None` for that fn's `counter` param, so this title keeps its pre-CS1 plain- +/// foreground look with no `[i/n]` counter; see [`changeset_title_spans`]'s doc comment), a +/// loading/failed line OR the per-file list + totals line. fn changeset_summary_lines( summary: &ChangesetSummary, height: usize, theme: &Palette, + icons: IconMode, ) -> Vec> { let mut lines = Vec::new(); - let mut title_spans = vec![TSpan::styled( - if summary.current { "\u{25CF} " } else { " " }, - Style::default().fg(FG_CURRENT), - )]; - title_spans.push(TSpan::styled( - summary.label.clone(), - Style::default() - .fg(theme.foreground) - .add_modifier(Modifier::BOLD), - )); - if summary.needs_restack { - title_spans.push(TSpan::styled(" \u{26A0}", Style::default().fg(FG_WARN))); - } - lines.push(Line::from(title_spans)); + lines.push(Line::from(changeset_title_spans( + &summary.label, + summary.current, + summary.needs_restack, + theme, + icons, + None, + ))); if summary.failed { let msg = summary @@ -929,14 +1417,14 @@ fn changeset_summary_lines( .as_deref() .unwrap_or("(no error message)"); lines.push(Line::from(TSpan::styled( - format!("\u{2717} {msg}"), - Style::default().fg(FG_ERROR), + format!("{} {msg}", error_marker(icons)), + Style::default().fg(theme.error_fg), ))); return lines; } if summary.loading { lines.push(Line::from(TSpan::styled( - "Loading\u{2026}", + format!("Loading{}", loading_marker(icons)), Style::default().fg(theme.dim), ))); return lines; @@ -949,15 +1437,27 @@ fn changeset_summary_lines( summary.total_dels, height, theme, + icons, ); lines } /// Build a [`DirSummary`]'s lines: a bold path title, a blank line, the per-file list, and the /// totals line — no current/restack/loading/failed markers (a directory carries none of those). -fn dir_summary_lines(summary: &DirSummary, height: usize, theme: &Palette) -> Vec> { +/// The title gets [`crate::icons::DIR_ICON`] in [`IconMode::Nerd`] mode, matching the +/// outline's own [`OutlineItem::Dir`] row (`build_outline_line`). +fn dir_summary_lines( + summary: &DirSummary, + height: usize, + theme: &Palette, + icons: IconMode, +) -> Vec> { + let dir_icon = match icons { + IconMode::Nerd => format!("{} ", crate::icons::DIR_ICON), + IconMode::None => String::new(), + }; let mut lines = vec![Line::from(TSpan::styled( - format!("{}/", summary.path), + format!("{dir_icon}{}/", summary.path), Style::default() .fg(theme.foreground) .add_modifier(Modifier::BOLD), @@ -969,6 +1469,7 @@ fn dir_summary_lines(summary: &DirSummary, height: usize, theme: &Palette) -> Ve summary.total_dels, height, theme, + icons, ); lines } @@ -978,11 +1479,17 @@ fn dir_summary_lines(summary: &DirSummary, height: usize, theme: &Palette) -> Ve /// line, per-file `"path +N -M"` rows (truncated to the pane height), and a totals line. A /// loading/failed Header shows its own inline state instead of a file list (see /// [`changeset_summary_lines`]). -fn render_summary(frame: &mut Frame, summary: &Summary, area: Rect, theme: &Palette) { +fn render_summary( + frame: &mut Frame, + summary: &Summary, + area: Rect, + theme: &Palette, + icons: IconMode, +) { let height = area.height as usize; let lines = match summary { - Summary::Changeset(cs) => changeset_summary_lines(cs, height, theme), - Summary::Dir(dir) => dir_summary_lines(dir, height, theme), + Summary::Changeset(cs) => changeset_summary_lines(cs, height, theme, icons), + Summary::Dir(dir) => dir_summary_lines(dir, height, theme, icons), }; frame.render_widget(Paragraph::new(lines), area); } @@ -994,7 +1501,7 @@ fn render_body(frame: &mut Frame, app: &mut App, area: Rect, theme: &Palette) { // diff-body rendering; `summary_target` returns `None` in both cases). if let Some(target) = app.summary_target() { let summary = app.summary_for(target); - render_summary(frame, &summary, area, theme); + render_summary(frame, &summary, area, theme, app.icon_mode()); return; } // ADR-037: the active changeset's diff hasn't been acquired (or failed to acquire) yet — @@ -1004,7 +1511,7 @@ fn render_body(frame: &mut Frame, app: &mut App, area: Rect, theme: &Palette) { if let Some(message) = app.current_failure() { let msg = format!("Failed to load this changeset: {message}"); frame.render_widget( - Paragraph::new(msg).style(Style::default().fg(FG_ERROR)), + Paragraph::new(msg).style(Style::default().fg(theme.error_fg)), area, ); return; @@ -1048,6 +1555,7 @@ fn render_body(frame: &mut Frame, app: &mut App, area: Rect, theme: &Palette) { match app.effective_zoom_for(idx) { EffectiveZoom::Single(role) => { app.pane_height = area.height as usize; + app.hit_regions.single = Some(region_from(area)); let scroll = app.scroll; let cursor = Some(app.cursor); // The single pane is the focused one, so it shows any active selection. @@ -1107,8 +1615,12 @@ fn render_body_split(frame: &mut Frame, app: &mut App, area: Rect, idx: usize, t }; app.pane_height = focused_h as usize; app.alt_height = unfocused_h as usize; - app.derive_scroll(); - app.derive_alt_scroll(); + app.hit_regions.unstaged = Some(region_from(unstaged_content)); + app.hit_regions.staged = Some(region_from(staged_content)); + // Bounds-clamp only (peek model — see render_outline's identical note); this also brings + // the split arm in line with the Single arm, which never re-derived at render time. + app.clamp_scroll(); + app.clamp_alt_scroll(); render_caption(frame.buffer_mut(), unstaged_caption, "UNSTAGED", theme); render_caption(frame.buffer_mut(), staged_caption, "STAGED", theme); @@ -1208,6 +1720,9 @@ fn render_pane_sbs( let old_area = hlayout[0]; let div_area = hlayout[1]; let new_area = hlayout[2]; + // One offset shared by every content pane (locked decision #1) — read once, before any of + // the `app` borrows below. + let hscroll = app.hscroll; let Some(view) = app.role_view_ref(idx, role) else { frame.render_widget(Paragraph::new("(failed to load file)"), old_area); @@ -1283,6 +1798,7 @@ fn render_pane_sbs( old_gutter_w, old_area.width as usize, theme, + hscroll, ); let new_line = build_pane_line( view, @@ -1295,6 +1811,7 @@ fn render_pane_sbs( new_gutter_w, new_area.width as usize, theme, + hscroll, ); // Cursor wins over selection on the same row (see [`Palette::selection_bg`]). let (old_line, new_line) = if is_cursor { @@ -1316,6 +1833,12 @@ fn render_pane_sbs( frame .buffer_mut() .set_line(new_area.x, y, &new_line, new_area.width); + // Right-edge hscroll marker (decision #7) — applied AFTER `set_line` (and thus + // after the cursor/selection wash above, which already painted the background) + // so it survives on a cursor/selected row; `apply_right_edge_marker` only sets + // `fg`, leaving whatever background the wash left in place. + apply_right_edge_marker(frame.buffer_mut(), old_area, y, &old_line, theme); + apply_right_edge_marker(frame.buffer_mut(), new_area, y, &new_line, theme); // The divider column was painted once for the whole pane height above, with the // default background; re-tint just this row's divider cell so the cursor wash // covers the full width (panes AND the `│` between them), like `render_gap_row`. @@ -1348,6 +1871,7 @@ fn gutter_field(n: Option, w: usize) -> String { /// rows show only the old-side column, `Add` rows only the new-side column — the other column is /// blank rather than reused for anything, so a scan down the gutter reads as two honest, /// independent line-number tracks. +#[allow(clippy::too_many_arguments)] fn build_inline_line( view: &FileView, row: &InlineRow, @@ -1356,6 +1880,7 @@ fn build_inline_line( old_gutter_w: usize, new_gutter_w: usize, theme: &Palette, + hscroll: usize, ) -> Line<'static> { let (old_opt, new_opt, text, hl, kind) = match *row { InlineRow::Context { old, new } => ( @@ -1406,6 +1931,7 @@ fn build_inline_line( word_spans, is_word_pair, theme, + hscroll, )); Line::from(spans) } @@ -1425,6 +1951,10 @@ fn render_pane_inline( selection: Option<(usize, usize)>, theme: &Palette, ) { + // One offset shared by every content pane (locked decision #1) — read once, before any of + // the `app` borrows below. + let hscroll = app.hscroll; + let Some(view) = app.role_view_ref(idx, role) else { frame.render_widget(Paragraph::new("(failed to load file)"), area); return; @@ -1486,6 +2016,7 @@ fn render_pane_inline( old_gutter_w, new_gutter_w, theme, + hscroll, ); // Cursor wins over selection on the same row (see [`Palette::selection_bg`]). let line = if is_cursor { @@ -1496,6 +2027,9 @@ fn render_pane_inline( line }; frame.buffer_mut().set_line(area.x, y, &line, area.width); + // Right-edge hscroll marker (decision #7) — see `render_pane_sbs`'s identical + // comment on ordering relative to the cursor/selection wash above. + apply_right_edge_marker(frame.buffer_mut(), area, y, &line, theme); } } } @@ -1505,11 +2039,14 @@ fn render_pane_inline( mod tests { use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; + use ratatui::style::Style; + use ratatui::text::Span as TSpan; use ratatui::Terminal; use git_workon_fixture::prelude::*; + use unicode_width::UnicodeWidthChar; - use super::render; + use super::{hscroll_cut, pan_spans, render, STATUS_PLACEHOLDER}; use crate::align::{DisplayRow, Row}; use crate::app::test_support::app_from_fixture; use crate::app::App; @@ -2226,8 +2763,48 @@ mod tests { .map(|x| cell_text(&buf, x, footer_y)) .collect(); assert!( - footer.contains("open") && footer.contains("mode") && footer.contains("? help"), - "expected the curated outline hint string in the footer, got: {footer:?}" + footer.contains("open") + && footer.contains(&format!( + "i \u{2192}{}", + crate::outline::OutlineMode::StackTree.label() + )) + && footer.contains("? help"), + "expected the curated outline hint string, with CS4's dynamic next-mode label \ + (Stack's default -> StackTree), in the footer, got: {footer:?}" + ); + } + + #[test] + fn footer_outline_hint_next_mode_label_updates_as_the_mode_cycles() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.toggle_outline(); + assert!(app.outline_focused()); + assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Stack); + + let footer_text = |app: &mut App| { + let buf = render_once(app, 80, 10); + let footer_y = buf.area.height - 1; + (0..buf.area.width) + .map(|x| cell_text(&buf, x, footer_y)) + .collect::() + }; + + let footer = footer_text(&mut app); + assert!( + footer.contains("i \u{2192}stack-tree"), + "Stack's next mode is StackTree; got: {footer:?}" + ); + + app.outline_cycle_mode(); + assert_eq!(app.outline_mode(), crate::outline::OutlineMode::StackTree); + let footer = footer_text(&mut app); + assert!( + footer.contains("i \u{2192}flat"), + "StackTree's next mode is Flat; got: {footer:?}" ); } @@ -2290,7 +2867,7 @@ mod tests { ); assert_eq!( buf.cell((0, footer_y)).unwrap().style().fg, - Some(super::FG_ERROR), + Some(Palette::dark().error_fg), "expected the error notice to render in the error fg color" ); } @@ -2430,11 +3007,54 @@ mod tests { let marker_x = header.find('⚠').expect("restack glyph present") as u16; assert_eq!( buf.cell((marker_x, 0)).unwrap().style().fg, - Some(super::FG_WARN), + Some(Palette::dark().warn_fg), "expected the restack glyph to carry the warning color, not the plain header color" ); } + #[test] + fn winbar_shows_a_tight_diffstat_for_the_active_changeset() { + // CS4: the winbar previously showed no diffstat at all — cs-b adds a single line + // (`b.txt`, one-line file, committed with no prior content) with nothing deleted. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + + let buf = render_once(&mut app, 80, 20); + let header: String = (0..buf.area.width).map(|x| cell_text(&buf, x, 0)).collect(); + assert!( + header.contains("+1") && header.contains("-0"), + "expected a tight '+N -M' diffstat fragment for cs-b's single added file, got: {header:?}" + ); + } + + #[test] + fn winbar_nerd_mode_swaps_the_restack_marker_and_diffstat_glyphs_and_shows_a_file_icon() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); // cs-b: current + needs_restack + app.set_icon_mode(crate::icons::IconMode::Nerd); + + let buf = render_once(&mut app, 80, 20); + let header: String = (0..buf.area.width).map(|x| cell_text(&buf, x, 0)).collect(); + assert!( + header.contains(super::NERD_WARN_MARKER) && !header.contains('\u{26A0}'), + "expected the nerd restack marker, not the plain unicode one, got: {header:?}" + ); + assert!( + header.contains(super::NERD_DIFF_ADDED) && header.contains(super::NERD_DIFF_REMOVED), + "expected nerd diffstat glyphs in the winbar, got: {header:?}" + ); + assert!( + header.contains(crate::icons::icon_for_path("b.txt", false).0), + "expected the active file's (b.txt) devicons icon in the winbar, got: {header:?}" + ); + } + #[test] fn winbar_uses_title_when_present() { let fixture = FixtureBuilder::new() @@ -2554,12 +3174,234 @@ mod tests { ); } - // ── M5 CS3: outline side pane ─────────────────────────────────────────────── + // ── diff-hscroll ───────────────────────────────────────────────────────────── - /// Every outline test renders at this width so the pane's fixed 35-col + 1-col-divider - /// layout is unambiguous: columns `0..35` are the outline, `35` the divider, `36..` the - /// diff. - const OUTLINE_TEST_WIDTH: u16 = 80; + #[test] + fn hscroll_cut_ascii() { + // "hello world" — cutting at column 6 lands right after the space, before "world". + assert_eq!(hscroll_cut("hello world", 6), (6, false)); + assert_eq!(hscroll_cut("hello world", 0), (0, false)); + } + + #[test] + fn hscroll_cut_multibyte_narrow() { + // "café" — 'é' is a single (narrow, non-ASCII) column, so cutting at column 3 lands + // exactly at its 2-byte UTF-8 start. + let text = "café"; + assert_eq!(UnicodeWidthChar::width('é'), Some(1)); + let (cut, pad) = hscroll_cut(text, 3); + assert_eq!(&text[cut..], "é"); + assert!(!pad); + } + + #[test] + fn hscroll_cut_wide_cjk_straddling_the_cut_skips_it_and_pads() { + // "a漢b" — 'a' (col 0), '漢' (cols 1-2, a wide CJK glyph), 'b' (col 3). Cutting at column + // 2 lands mid-glyph: the whole wide char is dropped and `pad` signals the caller to + // insert a one-column space to keep the remaining columns aligned. + let text = "a漢b"; + assert_eq!(UnicodeWidthChar::width('漢'), Some(2)); + let (cut, pad) = hscroll_cut(text, 2); + assert!( + pad, + "a wide char straddling the cut must request a pad column" + ); + assert_eq!(&text[cut..], "b"); + } + + #[test] + fn hscroll_cut_emoji() { + // Most terminal-emulator-relevant emoji are wide (2 columns), like CJK. + let text = "a🎉b"; + let w = UnicodeWidthChar::width('🎉').unwrap_or(0); + let (cut, _pad) = hscroll_cut(text, 1 + w); + assert_eq!(&text[cut..], "b"); + } + + #[test] + fn hscroll_cut_beyond_line_width_yields_empty() { + let (cut, pad) = hscroll_cut("short", 100); + assert_eq!(cut, "short".len()); + assert!(!pad); + assert_eq!(&"short"[cut..], ""); + } + + // ── mouse h-wheel + outline hscroll follow-up: `pan_spans` ───────────────────── + + fn spans_text(spans: &[TSpan<'static>]) -> String { + spans.iter().map(|s| s.content.as_ref()).collect() + } + + fn span(text: &str) -> TSpan<'static> { + TSpan::styled(text.to_string(), Style::default()) + } + + #[test] + fn pan_spans_at_zero_columns_is_a_pass_through() { + let theme = Palette::dark(); + let spans = vec![span("hello "), span("world")]; + let panned = pan_spans(spans.clone(), 0, &theme); + assert_eq!(spans_text(&panned), "hello world"); + assert_eq!(panned.len(), spans.len(), "unchanged, span for span"); + } + + #[test] + fn pan_spans_cuts_mid_span() { + // "hello world" panned 3 columns — the cut (plus the marker's reserved column) lands + // inside the FIRST span ("hello "), leaving its tail attached to the second span. + let theme = Palette::dark(); + let spans = vec![span("hello "), span("world")]; + let panned = pan_spans(spans, 3, &theme); + assert_eq!(spans_text(&panned), "…o world"); + } + + #[test] + fn pan_spans_cuts_exactly_at_a_span_boundary() { + // "abcdef" as three 2-char spans, panned 2 columns — the cut (plus the marker's reserved + // column) lands exactly on the boundary between the first and second span. + let theme = Palette::dark(); + let spans = vec![span("ab"), span("cd"), span("ef")]; + let panned = pan_spans(spans, 2, &theme); + assert_eq!(spans_text(&panned), "…def"); + } + + #[test] + fn pan_spans_wide_char_straddling_a_span_edge_drops_and_pads() { + // "a漢b" as two spans ("a", "漢b"), panned 1 column — the cut (plus the marker's reserved + // column) straddles the wide CJK glyph at the start of the second span: it's dropped + // whole and compensated with a one-column space. + let theme = Palette::dark(); + assert_eq!(UnicodeWidthChar::width('漢'), Some(2)); + let spans = vec![span("a"), span("漢b")]; + let panned = pan_spans(spans, 1, &theme); + assert_eq!(spans_text(&panned), "… b"); + } + + #[test] + fn pan_spans_beyond_total_width_yields_just_the_marker() { + let theme = Palette::dark(); + let spans = vec![span("ab"), span("cd")]; + let panned = pan_spans(spans, 100, &theme); + assert_eq!(spans_text(&panned), "…"); + } + + #[test] + fn pan_spans_on_empty_content_is_a_pass_through() { + let theme = Palette::dark(); + let spans = vec![span("")]; + let panned = pan_spans(spans, 5, &theme); + assert_eq!( + spans_text(&panned), + "", + "an empty line has nothing to cut, so no marker either" + ); + } + + /// Build a single unstaged-file `App` with one long line, for the hscroll rendering tests — + /// long enough that panning by [`crate::app::HSCROLL_STEP`]-sized steps has real room to move + /// (the tests don't reference that constant directly since it's private to `app.rs`; `200` + /// just needs to comfortably exceed a test pane's width either way). + fn app_with_a_long_line() -> App { + let long_line = "x".repeat(200); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("long.txt", "short\n", &format!("{long_line}\n")) + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app + } + + #[test] + fn panning_right_shows_the_left_edge_marker_and_shifted_content() { + let mut app = app_with_a_long_line(); + app.hscroll_right(); + assert!( + app.hscroll > 0, + "the long line must give hscroll room to pan" + ); + + let buf = render_once(&mut app, 60, 20); + // divider (1) + new-side gutter ("{n:>3} ", 4 chars) — the new pane's first content + // column. + let left_w = buf.area.width.saturating_sub(1) / 2; + let content_x = left_w + 1 + 4; + let row_y = (0..buf.area.height) + .find(|&y| cell_text(&buf, content_x, y) == "…") + .expect("the panned long line's first visible content column must show the marker"); + assert_eq!( + cell_text(&buf, content_x + 1, row_y), + "x", + "content immediately after the marker must be the (shifted) line body" + ); + } + + #[test] + fn a_line_wider_than_the_pane_shows_the_right_edge_marker() { + let mut app = app_with_a_long_line(); + // At `hscroll == 0` the long line already overflows a narrow pane's content width. + assert_eq!(app.hscroll, 0); + + let buf = render_once(&mut app, 60, 20); + let right_x = buf.area.width - 1; + assert!( + (0..buf.area.height).any(|y| cell_text(&buf, right_x, y) == "…"), + "a line wider than the pane must show the right-edge marker" + ); + } + + #[test] + fn winbar_shows_the_pan_offset_indicator_once_panned() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + assert_eq!(app.hscroll, 0); + + let buf_unpanned = render_once(&mut app, 80, 20); + let header_unpanned: String = (0..buf_unpanned.area.width) + .map(|x| cell_text(&buf_unpanned, x, 0)) + .collect(); + assert!( + !header_unpanned.contains('»'), + "no indicator at column 0, got: {header_unpanned:?}" + ); + + // The winbar test's fixture files are tiny (`a\n`/`b\n`) — nowhere near wide enough for + // `hscroll_right` to actually move `hscroll` off `0`. This checks the indicator's own + // render logic, not the pan mechanics (covered separately in `app.rs`), so setting the + // field directly is the more honest test: the indicator must key off `App::hscroll` + // exactly, with no dependency on how it got there. + app.hscroll = 42; + let buf = render_once(&mut app, 80, 20); + let header: String = (0..buf.area.width).map(|x| cell_text(&buf, x, 0)).collect(); + assert!( + header.contains("»42"), + "expected the pan offset indicator, got: {header:?}" + ); + } + + #[test] + fn single_changeset_header_shows_the_pan_offset_indicator_once_panned() { + let mut app = app_with_a_long_line(); + app.hscroll_right(); + + let buf = render_once(&mut app, 80, 20); + let header: String = (0..buf.area.width).map(|x| cell_text(&buf, x, 0)).collect(); + assert!( + header.contains(&format!("»{}", app.hscroll)), + "expected the pan offset indicator on the lone-changeset header, got: {header:?}" + ); + } + + // ── M5 CS3: outline side pane ─────────────────────────────────────────────── + + /// Every outline test renders at this width so the pane's fixed 35-col + 1-col-divider + /// layout is unambiguous: columns `0..35` are the outline, `35` the divider, `36..` the + /// diff. + const OUTLINE_TEST_WIDTH: u16 = 80; fn outline_row(buf: &Buffer, y: u16) -> String { (0..35).map(|x| cell_text(buf, x, y)).collect() @@ -2637,13 +3479,13 @@ mod tests { let content: Vec = (0..buf.area.height).map(|y| outline_row(&buf, y)).collect(); let row = content .iter() - .position(|r| r.contains('\u{25CF}')) + .position(|r| r.contains('\u{2022}')) .expect("current marker present in the outline"); - let marker_x = content[row].find('\u{25CF}').unwrap() as u16; + let marker_x = content[row].find('\u{2022}').unwrap() as u16; assert_eq!( buf.cell((marker_x, row as u16)).unwrap().style().fg, - Some(super::FG_CURRENT), - "expected the outline's current marker to carry FG_CURRENT" + Some(Palette::dark().current_fg), + "expected the outline's current marker to carry Palette::dark().current_fg" ); } @@ -2664,8 +3506,177 @@ mod tests { let marker_x = content[row].find('\u{26A0}').unwrap() as u16; assert_eq!( buf.cell((marker_x, row as u16)).unwrap().style().fg, - Some(super::FG_WARN), - "expected the outline's restack glyph to carry FG_WARN" + Some(Palette::dark().warn_fg), + "expected the outline's restack glyph to carry Palette::dark().warn_fg" + ); + } + + #[test] + fn outline_header_shows_true_position_counter_regardless_of_display_order() { + // CS1 (`outline-header-polish`): the `[i/n]` counter is the TRUE stack position + // (`cs_idx + 1`), never a display-order index — HeadFirst (the default) paints cs-b + // (true index 1) before cs-a (true index 0), so the counter must read `[2/2]` on cs-b's + // row and `[1/2]` on cs-a's, in that display order, not `[1/2]` then `[2/2]`. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + + // Skip y=0: the full-width winbar also renders a `[i/n] ` fragment for the + // CURRENT changeset (cs-b) — an unskipped search for "[2/2]" would false-positive on it. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row_b = content + .iter() + .position(|r| r.contains("[2/2]")) + .expect("cs-b (true index 1) must show counter [2/2]"); + let row_a = content + .iter() + .position(|r| r.contains("[1/2]")) + .expect("cs-a (true index 0) must show counter [1/2]"); + assert!( + row_b < row_a, + "HeadFirst shows cs-b's header before cs-a's, but the counter stays the true stack \ + position, not a display-order index — got:\n{}", + content.join("\n") + ); + } + + #[test] + fn outline_header_label_carries_the_heading_accent_color() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + + // Skip y=0: the full-width winbar ALSO names cs-b (it's `current`) via its own + // `[i/n] ` fragment — in plain foreground, not the outline's heading + // accent — so an unskipped search for "cs-b" would false-positive onto the winbar's own + // label instead of the outline header row this test means to inspect. `content`'s index + // `i` is buffer row `i + 1` (the skip), so every `buf` query below adds 1 back. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row = content + .iter() + .position(|r| r.contains("cs-b")) + .expect("cs-b's header row present (it has no title, so falls back to its name)"); + // `String::find` returns a BYTE offset, not a display column — the row has multi-byte + // glyphs (`•`/`⚠`) ahead of/around the label, so a byte offset would target the wrong + // cell. Every rendered cell here is exactly one column wide, so a `chars()` (not byte) + // position IS the display column. + let label_chars: Vec = "cs-b".chars().collect(); + let row_chars: Vec = content[row].chars().collect(); + let label_x = row_chars + .windows(label_chars.len()) + .position(|w| w == label_chars.as_slice()) + .expect("cs-b's label text present in its own header row") as u16; + assert_eq!( + buf.cell((label_x, row as u16 + 1)).unwrap().style().fg, + Some(Palette::dark().heading_fg), + "expected the outline header's label to carry Palette::dark().heading_fg" + ); + } + + #[test] + fn summary_panel_title_has_no_counter_and_keeps_the_plain_foreground_look() { + // CS1's Gotcha: the counter + accent are outline-only — the summary panel's title (shared + // via `changeset_title_spans`, `counter: None`) must render exactly as it did pre-CS1. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + app.toggle_outline(); + app.toggle_outline(); + let header_idx = app + .outline_items() + .iter() + .position(|it| matches!(it, OutlineItem::Header { cs_idx: 1, .. })) + .expect("cs-b's header row present in Stack mode") as i64; + let delta = header_idx - app.outline_cursor() as i64; + app.outline_move_by(delta); + app.focus_outline(); + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0: the full-width winbar spans every column (including the body's 36.. slice), + // and it too names the current changeset (cs-b) — same false-positive risk as the outline + // tests above. `body_rows`' index `i` is buffer row `i + 1` (the skip), so every `buf` + // query below adds 1 back. + let body_rows: Vec = (1..buf.area.height) + .map(|y| { + (36..buf.area.width) + .map(|x| cell_text(&buf, x, y)) + .collect::() + }) + .collect(); + let joined = body_rows.join("\n"); + assert!( + !joined.contains("[2/2]") && !joined.contains("[1/2]"), + "the outline-only counter must not leak into the summary panel's title, got:\n{joined}" + ); + let row = body_rows + .iter() + .position(|r| r.contains("cs-b")) + .expect("summary panel's title (cs-b's label) present"); + // `String::find` is a BYTE offset, not a display column (the title carries a multi-byte + // `•` marker ahead of the label, since cs-b is `current`) — a `chars()` position over the + // 36.. slice IS the column offset within that slice (every cell here is one column wide), + // so add the slice's own start column (36) back to get the absolute buffer column. + let label_chars: Vec = "cs-b".chars().collect(); + let row_chars: Vec = body_rows[row].chars().collect(); + let label_x = row_chars + .windows(label_chars.len()) + .position(|w| w == label_chars.as_slice()) + .expect("cs-b's label text present in the summary panel's title") + as u16 + + 36; + assert_eq!( + buf.cell((label_x, row as u16 + 1)).unwrap().style().fg, + Some(Palette::dark().foreground), + "the summary panel's title must keep its plain foreground look, not the outline's \ + heading accent" + ); + } + + #[test] + fn render_preserves_a_wheel_scrolled_outline_viewport() { + // The peek model's load-bearing render change: `render_outline` bounds-CLAMPS the + // outline scroll instead of re-deriving it from the cursor, so a wheel-scrolled + // viewport (cursor left outside it) survives the frame instead of snapping back. + use crate::app::Region; + + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + assert!(app.outline_open()); + // A 5-row frame leaves a 3-row outline viewport over this fixture's 4 outline rows + // (2 headers + 2 files): max scroll = 1. + app.outline_height = 3; + app.hit_regions.outline = Some(Region { + x: 0, + y: 1, + w: 34, + h: 3, + }); + let cursor_before = app.outline_cursor(); + + app.handle_wheel(2, 2, 3); // clamps to max scroll = 1 + assert_eq!(app.outline_scroll(), 1, "the wheel scrolled the viewport"); + assert_eq!( + app.outline_cursor(), + cursor_before, + "peek model: the wheel never moves the outline cursor" + ); + + render_once(&mut app, OUTLINE_TEST_WIDTH, 5); + assert_eq!( + app.outline_scroll(), + 1, + "a frame must not re-derive the wheeled scroll back to the cursor" ); } @@ -2697,8 +3708,7 @@ mod tests { .build() .unwrap(); let mut app = two_committed_changesets_app(&fixture); - app.outline_cycle_mode(); // Stack -> Tree - app.outline_cycle_mode(); // Tree -> StackTree + app.outline_cycle_mode(); // Stack -> StackTree app.outline_cycle_mode(); // StackTree -> Flat assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Flat); @@ -2775,7 +3785,9 @@ mod tests { if !app.outline_open() { app.toggle_outline(); } - app.outline_cycle_mode(); // Stack -> Tree + app.outline_cycle_mode(); // Stack -> StackTree + app.outline_cycle_mode(); // StackTree -> Flat + app.outline_cycle_mode(); // Flat -> Tree assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); @@ -2785,112 +3797,766 @@ mod tests { // buffer row starting at y=1 (y=0 is the winbar): `src/` (dir, root, NOT the root's last // child — `top.txt` follows), `a.txt` nested one level under `src/` (the only — hence // last — child of `src/`), then `top.txt` (file, root, IS the root's last child). - assert!( - content[1].contains('\u{251C}') && content[1].contains("src/"), - "expected row 1 to be the src/ directory row with a non-last '├─' guide, got:\n{}", + // + // CS2 tightens `tree_prefix` to 2 cols/level with no trailing space on the connector, so + // these are exact-column checks (not just `contains`) — every rendered cell here is one + // column wide, so `chars()` (not byte) indexing IS the display column (the guide glyphs + // themselves are multi-byte, which is exactly why byte indexing would be wrong). + let row1: Vec = content[1].chars().collect(); + assert_eq!( + row1[0..6], + ['\u{251C}', '\u{2500}', 's', 'r', 'c', '/'], + "expected row 1 to be a tight '├─src/' (2-col connector, no trailing space), got:\n{}", content.join("\n") ); - assert!( - content[2].contains('\u{2514}') && content[2].contains("a.txt"), - "expected row 2 to be src/a.txt, indented under src/ with its own last-child '└─' \ - guide, got:\n{}", + let row2: Vec = content[2].chars().collect(); + assert_eq!( + row2[0..4], + ['\u{2502}', ' ', '\u{2570}', '\u{2500}'], + "expected row 2's guide to be a tight '│ ╰─' (continuation + last-child connector, \ + both 2 cols), got:\n{}", content.join("\n") ); - assert!( - content[3].contains('\u{2514}') && content[3].contains("top.txt"), - "expected row 3 to be top.txt with a last-child '└─' guide, got:\n{}", + assert_eq!( + row2[7..12], + ['a', '.', 't', 'x', 't'], + "expected src/a.txt's basename to start immediately after the 4-col guide + 1-col \ + glyph + 1-col letter + 1-col space, got:\n{}", + content.join("\n") + ); + let row3: Vec = content[3].chars().collect(); + assert_eq!( + row3[0..2], + ['\u{2570}', '\u{2500}'], + "expected row 3's guide to be a tight '╰─' (root-level last-child, 2 cols, no \ + trailing space), got:\n{}", + content.join("\n") + ); + assert_eq!( + row3[5..12], + ['t', 'o', 'p', '.', 't', 'x', 't'], + "expected top.txt to start immediately after the 2-col guide + 1-col glyph + 1-col \ + letter + 1-col space, got:\n{}", content.join("\n") ); } - // ── CS5: file status letter + opt-in nerd-font icons ─────────────────────────── - #[test] - fn outline_file_row_shows_the_modified_change_letter_in_its_own_color() { + fn outline_file_row_tree_guide_carries_the_dim_color() { + // CS4: a File row's tree-guide connector (distinct from its status glyph, which keeps + // `theme.foreground`) is styled `theme.dim`, matching the Dir row's already-dim guides. let fixture = FixtureBuilder::new() .config("core.autocrlf", "false") - .unstaged_file("a.rs", "one\n", "one\nCHANGED\n") .build() .unwrap(); - let mut app = app_from_fixture(&fixture); - // A lone changeset defaults the outline closed — force it open so this render test can - // inspect its rows (same pattern as `outline_tree_mode_renders_directory_rows_with_tree_guides`). + let mut app = changeset_with_nested_paths(&fixture); if !app.outline_open() { app.toggle_outline(); } + app.outline_cycle_mode(); // Stack -> StackTree + app.outline_cycle_mode(); // StackTree -> Flat + app.outline_cycle_mode(); // Flat -> Tree + assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); - let content: Vec = (0..buf.area.height).map(|y| outline_row(&buf, y)).collect(); - // Skip y=0: the full-width winbar also names the file ("[1/1] a.rs"), so an unskipped - // search would match it instead of the outline's own row below it. - let row = content + // Row 3 is top.txt (see the test above) — a File row with a non-empty guide vector. + let row = outline_row(&buf, 3); + // `String::find` returns a BYTE offset, not a display column — the rounded guide glyph is + // multi-byte, so a `chars()` (not byte) position is what actually lines up with the + // column-indexed `buf.cell` lookup below (every rendered cell here is one column wide). + let row_chars: Vec = row.chars().collect(); + let guide_x = row_chars + .iter() + .position(|&c| c == '\u{2570}') + .expect("rounded guide present") as u16; + assert_eq!( + buf.cell((guide_x, 3)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected the File row's tree-guide connector to carry theme.dim, got: {row:?}" + ); + } + + // ── CS5 (`outline-fold`): collapse/expand marker ──────────────────────────────── + + #[test] + fn outline_collapsed_header_renders_a_trailing_dim_hidden_file_marker() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + app.set_outline_order(crate::outline::OutlineOrder::BaseFirst); + app.focus_outline(); + app.outline_top(); // cs-a's header row (BaseFirst: cs-a's header renders first) + app.outline_confirm(); // toggle fold — collapses cs-a, hiding its single file (a.txt) + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0 (the winbar) — it names the current file too (e.g. `[i/n] path`), which can + // false-positive a bare `contains` search, same gotcha `render_outline_file_row` already + // documents. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let (row_idx, header_row) = content .iter() .enumerate() - .skip(1) - .find(|(_, r)| r.contains("a.rs")) - .map(|(i, _)| i) - .expect("a.rs's file row present"); + .find(|(_, r)| r.contains("Add a")) + .map(|(i, r)| (i, r.clone())) + .expect("cs-a's header row present"); + let y = row_idx as u16 + 1; // +1 to undo the y=0 skip above. + assert!( + header_row.contains("\u{25b8} 1"), + "collapsed header must show its 1 hidden file, got: {header_row:?}" + ); assert!( - content[row].contains('M'), - "expected the Modified change letter 'M' in a.rs's row, got: {:?}", - content[row] + !content.iter().any(|r| r.contains("a.txt")), + "a.txt's row must be hidden while its header is collapsed, got:\n{}", + content.join("\n") ); - let letter_x = content[row].find('M').unwrap() as u16; + let row_chars: Vec = header_row.chars().collect(); + let marker_x = row_chars + .iter() + .position(|&c| c == '\u{25b8}') + .expect("marker glyph present") as u16; assert_eq!( - buf.cell((letter_x, row as u16)).unwrap().style().fg, - Some(Palette::dark().foreground), - "Modified is a change-to-existing-content status, so its letter must carry the \ - theme's neutral foreground, not an add/del tint" + buf.cell((marker_x, y)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected the collapsed marker to carry theme.dim, got: {header_row:?}" ); } #[test] - fn outline_icons_nerd_renders_the_rust_file_icon_and_the_dir_icon() { - use git2::Repository; - use workon::{Changeset, ChangesetSpan}; - - use crate::app::ChangesetView; - + fn outline_expanded_header_renders_no_chevron_marker() { let fixture = FixtureBuilder::new() .config("core.autocrlf", "false") .build() .unwrap(); - let root = fixture - .commit("main") - .file("root.txt", "r\n") - .create("root") - .unwrap(); - let head = fixture - .commit("main") - .file("src/main.rs", "fn main() {}\n") - .create("head") - .unwrap(); - let repo = fixture.repo().unwrap(); - let cs = Changeset { - name: "cs".to_string(), - span: ChangesetSpan::Committed { base: root, head }, - title: None, - current: true, - needs_restack: false, - }; - let view = ChangesetView::from_changeset_diff( - cs.clone(), - crate::acquire::diff_changeset(repo, &cs).unwrap(), - ); - let owned = Repository::open(repo.workdir().unwrap()).unwrap(); - let mut app = App::from_changesets(owned, vec![view]); - app.open_current(); - if !app.outline_open() { - app.toggle_outline(); - } - app.outline_cycle_mode(); // Stack -> Tree, so `src/` renders as its own Dir row - assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); - app.set_outline_icons(crate::icons::OutlineIcons::Nerd); + let mut app = two_committed_changesets_app(&fixture); let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); - let content: Vec = (0..buf.area.height).map(|y| outline_row(&buf, y)).collect(); - + // Skip y=0 (the winbar) — see the gotcha noted above. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + assert!( + !content.iter().any(|r| r.contains('\u{25b8}')), + "no row should carry the collapsed marker while every Header/Dir is expanded, got:\n{}", + content.join("\n") + ); + } + + #[test] + fn outline_collapsed_dir_renders_a_trailing_dim_hidden_file_marker() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = changeset_with_nested_paths(&fixture); + if !app.outline_open() { + app.toggle_outline(); + } + app.outline_cycle_mode(); // Stack -> StackTree + app.outline_cycle_mode(); // StackTree -> Flat + app.outline_cycle_mode(); // Flat -> Tree + assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); + + app.focus_outline(); + app.outline_top(); // src/ (dirs-before-files root ordering — see the tree-guide test above) + app.outline_confirm(); // toggle fold — collapses src/, hiding its one file (a.txt) + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0 (the winbar) — its OWN current-file label can itself contain `src/` (e.g. + // `[1/1] src/a.txt`) and false-positive the `contains("src/")` search below if included. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let dir_row = content + .iter() + .find(|r| r.contains("src/")) + .expect("src/ row present"); + assert!( + dir_row.contains("\u{25b8} 1"), + "collapsed src/ must show its 1 hidden file, got: {dir_row:?}" + ); + assert!( + !content.iter().any(|r| r.contains("a.txt")), + "a.txt must be hidden under collapsed src/, got:\n{}", + content.join("\n") + ); + assert!( + content.iter().any(|r| r.contains("top.txt")), + "top.txt (a sibling, not nested under src/) must remain visible, got:\n{}", + content.join("\n") + ); + } + + // ── CS2 (outline-row-shape): smart path render ───────────────────────────────── + + #[test] + fn outline_stack_mode_file_row_splits_basename_and_dim_dirname() { + // Stack mode keeps `guides` empty, so a nested path (`src/a.txt`) must split at render + // time into basename-first, then the dirname in `theme.dim` — ancestors don't carry the + // path here (unlike Tree mode), so the row has to spell it out itself. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = changeset_with_nested_paths(&fixture); + if !app.outline_open() { + app.toggle_outline(); + } + assert_eq!( + app.outline_mode(), + crate::outline::OutlineMode::Stack, + "sanity: default mode is Stack, so guides stay empty and this exercises CS2's split" + ); + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0: the full-width winbar also names the current file (possibly `src/a.txt` + // itself), so an unskipped search could false-positive onto it instead of the outline's + // own row below it. `content`'s index `i` is buffer row `i + 1` (the skip), so every + // `buf` query below adds 1 back. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row_idx = content + .iter() + .position(|r| r.contains("a.txt") && r.contains("src")) + .expect("src/a.txt's split row present"); + let row_chars: Vec = content[row_idx].chars().collect(); + let buf_y = row_idx as u16 + 1; + + let basename_x = row_chars + .windows(5) + .position(|w| w == ['a', '.', 't', 'x', 't']) + .expect("basename 'a.txt' present in its own row") as u16; + assert_eq!( + buf.cell((basename_x, buf_y)).unwrap().style().fg, + Some(Palette::dark().foreground), + "expected the basename to carry the plain (bright) foreground, got:\n{}", + content[row_idx] + ); + + // Two blank columns separate the basename from the dirname (CS2: "basename dim/ + // dirname"), so the dirname starts right after them. + let dirname_x = basename_x + 5 + 2; + assert_eq!( + row_chars[dirname_x as usize], 's', + "expected the dirname 'src' to start two columns after the basename, got:\n{}", + content[row_idx] + ); + assert_eq!( + buf.cell((dirname_x, buf_y)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected the dirname to carry theme.dim, got:\n{}", + content[row_idx] + ); + assert!( + basename_x < dirname_x, + "basename must render BEFORE the dim dirname (basename-first ordering is what makes \ + truncation eat the dirname first), got:\n{}", + content[row_idx] + ); + } + + #[test] + fn outline_root_level_file_gets_no_dirname_suffix() { + // A root-level file (no `/` in its path) gets no suffix at all — no "(root)" + // placeholder, just the bare basename. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = changeset_with_nested_paths(&fixture); // top.txt is root-level + if !app.outline_open() { + app.toggle_outline(); + } + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0: see the split test above — the winbar also names the current file. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row = content + .iter() + .find(|r| r.contains("top.txt")) + .expect("top.txt's row present"); + assert!( + row.trim_end().ends_with("top.txt"), + "a root-level file must render with no trailing suffix after its basename, got: \ + {row:?}" + ); + } + + #[test] + fn outline_flat_row_truncation_eats_the_dim_dirname_first() { + // A pane-width-exceeding Flat-mode row must truncate the (later, dim) dirname before it + // ever touches the (earlier, bright) basename — that ordering is the whole point of + // basename-first rendering (CS2 gotcha). + let long_dir = "reallyquiteverbosedirectoryname"; + let path = format!("{long_dir}/x.txt"); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file(&path, "content\n") + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + if !app.outline_open() { + app.toggle_outline(); + } + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0: see the split test above — the winbar also names the current file (the long + // path itself here), so an unskipped search would false-positive onto it. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row_idx = content + .iter() + .position(|r| r.contains("x.txt")) + .expect("x.txt's row present"); + assert!( + !content[row_idx].contains(long_dir), + "the full dirname must NOT fit/appear — truncation should have eaten part of it, \ + got: {:?}", + content[row_idx] + ); + // Column 34 is the outline's last column before the divider at 35 (see + // `OUTLINE_TEST_WIDTH`'s doc comment). `content`'s index is buffer row `+ 1` (the y=0 + // skip above). + assert_eq!( + cell_text(&buf, 34, row_idx as u16 + 1), + "\u{2026}", + "the truncated row must show the right-edge marker at the pane's last column" + ); + } + + // ── mouse h-wheel + outline hscroll follow-up: outline panning ───────────────── + + /// A single-changeset `App` with one file whose path is far wider than the outline's fixed + /// 35-column width, focused into the outline — for the outline hscroll rendering tests. + fn app_with_a_long_outline_path() -> App { + let long_path = format!("{}.txt", "a".repeat(80)); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file(&long_path, "content\n") + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.focus_outline(); // opens (a lone changeset defaults closed) and focuses. + app + } + + #[test] + fn outline_panning_shows_the_left_marker_shifted_text_and_the_right_edge_marker() { + let mut app = app_with_a_long_outline_path(); + app.outline_hscroll_right(); + assert!( + app.outline_hscroll() > 0, + "the long path must give outline hscroll room to pan" + ); + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let content: Vec = (0..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + // The header row's own label is short, but CS1's `[i/n] ` counter widens it enough that a + // single hscroll step no longer pans it fully off — it can now ALSO show a lone marker + // plus a stray `a` (from a branch name like `main`), so a bare "contains 'a'" check no + // longer picks out the PATH row unambiguously. Look for a run of the synthetic path's + // repeated `a`s instead (`app_with_a_long_outline_path`'s path is 80 `a`s + `.txt`) — no + // header label plausibly contains four `a`s in a row. + let row = content + .iter() + .position(|r| r.contains('…') && r.contains("aaaa")) + .expect("the panned path row must show the left-edge marker plus shifted content"); + // Column 34 is the outline's last column before the divider at 35 (see + // `OUTLINE_TEST_WIDTH`'s doc comment). + assert_eq!( + cell_text(&buf, 34, row as u16), + "…", + "a row wider than the outline pane must show the right-edge marker too" + ); + } + + #[test] + fn outline_render_side_clamp_caps_a_huge_pan_offset() { + let mut app = app_with_a_long_outline_path(); + for _ in 0..1000 { + app.outline_hscroll_right(); + } + assert!( + app.outline_hscroll() > 1000, + "sanity: `outline_hscroll_right` itself has no upper clamp" + ); + + render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + assert!( + app.outline_hscroll() < 1000, + "render_outline must clamp the huge offset down to the content width, got {}", + app.outline_hscroll() + ); + } + + // ── CS3 (`outline-status-xy`): git-style X/Y status matrix ───────────────────── + + /// Render `fixture` (a lone uncommitted changeset with one file at `path`) and return the + /// buffer row + its char cells for the file row matching `path`. Skips y=0 (the winbar also + /// names the current file, which can false-positive a `contains(path)` search). + fn render_outline_file_row(fixture: &Fixture, path: &str) -> (Buffer, u16, Vec) { + let mut app = app_from_fixture(fixture); + // A lone changeset defaults the outline closed — force it open so this render test can + // inspect its rows (same pattern as `outline_tree_mode_renders_directory_rows_with_tree_guides`). + if !app.outline_open() { + app.toggle_outline(); + } + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let (row_idx, row) = content + .iter() + .enumerate() + .find(|(_, r)| r.contains(path)) + .map(|(i, r)| (i, r.clone())) + .unwrap_or_else(|| panic!("{path}'s file row present")); + let y = row_idx as u16 + 1; // +1 to undo the y=0 skip above. + (buf, y, row.chars().collect()) + } + + #[test] + fn outline_unstaged_file_renders_the_y_column_letter_in_del_strong() { + // Unstaged-only (worktree change, no staged one): X is the placeholder, Y carries the + // change letter in del_strong (git convention: worktree column is red). + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("a.rs", "one\n", "one\nCHANGED\n") + .build() + .unwrap(); + let (buf, y, row) = render_outline_file_row(&fixture, "a.rs"); + + let x = row + .iter() + .position(|&c| c == STATUS_PLACEHOLDER) + .expect("expected the X (staged) column placeholder '·'") as u16; + assert_eq!( + row[x as usize + 1], + 'M', + "expected the Y (worktree) column to carry the Modified letter right after the \ + X placeholder, got: {:?}", + row + ); + assert_eq!( + buf.cell((x, y)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected the empty X placeholder to carry theme.dim" + ); + assert_eq!( + buf.cell((x + 1, y)).unwrap().style().fg, + Some(Palette::dark().del_strong), + "expected the Y column's Modified letter to carry theme.del_strong" + ); + } + + #[test] + fn outline_fully_staged_file_renders_the_x_column_letter_in_add_strong() { + // `staged_file` writes+stages a brand-new path (Added, not Modified — there's no prior + // commit for it to modify). Fully staged (index change, no worktree one): X carries the + // letter in add_strong, Y is the placeholder. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .staged_file("a.rs", "new content\n") + .build() + .unwrap(); + let (buf, y, row) = render_outline_file_row(&fixture, "a.rs"); + + let x = row + .iter() + .position(|&c| c == 'A') + .expect("expected the Added letter in the X (staged) column") as u16; + assert_eq!( + row[x as usize + 1], + STATUS_PLACEHOLDER, + "expected the Y (worktree) column to be the empty placeholder, got: {:?}", + row + ); + assert_eq!( + buf.cell((x, y)).unwrap().style().fg, + Some(Palette::dark().add_strong), + "expected the X column's Added letter to carry theme.add_strong" + ); + assert_eq!( + buf.cell((x + 1, y)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected the empty Y placeholder to carry theme.dim" + ); + } + + #[test] + fn outline_partially_staged_file_renders_mm_with_green_x_and_red_y() { + // Partially staged (both a staged AND an unstaged change): both columns show the change + // letter, X in add_strong (green), Y in del_strong (red). + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .partially_staged_file("a.rs", "one\n", "one\nSTAGED\n", "one\nSTAGED\nWORKTREE\n") + .build() + .unwrap(); + let (buf, y, row) = render_outline_file_row(&fixture, "a.rs"); + + let x = row + .iter() + .position(|&c| c == 'M') + .expect("expected the Modified letter in the X column") as u16; + assert_eq!( + row[x as usize + 1], + 'M', + "expected the Modified letter in the Y column too (partial = both axes), got: {:?}", + row + ); + assert_eq!( + buf.cell((x, y)).unwrap().style().fg, + Some(Palette::dark().add_strong), + "expected the X (staged) column's letter to carry theme.add_strong" + ); + assert_eq!( + buf.cell((x + 1, y)).unwrap().style().fg, + Some(Palette::dark().del_strong), + "expected the Y (worktree) column's letter to carry theme.del_strong" + ); + } + + #[test] + fn outline_untracked_file_renders_a_dim_double_question_mark() { + // Untracked overrides the staged-ness-derived matrix entirely: always a dim `??`, even + // though an untracked worktree file's StagedStatus is Unstaged under the hood. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file("new.txt", "brand new\n") + .build() + .unwrap(); + let (buf, y, row) = render_outline_file_row(&fixture, "new.txt"); + + let x = row + .iter() + .position(|&c| c == '?') + .expect("expected the untracked '??' marker") as u16; + assert_eq!( + row[x as usize + 1], + '?', + "expected '??' (both columns), got: {:?}", + row + ); + assert_eq!( + buf.cell((x, y)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected the untracked '?' to carry theme.dim, not an add/del tint" + ); + assert_eq!( + buf.cell((x + 1, y)).unwrap().style().fg, + Some(Palette::dark().dim), + "expected BOTH untracked '?' chars to carry theme.dim" + ); + } + + #[test] + fn outline_committed_modified_file_renders_a_single_amber_letter() { + // A committed changeset's file has StagedStatus::None — single letter + pad column, not + // the X/Y matrix. M/R/C get the dedicated `modified_fg` amber, distinct from `warn_fg`. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let base = fixture + .commit("main") + .file("a.rs", "one\n") + .create("base") + .unwrap(); + let head = fixture + .commit("main") + .file("a.rs", "one\nCHANGED\n") + .create("head") + .unwrap(); + let repo = fixture.repo().unwrap(); + let cs = workon::Changeset { + name: "cs".to_string(), + span: workon::ChangesetSpan::Committed { base, head }, + title: None, + current: true, + needs_restack: false, + }; + let view = crate::app::ChangesetView::from_changeset_diff( + cs.clone(), + crate::acquire::diff_changeset(repo, &cs).unwrap(), + ); + let owned = git2::Repository::open(repo.workdir().unwrap()).unwrap(); + let mut app = App::from_changesets(owned, vec![view]); + app.open_current(); + if !app.outline_open() { + app.toggle_outline(); + } + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row_idx = content + .iter() + .position(|r| r.contains("a.rs")) + .expect("a.rs's file row present"); + let row: Vec = content[row_idx].chars().collect(); + let y = row_idx as u16 + 1; + + let x = row + .iter() + .position(|&c| c == 'M') + .expect("expected the Modified letter") as u16; + assert_eq!( + row[x as usize + 1], + ' ', + "expected the pad column after a committed file's single letter to be a blank space, \ + got: {:?}", + row + ); + assert_eq!( + buf.cell((x, y)).unwrap().style().fg, + Some(Palette::dark().modified_fg), + "expected the committed Modified letter to carry theme.modified_fg (amber), got a \ + different color" + ); + assert_ne!( + buf.cell((x, y)).unwrap().style().fg, + Some(Palette::dark().warn_fg), + "modified_fg must stay a distinct field from warn_fg even though both default to amber" + ); + } + + #[test] + fn outline_committed_added_and_deleted_files_render_add_strong_and_del_strong() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let base = fixture + .commit("main") + .file("deleted.txt", "keep\n") + .create("base") + .unwrap(); + let stage = fixture + .commit("main") + .file("deleted.txt", "keep\n") + .file("added.txt", "new\n") + .create("stage") + .unwrap(); + let _ = stage; // only needed to move the branch tip forward before the manual deletion below + let repo = fixture.repo().unwrap(); + let workdir = repo.workdir().unwrap().to_path_buf(); + std::fs::remove_file(workdir.join("deleted.txt")).unwrap(); + let mut index = repo.index().unwrap(); + // `CommitBuilder::create` wrote the index/commit through its OWN `Repository::open` + // handle, so `repo`'s cached index is stale until forced to re-read from disk. + index.read(true).unwrap(); + index + .remove_path(std::path::Path::new("deleted.txt")) + .unwrap(); + index.write().unwrap(); + let tree = repo.find_tree(index.write_tree().unwrap()).unwrap(); + let sig = git2::Signature::now("Test User", "test@example.com").unwrap(); + let parent = repo.head().unwrap().peel_to_commit().unwrap(); + let head = repo + .commit(Some("HEAD"), &sig, &sig, "head", &tree, &[&parent]) + .unwrap(); + + let cs = workon::Changeset { + name: "cs".to_string(), + span: workon::ChangesetSpan::Committed { base, head }, + title: None, + current: true, + needs_restack: false, + }; + let view = crate::app::ChangesetView::from_changeset_diff( + cs.clone(), + crate::acquire::diff_changeset(repo, &cs).unwrap(), + ); + let owned = git2::Repository::open(repo.workdir().unwrap()).unwrap(); + let mut app = App::from_changesets(owned, vec![view]); + app.open_current(); + if !app.outline_open() { + app.toggle_outline(); + } + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + + let added_row_idx = content + .iter() + .position(|r| r.contains("added.txt")) + .expect("added.txt's file row present"); + let added_row: Vec = content[added_row_idx].chars().collect(); + let added_x = added_row + .iter() + .position(|&c| c == 'A') + .expect("expected the Added letter") as u16; + assert_eq!( + buf.cell((added_x, added_row_idx as u16 + 1)) + .unwrap() + .style() + .fg, + Some(Palette::dark().add_strong), + "expected a committed Added file's letter to carry theme.add_strong" + ); + + let deleted_row_idx = content + .iter() + .position(|r| r.contains("deleted.txt")) + .expect("deleted.txt's file row present"); + let deleted_row: Vec = content[deleted_row_idx].chars().collect(); + let deleted_x = deleted_row + .iter() + .position(|&c| c == 'D') + .expect("expected the Deleted letter") as u16; + assert_eq!( + buf.cell((deleted_x, deleted_row_idx as u16 + 1)) + .unwrap() + .style() + .fg, + Some(Palette::dark().del_strong), + "expected a committed Deleted file's letter to carry theme.del_strong" + ); + } + + #[test] + fn icon_mode_nerd_renders_the_rust_file_icon_and_the_dir_icon() { + use git2::Repository; + use workon::{Changeset, ChangesetSpan}; + + use crate::app::ChangesetView; + + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let root = fixture + .commit("main") + .file("root.txt", "r\n") + .create("root") + .unwrap(); + let head = fixture + .commit("main") + .file("src/main.rs", "fn main() {}\n") + .create("head") + .unwrap(); + let repo = fixture.repo().unwrap(); + let cs = Changeset { + name: "cs".to_string(), + span: ChangesetSpan::Committed { base: root, head }, + title: None, + current: true, + needs_restack: false, + }; + let view = ChangesetView::from_changeset_diff( + cs.clone(), + crate::acquire::diff_changeset(repo, &cs).unwrap(), + ); + let owned = Repository::open(repo.workdir().unwrap()).unwrap(); + let mut app = App::from_changesets(owned, vec![view]); + app.open_current(); + if !app.outline_open() { + app.toggle_outline(); + } + app.outline_cycle_mode(); // Stack -> StackTree + app.outline_cycle_mode(); // StackTree -> Flat + app.outline_cycle_mode(); // Flat -> Tree, so `src/` renders as its own Dir row + assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); + app.set_icon_mode(crate::icons::IconMode::Nerd); + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let content: Vec = (0..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + // Skip y=0 in both searches: the full-width winbar names the path ("[1/1] src/main.rs"), // so an unskipped search would match it instead of the outline's own rows below it. let dir_row = content @@ -2908,13 +4574,13 @@ mod tests { .find(|r| r.contains("main.rs")) .expect("main.rs file row present"); assert!( - file_row.contains(crate::icons::icon_for_path("main.rs")), + file_row.contains(crate::icons::icon_for_path("main.rs", false).0), "expected the rust file icon before main.rs, got: {file_row:?}" ); } #[test] - fn outline_icons_none_renders_neither_icon() { + fn icon_mode_none_renders_neither_icon() { let fixture = FixtureBuilder::new() .config("core.autocrlf", "false") .build() @@ -2923,11 +4589,13 @@ mod tests { if !app.outline_open() { app.toggle_outline(); } - app.outline_cycle_mode(); // Stack -> Tree + app.outline_cycle_mode(); // Stack -> StackTree + app.outline_cycle_mode(); // StackTree -> Flat + app.outline_cycle_mode(); // Flat -> Tree assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); assert_eq!( - app.outline_icons(), - crate::icons::OutlineIcons::None, + app.icon_mode(), + crate::icons::IconMode::None, "sanity: icons default to None" ); @@ -2948,6 +4616,108 @@ mod tests { ); } + // ── CS3: nerd-mode status/header/summary iconography ──────────────────────── + + #[test] + fn outline_header_nerd_markers_replace_the_unicode_defaults() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); // cs-b: current + needs_restack + app.set_icon_mode(crate::icons::IconMode::Nerd); + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + // Skip y=0: it's the full-width winbar, which ALSO renders a (still-unicode, CS4's job) + // "⚠ needs restack" marker — an unskipped search would false-positive on it. + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let joined = content.join("\n"); + + assert!( + joined.contains(super::NERD_CURRENT_MARKER), + "expected the nerd current-changeset marker, got:\n{joined}" + ); + assert!( + joined.contains(super::NERD_WARN_MARKER), + "expected the nerd needs-restack marker, got:\n{joined}" + ); + assert!( + !joined.contains('\u{2022}') && !joined.contains('\u{26A0}'), + "nerd mode must not leave the plain unicode markers behind in the outline pane, got:\n{joined}" + ); + assert!( + joined.contains(super::NERD_BRANCH_ICON), + "expected a branch glyph on the changeset header row, got:\n{joined}" + ); + } + + #[test] + fn outline_file_status_xy_column_is_unaffected_by_icon_mode() { + // CS3 retires StagedStatus's nerd/plain glyph split entirely — the X/Y status matrix is + // now plain letters + `STATUS_PLACEHOLDER`, icon-mode-independent (only the devicons + // per-file icon toggles on `IconMode::Nerd`). A fully staged file (`staged_file` writes a + // brand-new path, so it's Added, not Modified) still renders `A·` whether or not nerd + // icons are on. + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .staged_file("a.txt", "one\nCHANGED\n") + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.set_icon_mode(crate::icons::IconMode::Nerd); + if !app.outline_open() { + app.toggle_outline(); + } + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let content: Vec = (1..buf.area.height).map(|y| outline_row(&buf, y)).collect(); + let row = content + .iter() + .find(|r| r.contains("a.txt")) + .expect("a.txt's file row present"); + assert!( + row.contains(&format!("A{}", '\u{b7}')), + "expected the fully-staged 'A·' status pair to survive nerd icon mode, got: {row:?}" + ); + } + + #[test] + fn summary_panel_nerd_mode_renders_the_dir_icon_and_diffstat_glyphs() { + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = changeset_with_nested_paths(&fixture); + app.set_icon_mode(crate::icons::IconMode::Nerd); + app.focus_outline(); // opens (a lone changeset defaults closed) and focuses + app.outline_cycle_mode(); // Stack -> StackTree + app.outline_cycle_mode(); // StackTree -> Flat + app.outline_cycle_mode(); // Flat -> Tree, so a Dir row exists to focus + assert_eq!(app.outline_mode(), crate::outline::OutlineMode::Tree); + let dir_idx = app + .outline_items() + .iter() + .position(|it| matches!(it, OutlineItem::Dir { .. })) + .expect("a Dir row present in Tree mode") as i64; + let delta = dir_idx - app.outline_cursor() as i64; + app.outline_move_by(delta); + assert!(matches!( + app.outline_items()[app.outline_cursor()], + OutlineItem::Dir { .. } + )); + + let buf = render_once(&mut app, OUTLINE_TEST_WIDTH, 20); + let body = body_text(&buf); + assert!( + body.contains(crate::icons::DIR_ICON), + "expected the summary panel's dir title to carry the nerd dir icon, got:\n{body}" + ); + assert!( + body.contains(super::NERD_DIFF_ADDED) && body.contains(super::NERD_DIFF_REMOVED), + "expected nerd diffstat glyphs in the summary panel's totals line, got:\n{body}" + ); + } + // ── CS4: summary panel ─────────────────────────────────────────────────────── /// The body area's columns, for a render at [`OUTLINE_TEST_WIDTH`] (outline `0..35`, divider diff --git a/git-workon-review/src/summary.rs b/git-workon-review/src/summary.rs index b6eab27d..dea09925 100644 --- a/git-workon-review/src/summary.rs +++ b/git-workon-review/src/summary.rs @@ -9,8 +9,9 @@ //! //! `workon::Changeset` (see `git-workon-lib/src/changeset.rs`) exposes only `name`/`title` for a //! changeset today — no commit body/message. [`changeset_summary`] therefore renders the -//! label (title, falling back to name — the same rule the winbar/outline header already use) -//! plus the diffstat; there is no commit-message row. Surfacing the commit body would need a +//! label (`crate::app::display_label` — title falling back to name, with the uncommitted layer +//! rendered as "Uncommitted changes"; the same rule the winbar/outline header use) plus the +//! diffstat; there is no commit-message row. Surfacing the commit body would need a //! `repo.find_commit` lookup keyed off the changeset's head OID — left as a follow-up, not part //! of this changeset's scope. diff --git a/git-workon-review/src/theme.rs b/git-workon-review/src/theme.rs index a9b63ce2..f5a27c8e 100644 --- a/git-workon-review/src/theme.rs +++ b/git-workon-review/src/theme.rs @@ -6,7 +6,7 @@ //! CS5 adds [`Palette::light`] and wires [`crate::config::Theme`] to pick between them; CS6 adds the //! terminal-derivation probe for `auto`. //! -//! ## Hybrid boundary (ADR-035, revised) +//! ## Hybrid boundary (ADR-035, twice-revised) //! Colors that sit ON a tinted background — the diff add/del gradient, its staged variants, the //! cursor/selection washes, and syntax foreground — are theme-controlled base16 truecolor and live //! here, as before. The canvas background and chrome FOREGROUND (default text, dim labels, the @@ -15,9 +15,29 @@ //! look instead of bleeding the terminal's own bg/fg through. `auto` ([`Palette::from_terminal`]) //! still derives these four from the probed terminal colors — so it matches the terminal exactly — //! and leaves [`Palette::paint_canvas`] `false` so a transparent/backgrounded terminal isn't -//! painted over; the curated schemes and the probe's curated fallback set it `true`. Semantic -//! chrome that is never on a tint and never a theme knob — error/warn/current-marker colors — stays -//! ANSI/const in [`crate::render`] (`FG_ERROR`/`FG_WARN`/`FG_CURRENT`), unaffected by this boundary. +//! painted over; the curated schemes and the probe's curated fallback set it `true`. +//! +//! **CS2 revision:** semantic chrome — error/warn/current-marker colors — was previously ANSI/const +//! in `crate::render` (`FG_ERROR`/`FG_WARN`/`FG_CURRENT`), deliberately excluded from the palette on +//! the reasoning that these colors never sit on a tint and are never a theme knob. That boundary is +//! now revised: they ARE palette knobs ([`Palette::error_fg`]/[`Palette::warn_fg`]/ +//! [`Palette::current_fg`], mapped to base08/base0A/base0B), so a curated or probed theme can shift +//! them too. `dark()` keeps the three shipped RGB values verbatim (the same pixel-identity +//! precedent as its diff/cursor tints); `light()` takes `ONE_LIGHT`'s base08/base0A/base0B; +//! `from_terminal()` takes the probed scheme's base08/base0A/base0B directly (matching the syntax +//! slots' reasoning, not the curated-tint-borrowing the diff/cursor washes use). +//! +//! **CS1 addition (`outline-header-polish`):** [`Palette::heading_fg`] (base0C, cyan) is a fourth +//! semantic-chrome field, same reasoning and same three-scheme mapping as the CS2 trio above — +//! it's the outline's changeset-header-row accent, used only there (see +//! `render::changeset_title_spans`'s doc comment for the outline-only gating). +//! +//! **CS3 addition (`outline-status-xy`):** [`Palette::modified_fg`] (base09, orange/amber) is a +//! fifth semantic-chrome field, same three-scheme mapping again — the outline's committed-file +//! "modified" tint (M/R/C letters). Deliberately a NEW field rather than reusing +//! [`Palette::warn_fg`]: "this changeset needs restacking" and "this file was modified" are +//! unrelated facts that happen to both want an amber tone, and collapsing them onto one field +//! would make them un-independently themeable. use ratatui::style::Color; @@ -202,6 +222,32 @@ pub struct Palette { pub dim: Color, /// Gutter/divider foreground (base04) — line-number gutters and pane dividers. pub gutter: Color, + /// Footer text color for an [`crate::app::Severity::Error`] notice, a pending-discard confirm + /// prompt, and a Failed changeset's marker/message — a clearly-red tone (base08). Promoted + /// from `render.rs`'s `FG_ERROR` const (CS2, revising ADR-035's hybrid boundary — see this + /// module's doc comment). + pub error_fg: Color, + /// Warning tone for a needs-restack marker (locked decision #9) — an amber (base0A), distinct + /// from [`Palette::error_fg`]'s red: a stale-parent changeset is a heads-up to `gt restack`, + /// not a failure. Promoted from `render.rs`'s `FG_WARN` const (CS2). + pub warn_fg: Color, + /// Tone for the outline's "this is the lib-marked `current` changeset" marker (locked + /// decision #9's outline half) — a green (base0B), distinct from every other marker color so + /// "current" reads unambiguously at a glance. Promoted from `render.rs`'s `FG_CURRENT` const + /// (CS2). + pub current_fg: Color, + /// Accent tone for a changeset header row's label (CS1, `outline-header-polish`) — a cyan + /// (base0C), distinct from [`Palette::current_fg`]'s green so "this is a section heading" + /// reads independently of "this is the current changeset." Used ONLY by the outline's Header + /// rows (`render::changeset_title_spans`'s `counter` param gates it) — the summary panel's + /// changeset title keeps the plain [`Palette::foreground`] look. + pub heading_fg: Color, + /// Tone for a committed changeset's Modified/Renamed/Copied outline file-status letter (CS3, + /// `outline-status-xy`) — an amber (base09), distinct from [`Palette::warn_fg`]'s amber + /// (base0A) so "needs restack" and "modified" stay independently themeable even though both + /// default to the same amber family. Used ONLY by the outline's committed-file status column + /// (`render::committed_letter_color`). + pub modified_fg: Color, /// Whether [`crate::render::render`] should paint the whole frame with [`Palette::background`] /// before drawing panes. `true` for the curated [`Palette::dark`]/[`Palette::light`] schemes /// (and the probe's curated fallback); `false` for [`Palette::from_terminal`], so `auto` @@ -237,6 +283,17 @@ impl Palette { foreground: base.slot(5), dim: base.slot(3), gutter: base.slot(4), + // The shipped M3–M5 semantic-chrome colors, reproduced verbatim (the pixel-identity + // gate — CS2 promotes these from `render.rs` consts without changing a single value). + error_fg: Color::Rgb(220, 60, 60), + warn_fg: Color::Rgb(214, 158, 46), + current_fg: Color::Rgb(96, 200, 128), + // CS1: brand new (no historical `render.rs` const to reproduce), so this takes the + // scheme's base0C directly rather than an authored literal. + heading_fg: base.slot(12), + // CS3: brand new, same reasoning as `heading_fg` above — takes the scheme's base09 + // directly rather than an authored literal. + modified_fg: base.slot(9), paint_canvas: true, } } @@ -286,6 +343,11 @@ impl Palette { foreground: base.slot(5), dim: base.slot(3), gutter: base.slot(4), + error_fg: red, + warn_fg: base.slot(10), // base0A + current_fg: green, + heading_fg: cyan, + modified_fg: base.slot(9), // base09 paint_canvas: true, } } @@ -324,6 +386,13 @@ impl Palette { foreground: base.slot(5), dim: base.slot(3), gutter: base.slot(4), + // Semantic chrome also matches the terminal — probed base08/base0A/base0B, not the + // curated fallback's (mirrors the syntax slots' reasoning just above). + error_fg: base.slot(8), + warn_fg: base.slot(10), + current_fg: base.slot(11), + heading_fg: base.slot(12), + modified_fg: base.slot(9), // Unlike the curated schemes, `auto` must NOT paint over the terminal's own // background — base00 here IS the probed terminal bg, so painting a solid canvas // would defeat terminal transparency/background images for no benefit (the probed @@ -397,6 +466,37 @@ mod tests { assert_eq!(t.outline_cursor_unfocused_bg, Color::Rgb(35, 38, 55)); } + #[test] + fn dark_semantic_fg_matches_the_historical_render_rs_constants() { + // CS2's pixel-identity gate for the promoted `FG_ERROR`/`FG_WARN`/`FG_CURRENT` consts. + let t = Palette::dark(); + assert_eq!(t.error_fg, Color::Rgb(220, 60, 60)); + assert_eq!(t.warn_fg, Color::Rgb(214, 158, 46)); + assert_eq!(t.current_fg, Color::Rgb(96, 200, 128)); + } + + #[test] + fn dark_heading_fg_takes_the_eighties_dark_cyan_accent() { + // CS1: no historical constant to reproduce (this field is new) — unlike + // `dark_semantic_fg_matches_the_historical_render_rs_constants` above, it takes base0C + // straight from the scheme. + let t = Palette::dark(); + assert_eq!(t.heading_fg, Color::Rgb(0x66, 0xcc, 0xcc)); // base0C + } + + #[test] + fn dark_modified_fg_takes_the_eighties_dark_orange_accent() { + // CS3: no historical constant to reproduce (this field is new, same reasoning as + // `dark_heading_fg_takes_the_eighties_dark_cyan_accent` above) — takes base09 straight + // from the scheme. + let t = Palette::dark(); + assert_eq!(t.modified_fg, Color::Rgb(0xf9, 0x91, 0x57)); // base09 + assert_ne!( + t.modified_fg, t.warn_fg, + "modified_fg must stay independently themeable from warn_fg" + ); + } + #[test] fn dark_chrome_fields_match_the_eighties_dark_ramp_and_paint_the_canvas() { // `dark()`'s canvas/chrome must come from the SAME ramp `Palette::dark`'s syntax/tints @@ -502,6 +602,30 @@ mod tests { assert_eq!(color("variable"), Color::Rgb(0x38, 0x3a, 0x42)); // base05 fg } + #[test] + fn light_semantic_fg_takes_one_lights_base08_base0a_base0b() { + let t = Palette::light(); + assert_eq!(t.error_fg, Color::Rgb(0xca, 0x12, 0x43)); // base08 + assert_eq!(t.warn_fg, Color::Rgb(0xc1, 0x84, 0x01)); // base0A + assert_eq!(t.current_fg, Color::Rgb(0x50, 0xa1, 0x4f)); // base0B + } + + #[test] + fn light_heading_fg_takes_one_lights_cyan_accent() { + let t = Palette::light(); + assert_eq!(t.heading_fg, Color::Rgb(0x01, 0x84, 0xbc)); // base0C + } + + #[test] + fn light_modified_fg_takes_one_lights_orange_accent() { + let t = Palette::light(); + assert_eq!(t.modified_fg, Color::Rgb(0xd7, 0x5f, 0x00)); // base09 + assert_ne!( + t.modified_fg, t.warn_fg, + "modified_fg must stay independently themeable from warn_fg" + ); + } + /// A synthetic probed scheme with a distinct value in every slot and the given `base00`, so a /// test can assert `from_terminal`'s syntax slots came from the probed scheme (not a curated /// one) and read the base00 luminance branch. @@ -574,6 +698,21 @@ mod tests { assert!(!palette.paint_canvas); } + #[test] + fn from_terminal_takes_semantic_fg_from_the_probed_scheme_not_the_curated_fallback() { + // Same reasoning as syntax/chrome: `auto`'s error/warn/current colors should match the + // terminal, not borrow the curated dark/light fallback's (unlike the diff/cursor tints, + // which DO borrow — see `from_terminal_with_a_dark_background_borrows_darks_curated_tints`). + let probed = probed_base16(Color::Rgb(0x1a, 0x1a, 0x1a)); + let palette = Palette::from_terminal(probed); + assert_eq!(palette.error_fg, probed.slot(8)); + assert_eq!(palette.warn_fg, probed.slot(10)); + assert_eq!(palette.current_fg, probed.slot(11)); + assert_eq!(palette.heading_fg, probed.slot(12)); + assert_eq!(palette.modified_fg, probed.slot(9)); + assert_ne!(palette.error_fg, Palette::dark().error_fg); + } + #[test] fn is_light_background_splits_on_the_luminance_midpoint() { assert!(is_light_background(Base16::ONE_LIGHT.slot(0))); diff --git a/git-workon-review/src/tui.rs b/git-workon-review/src/tui.rs index 5d194a2c..5fd518b2 100644 --- a/git-workon-review/src/tui.rs +++ b/git-workon-review/src/tui.rs @@ -15,6 +15,13 @@ //! `event::poll`'s timeout to the channel's. The M4 index watcher's *semantics* are exactly //! unchanged by this move: it still compares [`workon_review::refresh::IndexSignature`] and //! re-diffs in place via [`App::on_tick`] on every `Tick`; only the beat's mechanism moved. +//! +//! CS10 turns the mouse on: [`Tui::acquire`] enables capture for the whole session (undone by +//! [`Tui::restore`] and, unconditionally, the panic hook), and [`map_terminal_event`] maps a +//! left-click or wheel-scroll into an [`AppEvent::Mouse`] the loop dispatches to +//! [`workon_review::app::App::handle_click`]/[`workon_review::app::App::handle_wheel`] — every +//! other mouse kind (drag, move, non-left buttons, button-up) is still dropped, same as key +//! release/repeat. use std::fs::File; use std::io::{self, Write}; @@ -23,7 +30,10 @@ use std::sync::mpsc; use std::thread; use std::time::Duration; -use crossterm::event::{self, Event, KeyCode, KeyEvent, KeyEventKind}; +use crossterm::event::{ + self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEvent, KeyEventKind, + MouseButton, MouseEvent, MouseEventKind, +}; use crossterm::execute; use crossterm::terminal::{ disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen, @@ -52,6 +62,10 @@ use workon_review::theme::Palette; pub enum AppEvent { Key(KeyEvent), Resize(u16, u16), + /// A left-click or wheel-scroll (CS10) — the only [`MouseEventKind`]s [`map_terminal_event`] + /// maps; drag, move, non-left buttons, and up events are dropped at the mapping step, exactly + /// like key release/repeat. + Mouse(MouseEvent), Tick, /// One [`LoadRequest`]'s result — ADR-037's loader-result variant. `gen`/`cs_idx`/`file_idx` /// echo the request's stamp; `result` is `Err` for a job that panicked or otherwise failed @@ -78,15 +92,20 @@ pub enum AppEvent { impl PartialEq for AppEvent { /// Manual, deliberately PARTIAL equality (can't derive — `FileReady`'s `LoadedViews` payload - /// isn't `PartialEq`, see the enum's doc comment): `Key`/`Resize`/`Tick` compare structurally, - /// exactly like the pre-ADR-037 derive did, for the input-thread tests that still assert - /// mapped-event shape via `assert_eq!`. Two `FileReady` events are never considered equal — - /// there's no sound definition of "the same loader result" once `FileView` can't be compared, - /// and nothing needs one; tests that care about a `FileReady`'s fields match on them directly. + /// isn't `PartialEq`, see the enum's doc comment): `Key`/`Resize`/`Mouse`/`Tick` compare + /// structurally, exactly like the pre-ADR-037 derive did, for the input-thread tests that + /// still assert mapped-event shape via `assert_eq!`. Two `FileReady` events are never + /// considered equal — there's no sound definition of "the same loader result" once `FileView` + /// can't be compared, and nothing needs one; tests that care about a `FileReady`'s fields + /// match on them directly. Every fully-comparable variant needs its own arm here: the + /// `_ => false` catch-all exists ONLY for `FileReady`/`ChangesetReady`, and letting a + /// comparable variant fall into it silently breaks reflexivity (`Mouse` did exactly that + /// when CS10 first added it — crossterm's `MouseEvent` derives `PartialEq` fine). fn eq(&self, other: &Self) -> bool { match (self, other) { (AppEvent::Key(a), AppEvent::Key(b)) => a == b, (AppEvent::Resize(w1, h1), AppEvent::Resize(w2, h2)) => w1 == w2 && h1 == h2, + (AppEvent::Mouse(a), AppEvent::Mouse(b)) => a == b, (AppEvent::Tick, AppEvent::Tick) => true, _ => false, } @@ -98,15 +117,28 @@ impl PartialEq for AppEvent { /// still observable, just relayed rather than swallowed). `Tick` never appears here. type InboxMessage = io::Result; -/// Map one crossterm terminal [`Event`] to the [`AppEvent`] the loop reacts to — key-press and -/// resize map; key release/repeat, mouse, paste, and focus events are skipped (`None`), exactly -/// like this module's pre-ADR-037 `next_event`/`drain_pending` read arms did. Pure and -/// independent of any thread or channel, so it's unit-tested directly; the input thread's loop -/// body is a thin wrapper around it. +/// Map one crossterm terminal [`Event`] to the [`AppEvent`] the loop reacts to — key-press, +/// resize, and (CS10, extended by the mouse h-wheel follow-up) a left-click or vertical/ +/// horizontal wheel-scroll map; key release/repeat, every other mouse kind (drag, move, non-left +/// buttons, button-up), paste, and focus events are skipped (`None`). Pure and independent of any +/// thread or channel, so it's unit-tested directly; the input thread's loop body is a thin wrapper +/// around it. fn map_terminal_event(event: Event) -> Option { match event { Event::Key(key) if key.kind == KeyEventKind::Press => Some(AppEvent::Key(key)), Event::Resize(w, h) => Some(AppEvent::Resize(w, h)), + Event::Mouse(m) + if matches!( + m.kind, + MouseEventKind::Down(MouseButton::Left) + | MouseEventKind::ScrollUp + | MouseEventKind::ScrollDown + | MouseEventKind::ScrollLeft + | MouseEventKind::ScrollRight + ) => + { + Some(AppEvent::Mouse(m)) + } _ => None, } } @@ -415,6 +447,8 @@ enum Action { StartSelection, ExpandGap, ExpandGapAll, + HscrollLeft, + HscrollRight, ToggleOutline, OutlineMoveBy(i64), OutlineConfirm, @@ -425,6 +459,12 @@ enum Action { OutlineBottom, OutlineStage, OutlineDiscard, + OutlineHscrollLeft, + OutlineHscrollRight, + OutlineNextChangeset, + OutlinePrevChangeset, + OutlineCollapseAll, + OutlineExpandAll, None, } @@ -455,6 +495,8 @@ fn command_to_action(command: Command, pane_height: usize) -> Action { Command::StartSelection => Action::StartSelection, Command::ExpandGap => Action::ExpandGap, Command::ExpandGapAll => Action::ExpandGapAll, + Command::HscrollLeft => Action::HscrollLeft, + Command::HscrollRight => Action::HscrollRight, Command::NextFile => Action::NextFile, Command::PrevFile => Action::PrevFile, Command::NextHunk => Action::NextHunk, @@ -471,6 +513,12 @@ fn command_to_action(command: Command, pane_height: usize) -> Action { Command::OutlineBottom => Action::OutlineBottom, Command::OutlineStage => Action::OutlineStage, Command::OutlineDiscard => Action::OutlineDiscard, + Command::OutlineHscrollLeft => Action::OutlineHscrollLeft, + Command::OutlineHscrollRight => Action::OutlineHscrollRight, + Command::OutlineNextChangeset => Action::OutlineNextChangeset, + Command::OutlinePrevChangeset => Action::OutlinePrevChangeset, + Command::OutlineCollapseAll => Action::OutlineCollapseAll, + Command::OutlineExpandAll => Action::OutlineExpandAll, } } @@ -582,16 +630,36 @@ fn apply_action(app: &mut App, action: Action) -> bool { Action::StartSelection => app.start_selection(), Action::ExpandGap => app.expand_gap_at_cursor(false), Action::ExpandGapAll => app.expand_gap_at_cursor(true), + Action::HscrollLeft => app.hscroll_left(), + Action::HscrollRight => app.hscroll_right(), Action::ToggleOutline => app.toggle_outline(), Action::OutlineMoveBy(delta) => app.outline_move_by(delta), Action::OutlineConfirm => app.outline_confirm(), Action::OutlineCycleMode => app.outline_cycle_mode(), - Action::FocusOutline => app.focus_outline(), + // `h`/`left` pans the diff back to column 0 first (mirroring the outline's own home + // position) and only actually focuses the outline once there — see the handoff's locked + // decision #2. Implemented here rather than in `App::focus_outline` itself, since that + // method is also called from the outline toggle (`App::toggle_outline`) and the mouse + // click/wheel paths (`App::handle_click`/`handle_wheel`), none of which should gain pan + // behavior. + Action::FocusOutline => { + if app.hscroll > 0 { + app.hscroll_left(); + } else { + app.focus_outline(); + } + } Action::FocusDiff => app.focus_diff(), Action::OutlineTop => app.outline_top(), Action::OutlineBottom => app.outline_bottom(), Action::OutlineStage => app.outline_stage(), Action::OutlineDiscard => app.outline_discard(), + Action::OutlineHscrollLeft => app.outline_hscroll_left(), + Action::OutlineHscrollRight => app.outline_hscroll_right(), + Action::OutlineNextChangeset => app.outline_next_changeset(), + Action::OutlinePrevChangeset => app.outline_prev_changeset(), + Action::OutlineCollapseAll => app.outline_collapse_all(), + Action::OutlineExpandAll => app.outline_expand_all(), Action::None => {} } false @@ -695,6 +763,23 @@ fn update(app: &mut App, keymap: &Keymap, pending: &mut Vec, event: Ap KeyOutcome::Handled => false, KeyOutcome::Action(action) => apply_action(app, action), }, + // CS10: both modals swallow mouse input exactly like they swallow keys (cases 1-2 above) + // — a click/wheel while a discard confirm or the help overlay is up does nothing. + AppEvent::Mouse(_) if app.pending_confirm.is_some() || app.help_visible => false, + AppEvent::Mouse(m) => { + app.clear_notice(); + match m.kind { + MouseEventKind::Down(MouseButton::Left) => app.handle_click(m.column, m.row), + MouseEventKind::ScrollDown => app.handle_wheel(m.column, m.row, 3), + MouseEventKind::ScrollUp => app.handle_wheel(m.column, m.row, -3), + // 4 columns per tick — finer than `HSCROLL_STEP` since trackpads emit streams of + // ticks (see `App::handle_hwheel`'s doc comment). + MouseEventKind::ScrollRight => app.handle_hwheel(m.column, m.row, 4), + MouseEventKind::ScrollLeft => app.handle_hwheel(m.column, m.row, -4), + _ => {} + } + false + } AppEvent::Tick => { app.on_tick(); false @@ -878,7 +963,11 @@ fn install_panic_hook() { std::panic::set_hook(Box::new(move |info| { let _ = disable_raw_mode(); let mut out = terminal_writer(); - let _ = execute!(out, LeaveAlternateScreen); + // CS10: disable mouse capture unconditionally, same as `Tui::restore` — a stray disable + // sequence when capture was never enabled (a panic before `Tui::acquire` reaches its own + // `EnableMouseCapture`) is harmless, and there's no cheaper way from here to know whether + // capture is currently on. + let _ = execute!(out, DisableMouseCapture, LeaveAlternateScreen); default_hook(info); })); } @@ -903,7 +992,10 @@ impl Tui { install_panic_hook(); enable_raw_mode()?; let mut out = terminal_writer(); - execute!(out, EnterAlternateScreen)?; + // CS10: capture the mouse for the whole session — `map_terminal_event` only ever lets a + // left-click or wheel-scroll through, so this doesn't cost the terminal's normal + // text-selection UX beyond what most terminals' shift-click bypass already covers. + execute!(out, EnterAlternateScreen, EnableMouseCapture)?; let backend = CrosstermBackend::new(out); let terminal = Terminal::new(backend)?; Ok(Self { @@ -1014,7 +1106,14 @@ impl Tui { } self.restored = true; disable_raw_mode()?; - execute!(self.terminal.backend_mut(), LeaveAlternateScreen)?; + // CS10: disable mouse capture before leaving the alternate screen — same ordering + // convention as the raw-mode/alternate-screen pair, undone in the reverse order acquire + // set them up in. + execute!( + self.terminal.backend_mut(), + DisableMouseCapture, + LeaveAlternateScreen + )?; self.terminal.show_cursor() } } @@ -1144,9 +1243,18 @@ mod tests { ); } + fn mouse(kind: MouseEventKind) -> MouseEvent { + MouseEvent { + kind, + column: 5, + row: 7, + modifiers: KeyModifiers::NONE, + } + } + #[test] - fn map_terminal_event_skips_release_repeat_mouse_paste_and_focus() { - use crossterm::event::{KeyEventState, MouseEvent, MouseEventKind}; + fn map_terminal_event_skips_release_repeat_paste_and_focus() { + use crossterm::event::KeyEventState; let release = KeyEvent::new_with_kind( KeyCode::Char('q'), @@ -1163,20 +1271,75 @@ mod tests { ); assert_eq!(map_terminal_event(Event::Key(repeat)), None); - assert_eq!( - map_terminal_event(Event::Mouse(MouseEvent { - kind: MouseEventKind::Moved, - column: 0, - row: 0, - modifiers: KeyModifiers::NONE, - })), - None - ); assert_eq!(map_terminal_event(Event::Paste("pasted".to_string())), None); assert_eq!(map_terminal_event(Event::FocusGained), None); assert_eq!(map_terminal_event(Event::FocusLost), None); } + /// CS10: `map_terminal_event` maps ONLY a left-click-down or a wheel-scroll to + /// `AppEvent::Mouse`; every other mouse kind — drag, move, button-up, and non-left buttons — + /// is still dropped, exactly like the pre-CS10 version dropped every mouse event outright. + /// This supersedes the old `map_terminal_event_skips_release_repeat_mouse_paste_and_focus` + /// pin (split above into the non-mouse skip cases, which are unchanged by CS10). + #[test] + fn map_terminal_event_maps_left_down_and_scroll_but_drops_other_mouse_kinds() { + let left_down = mouse(MouseEventKind::Down(MouseButton::Left)); + assert!(matches!( + map_terminal_event(Event::Mouse(left_down)), + Some(AppEvent::Mouse(m)) if m == left_down + )); + + let scroll_up = mouse(MouseEventKind::ScrollUp); + assert!(matches!( + map_terminal_event(Event::Mouse(scroll_up)), + Some(AppEvent::Mouse(m)) if m == scroll_up + )); + + let scroll_down = mouse(MouseEventKind::ScrollDown); + assert!(matches!( + map_terminal_event(Event::Mouse(scroll_down)), + Some(AppEvent::Mouse(m)) if m == scroll_down + )); + + // Dropped: drag, move, button-up, and a right-click-down. + assert_eq!( + map_terminal_event(Event::Mouse(mouse(MouseEventKind::Drag(MouseButton::Left)))), + None + ); + assert_eq!( + map_terminal_event(Event::Mouse(mouse(MouseEventKind::Moved))), + None + ); + assert_eq!( + map_terminal_event(Event::Mouse(mouse(MouseEventKind::Up(MouseButton::Left)))), + None + ); + assert_eq!( + map_terminal_event(Event::Mouse(mouse(MouseEventKind::Down( + MouseButton::Right + )))), + None + ); + } + + /// Mouse h-wheel follow-up: `ScrollLeft`/`ScrollRight` (trackpad h-scroll, or a shift-wheel + /// the terminal reports this way) map through exactly like the vertical `ScrollUp`/ + /// `ScrollDown` pair above. + #[test] + fn map_terminal_event_maps_scroll_left_and_right() { + let scroll_left = mouse(MouseEventKind::ScrollLeft); + assert!(matches!( + map_terminal_event(Event::Mouse(scroll_left)), + Some(AppEvent::Mouse(m)) if m == scroll_left + )); + + let scroll_right = mouse(MouseEventKind::ScrollRight); + assert!(matches!( + map_terminal_event(Event::Mouse(scroll_right)), + Some(AppEvent::Mouse(m)) if m == scroll_right + )); + } + /// `AppEvent` dropped `PartialEq`/`Eq` in ADR-037 (`FileReady`'s `LoadedViews` payload wraps /// `FileView`, which has neither) — this test-only helper is the `matches!`-based replacement /// for the `assert_eq!(event, AppEvent::Key(key(...)))` shape used throughout this module's @@ -1881,6 +2044,50 @@ mod tests { repo.assert(predicate::repo::workdir_file_equals("a.txt", "one\ntwo\n")); } + /// CS10: a pending discard confirm swallows a mouse event exactly like it swallows a key — + /// mirrors `pending_confirm_captures_y_and_n_and_ignores_other_keys` above. A click inside a + /// live hit region must not move the cursor or resolve the confirm. + #[test] + fn pending_confirm_swallows_a_mouse_click() { + use workon_review::app::{PendingOp, Region}; + + let fixture = git_workon_fixture::prelude::FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("a.txt", "one\ntwo\nthree\n", "one\nCHANGED\nthree\n") + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.pane_height = 10; + app.hit_regions.single = Some(Region { + x: 0, + y: 0, + w: 40, + h: 10, + }); + let km = Keymap::defaults(); + let mut pending: Vec = Vec::new(); + + app.request_confirm("Discard? (y/n)", PendingOp::DiscardFile { file_idx: 0 }); + let cursor_before = app.cursor; + let quit = update( + &mut app, + &km, + &mut pending, + AppEvent::Mouse(mouse(MouseEventKind::Down(MouseButton::Left))), + ); + + assert!(!quit); + assert!( + app.pending_confirm.is_some(), + "a mouse event must not resolve the confirm" + ); + assert_eq!( + app.cursor, cursor_before, + "a swallowed click must not move the cursor" + ); + } + // ── M5 CS3: outline pane key routing ───────────────────────────────────── /// A two-committed-changeset stack, built the same way as `app.rs`/`render.rs`'s own M5 @@ -2223,15 +2430,26 @@ mod tests { .build() .unwrap(); let mut app = two_committed_changesets_app(&fixture); - // CS3: pin BaseFirst explicitly — this test exercises Enter's header-jump + focus - // return, which is orthogonal to display order, but the `-3` row offset below assumes - // the base->head row layout. + // CS3: pin BaseFirst explicitly — this test exercises Enter's File-row jump + focus + // return, which is orthogonal to display order, but the row offset below assumes the + // base->head row layout. app.set_outline_order(workon_review::outline::OutlineOrder::BaseFirst); app.toggle_outline(); // close app.toggle_outline(); // open + focus, cursor synced onto cs-b's file row assert!(app.outline_focused()); - // Move the outline cursor up onto cs-a's header row. - app.outline_move_by(-3); + // Move the outline cursor onto cs-a's FILE row (rows, BaseFirst: [Header a, File a.txt, + // Header b, File b.txt] — cursor starts at 3; -2 lands on File a.txt at row 1). CS5 + // (`outline-fold`) removed Enter's old header-jump behavior — see + // `enter_on_a_header_row_toggles_fold_and_keeps_focus` below for that case — so this + // keybinding-dispatch test needs a File row to still exercise a real jump+unfocus. + app.outline_move_by(-2); + assert_eq!( + app.current_cs(), + 0, + "sanity: the move itself already landed on cs-a's file (moving onto a File row \ + always jumps, per `outline_move_by`'s own contract) — Enter below re-confirms the \ + same jump through the real keybinding-dispatch path" + ); let km = Keymap::defaults(); let mut pending: Vec = Vec::new(); @@ -2245,12 +2463,63 @@ mod tests { assert_eq!( app.current_cs(), 0, - "Enter on cs-a's header must jump there" + "Enter on a File row must (still) land on cs-a" ); - assert_eq!(app.current, 0, "...landing on its first file"); + assert_eq!(app.current, 0, "...landing on its file"); assert!( !app.outline_focused(), - "Enter returns focus to the diff after jumping" + "Enter on a File row returns focus to the diff" + ); + } + + #[test] + fn enter_on_a_header_row_toggles_fold_and_keeps_focus() { + // CS5 (`outline-fold`): Enter on a Header/Dir row no longer jumps+unfocuses — it toggles + // that row's fold and deliberately keeps focus. This is the header-row counterpart to + // `enter_confirms_an_outline_jump_and_returns_focus_to_the_diff` above, verified through + // the same real keybinding-dispatch path (`update`/`map_key`), not a direct + // `App::outline_confirm()` call. + use git_workon_fixture::prelude::*; + + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .build() + .unwrap(); + let mut app = two_committed_changesets_app(&fixture); + app.set_outline_order(workon_review::outline::OutlineOrder::BaseFirst); + app.toggle_outline(); // close + app.toggle_outline(); // open + focus, cursor synced onto cs-b's file row + // Move the outline cursor onto cs-a's header row (rows, BaseFirst: [Header a, File a.txt, + // Header b, File b.txt] — cursor starts at 3; -3 lands on Header a at row 0, which never + // jumps). + app.outline_move_by(-3); + let before_cs = app.current_cs(); + let before_file = app.current; + let rows_before = app.outline_items().len(); + + let km = Keymap::defaults(); + let mut pending: Vec = Vec::new(); + update( + &mut app, + &km, + &mut pending, + AppEvent::Key(key(KeyCode::Enter)), + ); + + assert_eq!( + app.current_cs(), + before_cs, + "Enter on a header must NOT jump the diff (CS5)" + ); + assert_eq!(app.current, before_file); + assert!( + app.outline_focused(), + "Enter on a header toggles its fold and keeps focus (CS5), rather than confirming a \ + jump" + ); + assert!( + app.outline_items().len() < rows_before, + "cs-a's file row must now be hidden under its collapsed header" ); } @@ -3089,4 +3358,90 @@ mod tests { "the active file's view must be cached after its FileReady lands" ); } + + // ── diff-hscroll: `Action::FocusOutline` pans home before focusing ───────────── + + /// Locked decision #2: `h`/`left` (`Action::FocusOutline`) pans the diff back toward column + /// `0` first while panned, and only actually focuses the outline once there — implemented in + /// this dispatch arm rather than in `App::focus_outline` itself (see that arm's comment), so + /// this is only testable at the `apply_action` layer, not through `App` alone. + #[test] + fn focus_outline_action_pans_home_before_focusing_when_panned() { + use git_workon_fixture::prelude::*; + + let long_line = "x".repeat(200); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .unstaged_file("a.txt", "short\n", &format!("{long_line}\n")) + .build() + .unwrap(); + + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.hscroll_right(); + assert!( + app.hscroll > 0, + "the long line must give hscroll room to pan" + ); + assert!(!app.outline_focused()); + + // Panned: the first press pans back toward column 0 rather than focusing the outline. + apply_action(&mut app, Action::FocusOutline); + assert_eq!( + app.hscroll, 0, + "one press from a single hscroll step returns to column 0" + ); + assert!( + !app.outline_focused(), + "still unfocused — this press only panned" + ); + + // Already at column 0: the next press focuses the outline as normal. + apply_action(&mut app, Action::FocusOutline); + assert!(app.outline_focused()); + } + + /// Mouse h-wheel follow-up: a `ScrollRight` event reaches `App::handle_hwheel` (not the + /// vertical `App::handle_wheel`) when dispatched through the full `update` path — mirroring + /// how the existing vertical-wheel tests exercise `App::handle_wheel` directly, but this one + /// goes through `map_terminal_event` + `update`'s mouse arm to also pin the event mapping. + #[test] + fn scroll_right_event_reaches_handle_hwheel_via_update() { + use git_workon_fixture::prelude::*; + use workon_review::app::Region; + + let lines: String = (1..=40).map(|n| format!("l{n}\n")).collect(); + let fixture = FixtureBuilder::new() + .config("core.autocrlf", "false") + .untracked_file("big.txt", &lines) + .build() + .unwrap(); + let mut app = app_from_fixture(&fixture); + app.open_current(); + app.pane_height = 10; + app.hit_regions.single = Some(Region { + x: 0, + y: 0, + w: 40, + h: 10, + }); + let km = Keymap::defaults(); + let mut pending: Vec = Vec::new(); + assert_eq!(app.hscroll, 0); + + let raw = MouseEvent { + kind: MouseEventKind::ScrollRight, + column: 10, + row: 3, + modifiers: KeyModifiers::NONE, + }; + // Round-trip through the real mapping first, matching how the input thread feeds `update`. + let mapped = map_terminal_event(Event::Mouse(raw)).expect("ScrollRight must map"); + update(&mut app, &km, &mut pending, mapped); + + assert!( + app.hscroll > 0, + "a ScrollRight event over the diff pane must pan App::hscroll via handle_hwheel" + ); + } }