diff --git a/Cargo.lock b/Cargo.lock index 344412c..366190b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2311,6 +2311,7 @@ dependencies = [ "gitr-graph", "gitr-vcs", "gpui", + "gpui-base", "gpui-component", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 96adb4b..abcb2e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ gpui = { git = "https://github.com/zed-industries/zed" } gpui_platform = { git = "https://github.com/zed-industries/zed", features = ["font-kit"] } gpui-component = { git = "https://github.com/longbridge/gpui-component", features = ["tree-sitter", "tree-sitter-diff", "tree-sitter-rust"] } gpui-component-assets = { git = "https://github.com/longbridge/gpui-component" } +gpui-base = { git = "https://github.com/longbridge/gpui-component" } gix = "0.86" notify = "8.2" diff --git a/crates/ui/Cargo.toml b/crates/ui/Cargo.toml index 6fe506c..21922ab 100644 --- a/crates/ui/Cargo.toml +++ b/crates/ui/Cargo.toml @@ -17,6 +17,7 @@ graph.workspace = true vcs.workspace = true gpui.workspace = true gpui-component.workspace = true +gpui-base.workspace = true anyhow.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/ui/src/detail/decorations.rs b/crates/ui/src/detail/decorations.rs deleted file mode 100644 index 077480d..0000000 --- a/crates/ui/src/detail/decorations.rs +++ /dev/null @@ -1,138 +0,0 @@ -//! Line-background colours for the diff editor, and the [`TextDecoration`]s that paint -//! them onto [`format::DiffLineRanges`]. -//! -//! The theme's syntax styles cannot express a background — [`gpui_component::ThemeStyle`] -//! carries only `color`, `font_style` and `font_weight` — so a per-line background has to -//! go through the editor's decorations collection instead (see `super::mod` for why). -//! [`LIGHT_ADDITION_BACKGROUND`] and [`LIGHT_DELETION_BACKGROUND`] are the exact values -//! given for the light theme; [`DARK_ADDITION_BACKGROUND`] and [`DARK_DELETION_BACKGROUND`] -//! are this module's own pair; see [`line_backgrounds`] for why dark needs one. - -use gpui::{HighlightStyle, Hsla, rgb}; -use gpui_component::ThemeMode; -use gpui_component::input::TextDecoration; - -use super::format::DiffLineRanges; - -const LIGHT_ADDITION_BACKGROUND: u32 = 0xdafbe1; -const LIGHT_DELETION_BACKGROUND: u32 = 0xffebe9; - -/// A tint has to clear two bars, and only one of them is legibility. -/// -/// The light pair is unreadable under Catppuccin Frappé — its addition syntax colour -/// `#a6d189` sits at 1.56:1 against [`LIGHT_ADDITION_BACKGROUND`], pale on pale. But an -/// earlier dark green picked purely for legibility against that text landed at 1.00:1 -/// against Frappé's own `#303446` background: identical luminance, so the band was -/// invisible and the tint may as well not have been drawn. This value clears both — 1.58:1 -/// against the background so the band reads, 4.50:1 under the text so the code stays -/// legible on it. -const DARK_ADDITION_BACKGROUND: u32 = 0x355a40; - -/// Mirrors [`DARK_ADDITION_BACKGROUND`]'s two-bar reasoning for Frappé's deletion syntax -/// colour `#e78284`: 1.30:1 against the background, 3.57:1 under the text. Red text is -/// lighter than green here, so the two bars pull harder against each other and this sits -/// where they meet. -const DARK_DELETION_BACKGROUND: u32 = 0x5f3c45; - -pub(super) struct LineBackgrounds { - pub added: Hsla, - pub deleted: Hsla, -} - -pub(super) fn line_backgrounds(mode: ThemeMode) -> LineBackgrounds { - let (added, deleted) = if mode.is_dark() { - (DARK_ADDITION_BACKGROUND, DARK_DELETION_BACKGROUND) - } else { - (LIGHT_ADDITION_BACKGROUND, LIGHT_DELETION_BACKGROUND) - }; - LineBackgrounds { - added: rgb(added).into(), - deleted: rgb(deleted).into(), - } -} - -pub(super) fn build_decorations( - ranges: &DiffLineRanges, - colors: &LineBackgrounds, -) -> Vec { - let added_style = HighlightStyle { - background_color: Some(colors.added), - ..Default::default() - }; - let deleted_style = HighlightStyle { - background_color: Some(colors.deleted), - ..Default::default() - }; - - ranges - .additions - .iter() - .cloned() - .map(|range| TextDecoration::new(range, added_style)) - .chain( - ranges - .deletions - .iter() - .cloned() - .map(|range| TextDecoration::new(range, deleted_style)), - ) - .collect() -} - -#[cfg(test)] -mod tests { - use super::*; - - fn approx_eq(a: gpui::Rgba, b: gpui::Rgba) -> bool { - (a.r - b.r).abs() < 1e-3 - && (a.g - b.g).abs() < 1e-3 - && (a.b - b.b).abs() < 1e-3 - && (a.a - b.a).abs() < 1e-3 - } - - #[test] - fn light_mode_uses_the_exact_given_hex_values() { - let colors = line_backgrounds(ThemeMode::Light); - assert!(approx_eq( - colors.added.into(), - rgb(LIGHT_ADDITION_BACKGROUND) - )); - assert!(approx_eq( - colors.deleted.into(), - rgb(LIGHT_DELETION_BACKGROUND) - )); - } - - #[test] - fn dark_mode_does_not_reuse_the_light_pair() { - let light = line_backgrounds(ThemeMode::Light); - let dark = line_backgrounds(ThemeMode::Dark); - assert!(!approx_eq(dark.added.into(), light.added.into())); - assert!(!approx_eq(dark.deleted.into(), light.deleted.into())); - } - - #[test] - fn build_decorations_pairs_each_range_with_its_own_background() { - let ranges = DiffLineRanges { - additions: vec![0..3, 10..14], - deletions: vec![5..8, 15..18], - }; - let colors = line_backgrounds(ThemeMode::Light); - - let decorations = build_decorations(&ranges, &colors); - - assert_eq!(decorations.len(), 4); - let additions: Vec<_> = decorations - .iter() - .filter(|d| d.style.background_color == Some(colors.added)) - .map(|d| d.range.clone()) - .collect(); - let deletions: Vec<_> = decorations - .iter() - .filter(|d| d.style.background_color == Some(colors.deleted)) - .map(|d| d.range.clone()) - .collect(); - assert_eq!(additions, ranges.additions); - assert_eq!(deletions, ranges.deletions); - } -} diff --git a/crates/ui/src/detail/diff.rs b/crates/ui/src/detail/diff.rs deleted file mode 100644 index 88c01fd..0000000 --- a/crates/ui/src/detail/diff.rs +++ /dev/null @@ -1,49 +0,0 @@ -//! Renders a `Patch` inside a single readonly code editor, instead of one hand-built -//! `div` per diff line. -//! -//! A hand-built row per line means every element for the whole patch is constructed on -//! every render, selects nothing, and highlights nothing beyond a full-width background -//! tint. Feeding [`super::format::unified_diff_text`] into a real -//! [`gpui_component::input::Editor`] gets text selection, `tree-sitter-diff` syntax -//! highlighting, and virtualised scrolling that only lays out the lines actually on -//! screen — for free, from the editor. -//! -//! One [`Editor`] holds the whole patch rather than one per file: a diff reads as a -//! single continuous document (this is how `git diff` and a GitHub raw patch view both -//! present it), a single scrollbar matches the rest of the panel, and it avoids creating -//! and tearing down one [`EditorState`] entity per file on every commit selection. The -//! trade-off is that per-file collapsing isn't available; nothing in this panel asks for -//! it. -//! -//! [`super::DetailPanel`] owns the [`EditorState`] entity and is the one that pushes new -//! text into it — this module only builds the element each render, exactly like the rest -//! of the panel's view functions. - -use domain::Patch; -use gpui::{AnyElement, App, Entity, IntoElement, ParentElement as _, Styled as _, div}; -use gpui_component::{ - ActiveTheme as _, - input::{Editor, EditorState}, -}; - -pub(super) fn render(patch: &Patch, diff_editor: &Entity, cx: &App) -> AnyElement { - if patch.files.is_empty() { - return div() - .size_full() - .flex() - .items_center() - .justify_center() - .text_color(cx.theme().muted_foreground) - .child("This commit changes nothing.") - .into_any_element(); - } - - Editor::new(diff_editor) - .appearance(false) - .bordered(false) - .readonly(true) - .font_family(cx.theme().mono_font_family.clone()) - .text_size(cx.theme().mono_font_size) - .size_full() - .into_any_element() -} diff --git a/crates/ui/src/detail/diff/body.rs b/crates/ui/src/detail/diff/body.rs new file mode 100644 index 0000000..fd44881 --- /dev/null +++ b/crates/ui/src/detail/diff/body.rs @@ -0,0 +1,1250 @@ +use std::ops::{Range, RangeInclusive}; +use std::rc::Rc; + +use domain::LineOrigin; +use gpui::{ + App, Bounds, Element, ElementId, FlexDirection, GlobalElementId, Half as _, HighlightStyle, + Hitbox, HitboxBehavior, Hsla, InspectorElementId, IntoElement, LayoutId, Length, Pixels, Point, + ScrollHandle, ShapedLine, SharedString, Style, StyledText, TextAlign, TextLayout, TextStyle, + Window, fill, point, px, relative, size, +}; +use gpui_base::{TextSelectionHandle, TextSelectionRegistration, TextSelectionRun}; +use gpui_component::{ThemeColor, ThemeMode}; + +use super::DiffContent; +use super::model::{Row, header_line}; +use super::pairing::SideLine; +use super::palette::line_colors; +use super::split::SplitRow; + +pub(super) const ROW_HEIGHT: f32 = 18.; + +const GUTTER_WIDTH: f32 = 44.; +const MARKER_WIDTH: f32 = 16.; +const GUTTER_PADDING: f32 = 8.; +const MARKER_PADDING: f32 = 5.; +const CODE_LEFT: f32 = 2. * GUTTER_WIDTH + MARKER_WIDTH; +const SPLIT_CODE_LEFT: f32 = GUTTER_WIDTH + MARKER_WIDTH; +const COLUMN_RULE_WIDTH: f32 = 1.; +const TRAILING_SPACE: f32 = 16.; +const UNMEASURED_ROWS: usize = 100; + +pub(super) enum Rows { + Unified(Vec), + Split(Vec), +} + +impl Rows { + fn len(&self) -> usize { + match self { + Rows::Unified(rows) => rows.len(), + Rows::Split(rows) => rows.len(), + } + } + + pub(super) fn is_empty(&self) -> bool { + self.len() == 0 + } + + fn columns(&self) -> usize { + match self { + Rows::Unified(_) => 1, + Rows::Split(_) => 2, + } + } + + fn code_left(&self) -> f32 { + match self { + Rows::Unified(_) => CODE_LEFT, + Rows::Split(_) => SPLIT_CODE_LEFT, + } + } + + fn cells(&self) -> usize { + self.len() * self.columns() + } + + fn side(&self, row: usize, column: usize) -> Option<&SideLine> { + let Rows::Split(rows) = self else { + return None; + }; + let SplitRow::Sides { left, right } = &rows[row] else { + return None; + }; + if column == 0 { + left.as_ref() + } else { + right.as_ref() + } + } +} + +pub(super) fn cell_strings(rows: &Rows) -> Vec { + (0..rows.cells()) + .map(|cell| cell_text(rows, cell)) + .collect() +} + +pub(super) struct DiffBody { + content: Rc, + select_all: bool, + selection: TextSelectionHandle, + scroll: ScrollHandle, + theme: ThemeColor, + mode: ThemeMode, + visible: Range, + texts: Vec, + cell_bounds: Vec>, +} + +pub(super) fn body( + content: Rc, + select_all: bool, + selection: TextSelectionHandle, + scroll: ScrollHandle, + theme: ThemeColor, + mode: ThemeMode, +) -> DiffBody { + DiffBody { + content, + select_all, + selection, + scroll, + theme, + mode, + visible: 0..0, + texts: Vec::new(), + cell_bounds: Vec::new(), + } +} + +fn cell_text(rows: &Rows, cell: usize) -> SharedString { + let columns = rows.columns(); + let (row, column) = (cell / columns, cell % columns); + match rows { + Rows::Unified(rows) => row_text(&rows[row]), + Rows::Split(split) => match &split[row] { + SplitRow::Full(full) if column == 0 => row_text(full), + SplitRow::Full(_) => SharedString::default(), + SplitRow::Sides { left, right } => { + let side = if column == 0 { left } else { right }; + side.as_ref() + .map(|side| SharedString::from(side.content.clone())) + .unwrap_or_default() + } + }, + } +} + +fn row_text(row: &Row) -> SharedString { + match row { + Row::FileHeader { + path, + status, + added, + deleted, + } => header_line(path, status, *added, *deleted).into(), + Row::HunkHeader { text } => text.clone().into(), + Row::Line { content, .. } => content.clone().into(), + Row::Placeholder { message } => (*message).into(), + } +} + +fn styled_cell(rows: &Rows, cell: usize, text: SharedString, theme: &ThemeColor) -> StyledText { + let range = 0..text.len(); + let highlight = HighlightStyle { + color: Some(cell_foreground(rows, cell, theme)), + ..Default::default() + }; + StyledText::new(text).with_highlights([(range, highlight)]) +} + +fn cell_foreground(rows: &Rows, cell: usize, theme: &ThemeColor) -> Hsla { + let row = cell / rows.columns(); + match rows { + Rows::Unified(rows) => row_foreground(&rows[row], theme), + Rows::Split(split) => match &split[row] { + SplitRow::Full(full) => row_foreground(full, theme), + SplitRow::Sides { .. } => theme.foreground, + }, + } +} + +fn row_foreground(row: &Row, theme: &ThemeColor) -> Hsla { + match row { + Row::FileHeader { .. } | Row::Line { .. } => theme.foreground, + Row::HunkHeader { .. } | Row::Placeholder { .. } => theme.muted_foreground, + } +} + +fn row_background(row: &Row, theme: &ThemeColor, mode: ThemeMode) -> Option { + match row { + Row::FileHeader { .. } => Some(theme.secondary), + Row::HunkHeader { .. } => Some(theme.muted), + Row::Placeholder { .. } => None, + Row::Line { origin, .. } => line_colors(*origin, mode, theme).background, + } +} + +fn marker(origin: LineOrigin) -> &'static str { + match origin { + LineOrigin::Addition => "+", + LineOrigin::Deletion => "\u{2212}", + LineOrigin::Context => " ", + } +} + +fn column_width(bounds: Bounds, columns: usize) -> Pixels { + bounds.size.width / columns as f32 +} + +fn column_left(bounds: Bounds, columns: usize, column: usize) -> Pixels { + bounds.origin.x + column_width(bounds, columns) * column as f32 +} + +fn bounds_for_cell( + bounds: Bounds, + columns: usize, + code_left: Pixels, + cell: usize, +) -> Bounds { + Bounds::new( + point( + column_left(bounds, columns, cell % columns) + code_left, + bounds.origin.y + px((cell / columns) as f32 * ROW_HEIGHT), + ), + size( + (column_width(bounds, columns) - code_left).max(px(0.)), + px(ROW_HEIGHT), + ), + ) +} + +fn selection_quad_bounds( + start: Point, + end: Point, + bounds: Bounds, + line_height: Pixels, +) -> Vec> { + if start.y == end.y { + return vec![Bounds::from_corners( + start, + Point::new(end.x, end.y + line_height), + )]; + } + + let mut quads = vec![Bounds::from_corners( + start, + Point::new(bounds.right(), start.y + line_height), + )]; + if end.y > start.y + line_height { + quads.push(Bounds::from_corners( + Point::new(bounds.left(), start.y + line_height), + Point::new(bounds.right(), end.y), + )); + } + quads.push(Bounds::from_corners( + Point::new(bounds.left(), end.y), + Point::new(end.x, end.y + line_height), + )); + quads +} + +fn row_window(offset_y: Pixels, viewport: Pixels, rows: usize) -> Range { + if viewport <= px(0.) { + return 0..rows.min(UNMEASURED_ROWS); + } + let first = ((-offset_y) / px(ROW_HEIGHT)).floor().max(0.) as usize; + let count = (viewport / px(ROW_HEIGHT)).ceil() as usize + 2; + first.min(rows)..first.saturating_add(count).min(rows) +} + +fn selected_rows( + origin_y: Pixels, + top: Pixels, + bottom: Pixels, + rows: usize, +) -> Option> { + let last_row = rows.checked_sub(1)?; + let row_of = |y: Pixels| ((y - origin_y) / px(ROW_HEIGHT)).floor(); + let first = row_of(top); + let last = row_of(bottom); + if first > last_row as f32 || last < 0. { + return None; + } + Some((first.max(0.) as usize)..=(last as usize).min(last_row)) +} + +#[derive(Clone, Copy, Debug, PartialEq)] +enum Band { + Whole, + From(Pixels), + To(Pixels), + Between(Pixels, Pixels), +} + +impl Band { + fn holds(self, x: Pixels) -> bool { + match self { + Band::Whole => true, + Band::From(low) => x >= low, + Band::To(high) => x <= high, + Band::Between(low, high) => x >= low && x <= high, + } + } +} + +fn selection_band(row_top: Pixels, anchor: Point, cursor: Point) -> Option { + let height = px(ROW_HEIGHT); + let in_row = |point: Point| point.y >= row_top && point.y < row_top + height; + if row_top + height <= anchor.y.min(cursor.y) || row_top > anchor.y.max(cursor.y) { + return None; + } + if in_row(anchor) && in_row(cursor) { + return Some(Band::Between( + anchor.x.min(cursor.x), + anchor.x.max(cursor.x), + )); + } + let (start, end) = if anchor.y < cursor.y { + (anchor, cursor) + } else { + (cursor, anchor) + }; + if in_row(start) { + Some(Band::From(start.x)) + } else if in_row(end) { + Some(Band::To(end.x)) + } else { + Some(Band::Whole) + } +} + +fn selected_range( + text: &str, + band: Band, + left: Pixels, + line: impl FnOnce() -> ShapedLine, +) -> Option> { + if band == Band::Whole { + return (!text.is_empty()).then_some(0..text.len()); + } + let line = line(); + if text.len() != line.len() { + return None; + } + let mut range: Option> = None; + let mut start = line.x_for_index(0); + for (offset, character) in text.char_indices() { + let next = offset + character.len_utf8(); + let end = line.x_for_index(next); + if band.holds(left + start + (end - start).half()) { + range.get_or_insert(offset..offset).end = next; + } + start = end; + } + range +} + +fn copy_text( + strings: &[SharedString], + ranges: &[Option>], + start: usize, + columns: usize, +) -> String { + let (Some(first), Some(last)) = ( + ranges.iter().position(Option::is_some), + ranges.iter().rposition(Option::is_some), + ) else { + return String::new(); + }; + + let mut text = String::new(); + for cell in first..=last { + if cell > first { + text.push(if (start + cell).is_multiple_of(columns) { + '\n' + } else { + '\t' + }); + } + if let Some(range) = &ranges[cell] { + text.push_str(&strings[cell][range.clone()]); + } + } + text +} + +fn paint_selection(layout: &TextLayout, range: Range, color: Hsla, window: &mut Window) { + let (Some(start), Some(end)) = ( + layout.position_for_index(range.start), + layout.position_for_index(range.end), + ) else { + return; + }; + for bounds in selection_quad_bounds(start, end, layout.bounds(), layout.line_height()) { + window.paint_quad(fill(bounds, color)); + } +} + +struct Pen { + style: TextStyle, + font_size: Pixels, +} + +impl Pen { + fn new(window: &Window) -> Self { + let style = window.text_style(); + let font_size = style.font_size.to_pixels(window.rem_size()); + Self { style, font_size } + } + + fn shape(&self, text: SharedString, color: Hsla, window: &Window) -> ShapedLine { + let mut run = self.style.to_run(text.len()); + run.color = color; + window + .text_system() + .shape_line(text, self.font_size, &[run], None) + } + + fn measure(&self, text: SharedString, window: &Window) -> ShapedLine { + self.shape(text, self.style.color, window) + } + + fn width(&self, text: SharedString, window: &Window) -> Pixels { + self.measure(text, window).width() + } +} + +fn paint_line(line: &ShapedLine, origin: Point, window: &mut Window, cx: &mut App) { + let _ = line.paint(origin, px(ROW_HEIGHT), TextAlign::Left, None, window, cx); +} + +fn paint_number( + number: Option, + right: Pixels, + top: Pixels, + color: Hsla, + pen: &Pen, + window: &mut Window, + cx: &mut App, +) { + let Some(number) = number else { + return; + }; + let line = pen.shape(number.to_string().into(), color, window); + let origin = point(right - px(GUTTER_PADDING) - line.width(), top); + paint_line(&line, origin, window, cx); +} + +impl DiffBody { + fn rows(&self) -> &Rows { + &self.content.rows + } + + fn strings(&self) -> &[SharedString] { + &self.content.strings + } + + fn cell_bounds_at(&self, bounds: Bounds, cell: usize) -> Bounds { + bounds_for_cell( + bounds, + self.rows().columns(), + px(self.rows().code_left()), + cell, + ) + } + + fn content_width(&self, window: &Window) -> Pixels { + let pen = Pen::new(window); + let mut widest = px(0.); + for text in self.strings() { + widest = widest.max(pen.width(text.clone(), window)); + } + (px(self.rows().code_left()) + widest + px(TRAILING_SPACE)) * self.rows().columns() as f32 + } + + fn visible_rows(&self) -> Range { + row_window( + self.scroll.offset().y, + self.scroll.bounds().size.height, + self.rows().len(), + ) + } + + fn visible_cells(&self) -> Range { + let columns = self.rows().columns(); + self.visible.start * columns..self.visible.end * columns + } + + fn whole_text(&self) -> String { + let ranges: Vec>> = self + .strings() + .iter() + .map(|text| Some(0..text.len())) + .collect(); + copy_text(self.strings(), &ranges, 0, self.rows().columns()) + } + + fn copy_selection( + &self, + bounds: Bounds, + projected: &[Option>], + pen: &Pen, + window: &Window, + cx: &App, + ) -> String { + if self.select_all { + return self.whole_text(); + } + let Some(points) = self + .selection + .snapshot(cx) + .and_then(|snapshot| snapshot.window_points()) + else { + return String::new(); + }; + let anchor = points.anchor(); + let cursor = points.cursor(); + let Some(rows) = selected_rows( + bounds.origin.y, + anchor.y.min(cursor.y), + anchor.y.max(cursor.y), + self.rows().len(), + ) else { + return String::new(); + }; + + let columns = self.rows().columns(); + let visible = self.visible_cells(); + let cells = rows.start() * columns..(rows.end() + 1) * columns; + let ranges: Vec>> = cells + .clone() + .map(|cell| { + if visible.contains(&cell) { + return projected.get(cell - visible.start).and_then(Clone::clone); + } + let cell_bounds = self.cell_bounds_at(bounds, cell); + let band = selection_band(cell_bounds.origin.y, anchor, cursor)?; + let text = &self.strings()[cell]; + selected_range(text, band, cell_bounds.origin.x, || { + pen.measure(text.clone(), window) + }) + }) + .collect(); + + copy_text( + &self.strings()[cells.clone()], + &ranges, + cells.start, + columns, + ) + } + + fn paint_background( + &self, + row: usize, + bounds: Bounds, + top: Pixels, + window: &mut Window, + ) { + let columns = self.rows().columns(); + let full_width = match self.rows() { + Rows::Unified(rows) => Some(&rows[row]), + Rows::Split(split) => match &split[row] { + SplitRow::Full(full) => Some(full), + SplitRow::Sides { .. } => None, + }, + }; + + match full_width { + Some(full) => { + if let Some(background) = row_background(full, &self.theme, self.mode) { + let band = Bounds::new( + point(bounds.origin.x, top), + size(bounds.size.width, px(ROW_HEIGHT)), + ); + window.paint_quad(fill(band, background)); + } + } + None => { + for column in 0..columns { + let Some(background) = self.rows().side(row, column).and_then(|side| { + line_colors(side.origin, self.mode, &self.theme).background + }) else { + continue; + }; + let band = Bounds::new( + point(column_left(bounds, columns, column), top), + size(column_width(bounds, columns), px(ROW_HEIGHT)), + ); + window.paint_quad(fill(band, background)); + } + + for column in 1..columns { + let rule = Bounds::new( + point(column_left(bounds, columns, column), top), + size(px(COLUMN_RULE_WIDTH), px(ROW_HEIGHT)), + ); + window.paint_quad(fill(rule, self.theme.border)); + } + } + } + } + + fn paint_gutter( + &self, + row: usize, + column: usize, + cell_bounds: Bounds, + pen: &Pen, + window: &mut Window, + cx: &mut App, + ) { + let left = cell_bounds.origin.x - px(self.rows().code_left()); + let top = cell_bounds.origin.y; + let muted = self.theme.muted_foreground; + let (origin, marker_left) = match self.rows() { + Rows::Unified(rows) => { + let Row::Line { + origin, + old_number, + new_number, + .. + } = rows[row] + else { + return; + }; + paint_number( + old_number, + left + px(GUTTER_WIDTH), + top, + muted, + pen, + window, + cx, + ); + paint_number( + new_number, + left + px(2. * GUTTER_WIDTH), + top, + muted, + pen, + window, + cx, + ); + (origin, left + px(2. * GUTTER_WIDTH + MARKER_PADDING)) + } + Rows::Split(_) => { + let Some(side) = self.rows().side(row, column) else { + return; + }; + paint_number( + side.number, + left + px(GUTTER_WIDTH), + top, + muted, + pen, + window, + cx, + ); + (side.origin, left + px(GUTTER_WIDTH + MARKER_PADDING)) + } + }; + + let marker_color = line_colors(origin, self.mode, &self.theme).foreground; + let line = pen.shape(marker(origin).into(), marker_color, window); + paint_line(&line, point(marker_left, top), window, cx); + } +} + +impl IntoElement for DiffBody { + type Element = Self; + + fn into_element(self) -> Self::Element { + self + } +} + +impl Element for DiffBody { + type RequestLayoutState = (); + type PrepaintState = Hitbox; + + fn id(&self) -> Option { + None + } + + fn source_location(&self) -> Option<&'static std::panic::Location<'static>> { + None + } + + fn request_layout( + &mut self, + _: Option<&GlobalElementId>, + _: Option<&InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, Self::RequestLayoutState) { + self.visible = self.visible_rows(); + self.texts = self + .visible_cells() + .map(|cell| { + styled_cell( + self.rows(), + cell, + self.content.strings[cell].clone(), + &self.theme, + ) + }) + .collect(); + + let children: Vec = self + .texts + .iter_mut() + .map(|text| text.request_layout(None, None, window, cx).0) + .collect(); + + let width = self.content_width(window); + let style = Style { + flex_direction: FlexDirection::Column, + flex_shrink: 0., + size: size( + width.into(), + px(self.rows().len() as f32 * ROW_HEIGHT).into(), + ), + min_size: size(relative(1.).into(), Length::Auto), + ..Default::default() + }; + + (window.request_layout(style, children, cx), ()) + } + + fn prepaint( + &mut self, + _: Option<&GlobalElementId>, + _: Option<&InspectorElementId>, + bounds: Bounds, + _: &mut Self::RequestLayoutState, + window: &mut Window, + cx: &mut App, + ) -> Self::PrepaintState { + self.cell_bounds = self + .visible_cells() + .map(|cell| self.cell_bounds_at(bounds, cell)) + .collect(); + for (text, cell_bounds) in self.texts.iter_mut().zip(&self.cell_bounds) { + text.prepaint(None, None, *cell_bounds, &mut (), window, cx); + } + + let viewport = self.scroll.bounds(); + let hitbox = window.insert_hitbox(bounds, HitboxBehavior::Normal); + self.selection.register( + TextSelectionRegistration::new(hitbox.clone(), viewport) + .with_scroll_offset(bounds.origin - viewport.origin) + .with_document_order(0) + .with_text_bounds(self.cell_bounds.clone()), + window, + cx, + ); + hitbox + } + + fn paint( + &mut self, + _: Option<&GlobalElementId>, + _: Option<&InspectorElementId>, + bounds: Bounds, + _: &mut Self::RequestLayoutState, + _: &mut Self::PrepaintState, + window: &mut Window, + cx: &mut App, + ) { + let first_cell = self.visible_cells().start; + let runs: Vec = self + .texts + .iter() + .enumerate() + .map(|(offset, text)| { + let cell = first_cell + offset; + TextSelectionRun::new( + self.content.strings[cell].clone(), + text.layout().clone(), + self.cell_bounds[offset], + ) + .with_document_order(cell as u64) + }) + .collect(); + let projection = self.selection.update_runs(&runs, cx); + + let pen = Pen::new(window); + let selected = self.copy_selection(bounds, projection.ranges(), &pen, window, cx); + self.selection.set_fallback_copy_text(selected, cx); + + let columns = self.rows().columns(); + for (offset, row) in self.visible.clone().enumerate() { + let top = self.cell_bounds[offset * columns].origin.y; + self.paint_background(row, bounds, top, window); + + for column in 0..columns { + let cell_offset = offset * columns + column; + let cell_bounds = self.cell_bounds[cell_offset]; + let range = if self.select_all { + Some(0..self.content.strings[first_cell + cell_offset].len()) + } else { + projection.ranges().get(cell_offset).and_then(Clone::clone) + }; + if let Some(range) = range { + paint_selection( + self.texts[cell_offset].layout(), + range, + self.theme.selection, + window, + ); + } + + self.paint_gutter(row, column, cell_bounds, &pen, window, cx); + self.texts[cell_offset].paint( + None, + None, + cell_bounds, + &mut (), + &mut (), + window, + cx, + ); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use domain::FileStatus; + + fn file_header() -> Row { + Row::FileHeader { + path: "src/main.rs".to_string(), + status: FileStatus::Modified, + added: 3, + deleted: 1, + } + } + + fn hunk_header() -> Row { + Row::HunkHeader { + text: "@@ -1,3 +1,4 @@".to_string(), + } + } + + fn line(origin: LineOrigin, content: &str) -> Row { + Row::Line { + origin, + old_number: Some(1), + new_number: Some(1), + content: content.to_string(), + } + } + + fn side(origin: LineOrigin, content: &str) -> SideLine { + SideLine { + number: Some(1), + origin, + content: content.to_string(), + } + } + + fn split_rows() -> Rows { + Rows::Split(vec![ + SplitRow::Full(hunk_header()), + SplitRow::Sides { + left: Some(side(LineOrigin::Deletion, "gone")), + right: None, + }, + ]) + } + + #[test] + fn a_unified_view_has_one_column_and_a_split_view_two() { + let unified = Rows::Unified(vec![file_header(), line(LineOrigin::Context, "keep")]); + let split = split_rows(); + + assert_eq!(unified.columns(), 1); + assert_eq!(unified.cells(), 2); + assert_eq!(unified.code_left(), CODE_LEFT); + + assert_eq!(split.columns(), 2); + assert_eq!(split.cells(), 4); + assert_eq!(split.code_left(), SPLIT_CODE_LEFT); + } + + #[test] + fn an_empty_row_list_is_empty_in_either_view() { + assert!(Rows::Unified(Vec::new()).is_empty()); + assert!(Rows::Split(Vec::new()).is_empty()); + assert!(!Rows::Unified(vec![file_header()]).is_empty()); + } + + #[test] + fn only_a_split_row_with_two_sides_has_a_side() { + let rows = split_rows(); + + assert_eq!(rows.side(0, 0), None, "a full-width row has no side"); + assert_eq!( + rows.side(1, 0).map(|side| side.content.as_str()), + Some("gone") + ); + assert_eq!(rows.side(1, 1), None, "the padded side is absent"); + assert_eq!( + Rows::Unified(vec![file_header()]).side(0, 0), + None, + "a unified row has no sides at all" + ); + } + + #[test] + fn a_row_renders_its_own_kind_of_text() { + assert_eq!( + row_text(&file_header()), + SharedString::from("src/main.rs +3 \u{2212}1") + ); + assert_eq!( + row_text(&hunk_header()), + SharedString::from("@@ -1,3 +1,4 @@") + ); + assert_eq!( + row_text(&line(LineOrigin::Addition, "let x = 1;")), + SharedString::from("let x = 1;") + ); + assert_eq!( + row_text(&Row::Placeholder { + message: "Binary file not shown." + }), + SharedString::from("Binary file not shown.") + ); + } + + #[test] + fn a_unified_cell_is_its_row() { + let rows = Rows::Unified(vec![file_header(), line(LineOrigin::Deletion, "gone")]); + assert_eq!( + cell_text(&rows, 0), + SharedString::from("src/main.rs +3 \u{2212}1") + ); + assert_eq!(cell_text(&rows, 1), SharedString::from("gone")); + } + + #[test] + fn a_full_width_split_row_renders_in_the_first_column_and_blank_in_the_second() { + let rows = split_rows(); + assert_eq!(cell_text(&rows, 0), SharedString::from("@@ -1,3 +1,4 @@")); + assert_eq!(cell_text(&rows, 1), SharedString::default()); + } + + #[test] + fn a_split_row_puts_each_side_in_its_own_column_and_pads_the_missing_one() { + let rows = split_rows(); + assert_eq!(cell_text(&rows, 2), SharedString::from("gone")); + assert_eq!(cell_text(&rows, 3), SharedString::default()); + } + + #[test] + fn a_marker_names_the_origin_and_a_context_line_keeps_the_column_wide() { + assert_eq!(marker(LineOrigin::Addition), "+"); + assert_eq!(marker(LineOrigin::Deletion), "\u{2212}"); + assert_eq!(marker(LineOrigin::Context), " "); + } + + #[test] + fn only_a_changed_line_and_the_two_headers_are_banded() { + let theme = ThemeColor::light(); + let mode = ThemeMode::Light; + + assert_eq!( + row_background(&file_header(), &theme, mode), + Some(theme.secondary) + ); + assert_eq!( + row_background(&hunk_header(), &theme, mode), + Some(theme.muted) + ); + assert_eq!( + row_background( + &Row::Placeholder { + message: "No content changes." + }, + &theme, + mode + ), + None + ); + assert_eq!( + row_background(&line(LineOrigin::Context, "keep"), &theme, mode), + None + ); + assert_eq!( + row_background(&line(LineOrigin::Addition, "new"), &theme, mode), + line_colors(LineOrigin::Addition, mode, &theme).background + ); + } + + #[test] + fn a_column_takes_an_equal_share_of_the_width_from_left_to_right() { + let bounds = Bounds::new(point(px(10.), px(20.)), size(px(200.), px(400.))); + + assert_eq!(column_width(bounds, 1), px(200.)); + assert_eq!(column_width(bounds, 2), px(100.)); + assert_eq!(column_left(bounds, 2, 0), px(10.)); + assert_eq!(column_left(bounds, 2, 1), px(110.)); + } + + #[test] + fn a_cell_sits_at_its_column_past_the_gutters_and_at_its_row() { + let bounds = Bounds::new(point(px(10.), px(20.)), size(px(200.), px(400.))); + + assert_eq!( + bounds_for_cell(bounds, 2, px(60.), 3), + Bounds::new( + point(px(170.), px(20. + ROW_HEIGHT)), + size(px(40.), px(ROW_HEIGHT)) + ), + "cell 3 is the right column of the second row" + ); + assert_eq!( + bounds_for_cell(bounds, 1, px(104.), 0), + Bounds::new(point(px(114.), px(20.)), size(px(96.), px(ROW_HEIGHT))) + ); + } + + #[test] + fn a_column_narrower_than_its_gutters_leaves_no_width_rather_than_a_negative_one() { + let bounds = Bounds::new(point(px(0.), px(0.)), size(px(80.), px(400.))); + assert_eq!(bounds_for_cell(bounds, 2, px(60.), 0).size.width, px(0.)); + } + + #[test] + fn copy_text_joins_the_selected_rows_with_newlines() { + let strings = vec![ + SharedString::from("one"), + SharedString::from("two"), + SharedString::from("three"), + ]; + let ranges = vec![Some(1..3), Some(0..3), None]; + assert_eq!(copy_text(&strings, &ranges, 0, 1), "ne\ntwo"); + } + + #[test] + fn copy_text_keeps_a_blank_row_inside_the_selection() { + let strings = vec![ + SharedString::from("one"), + SharedString::from(""), + SharedString::from("three"), + ]; + let ranges = vec![Some(0..3), None, Some(0..5)]; + assert_eq!(copy_text(&strings, &ranges, 0, 1), "one\n\nthree"); + } + + #[test] + fn copy_text_of_an_empty_projection_is_empty() { + let strings = vec![SharedString::from("one")]; + assert_eq!(copy_text(&strings, &[None], 0, 1), ""); + } + + #[test] + fn copy_text_separates_two_columns_of_the_same_row_with_a_tab() { + let strings = vec![ + SharedString::from("gone"), + SharedString::from("new"), + SharedString::from("keep"), + SharedString::from("keep"), + ]; + let ranges = vec![Some(0..4), Some(0..3), Some(0..4), Some(0..4)]; + assert_eq!(copy_text(&strings, &ranges, 0, 2), "gone\tnew\nkeep\tkeep"); + } + + #[test] + fn copy_text_of_a_padded_column_inside_the_selection_keeps_its_empty_field() { + let strings = vec![ + SharedString::from("gone"), + SharedString::from(""), + SharedString::from("keep"), + SharedString::from("keep"), + ]; + let ranges = vec![Some(0..4), None, Some(0..4), Some(0..4)]; + assert_eq!(copy_text(&strings, &ranges, 0, 2), "gone\t\nkeep\tkeep"); + } + + #[test] + fn copy_text_starts_at_the_first_selected_column_rather_than_at_a_leading_pad() { + let strings = vec![SharedString::from(""), SharedString::from("new")]; + let ranges = vec![None, Some(0..3)]; + assert_eq!(copy_text(&strings, &ranges, 0, 2), "new"); + } + + #[test] + fn copy_text_of_a_span_starting_mid_row_keeps_the_row_boundaries_aligned() { + let strings = vec![ + SharedString::from("new"), + SharedString::from("keep"), + SharedString::from("keep"), + ]; + let ranges = vec![Some(0..3), Some(0..4), Some(0..4)]; + assert_eq!(copy_text(&strings, &ranges, 1, 2), "new\nkeep\tkeep"); + } + + #[test] + fn a_wrapped_selection_covers_the_full_width_of_the_middle_lines() { + let bounds = Bounds::new(point(px(10.), px(20.)), size(px(100.), px(100.))); + let quads = selection_quad_bounds( + point(px(40.), px(20.)), + point(px(30.), px(80.)), + bounds, + px(20.), + ); + + assert_eq!( + quads, + vec![ + Bounds::from_corners(point(px(40.), px(20.)), point(px(110.), px(40.))), + Bounds::from_corners(point(px(10.), px(40.)), point(px(110.), px(80.))), + Bounds::from_corners(point(px(10.), px(80.)), point(px(30.), px(100.))), + ] + ); + } + + #[test] + fn the_window_is_one_screenful_until_the_viewport_has_been_measured() { + assert_eq!(row_window(px(0.), px(0.), 500), 0..UNMEASURED_ROWS); + assert_eq!(row_window(px(0.), px(0.), 12), 0..12); + } + + #[test] + fn the_window_starts_at_the_first_row_the_viewport_cuts_through() { + let window = row_window(px(-3. * ROW_HEIGHT - 4.), px(10. * ROW_HEIGHT), 500); + assert_eq!(window.start, 3); + assert_eq!(window.end, 3 + 12); + } + + #[test] + fn the_window_starts_at_the_first_row_when_the_view_is_over_scrolled_upwards() { + let window = row_window(px(60.), px(10. * ROW_HEIGHT), 500); + assert_eq!(window.start, 0); + assert_eq!(window.end, 12); + } + + #[test] + fn the_window_never_runs_past_the_last_row() { + assert_eq!( + row_window(px(-490. * ROW_HEIGHT), px(10. * ROW_HEIGHT), 500).end, + 500 + ); + assert_eq!( + row_window(px(-900. * ROW_HEIGHT), px(10. * ROW_HEIGHT), 500), + 500..500 + ); + } + + #[test] + fn a_selection_spans_the_rows_its_two_endpoints_land_in() { + assert_eq!( + selected_rows( + px(100.), + px(100. + 2.5 * ROW_HEIGHT), + px(100. + 7.1 * ROW_HEIGHT), + 20 + ), + Some(2..=7) + ); + } + + #[test] + fn a_selection_reaching_past_the_body_is_clamped_to_it() { + assert_eq!( + selected_rows(px(100.), px(-500.), px(100. + 900. * ROW_HEIGHT), 20), + Some(0..=19) + ); + } + + #[test] + fn a_selection_entirely_outside_the_body_spans_no_rows() { + assert_eq!(selected_rows(px(100.), px(-500.), px(-400.), 20), None); + assert_eq!( + selected_rows( + px(100.), + px(100. + 40. * ROW_HEIGHT), + px(100. + 50. * ROW_HEIGHT), + 20 + ), + None + ); + assert_eq!(selected_rows(px(100.), px(100.), px(200.), 0), None); + } + + #[test] + fn a_row_holding_both_endpoints_is_bounded_by_them() { + let band = selection_band(px(36.), point(px(80.), px(40.)), point(px(20.), px(50.))); + assert_eq!(band, Some(Band::Between(px(20.), px(80.)))); + } + + #[test] + fn the_first_row_of_a_selection_runs_from_its_endpoint_to_the_end_of_the_line() { + let band = selection_band(px(36.), point(px(80.), px(40.)), point(px(20.), px(200.))); + assert_eq!(band, Some(Band::From(px(80.)))); + } + + #[test] + fn the_last_row_of_a_selection_runs_from_the_start_of_the_line_to_its_endpoint() { + let band = selection_band(px(36.), point(px(80.), px(-10.)), point(px(20.), px(40.))); + assert_eq!(band, Some(Band::To(px(20.)))); + } + + #[test] + fn a_row_between_the_endpoints_is_selected_whole() { + let band = selection_band(px(36.), point(px(80.), px(-10.)), point(px(20.), px(200.))); + assert_eq!(band, Some(Band::Whole)); + } + + #[test] + fn a_drag_upwards_bands_its_rows_exactly_as_the_same_drag_downwards() { + let low = point(px(80.), px(40.)); + let high = point(px(20.), px(200.)); + let above = point(px(20.), px(-10.)); + + assert_eq!( + selection_band(px(36.), high, low), + selection_band(px(36.), low, high) + ); + assert_eq!( + selection_band(px(36.), low, above), + selection_band(px(36.), above, low) + ); + assert_eq!( + selection_band(px(36.), high, above), + selection_band(px(36.), above, high) + ); + assert_eq!( + selection_band(px(36.), high, low), + Some(Band::From(px(80.))) + ); + assert_eq!(selection_band(px(36.), low, above), Some(Band::To(px(80.)))); + assert_eq!(selection_band(px(36.), high, above), Some(Band::Whole)); + } + + #[test] + fn a_row_outside_the_selection_has_no_band() { + let above = selection_band(px(0.), point(px(80.), px(40.)), point(px(20.), px(50.))); + let below = selection_band(px(90.), point(px(80.), px(40.)), point(px(20.), px(50.))); + assert_eq!(above, None); + assert_eq!(below, None); + } + + #[test] + fn a_whole_row_needs_no_shaping_and_an_empty_one_selects_nothing() { + let shape = || unreachable!("a whole row must not be shaped"); + assert_eq!( + selected_range("one", Band::Whole, px(0.), shape), + Some(0..3) + ); + assert_eq!(selected_range("", Band::Whole, px(0.), shape), None); + } +} diff --git a/crates/ui/src/detail/diff/mod.rs b/crates/ui/src/detail/diff/mod.rs new file mode 100644 index 0000000..a870240 --- /dev/null +++ b/crates/ui/src/detail/diff/mod.rs @@ -0,0 +1,187 @@ +//! Renders a `Patch` as rows painted by one custom element, instead of feeding a +//! reconstructed unified diff to a code editor. +//! +//! The editor gave selection, syntax highlighting and virtualised scrolling for free, and +//! it also fixed three things this view needs to control. Its gutter is always +//! `buffer_line + 1` in a single column, so a GitHub old/new pair is unreachable; it has no +//! full-width row background outside the cursor line, so an addition's tint hugs the glyphs +//! and a blank added line gets none at all; and it has no per-line element hook to work +//! around either. The `+`/`-` markers were carrying the signal the colour only half +//! carried. All three follow from the diff being one text document, so the rows are built +//! here instead — see the design note under `docs/superpowers/specs/`. +//! +//! A row carries one cell in [`DiffViewMode::Unified`] and two in [`DiffViewMode::Split`], +//! which is the only difference between the two views: [`body::Rows`] answers how many +//! columns a body has, and every horizontal position — the content width, a cell's bounds, +//! a gutter's origin — is that column's share of the element's width. A file, hunk or +//! placeholder row keeps one full-width cell in either view, so the columns stay uniform +//! and a cell is `row * columns + column` rather than a lookup. Because a column is laid +//! out against `content_width / columns` and the width is measured over every cell, a +//! full-width header still fits in the half it is drawn in. +//! +//! Selection comes back through `gpui-base`'s window-level participant system rather than +//! from the editor. [`body`] is the element that joins it: it registers one participant and +//! declares one run per cell on screen, left before right within a row, and only the code +//! text becomes a run — the gutters and the marker are painted directly and never +//! registered, which is what keeps line numbers and markers out of the clipboard. The rows +//! scroll on both axes rather than soft-wrapping, +//! matching GitHub. `restrict_scroll_to_axis` still earns its place here, but not for the +//! reason it usually does: the container's `overflow_scroll()` puts both axes in +//! `Overflow::Scroll`, and gpui's vertical-onto-horizontal remap (`div.rs:3220-3224`, +//! `:3229-3233`) only fires when one axis is *not* `Overflow::Scroll`, so that remap is +//! unreachable here with or without the flag. What the flag does is axis-lock a precise +//! trackpad gesture through `ongoing_scroll.filter(..)` (`div.rs:3209-3216`), which is live +//! because `track_scroll` populates `ongoing_scroll` from the tracked handle (`div.rs:2165`). +//! +//! "On screen" is decided in `request_layout`, not in `paint`, because laying a row out is +//! the expensive half and nothing downstream can be narrowed without it. That ordering is +//! forced rather than chosen: every `TextLayout` accessor panics on a row that was skipped, +//! `len` and `line_height` on the cell the measure closure fills +//! (`gpui/src/elements/text.rs:935-942`) and `bounds` and `position_for_index` on that one +//! and on the cell prepaint fills as well (`:864-871`, `:930-932`). `selection_range_for_run` +//! reads `layout.len()` on *every* run it is handed, before any geometry +//! (`text_selection.rs:388`), so a row whose `StyledText` was skipped this frame cannot be +//! declared as a run at all — declaring it panics on the first scroll that has a live +//! selection. +//! +//! The copied text is therefore not read off that projection. A selection dragged past the +//! bottom edge would come back holding only the rows that happened to be on screen, and +//! nothing would report that it had been cut short. `body::DiffBody::copy_selection` derives +//! the row span from the selection's own window points instead — `body::selected_rows` — and +//! asks `body::selection_band` and `body::selected_range` for each cell's byte range, shaping +//! only the cells of the at most two rows whose ends the selection cuts through; every row +//! between them is whole. A band is a property of the row, so both cells of a row share it +//! and a cell's own column offset is what turns it into a range — which is exactly what +//! `point_in_selection_band` does to two runs that share a `y`, so the arithmetic and the +//! projection still agree cell for cell. Endpoints survive scrolling because `gpui-base` +//! stores them relative to `bounds.origin + scroll_offset`, and the participant reports the +//! two so that their sum is this element's own origin, which already carries the scroll: a +//! point off the top of the viewport is then a negative `y` rather than a lost one. Rows +//! that *are* on screen keep the projection's +//! own range, so what is highlighted and what is copied cannot drift apart. Select All is +//! the one selection with no window points to derive anything from — it is participant-local +//! (`set_local_selection`), so `copy_selection` answers the whole document for it directly +//! and every visible cell is highlighted whole. +//! +//! What stays unwindowed is the content width: `body::DiffBody::content_width` shapes every +//! cell on every layout pass, because the horizontal scroll extent has to consider rows that +//! are not on screen or the scrollbar would resize as the view scrolls vertically, and +//! because a cell is laid out against a column width that provably exceeds every cell's +//! natural width — which is what keeps a row one line tall and `ROW_HEIGHT` true. That is +//! affordable only because the text it measures is not rebuilt with it: [`DiffContent`] +//! holds the rows and one [`gpui::SharedString`] per cell, derived by [`content`] when the +//! patch or the view mode changes and handed to the element behind an `Rc` thereafter. So +//! after the first frame each cell is a hit in gpui's line-layout cache and the steady-state +//! cost is a hash of bytes that are already there, with no allocation and no reshape. +//! +//! Copying depends on a fact `gpui-base`'s own doc comment does not state. A participant's +//! runs concatenate with no separator when `update_runs` projects them +//! (`text_selection.rs:593`); only whole participants are joined with `"\n"` +//! (`resolve_copy_items`, `:513`). So the separators are computed here, in +//! [`body::copy_text`], and published through `TextSelectionHandle::set_fallback_copy_text` +//! right after `update_runs` — which works only because that setter also clears +//! `projected_copy_text` (`:559`), the field `update_runs` just set, so `copy_item` falls +//! through to our fallback instead of the unseparated projection. `gpui-base` tracks a git +//! default branch pinned solely by `Cargo.lock`; if that clearing behaviour ever moves, +//! copying silently goes back to gluing rows together with no separator, and nothing fails +//! to say so. +//! +//! A row ends with `"\n"` and a column with `"\t"`, which makes a split copy the table the +//! reader is looking at. The alternative — a newline between the two columns as well — was +//! rejected because the band rule takes both cells of every row a selection passes through +//! whole, so a drag down one column still copies the other; newlines would interleave the +//! two sides and repeat every context line, and neither separator can yield compilable code +//! out of a two-column view. Narrowing the copy to one column was rejected for a harder +//! reason: the highlight comes from the projection, and dropping a cell the projection +//! selected is exactly the drift between clipboard and screen the paragraph above exists to +//! prevent. + +mod body; +mod model; +mod pairing; +mod palette; +mod split; + +use std::rc::Rc; + +use domain::Patch; +use gpui::{ + AnyElement, App, InteractiveElement as _, IntoElement, ParentElement as _, ScrollHandle, + SharedString, StatefulInteractiveElement as _, Styled as _, div, px, +}; +use gpui_base::TextSelectionHandle; +use gpui_component::{ + ActiveTheme as _, + scroll::{ScrollableElement as _, ScrollbarAxis}, +}; + +use crate::diff_view_mode::DiffViewMode; + +use body::{ROW_HEIGHT, Rows, body, cell_strings}; +use model::rows; +use split::split_rows; + +pub(super) struct DiffContent { + rows: Rows, + strings: Vec, +} + +impl DiffContent { + pub(super) fn is_empty(&self) -> bool { + self.rows.is_empty() + } +} + +pub(super) fn content(patch: &Patch, mode: DiffViewMode) -> DiffContent { + let rows = match mode { + DiffViewMode::Unified => Rows::Unified(rows(patch)), + DiffViewMode::Split => Rows::Split(split_rows(patch)), + }; + let strings = cell_strings(&rows); + DiffContent { rows, strings } +} + +pub(super) fn render( + content: Option<&Rc>, + select_all: bool, + selection: &TextSelectionHandle, + scroll: &ScrollHandle, + cx: &App, +) -> AnyElement { + let Some(content) = content.filter(|content| !content.is_empty()) else { + return div() + .size_full() + .flex() + .items_center() + .justify_center() + .text_color(cx.theme().muted_foreground) + .child("This commit changes nothing.") + .into_any_element(); + }; + + let theme = cx.theme(); + div() + .relative() + .size_full() + .child( + div() + .id("detail-diff-scroll") + .size_full() + .overflow_scroll() + .restrict_scroll_to_axis() + .track_scroll(scroll) + .font_family(theme.mono_font_family.clone()) + .text_size(theme.mono_font_size) + .line_height(px(ROW_HEIGHT)) + .child(body( + Rc::clone(content), + select_all, + selection.clone(), + scroll.clone(), + theme.colors, + theme.mode, + )), + ) + .scrollbar(scroll, ScrollbarAxis::Both) + .into_any_element() +} diff --git a/crates/ui/src/detail/diff/model.rs b/crates/ui/src/detail/diff/model.rs new file mode 100644 index 0000000..f691e5b --- /dev/null +++ b/crates/ui/src/detail/diff/model.rs @@ -0,0 +1,342 @@ +use std::fmt::Write as _; + +use domain::{DiffLine, FilePatch, FileStatus, Hunk, LineOrigin, Patch}; + +use crate::detail::format; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) enum Row { + FileHeader { + path: String, + status: FileStatus, + added: usize, + deleted: usize, + }, + HunkHeader { + text: String, + }, + Line { + origin: LineOrigin, + old_number: Option, + new_number: Option, + content: String, + }, + Placeholder { + message: &'static str, + }, +} + +pub(super) fn rows(patch: &Patch) -> Vec { + let mut rows = Vec::new(); + for file in &patch.files { + rows.push(file_header(file)); + push_body(&mut rows, file); + } + rows +} + +pub(super) fn file_header(file: &FilePatch) -> Row { + Row::FileHeader { + path: header_path(file), + status: file.status.clone(), + added: file.added_lines(), + deleted: file.deleted_lines(), + } +} + +pub(super) fn header_line(path: &str, status: &FileStatus, added: usize, deleted: usize) -> String { + let mut text = path.to_string(); + if let Some(label) = status_label(status) { + let _ = write!(text, " {label}"); + } + let _ = write!(text, " +{added} \u{2212}{deleted}"); + text +} + +fn header_path(file: &FilePatch) -> String { + let moved = matches!( + file.status, + FileStatus::Renamed { .. } | FileStatus::Copied { .. } + ); + match (&file.old_path, &file.new_path) { + (Some(old), Some(new)) if moved && old != new => { + format!("{} \u{2192} {}", old.display(), new.display()) + } + _ => file + .display_path() + .map(|path| path.display().to_string()) + .unwrap_or_default(), + } +} + +fn status_label(status: &FileStatus) -> Option { + match status { + FileStatus::Modified => None, + FileStatus::Added => Some("added".to_string()), + FileStatus::Deleted => Some("deleted".to_string()), + FileStatus::Renamed { similarity } => Some(format!("renamed {similarity}%")), + FileStatus::Copied { similarity } => Some(format!("copied {similarity}%")), + FileStatus::TypeChanged => Some("type changed".to_string()), + } +} + +pub(super) fn placeholder(file: &FilePatch) -> Option { + if file.is_binary { + return Some(Row::Placeholder { + message: "Binary file not shown.", + }); + } + if file.hunks.is_empty() { + return Some(Row::Placeholder { + message: "No content changes.", + }); + } + None +} + +pub(super) fn hunk_header(hunk: &Hunk) -> Row { + Row::HunkHeader { + text: format::hunk_heading(hunk), + } +} + +fn push_body(rows: &mut Vec, file: &FilePatch) { + if let Some(placeholder) = placeholder(file) { + rows.push(placeholder); + return; + } + for hunk in &file.hunks { + rows.push(hunk_header(hunk)); + rows.extend(hunk.lines.iter().map(line_row)); + } +} + +fn line_row(line: &DiffLine) -> Row { + Row::Line { + origin: line.origin, + old_number: line.old_number, + new_number: line.new_number, + content: line.content.clone(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use domain::{FileStatus, Hunk}; + use std::path::PathBuf; + + fn line(origin: LineOrigin, old: Option, new: Option, content: &str) -> DiffLine { + DiffLine { + origin, + old_number: old, + new_number: new, + content: content.to_string(), + } + } + + fn file(hunks: Vec, is_binary: bool) -> FilePatch { + FilePatch { + old_path: Some(PathBuf::from("src/main.rs")), + new_path: Some(PathBuf::from("src/main.rs")), + status: FileStatus::Modified, + is_binary, + hunks, + } + } + + fn moved(old: &str, new: &str, status: FileStatus) -> FilePatch { + FilePatch { + old_path: Some(PathBuf::from(old)), + new_path: Some(PathBuf::from(new)), + status, + is_binary: false, + hunks: Vec::new(), + } + } + + fn hunk(lines: Vec) -> Hunk { + Hunk { + old_start: 1, + old_lines: 1, + new_start: 1, + new_lines: 1, + heading: String::new(), + lines, + } + } + + #[test] + fn a_modified_file_yields_a_header_a_hunk_header_and_one_row_per_line() { + let patch = Patch { + files: vec![file( + vec![hunk(vec![ + line(LineOrigin::Context, Some(1), Some(1), "keep"), + line(LineOrigin::Deletion, Some(2), None, "gone"), + line(LineOrigin::Addition, None, Some(2), "new"), + ])], + false, + )], + }; + + let rows = rows(&patch); + + assert!(matches!(rows[0], Row::FileHeader { .. })); + assert!(matches!(rows[1], Row::HunkHeader { .. })); + assert_eq!(rows.len(), 5); + } + + #[test] + fn a_line_row_carries_both_numbers_and_the_bare_content() { + let patch = Patch { + files: vec![file( + vec![hunk(vec![line( + LineOrigin::Deletion, + Some(7), + None, + "-not a marker", + )])], + false, + )], + }; + + let rows = rows(&patch); + + assert_eq!( + rows[2], + Row::Line { + origin: LineOrigin::Deletion, + old_number: Some(7), + new_number: None, + content: "-not a marker".to_string(), + }, + "content is stored bare by the parser and must not be re-marked here" + ); + } + + #[test] + fn a_binary_file_yields_a_placeholder_instead_of_lines() { + let patch = Patch { + files: vec![file(Vec::new(), true)], + }; + let rows = rows(&patch); + assert_eq!( + rows[1], + Row::Placeholder { + message: "Binary file not shown." + } + ); + } + + #[test] + fn a_file_with_no_hunks_yields_a_no_change_placeholder() { + let patch = Patch { + files: vec![file(Vec::new(), false)], + }; + let rows = rows(&patch); + assert_eq!( + rows[1], + Row::Placeholder { + message: "No content changes." + } + ); + } + + #[test] + fn a_rename_keeps_both_paths_and_its_similarity() { + let file = moved( + "src/old.rs", + "src/new.rs", + FileStatus::Renamed { similarity: 87 }, + ); + let Row::FileHeader { + path, + status, + added, + deleted, + } = file_header(&file) + else { + panic!("a file yields a header row"); + }; + + assert_eq!(path, "src/old.rs \u{2192} src/new.rs"); + assert_eq!(status, FileStatus::Renamed { similarity: 87 }); + assert_eq!( + header_line(&path, &status, added, deleted), + "src/old.rs \u{2192} src/new.rs renamed 87% +0 \u{2212}0" + ); + } + + #[test] + fn a_copy_reads_as_a_copy_rather_than_as_a_rename() { + let file = moved( + "src/old.rs", + "src/copy.rs", + FileStatus::Copied { similarity: 100 }, + ); + let Row::FileHeader { + path, + status, + added, + deleted, + } = file_header(&file) + else { + panic!("a file yields a header row"); + }; + + assert_eq!(path, "src/old.rs \u{2192} src/copy.rs"); + assert_eq!( + header_line(&path, &status, added, deleted), + "src/old.rs \u{2192} src/copy.rs copied 100% +0 \u{2212}0" + ); + } + + #[test] + fn a_rename_that_only_changed_the_content_shows_one_path() { + let file = moved( + "src/a.rs", + "src/a.rs", + FileStatus::Renamed { similarity: 100 }, + ); + let Row::FileHeader { path, .. } = file_header(&file) else { + panic!("a file yields a header row"); + }; + assert_eq!(path, "src/a.rs"); + } + + #[test] + fn a_modified_file_carries_no_status_label() { + let patch = Patch { + files: vec![file( + vec![hunk(vec![line(LineOrigin::Addition, None, Some(1), "new")])], + false, + )], + }; + let Row::FileHeader { + path, + status, + added, + deleted, + } = rows(&patch).remove(0) + else { + panic!("the first row is the file header"); + }; + + assert_eq!( + header_line(&path, &status, added, deleted), + "src/main.rs +1 \u{2212}0" + ); + } + + #[test] + fn every_file_contributes_its_own_header() { + let patch = Patch { + files: vec![file(Vec::new(), true), file(Vec::new(), true)], + }; + let headers = rows(&patch) + .iter() + .filter(|r| matches!(r, Row::FileHeader { .. })) + .count(); + assert_eq!(headers, 2); + } +} diff --git a/crates/ui/src/detail/diff/pairing.rs b/crates/ui/src/detail/diff/pairing.rs new file mode 100644 index 0000000..ca81122 --- /dev/null +++ b/crates/ui/src/detail/diff/pairing.rs @@ -0,0 +1,182 @@ +use domain::{DiffLine, LineOrigin}; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) struct SideLine { + pub number: Option, + pub origin: LineOrigin, + pub content: String, +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(super) struct SplitRow { + pub left: Option, + pub right: Option, +} + +pub(super) fn pair(lines: &[DiffLine]) -> Vec { + let mut rows = Vec::new(); + let mut deletions: Vec<&DiffLine> = Vec::new(); + let mut additions: Vec<&DiffLine> = Vec::new(); + + for line in lines { + match line.origin { + LineOrigin::Deletion => deletions.push(line), + LineOrigin::Addition => additions.push(line), + LineOrigin::Context => { + flush(&mut rows, &mut deletions, &mut additions); + rows.push(SplitRow { + left: Some(side(line, line.old_number)), + right: Some(side(line, line.new_number)), + }); + } + } + } + flush(&mut rows, &mut deletions, &mut additions); + rows +} + +fn flush(rows: &mut Vec, deletions: &mut Vec<&DiffLine>, additions: &mut Vec<&DiffLine>) { + let paired = deletions.len().max(additions.len()); + for index in 0..paired { + rows.push(SplitRow { + left: deletions.get(index).map(|line| side(line, line.old_number)), + right: additions.get(index).map(|line| side(line, line.new_number)), + }); + } + deletions.clear(); + additions.clear(); +} + +fn side(line: &DiffLine, number: Option) -> SideLine { + SideLine { + number, + origin: line.origin, + content: line.content.clone(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn line(origin: LineOrigin, old: Option, new: Option, content: &str) -> DiffLine { + DiffLine { + origin, + old_number: old, + new_number: new, + content: content.to_string(), + } + } + + fn deletion(number: u32, content: &str) -> DiffLine { + line(LineOrigin::Deletion, Some(number), None, content) + } + + fn addition(number: u32, content: &str) -> DiffLine { + line(LineOrigin::Addition, None, Some(number), content) + } + + fn context(number: u32, content: &str) -> DiffLine { + line(LineOrigin::Context, Some(number), Some(number), content) + } + + #[test] + fn a_context_line_is_the_same_on_both_sides() { + let rows = pair(&[context(1, "keep")]); + assert_eq!(rows.len(), 1); + assert_eq!( + rows[0].left.as_ref().map(|s| s.content.as_str()), + Some("keep") + ); + assert_eq!( + rows[0].right.as_ref().map(|s| s.content.as_str()), + Some("keep") + ); + } + + #[test] + fn an_equal_length_replacement_pairs_line_for_line() { + let rows = pair(&[ + deletion(1, "a"), + deletion(2, "b"), + addition(1, "x"), + addition(2, "y"), + ]); + assert_eq!(rows.len(), 2); + assert_eq!(rows[0].left.as_ref().map(|s| s.content.as_str()), Some("a")); + assert_eq!( + rows[0].right.as_ref().map(|s| s.content.as_str()), + Some("x") + ); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("b")); + assert_eq!( + rows[1].right.as_ref().map(|s| s.content.as_str()), + Some("y") + ); + } + + #[test] + fn more_additions_than_deletions_pads_the_left() { + let rows = pair(&[deletion(1, "a"), addition(1, "x"), addition(2, "y")]); + assert_eq!(rows.len(), 2); + assert!( + rows[1].left.is_none(), + "the extra addition has nothing to pair with" + ); + assert_eq!( + rows[1].right.as_ref().map(|s| s.content.as_str()), + Some("y") + ); + } + + #[test] + fn more_deletions_than_additions_pads_the_right() { + let rows = pair(&[deletion(1, "a"), deletion(2, "b"), addition(1, "x")]); + assert_eq!(rows.len(), 2); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("b")); + assert!(rows[1].right.is_none()); + } + + #[test] + fn a_pure_addition_leaves_the_left_side_empty() { + let rows = pair(&[addition(1, "x"), addition(2, "y")]); + assert_eq!(rows.len(), 2); + assert!(rows.iter().all(|row| row.left.is_none())); + } + + #[test] + fn a_pure_deletion_leaves_the_right_side_empty() { + let rows = pair(&[deletion(1, "a")]); + assert_eq!(rows.len(), 1); + assert!(rows[0].right.is_none()); + } + + #[test] + fn a_run_is_flushed_when_a_context_line_ends_it() { + let rows = pair(&[deletion(1, "a"), addition(1, "x"), context(2, "keep")]); + assert_eq!(rows.len(), 2); + assert_eq!( + rows[1].left.as_ref().map(|s| s.content.as_str()), + Some("keep") + ); + } + + #[test] + fn a_run_at_the_very_end_is_flushed_without_trailing_context() { + let rows = pair(&[context(1, "keep"), deletion(2, "a")]); + assert_eq!(rows.len(), 2, "the trailing run must not be dropped"); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("a")); + } + + #[test] + fn an_empty_hunk_yields_no_rows() { + assert!(pair(&[]).is_empty()); + } + + #[test] + fn a_side_line_keeps_its_own_number() { + let rows = pair(&[deletion(7, "a"), addition(9, "x")]); + assert_eq!(rows[0].left.as_ref().and_then(|s| s.number), Some(7)); + assert_eq!(rows[0].right.as_ref().and_then(|s| s.number), Some(9)); + } +} diff --git a/crates/ui/src/detail/diff/palette.rs b/crates/ui/src/detail/diff/palette.rs new file mode 100644 index 0000000..74015ff --- /dev/null +++ b/crates/ui/src/detail/diff/palette.rs @@ -0,0 +1,110 @@ +use domain::LineOrigin; +use gpui::{Hsla, rgb}; +use gpui_component::{ThemeColor, ThemeMode}; + +const LIGHT_ADDITION_BACKGROUND: u32 = 0xdafbe1; +const LIGHT_DELETION_BACKGROUND: u32 = 0xffebe9; + +/// A tint has to clear two bars, and only one of them is legibility. +/// +/// The light pair is unusable under Catppuccin Frappé: it is pale, and the code above it +/// is painted in `theme.foreground`, `#c6d0f5` — pale on pale. But an earlier dark green +/// picked purely for legibility against that text landed at 1.00:1 against Frappé's own +/// `#303446` background: identical luminance, so the band was invisible and the tint may +/// as well not have been drawn. This value clears both — 1.58:1 against the background so +/// the band reads, 5.11:1 under `#c6d0f5` so the code stays legible on it. +const DARK_ADDITION_BACKGROUND: u32 = 0x355a40; + +/// Mirrors [`DARK_ADDITION_BACKGROUND`]'s two-bar reasoning: 1.30:1 against Frappé's +/// background, 6.20:1 under the same `#c6d0f5`. A deletion can afford a darker plate than +/// an addition because nothing has to read *as* red on it — only the marker is tinted, and +/// it is drawn in `theme.red` rather than in the plate's own hue. +const DARK_DELETION_BACKGROUND: u32 = 0x5f3c45; + +/// The plate a line of a given origin sits on, and the colour of its `+`/`−` marker. +/// +/// `foreground` is the marker's colour alone. The code itself is painted in +/// `theme.foreground` whatever its origin — which is the measurement the two dark +/// constants above are chosen against. +pub(super) struct LineColors { + pub background: Option, + pub foreground: Hsla, +} + +pub(super) fn line_colors(origin: LineOrigin, mode: ThemeMode, theme: &ThemeColor) -> LineColors { + let background = match (origin, mode.is_dark()) { + (LineOrigin::Context, _) => None, + (LineOrigin::Addition, false) => Some(rgb(LIGHT_ADDITION_BACKGROUND).into()), + (LineOrigin::Addition, true) => Some(rgb(DARK_ADDITION_BACKGROUND).into()), + (LineOrigin::Deletion, false) => Some(rgb(LIGHT_DELETION_BACKGROUND).into()), + (LineOrigin::Deletion, true) => Some(rgb(DARK_DELETION_BACKGROUND).into()), + }; + let foreground = match origin { + LineOrigin::Context => theme.muted_foreground, + LineOrigin::Addition => theme.green, + LineOrigin::Deletion => theme.red, + }; + LineColors { + background, + foreground, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn light_mode_uses_the_exact_given_hex_values() { + let theme = ThemeColor::light(); + assert_eq!( + line_colors(LineOrigin::Addition, ThemeMode::Light, &theme).background, + Some(rgb(0xdafbe1).into()) + ); + assert_eq!( + line_colors(LineOrigin::Deletion, ThemeMode::Light, &theme).background, + Some(rgb(0xffebe9).into()) + ); + } + + #[test] + fn a_marker_is_tinted_by_its_origin_rather_than_by_the_code_colour() { + let theme = ThemeColor::dark(); + let added = line_colors(LineOrigin::Addition, ThemeMode::Dark, &theme).foreground; + let deleted = line_colors(LineOrigin::Deletion, ThemeMode::Dark, &theme).foreground; + let context = line_colors(LineOrigin::Context, ThemeMode::Dark, &theme).foreground; + + assert_eq!(added, theme.green); + assert_eq!(deleted, theme.red); + assert_eq!(context, theme.muted_foreground); + assert_ne!(added, theme.foreground); + assert_ne!(deleted, theme.foreground); + } + + #[test] + fn a_context_line_has_no_background() { + let theme = ThemeColor::light(); + assert!( + line_colors(LineOrigin::Context, ThemeMode::Light, &theme) + .background + .is_none() + ); + } + + #[test] + fn an_addition_and_a_deletion_do_not_share_a_background() { + let theme = ThemeColor::light(); + let added = line_colors(LineOrigin::Addition, ThemeMode::Light, &theme).background; + let deleted = line_colors(LineOrigin::Deletion, ThemeMode::Light, &theme).background; + assert!(added.is_some() && deleted.is_some()); + assert_ne!(added, deleted); + } + + #[test] + fn dark_mode_does_not_reuse_the_light_pair() { + let theme = ThemeColor::dark(); + let light = line_colors(LineOrigin::Addition, ThemeMode::Light, &theme).background; + let dark = line_colors(LineOrigin::Addition, ThemeMode::Dark, &theme).background; + assert_ne!(light, dark); + } +} diff --git a/crates/ui/src/detail/diff/split.rs b/crates/ui/src/detail/diff/split.rs new file mode 100644 index 0000000..f711c83 --- /dev/null +++ b/crates/ui/src/detail/diff/split.rs @@ -0,0 +1,276 @@ +use domain::Patch; + +use super::model::{Row, file_header, hunk_header, placeholder}; +use super::pairing::{SideLine, pair}; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) enum SplitRow { + Full(Row), + Sides { + left: Option, + right: Option, + }, +} + +pub(super) fn split_rows(patch: &Patch) -> Vec { + let mut rows = Vec::new(); + for file in &patch.files { + rows.push(SplitRow::Full(file_header(file))); + if let Some(placeholder) = placeholder(file) { + rows.push(SplitRow::Full(placeholder)); + continue; + } + for hunk in &file.hunks { + rows.push(SplitRow::Full(hunk_header(hunk))); + rows.extend(pair(&hunk.lines).into_iter().map(|row| SplitRow::Sides { + left: row.left, + right: row.right, + })); + } + } + rows +} + +#[cfg(test)] +mod tests { + use super::*; + use domain::{DiffLine, FilePatch, FileStatus, Hunk, LineOrigin}; + use std::path::PathBuf; + + fn line(origin: LineOrigin, old: Option, new: Option, content: &str) -> DiffLine { + DiffLine { + origin, + old_number: old, + new_number: new, + content: content.to_string(), + } + } + + fn hunk(lines: Vec) -> Hunk { + Hunk { + old_start: 1, + old_lines: 1, + new_start: 1, + new_lines: 1, + heading: String::new(), + lines, + } + } + + fn file(path: &str, hunks: Vec, is_binary: bool) -> FilePatch { + FilePatch { + old_path: Some(PathBuf::from(path)), + new_path: Some(PathBuf::from(path)), + status: FileStatus::Modified, + is_binary, + hunks, + } + } + + fn replacement() -> Hunk { + hunk(vec![ + line(LineOrigin::Deletion, Some(1), None, "gone"), + line(LineOrigin::Addition, None, Some(1), "new"), + ]) + } + + fn moved(old: &str, new: &str, status: FileStatus) -> FilePatch { + FilePatch { + old_path: Some(PathBuf::from(old)), + new_path: Some(PathBuf::from(new)), + status, + is_binary: false, + hunks: Vec::new(), + } + } + + fn paths(rows: &[SplitRow]) -> Vec { + rows.iter() + .filter_map(|row| match row { + SplitRow::Full(Row::FileHeader { path, .. }) => Some(path.clone()), + _ => None, + }) + .collect() + } + + #[test] + fn a_two_file_patch_yields_both_files_in_order_each_behind_its_own_header() { + let patch = Patch { + files: vec![ + file("src/a.rs", vec![replacement()], false), + file("src/b.rs", vec![replacement()], false), + ], + }; + + let rows = split_rows(&patch); + + assert_eq!(paths(&rows), vec!["src/a.rs", "src/b.rs"]); + assert!(matches!(rows[1], SplitRow::Full(Row::HunkHeader { .. }))); + assert!(matches!(rows[2], SplitRow::Sides { .. })); + assert_eq!(rows.len(), 6); + } + + #[test] + fn a_replacement_pairs_the_deletion_against_the_addition_on_one_row() { + let patch = Patch { + files: vec![file("src/a.rs", vec![replacement()], false)], + }; + + let rows = split_rows(&patch); + + let SplitRow::Sides { left, right } = &rows[2] else { + panic!("the third row is the paired line"); + }; + assert_eq!( + left.as_ref().map(|side| side.content.as_str()), + Some("gone") + ); + assert_eq!( + right.as_ref().map(|side| side.content.as_str()), + Some("new") + ); + } + + #[test] + fn a_pure_addition_leaves_the_left_column_empty_rather_than_collapsing_the_row() { + let patch = Patch { + files: vec![file( + "src/a.rs", + vec![hunk(vec![line(LineOrigin::Addition, None, Some(1), "new")])], + false, + )], + }; + + let rows = split_rows(&patch); + + let SplitRow::Sides { left, right } = &rows[2] else { + panic!("the third row is the added line"); + }; + assert!(left.is_none()); + assert!(right.is_some()); + } + + #[test] + fn a_pure_deletion_leaves_the_right_column_empty_rather_than_collapsing_the_row() { + let patch = Patch { + files: vec![file( + "src/a.rs", + vec![hunk(vec![line( + LineOrigin::Deletion, + Some(1), + None, + "gone", + )])], + false, + )], + }; + + let rows = split_rows(&patch); + + let SplitRow::Sides { left, right } = &rows[2] else { + panic!("the third row is the deleted line"); + }; + assert!(left.is_some()); + assert!(right.is_none()); + } + + #[test] + fn a_binary_file_yields_a_full_width_placeholder_instead_of_columns() { + let patch = Patch { + files: vec![file("src/a.png", Vec::new(), true)], + }; + + let rows = split_rows(&patch); + + assert_eq!( + rows[1], + SplitRow::Full(Row::Placeholder { + message: "Binary file not shown." + }) + ); + assert_eq!(rows.len(), 2); + } + + #[test] + fn a_file_with_no_hunks_yields_a_no_change_placeholder() { + let patch = Patch { + files: vec![file("src/a.rs", Vec::new(), false)], + }; + + let rows = split_rows(&patch); + + assert_eq!( + rows[1], + SplitRow::Full(Row::Placeholder { + message: "No content changes." + }) + ); + } + + #[test] + fn a_rename_with_no_content_change_still_names_both_paths() { + let patch = Patch { + files: vec![moved( + "src/old.rs", + "src/new.rs", + FileStatus::Renamed { similarity: 87 }, + )], + }; + + let rows = split_rows(&patch); + + assert_eq!(paths(&rows), vec!["src/old.rs \u{2192} src/new.rs"]); + assert_eq!( + rows[0], + SplitRow::Full(Row::FileHeader { + path: "src/old.rs \u{2192} src/new.rs".to_string(), + status: FileStatus::Renamed { similarity: 87 }, + added: 0, + deleted: 0, + }) + ); + assert_eq!( + rows[1], + SplitRow::Full(Row::Placeholder { + message: "No content changes." + }) + ); + } + + #[test] + fn a_copy_carries_its_own_status_into_the_header_row() { + let patch = Patch { + files: vec![moved( + "src/old.rs", + "src/copy.rs", + FileStatus::Copied { similarity: 100 }, + )], + }; + + let rows = split_rows(&patch); + + assert_eq!( + rows[0], + SplitRow::Full(Row::FileHeader { + path: "src/old.rs \u{2192} src/copy.rs".to_string(), + status: FileStatus::Copied { similarity: 100 }, + added: 0, + deleted: 0, + }) + ); + } + + #[test] + fn every_hunk_of_a_file_keeps_its_own_header() { + let patch = Patch { + files: vec![file("src/a.rs", vec![replacement(), replacement()], false)], + }; + + let headers = split_rows(&patch) + .iter() + .filter(|row| matches!(row, SplitRow::Full(Row::HunkHeader { .. }))) + .count(); + + assert_eq!(headers, 2); + } +} diff --git a/crates/ui/src/detail/format.rs b/crates/ui/src/detail/format.rs index c0860df..5888f11 100644 --- a/crates/ui/src/detail/format.rs +++ b/crates/ui/src/detail/format.rs @@ -3,20 +3,7 @@ //! Nothing here touches gpui's `App` or `Window`: every function is a plain transformation //! from domain types to strings, so it is testable without a running window. -use std::fmt::Write as _; -use std::ops::Range; -use std::path::PathBuf; - -use domain::{FilePatch, FileStatus, Hunk, LineOrigin, ObjectId, Patch, Timestamp}; - -/// The UTF-8 byte range, into the text [`unified_diff_text_with_line_ranges`] produces, -/// of every added and every deleted line — everything else (file headers, hunk headers, -/// context lines) is left undecorated. -#[derive(Debug, Default, Clone, PartialEq, Eq)] -pub struct DiffLineRanges { - pub additions: Vec>, - pub deletions: Vec>, -} +use domain::{Hunk, ObjectId, Timestamp}; /// Hexadecimal characters kept when an identifier is shown abbreviated, matching Git's /// own default abbreviation length. @@ -105,166 +92,22 @@ pub fn hunk_heading(hunk: &Hunk) -> String { } } -fn git_path(path: Option<&PathBuf>) -> Option { - path.map(|path| path.display().to_string()) -} - -/// The path a `diff --git a/ b/` line shows for one side. -/// -/// Git always prints *some* path on both sides of that line, falling back to the other -/// side's path when this one doesn't exist (an added or deleted file) — the two sides -/// only ever truly differ for a rename or a copy. -fn git_header_path(path: Option<&PathBuf>, other: Option<&PathBuf>) -> String { - git_path(path) - .or_else(|| git_path(other)) - .unwrap_or_default() -} - -/// The `a/` / `b/` / `/dev/null` form Git uses on a `---`, `+++` or `Binary -/// files` line, where — unlike [`git_header_path`] — a missing side stays `/dev/null` -/// rather than borrowing the other side's path. -fn side_path(path: Option<&PathBuf>, prefix: char) -> String { - match git_path(path) { - Some(path) => format!("{prefix}/{path}"), - None => "/dev/null".to_string(), - } -} - -/// Rebuilds the unified diff text `git diff` would print for `patch` — so it can be fed -/// to a real editor for syntax highlighting, selection and virtualised scrolling — paired -/// with the byte range of every added and deleted line, computed in the same pass so the -/// text and the ranges can never disagree about where a line starts. Reconstructing the -/// text and then re-scanning it for `+`/`-` lines would be a second implementation of the -/// same rule, and a line whose own content begins with `+` or `-` is exactly where the -/// two could differ. -/// -/// Mode lines (`new file mode`, `deleted file mode`) are Git prints that [`FilePatch`] has -/// no data for, and are left out rather than fabricated. Each range starts at the line's -/// leading marker byte, read from [`LineOrigin`] structurally rather than from the first -/// byte of `line.content` — a line about a diff can itself start with `+` or `-`. -pub fn unified_diff_text_with_line_ranges(patch: &Patch) -> (String, DiffLineRanges) { - let mut text = String::new(); - let mut ranges = DiffLineRanges::default(); - for file in &patch.files { - write_file(&mut text, file, &mut ranges); - } - (text, ranges) -} - -fn write_file(text: &mut String, file: &FilePatch, ranges: &mut DiffLineRanges) { - let old = file.old_path.as_ref(); - let new = file.new_path.as_ref(); - - let _ = writeln!( - text, - "diff --git a/{} b/{}", - git_header_path(old, new), - git_header_path(new, old) - ); - - match &file.status { - FileStatus::Renamed { similarity } => { - let _ = writeln!(text, "similarity index {similarity}%"); - if let (Some(old), Some(new)) = (git_path(old), git_path(new)) { - let _ = writeln!(text, "rename from {old}"); - let _ = writeln!(text, "rename to {new}"); - } - } - FileStatus::Copied { similarity } => { - let _ = writeln!(text, "similarity index {similarity}%"); - if let (Some(old), Some(new)) = (git_path(old), git_path(new)) { - let _ = writeln!(text, "copy from {old}"); - let _ = writeln!(text, "copy to {new}"); - } - } - FileStatus::Added - | FileStatus::Deleted - | FileStatus::Modified - | FileStatus::TypeChanged => {} - } - - if file.is_binary { - let _ = writeln!( - text, - "Binary files {} and {} differ", - side_path(old, 'a'), - side_path(new, 'b') - ); - return; - } - - if file.hunks.is_empty() { - return; - } - - let _ = writeln!(text, "--- {}", side_path(old, 'a')); - let _ = writeln!(text, "+++ {}", side_path(new, 'b')); - for hunk in &file.hunks { - let _ = writeln!(text, "{}", hunk_heading(hunk)); - for line in &hunk.lines { - let marker = match line.origin { - LineOrigin::Addition => '+', - LineOrigin::Deletion => '-', - LineOrigin::Context => ' ', - }; - let start = text.len(); - let _ = writeln!(text, "{marker}{}", line.content); - let end = text.len() - 1; - match line.origin { - LineOrigin::Addition => ranges.additions.push(start..end), - LineOrigin::Deletion => ranges.deletions.push(start..end), - LineOrigin::Context => {} - } - } - } -} - #[cfg(test)] mod tests { use super::*; - use domain::DiffLine; - - fn unified_diff_text(patch: &Patch) -> String { - unified_diff_text_with_line_ranges(patch).0 - } fn id(nibble: char) -> ObjectId { nibble.to_string().repeat(40).parse().unwrap() } - fn file( - old_path: Option<&str>, - new_path: Option<&str>, - status: FileStatus, - is_binary: bool, - hunks: Vec, - ) -> FilePatch { - FilePatch { - old_path: old_path.map(PathBuf::from), - new_path: new_path.map(PathBuf::from), - status, - is_binary, - hunks, - } - } - - fn hunk(lines: Vec) -> Hunk { + fn hunk() -> Hunk { Hunk { old_start: 1, old_lines: 1, new_start: 1, new_lines: 2, heading: "fn existing()".to_string(), - lines, - } - } - - fn line(origin: LineOrigin, content: &str) -> DiffLine { - DiffLine { - origin, - old_number: None, - new_number: None, - content: content.to_string(), + lines: vec![], } } @@ -311,12 +154,12 @@ mod tests { #[test] fn hunk_heading_includes_the_function_context_when_git_found_one() { - assert_eq!(hunk_heading(&hunk(vec![])), "@@ -1,1 +1,2 @@ fn existing()"); + assert_eq!(hunk_heading(&hunk()), "@@ -1,1 +1,2 @@ fn existing()"); } #[test] fn hunk_heading_omits_the_trailing_space_when_git_found_no_context() { - let mut hunk = hunk(vec![]); + let mut hunk = hunk(); hunk.heading = String::new(); assert_eq!(hunk_heading(&hunk), "@@ -1,1 +1,2 @@"); } @@ -341,217 +184,4 @@ mod tests { "caf\u{e9} \u{2014} r\u{e9}sum\u{e9}" ); } - - #[test] - fn unified_diff_text_of_an_empty_patch_is_empty() { - let patch = Patch { files: vec![] }; - assert_eq!(unified_diff_text(&patch), ""); - } - - #[test] - fn unified_diff_text_reconstructs_a_single_hunk() { - let patch = Patch { - files: vec![file( - Some("src/lib.rs"), - Some("src/lib.rs"), - FileStatus::Modified, - false, - vec![hunk(vec![ - line(LineOrigin::Context, "fn existing() {"), - line(LineOrigin::Deletion, " old();"), - line(LineOrigin::Addition, " new();"), - line(LineOrigin::Addition, " more();"), - ])], - )], - }; - assert_eq!( - unified_diff_text(&patch), - "diff --git a/src/lib.rs b/src/lib.rs\n".to_string() - + "--- a/src/lib.rs\n" - + "+++ b/src/lib.rs\n" - + "@@ -1,1 +1,2 @@ fn existing()\n" - + " fn existing() {\n" - + "- old();\n" - + "+ new();\n" - + "+ more();\n" - ); - } - - #[test] - fn unified_diff_text_uses_dev_null_for_an_added_file() { - let patch = Patch { - files: vec![file( - None, - Some("new.rs"), - FileStatus::Added, - false, - vec![hunk(vec![line(LineOrigin::Addition, "fn new() {}")])], - )], - }; - let text = unified_diff_text(&patch); - assert!(text.starts_with("diff --git a/new.rs b/new.rs\n")); - assert!(text.contains("--- /dev/null\n")); - assert!(text.contains("+++ b/new.rs\n")); - } - - #[test] - fn unified_diff_text_uses_dev_null_for_a_deleted_file() { - let patch = Patch { - files: vec![file( - Some("old.rs"), - None, - FileStatus::Deleted, - false, - vec![hunk(vec![line(LineOrigin::Deletion, "fn old() {}")])], - )], - }; - let text = unified_diff_text(&patch); - assert!(text.starts_with("diff --git a/old.rs b/old.rs\n")); - assert!(text.contains("--- a/old.rs\n")); - assert!(text.contains("+++ /dev/null\n")); - } - - #[test] - fn unified_diff_text_marks_binary_files_without_a_hunk() { - let patch = Patch { - files: vec![file( - Some("image.png"), - Some("image.png"), - FileStatus::Modified, - true, - vec![], - )], - }; - assert_eq!( - unified_diff_text(&patch), - "diff --git a/image.png b/image.png\nBinary files a/image.png and b/image.png differ\n" - ); - } - - #[test] - fn unified_diff_text_includes_rename_headers_and_omits_hunks_for_a_pure_rename() { - let patch = Patch { - files: vec![file( - Some("old.rs"), - Some("new.rs"), - FileStatus::Renamed { similarity: 87 }, - false, - vec![], - )], - }; - assert_eq!( - unified_diff_text(&patch), - "diff --git a/old.rs b/new.rs\n\ - similarity index 87%\n\ - rename from old.rs\n\ - rename to new.rs\n" - ); - } - - #[test] - fn unified_diff_text_includes_copy_headers() { - let patch = Patch { - files: vec![file( - Some("old.rs"), - Some("copy.rs"), - FileStatus::Copied { similarity: 100 }, - false, - vec![], - )], - }; - let text = unified_diff_text(&patch); - assert!(text.contains("copy from old.rs\n")); - assert!(text.contains("copy to copy.rs\n")); - } - - #[test] - fn unified_diff_text_concatenates_multiple_files_in_order() { - let patch = Patch { - files: vec![ - file( - Some("a.rs"), - Some("a.rs"), - FileStatus::Modified, - false, - vec![hunk(vec![line(LineOrigin::Addition, "a")])], - ), - file( - Some("b.rs"), - Some("b.rs"), - FileStatus::Modified, - false, - vec![hunk(vec![line(LineOrigin::Addition, "b")])], - ), - ], - }; - let text = unified_diff_text(&patch); - let a_pos = text.find("diff --git a/a.rs").unwrap(); - let b_pos = text.find("diff --git a/b.rs").unwrap(); - assert!(a_pos < b_pos); - } - - #[test] - fn line_ranges_cover_additions_and_deletions_and_nothing_else() { - let patch = Patch { - files: vec![file( - Some("src/lib.rs"), - Some("src/lib.rs"), - FileStatus::Modified, - false, - vec![hunk(vec![ - line(LineOrigin::Context, "fn existing() {"), - line(LineOrigin::Deletion, " old();"), - line(LineOrigin::Addition, " new();"), - ])], - )], - }; - let (text, ranges) = unified_diff_text_with_line_ranges(&patch); - - assert_eq!(ranges.additions.len(), 1); - assert_eq!(ranges.deletions.len(), 1); - - let addition = &text[ranges.additions[0].clone()]; - assert_eq!(addition, "+ new();"); - - let deletion = &text[ranges.deletions[0].clone()]; - assert_eq!(deletion, "- old();"); - - let file_header_pos = text.find("diff --git").unwrap(); - let hunk_header_pos = text.find("@@").unwrap(); - let context_pos = text.find(" fn existing() {").unwrap(); - for range in ranges.additions.iter().chain(&ranges.deletions) { - assert!(!range.contains(&file_header_pos)); - assert!(!range.contains(&hunk_header_pos)); - assert!(!range.contains(&context_pos)); - } - } - - #[test] - fn line_ranges_are_read_from_the_marker_column_not_the_lines_own_content() { - let patch = Patch { - files: vec![file( - Some("src/lib.rs"), - Some("src/lib.rs"), - FileStatus::Modified, - false, - vec![hunk(vec![ - line(LineOrigin::Context, "+not actually an addition"), - line(LineOrigin::Addition, "-not actually a deletion"), - line(LineOrigin::Deletion, "+not actually an addition either"), - ])], - )], - }; - let (text, ranges) = unified_diff_text_with_line_ranges(&patch); - - assert_eq!(ranges.additions.len(), 1); - assert_eq!(ranges.deletions.len(), 1); - - let addition = &text[ranges.additions[0].clone()]; - assert!(addition.starts_with('+')); - assert_eq!(addition, "+-not actually a deletion"); - - let deletion = &text[ranges.deletions[0].clone()]; - assert!(deletion.starts_with('-')); - assert_eq!(deletion, "-+not actually an addition either"); - } } diff --git a/crates/ui/src/detail/metadata.rs b/crates/ui/src/detail/metadata.rs index 06e1e76..65f911b 100644 --- a/crates/ui/src/detail/metadata.rs +++ b/crates/ui/src/detail/metadata.rs @@ -1,9 +1,8 @@ //! Renders the commit metadata header (subject, identifier, parents and author) and, -//! separately, the commit message body — split because the header renders first -//! above the detail panel's scroll region while [`render_description`] scrolls -//! together with the header inside the tab's own scroll region, so an unusually -//! long body cannot squeeze the diff editor beneath it out of the panel. See -//! `detail::ready_state` for where the two are recombined. +//! separately, the commit message body — split because [`render_description`] answers +//! `None` for a commit that has nothing beyond its subject line, which renders no row +//! rather than an empty one. Both scroll together inside the General tab's single scroll +//! region; see `detail::general_tab` for where the two are recombined. //! //! Every value here goes through [`gpui_component::text::markdown`] rather than a plain //! `div`, which is what makes the commit's identifier — the thing most worth copying in @@ -82,7 +81,7 @@ pub(super) fn render_description(commit: &Commit, cx: &App) -> Option>, - pending_diff: Option<(SharedString, DiffLineRanges)>, - diff_editor: Entity, - diff_decorations: TextDecorationCollection, + diff_content: Option>, + diff_selection: TextSelectionHandle, + diff_select_all: bool, + diff_auto_scroll: AutoScroll, + diff_view_mode: DiffViewMode, selected_tab: DetailTab, general_scroll_handle: ScrollHandle, + diff_scroll_handle: ScrollHandle, focus_handle: FocusHandle, } impl DetailPanel { pub fn new(window: &mut Window, cx: &mut Context) -> Self { - let diff_editor = cx.new(|cx| EditorState::new(window, cx).language("diff")); - let diff_decorations = diff_editor.update(cx, |state, cx| { - state.create_decorations_collection(Vec::new(), cx) - }); + let diff_selection = TextSelectionHandle::new("", cx); + diff_selection.refresh_window_on_change(window, cx).detach(); + + let focus_handle = cx.focus_handle(); + let focus = focus_handle.clone(); + diff_selection.focus_with(move |window, cx| focus.focus(window, cx), cx); + + let panel = cx.weak_entity(); + diff_selection + .subscribe( + move |event, cx| { + let _ = match event { + TextSelectionEvent::AutoScroll(delta) => { + let delta = *delta; + panel.update(cx, |panel, cx| panel.auto_scroll_diff(delta, cx)) + } + TextSelectionEvent::Cleared => { + panel.update(cx, |panel, cx| panel.forget_select_all(cx)) + } + TextSelectionEvent::SelectionChanged(_) => Ok(()), + }; + }, + cx, + ) + .detach(); + Self { detail: LoadState::Idle, - pending_diff: None, - diff_editor, - diff_decorations, + diff_content: None, + diff_selection, + diff_select_all: false, + diff_auto_scroll: AutoScroll::default(), + diff_view_mode: persistence::load_diff_view_mode().unwrap_or_default(), selected_tab: DetailTab::default(), general_scroll_handle: ScrollHandle::new(), - focus_handle: cx.focus_handle(), + diff_scroll_handle: ScrollHandle::new(), + focus_handle, } } - pub fn set_detail(&mut self, detail: LoadState>, cx: &mut Context) { - if let LoadState::Ready(commit_detail) = &detail - && !commit_detail.patch.files.is_empty() - { - let (text, ranges) = format::unified_diff_text_with_line_ranges(&commit_detail.patch); - self.pending_diff = Some((text.into(), ranges)); - } + pub fn set_detail( + &mut self, + detail: LoadState>, + window: &mut Window, + cx: &mut Context, + ) { self.detail = detail; + self.rebuild_diff_content(); + self.reset_diff_view(window, cx); + cx.notify(); + } + + fn set_diff_view_mode( + &mut self, + mode: DiffViewMode, + window: &mut Window, + cx: &mut Context, + ) { + if mode == self.diff_view_mode { + return; + } + self.diff_view_mode = mode; + self.rebuild_diff_content(); + self.reset_diff_view(window, cx); + + cx.background_executor() + .spawn(async move { + if let Err(error) = persistence::save_diff_view_mode(&mode) { + eprintln!("gitr: failed to save diff view mode: {error:#}"); + } + }) + .detach(); + + cx.notify(); + } + + fn rebuild_diff_content(&mut self) { + self.diff_content = match &self.detail { + LoadState::Ready(detail) => { + Some(Rc::new(diff::content(&detail.patch, self.diff_view_mode))) + } + _ => None, + }; + } + + fn reset_diff_view(&mut self, window: &mut Window, cx: &mut Context) { + self.diff_auto_scroll.stop(); + self.diff_select_all = false; + self.diff_scroll_handle.set_offset(Point::default()); + TextSelection::clear(window, cx); + } + + fn forget_select_all(&mut self, cx: &mut Context) { + if !self.diff_select_all { + return; + } + self.diff_select_all = false; + cx.notify(); + } + + fn auto_scroll_diff(&mut self, delta: Option, cx: &mut Context) { + self.diff_auto_scroll.set(delta, cx, |delta, panel, cx| { + let offset = panel.diff_scroll_handle.offset(); + panel + .diff_scroll_handle + .set_offset(offset - point(px(0.), delta)); + cx.notify(); + }); + } + + fn on_copy(&mut self, _: &Copy, window: &mut Window, cx: &mut Context) { + let text = TextSelection::selected_text(window, cx); + if text.is_empty() { + cx.propagate(); + return; + } + cx.write_to_clipboard(ClipboardItem::new_string(text)); + } + + fn on_select_all(&mut self, _: &SelectAll, _: &mut Window, cx: &mut Context) { + let selectable = self.selected_tab == DetailTab::Diff + && self + .diff_content + .as_ref() + .is_some_and(|content| !content.is_empty()); + if !selectable { + cx.propagate(); + return; + } + + self.diff_select_all = true; + self.diff_selection.set_local_selection(true, cx); cx.notify(); } } @@ -131,21 +275,17 @@ impl Focusable for DetailPanel { } impl Render for DetailPanel { - fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - if let Some((text, ranges)) = self.pending_diff.take() { - let colors = decorations::line_backgrounds(cx.theme().mode); - let new_decorations = decorations::build_decorations(&ranges, &colors); - self.diff_editor - .update(cx, |state, cx| state.set_value(text, window, cx)); - self.diff_decorations.set(new_decorations, cx); - } - + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { let selected_tab = self.selected_tab; + let diff_view_mode = self.diff_view_mode; div() + .track_focus(&self.focus_handle) + .on_action(cx.listener(Self::on_copy)) + .on_action(cx.listener(Self::on_select_all)) .size_full() .flex() .flex_col() - .child(tab_bar(selected_tab, cx)) + .child(tab_bar(selected_tab, diff_view_mode, cx)) .child(match &self.detail { LoadState::Idle => centered_message(cx, "Select a commit to see its details."), LoadState::Loading => loading_state(cx), @@ -153,15 +293,22 @@ impl Render for DetailPanel { LoadState::Ready(detail) => ready_state( detail, selected_tab, - &self.diff_editor, + self.diff_content.as_ref(), + self.diff_select_all, + &self.diff_selection, &self.general_scroll_handle, + &self.diff_scroll_handle, cx, ), }) } } -fn tab_bar(selected: DetailTab, cx: &mut Context) -> AnyElement { +fn tab_bar( + selected: DetailTab, + diff_view_mode: DiffViewMode, + cx: &mut Context, +) -> AnyElement { let mut tabs = TabBar::new("detail-tabs") .segmented() .small() @@ -176,6 +323,7 @@ fn tab_bar(selected: DetailTab, cx: &mut Context) -> AnyElement { div() .flex_shrink_0() + .flex() .items_center() .gap_2() .px_2() @@ -183,9 +331,26 @@ fn tab_bar(selected: DetailTab, cx: &mut Context) -> AnyElement { .border_b_1() .border_color(cx.theme().border) .child(tabs) + .when(selected == DetailTab::Diff, |row| { + row.child(diff_view_mode_bar(diff_view_mode, cx)) + }) .into_any_element() } +fn diff_view_mode_bar(selected: DiffViewMode, cx: &mut Context) -> AnyElement { + let mut modes = TabBar::new("diff-view-mode") + .segmented() + .small() + .selected_index(selected.index()) + .on_click(cx.listener(|this, index: &usize, window, cx| { + this.set_diff_view_mode(DiffViewMode::from_index(*index), window, cx); + })); + for mode in DiffViewMode::ALL { + modes = modes.child(Tab::new().label(mode.label())); + } + modes.into_any_element() +} + fn centered_message(cx: &App, message: &str) -> AnyElement { div() .flex_1() @@ -224,16 +389,26 @@ fn failed_state(message: &str) -> AnyElement { .into_any_element() } +#[allow(clippy::too_many_arguments)] fn ready_state( detail: &CommitDetail, selected_tab: DetailTab, - diff_editor: &Entity, - scroll_handle: &ScrollHandle, + diff_content: Option<&Rc>, + diff_select_all: bool, + diff_selection: &TextSelectionHandle, + general_scroll_handle: &ScrollHandle, + diff_scroll_handle: &ScrollHandle, cx: &App, ) -> AnyElement { match selected_tab { - DetailTab::General => general_tab(detail, scroll_handle, cx), - DetailTab::Diff => diff_tab(detail, diff_editor, cx), + DetailTab::General => general_tab(detail, general_scroll_handle, cx), + DetailTab::Diff => diff_tab( + diff_content, + diff_select_all, + diff_selection, + diff_scroll_handle, + cx, + ), } } @@ -262,11 +437,23 @@ fn general_tab(detail: &CommitDetail, scroll_handle: &ScrollHandle, cx: &App) -> .into_any_element() } -fn diff_tab(detail: &CommitDetail, diff_editor: &Entity, cx: &App) -> AnyElement { +fn diff_tab( + content: Option<&Rc>, + select_all: bool, + selection: &TextSelectionHandle, + scroll_handle: &ScrollHandle, + cx: &App, +) -> AnyElement { div() .flex_1() .min_h_0() .min_w_0() - .child(diff::render(&detail.patch, diff_editor, cx)) + .child(diff::render( + content, + select_all, + selection, + scroll_handle, + cx, + )) .into_any_element() } diff --git a/crates/ui/src/diff_view_mode.rs b/crates/ui/src/diff_view_mode.rs new file mode 100644 index 0000000..a28c71a --- /dev/null +++ b/crates/ui/src/diff_view_mode.rs @@ -0,0 +1,50 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DiffViewMode { + #[default] + Unified, + Split, +} + +impl DiffViewMode { + pub const ALL: [DiffViewMode; 2] = [Self::Unified, Self::Split]; + + pub fn index(self) -> usize { + Self::ALL.iter().position(|mode| *mode == self).unwrap_or(0) + } + + pub fn from_index(index: usize) -> Self { + Self::ALL.get(index).copied().unwrap_or_default() + } + + pub fn label(self) -> &'static str { + match self { + Self::Unified => "Unified", + Self::Split => "Split", + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_default_is_unified() { + assert_eq!(DiffViewMode::default(), DiffViewMode::Unified); + } + + #[test] + fn every_mode_round_trips_through_its_index() { + for mode in DiffViewMode::ALL { + assert_eq!(DiffViewMode::from_index(mode.index()), mode); + } + } + + #[test] + fn an_out_of_range_index_falls_back_to_the_default() { + assert_eq!(DiffViewMode::from_index(99), DiffViewMode::default()); + } +} diff --git a/crates/ui/src/lib.rs b/crates/ui/src/lib.rs index 658f510..8e56e99 100644 --- a/crates/ui/src/lib.rs +++ b/crates/ui/src/lib.rs @@ -15,6 +15,7 @@ pub mod actions; pub mod branch_actions; pub mod density; pub mod detail; +pub mod diff_view_mode; pub mod graph_palette; pub mod history; pub mod persistence; diff --git a/crates/ui/src/persistence.rs b/crates/ui/src/persistence.rs index e17b019..28b3f77 100644 --- a/crates/ui/src/persistence.rs +++ b/crates/ui/src/persistence.rs @@ -15,12 +15,14 @@ use std::path::{Path, PathBuf}; use gpui_component::dock::DockAreaState; +use crate::diff_view_mode::DiffViewMode; use crate::project::ProjectList; use crate::theme_preference::ThemePreference; const APPLICATION_SUPPORT_DIR: &str = "Library/Application Support/gitr"; const DOCK_LAYOUT_FILE: &str = "dock-layout.json"; const THEME_PREFERENCE_FILE: &str = "theme-preference.json"; +const DIFF_VIEW_MODE_FILE: &str = "diff-view-preference.json"; const PROJECTS_FILE: &str = "projects.json"; const REMOTE_CACHE_DIR: &str = "remotes"; @@ -106,6 +108,33 @@ pub fn load_theme_preference() -> Option { load_theme_preference_from(&theme_preference_path()?).ok() } +pub fn diff_view_mode_path() -> Option { + Some(application_support_dir()?.join(DIFF_VIEW_MODE_FILE)) +} + +pub fn save_diff_view_mode_to(path: &Path, mode: &DiffViewMode) -> anyhow::Result<()> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + let json = serde_json::to_string_pretty(mode)?; + std::fs::write(path, json)?; + Ok(()) +} + +pub fn load_diff_view_mode_from(path: &Path) -> anyhow::Result { + let json = std::fs::read_to_string(path)?; + Ok(serde_json::from_str(&json)?) +} + +pub fn save_diff_view_mode(mode: &DiffViewMode) -> anyhow::Result<()> { + let path = diff_view_mode_path().ok_or_else(|| anyhow::anyhow!("$HOME is not set"))?; + save_diff_view_mode_to(&path, mode) +} + +pub fn load_diff_view_mode() -> Option { + load_diff_view_mode_from(&diff_view_mode_path()?).ok() +} + /// Where the project list lives for the signed-in user, or `None` if `$HOME` is unset. /// See [`dock_layout_path`] — same directory, same not-cached reasoning. pub fn project_list_path() -> Option { @@ -279,6 +308,17 @@ mod tests { let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn a_diff_view_mode_round_trips_through_a_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("diff-view-preference.json"); + save_diff_view_mode_to(&path, &DiffViewMode::Split).expect("save"); + assert_eq!( + load_diff_view_mode_from(&path).expect("load"), + DiffViewMode::Split + ); + } + fn sample_project_list() -> ProjectList { let a = Project::local(PathBuf::from("/repos/a")); let b = Project::local(PathBuf::from("/repos/b")); diff --git a/crates/ui/src/workspace.rs b/crates/ui/src/workspace.rs index 011dfbe..685a736 100644 --- a/crates/ui/src/workspace.rs +++ b/crates/ui/src/workspace.rs @@ -295,7 +295,7 @@ impl Workspace { let this = cx.entity().downgrade(); history_panel.update(cx, |panel, cx| panel.set_workspace(this, cx)); - sync_panels_from_repository(&repository, &history_panel, &detail_panel, cx); + sync_panels_from_repository(&repository, &history_panel, &detail_panel, window, cx); let repository_subscription = cx.subscribe_in(&repository, window, Self::on_repository_event); @@ -668,7 +668,7 @@ impl Workspace { RepositoryEvent::SelectionChanged => { let detail = repository.read(cx).detail().clone(); self.detail_panel - .update(cx, |panel, cx| panel.set_detail(detail, cx)); + .update(cx, |panel, cx| panel.set_detail(detail, window, cx)); if repository.read(cx).selected().is_none() { self.dismiss_detail(window, cx); } @@ -825,7 +825,13 @@ impl Workspace { let (path, watch) = repository_path_and_watch(&project.source); let repository = cx.new(|cx| RepositoryState::open(path, watch, cx)); - sync_panels_from_repository(&repository, &self.history_panel, &self.detail_panel, cx); + sync_panels_from_repository( + &repository, + &self.history_panel, + &self.detail_panel, + window, + cx, + ); self.history_panel .update(cx, |panel, cx| panel.reset_for_new_repository(cx)); @@ -1293,6 +1299,7 @@ fn sync_panels_from_repository( repository: &Entity, history_panel: &Entity, detail_panel: &Entity, + window: &mut Window, cx: &mut Context, ) { let history = repository.read(cx).history().clone(); @@ -1303,7 +1310,7 @@ fn sync_panels_from_repository( panel.set_history(history, cx); panel.set_head(deletion, head_commit(&head), cx); }); - detail_panel.update(cx, |panel, cx| panel.set_detail(detail, cx)); + detail_panel.update(cx, |panel, cx| panel.set_detail(detail, window, cx)); } fn deletion_context(repository: &Entity, cx: &App) -> Deletion { @@ -1606,10 +1613,12 @@ fn theme_preference_menu_item( /// The whole native macOS menu bar, rebuilt from scratch on every call — see /// [`Workspace::refresh_application_menus`] for when. `Cut`, `Copy`, `Paste` and /// `Select All` carry `gpui_component::input`'s own actions and matching [`OsAction`], -/// not an action this crate defines: the project search box, the "add from URL" field -/// and the readonly diff editor each register a handler for those every time they -/// paint, so the menu item reaches whichever one currently has focus exactly as the -/// keyboard shortcut already does. +/// not an action this crate defines: the project search box and the "add from URL" field +/// register a handler for those every time they paint, and `DetailPanel` registers its +/// own `Copy` and `Select All` for the diff, so the menu item reaches whichever one +/// currently has focus exactly as the keyboard shortcut already does. The diff needs its +/// own `Copy` rather than `gpui_component::Root`'s: that one trims the copied string +/// (`root.rs:552-555`), which eats the indentation of the first selected line. fn application_menus(theme_preference: ThemePreference) -> Vec { vec![ Menu { diff --git a/docs/superpowers/plans/2026-08-29-github-style-diff-view.md b/docs/superpowers/plans/2026-08-29-github-style-diff-view.md new file mode 100644 index 0000000..524658f --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-github-style-diff-view.md @@ -0,0 +1,971 @@ +# GitHub-style diff view — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the editor-backed diff with a rendered one that reads like GitHub — old/new line-number gutters, a full-width tint per line, the `+`/`-` in its own column, unified and side-by-side views, and text selection that crosses rows. + +**Architecture:** The diff stops being a text document fed to `EditorState` and becomes a derived `Vec` painted by one custom `Element`. That element does its own visible-range windowing and declares every visible row's text as runs of a single selection participant, because `gpui-base`'s selection API projects one range per run from one `update_runs` call. Row derivation and left/right pairing are pure functions and carry the tests; the element carries the risk. + +**Tech Stack:** Rust 2024, gpui (zed default branch), gpui-component 0.5.2 @ `7acfc18`, `gpui-base` (new direct dependency, same git source, no `rev`). + +**Spec:** `docs/superpowers/specs/2026-08-29-github-style-diff-view-design.md` + +## Global Constraints + +- Crates are imported unprefixed: `use domain::…`, never `use gitr_domain::…`. +- `cargo test -p gitr-domain -p gitr-graph` is the fast loop; `cargo test --workspace` is the exit check. +- `cargo clippy --workspace --all-targets -- -D warnings` and `cargo fmt --all --check` must pass at every commit. +- `[workspace.lints.clippy]` sets `todo = "deny"` and `dbg_macro = "deny"`. No `todo!()` scaffolding — every task compiles for real. +- Do not add comments to the code. Doc comments (`///`) on new functions and fields included. Reasoning goes in the commit message. +- Commit convention: `(): `. Never add an AI co-author trailer. +- Never pin `gpui` or any gpui-component crate to a `rev`. `Cargo.lock` is the pin. +- Cargo holds one lock per target directory — do not run `clippy` and `test` concurrently. +- `domain` and `vcs` are not touched by any task in this plan. + +--- + +### Task 1: Row model + +Derives the flat row list the unified view paints. Pure — no gpui, no window. + +**Files:** +- Create: `crates/ui/src/detail/diff/model.rs` +- Create: `crates/ui/src/detail/diff/mod.rs` +- Delete: `crates/ui/src/detail/diff.rs` (its body moves to `mod.rs` unchanged for now) + +**Interfaces:** +- Consumes: `domain::{DiffLine, FilePatch, FileStatus, LineOrigin, Patch}`. +- Produces: `pub(super) enum Row`, `pub(super) fn rows(patch: &Patch) -> Vec`. Task 4 paints `Row`; Task 2 reuses `file_stat`. + +- [ ] **Step 1: Turn the module into a directory** + +```bash +mkdir -p crates/ui/src/detail/diff +git mv crates/ui/src/detail/diff.rs crates/ui/src/detail/diff/mod.rs +``` + +Then add to the top of `crates/ui/src/detail/diff/mod.rs`: + +```rust +mod model; +``` + +- [ ] **Step 2: Write the failing tests** + +Create `crates/ui/src/detail/diff/model.rs` containing only this test module plus the `use super::*;` it needs: + +```rust +#[cfg(test)] +mod tests { + use super::*; + use domain::{FileStatus, Hunk}; + use std::path::PathBuf; + + fn line(origin: LineOrigin, old: Option, new: Option, content: &str) -> DiffLine { + DiffLine { origin, old_number: old, new_number: new, content: content.to_string() } + } + + fn file(hunks: Vec, is_binary: bool) -> FilePatch { + FilePatch { + old_path: Some(PathBuf::from("src/main.rs")), + new_path: Some(PathBuf::from("src/main.rs")), + status: FileStatus::Modified, + is_binary, + hunks, + } + } + + fn hunk(lines: Vec) -> Hunk { + Hunk { old_start: 1, old_lines: 1, new_start: 1, new_lines: 1, heading: String::new(), lines } + } + + #[test] + fn a_modified_file_yields_a_header_a_hunk_header_and_one_row_per_line() { + let patch = Patch { files: vec![file(vec![hunk(vec![ + line(LineOrigin::Context, Some(1), Some(1), "keep"), + line(LineOrigin::Deletion, Some(2), None, "gone"), + line(LineOrigin::Addition, None, Some(2), "new"), + ])], false)] }; + + let rows = rows(&patch); + + assert!(matches!(rows[0], Row::FileHeader { .. })); + assert!(matches!(rows[1], Row::HunkHeader { .. })); + assert_eq!(rows.len(), 5); + } + + #[test] + fn a_line_row_carries_both_numbers_and_the_bare_content() { + let patch = Patch { files: vec![file(vec![hunk(vec![ + line(LineOrigin::Deletion, Some(7), None, "-not a marker"), + ])], false)] }; + + let rows = rows(&patch); + + assert_eq!( + rows[2], + Row::Line { + origin: LineOrigin::Deletion, + old_number: Some(7), + new_number: None, + content: "-not a marker".to_string(), + }, + "content is stored bare by the parser and must not be re-marked here" + ); + } + + #[test] + fn a_binary_file_yields_a_placeholder_instead_of_lines() { + let patch = Patch { files: vec![file(Vec::new(), true)] }; + let rows = rows(&patch); + assert_eq!(rows[1], Row::Placeholder { message: "Binary file not shown." }); + } + + #[test] + fn a_file_with_no_hunks_yields_a_no_change_placeholder() { + let patch = Patch { files: vec![file(Vec::new(), false)] }; + let rows = rows(&patch); + assert_eq!(rows[1], Row::Placeholder { message: "No content changes." }); + } + + #[test] + fn every_file_contributes_its_own_header() { + let patch = Patch { files: vec![file(Vec::new(), true), file(Vec::new(), true)] }; + let headers = rows(&patch).iter().filter(|r| matches!(r, Row::FileHeader { .. })).count(); + assert_eq!(headers, 2); + } +} +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `cargo test -p gitr-ui --lib detail::diff::model` +Expected: FAIL to compile — `Row` and `rows` are not defined. + +- [ ] **Step 4: Write the implementation** + +Prepend to `crates/ui/src/detail/diff/model.rs`: + +```rust +use domain::{DiffLine, FilePatch, LineOrigin, Patch}; + +use crate::detail::format; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) enum Row { + FileHeader { path: String, stat: String }, + HunkHeader { text: String }, + Line { + origin: LineOrigin, + old_number: Option, + new_number: Option, + content: String, + }, + Placeholder { message: &'static str }, +} + +pub(super) fn rows(patch: &Patch) -> Vec { + let mut rows = Vec::new(); + for file in &patch.files { + rows.push(Row::FileHeader { + path: file.display_path(), + stat: file_stat(file), + }); + push_body(&mut rows, file); + } + rows +} + +pub(super) fn file_stat(file: &FilePatch) -> String { + format!("+{} \u{2212}{}", file.added_lines(), file.deleted_lines()) +} + +fn push_body(rows: &mut Vec, file: &FilePatch) { + if file.is_binary { + rows.push(Row::Placeholder { message: "Binary file not shown." }); + return; + } + if file.hunks.is_empty() { + rows.push(Row::Placeholder { message: "No content changes." }); + return; + } + for hunk in &file.hunks { + rows.push(Row::HunkHeader { text: format::hunk_heading(hunk) }); + rows.extend(hunk.lines.iter().map(line_row)); + } +} + +fn line_row(line: &DiffLine) -> Row { + Row::Line { + origin: line.origin, + old_number: line.old_number, + new_number: line.new_number, + content: line.content.clone(), + } +} +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo test -p gitr-ui --lib detail::diff::model` +Expected: PASS, 5 tests. + +- [ ] **Step 6: Check `display_path` returns a `String`** + +Run: `grep -n "fn display_path" -A 6 crates/domain/src/patch.rs` +If it returns `&Path` or `Option<&Path>` rather than `String`, adjust the `path:` field construction to `.display().to_string()`. Do not change `domain`. + +- [ ] **Step 7: Lint, format, commit** + +```bash +cargo fmt --all +cargo clippy -p gitr-ui --all-targets -- -D warnings +git add crates/ui/src/detail/diff/ +git commit -m "feat(diff): derive a flat row model from a patch" +``` + +--- + +### Task 2: Side-by-side pairing + +The only real algorithm in this plan. Pure. + +**Files:** +- Create: `crates/ui/src/detail/diff/pairing.rs` +- Modify: `crates/ui/src/detail/diff/mod.rs` — add `mod pairing;` + +**Interfaces:** +- Consumes: `domain::{DiffLine, LineOrigin}`. +- Produces: `pub(super) struct SplitRow { pub left: Option, pub right: Option }`, `pub(super) struct SideLine { pub number: Option, pub origin: LineOrigin, pub content: String }`, `pub(super) fn pair(lines: &[DiffLine]) -> Vec`. Task 7 paints these. + +- [ ] **Step 1: Write the failing tests** + +Create `crates/ui/src/detail/diff/pairing.rs` with this test module: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + fn line(origin: LineOrigin, old: Option, new: Option, content: &str) -> DiffLine { + DiffLine { origin, old_number: old, new_number: new, content: content.to_string() } + } + + fn deletion(number: u32, content: &str) -> DiffLine { + line(LineOrigin::Deletion, Some(number), None, content) + } + + fn addition(number: u32, content: &str) -> DiffLine { + line(LineOrigin::Addition, None, Some(number), content) + } + + fn context(number: u32, content: &str) -> DiffLine { + line(LineOrigin::Context, Some(number), Some(number), content) + } + + #[test] + fn a_context_line_is_the_same_on_both_sides() { + let rows = pair(&[context(1, "keep")]); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].left.as_ref().map(|s| s.content.as_str()), Some("keep")); + assert_eq!(rows[0].right.as_ref().map(|s| s.content.as_str()), Some("keep")); + } + + #[test] + fn an_equal_length_replacement_pairs_line_for_line() { + let rows = pair(&[deletion(1, "a"), deletion(2, "b"), addition(1, "x"), addition(2, "y")]); + assert_eq!(rows.len(), 2); + assert_eq!(rows[0].left.as_ref().map(|s| s.content.as_str()), Some("a")); + assert_eq!(rows[0].right.as_ref().map(|s| s.content.as_str()), Some("x")); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("b")); + assert_eq!(rows[1].right.as_ref().map(|s| s.content.as_str()), Some("y")); + } + + #[test] + fn more_additions_than_deletions_pads_the_left() { + let rows = pair(&[deletion(1, "a"), addition(1, "x"), addition(2, "y")]); + assert_eq!(rows.len(), 2); + assert!(rows[1].left.is_none(), "the extra addition has nothing to pair with"); + assert_eq!(rows[1].right.as_ref().map(|s| s.content.as_str()), Some("y")); + } + + #[test] + fn more_deletions_than_additions_pads_the_right() { + let rows = pair(&[deletion(1, "a"), deletion(2, "b"), addition(1, "x")]); + assert_eq!(rows.len(), 2); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("b")); + assert!(rows[1].right.is_none()); + } + + #[test] + fn a_pure_addition_leaves_the_left_side_empty() { + let rows = pair(&[addition(1, "x"), addition(2, "y")]); + assert_eq!(rows.len(), 2); + assert!(rows.iter().all(|row| row.left.is_none())); + } + + #[test] + fn a_pure_deletion_leaves_the_right_side_empty() { + let rows = pair(&[deletion(1, "a")]); + assert_eq!(rows.len(), 1); + assert!(rows[0].right.is_none()); + } + + #[test] + fn a_run_is_flushed_when_a_context_line_ends_it() { + let rows = pair(&[deletion(1, "a"), addition(1, "x"), context(2, "keep")]); + assert_eq!(rows.len(), 2); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("keep")); + } + + #[test] + fn a_run_at_the_very_end_is_flushed_without_trailing_context() { + let rows = pair(&[context(1, "keep"), deletion(2, "a")]); + assert_eq!(rows.len(), 2, "the trailing run must not be dropped"); + assert_eq!(rows[1].left.as_ref().map(|s| s.content.as_str()), Some("a")); + } + + #[test] + fn an_empty_hunk_yields_no_rows() { + assert!(pair(&[]).is_empty()); + } + + #[test] + fn a_side_line_keeps_its_own_number() { + let rows = pair(&[deletion(7, "a"), addition(9, "x")]); + assert_eq!(rows[0].left.as_ref().and_then(|s| s.number), Some(7)); + assert_eq!(rows[0].right.as_ref().and_then(|s| s.number), Some(9)); + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test -p gitr-ui --lib detail::diff::pairing` +Expected: FAIL to compile — `pair`, `SplitRow`, `SideLine` are not defined. + +- [ ] **Step 3: Write the implementation** + +Prepend to `crates/ui/src/detail/diff/pairing.rs`: + +```rust +use domain::{DiffLine, LineOrigin}; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(super) struct SideLine { + pub number: Option, + pub origin: LineOrigin, + pub content: String, +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub(super) struct SplitRow { + pub left: Option, + pub right: Option, +} + +pub(super) fn pair(lines: &[DiffLine]) -> Vec { + let mut rows = Vec::new(); + let mut deletions: Vec<&DiffLine> = Vec::new(); + let mut additions: Vec<&DiffLine> = Vec::new(); + + for line in lines { + match line.origin { + LineOrigin::Deletion => deletions.push(line), + LineOrigin::Addition => additions.push(line), + LineOrigin::Context => { + flush(&mut rows, &mut deletions, &mut additions); + rows.push(SplitRow { + left: Some(side(line, line.old_number)), + right: Some(side(line, line.new_number)), + }); + } + } + } + flush(&mut rows, &mut deletions, &mut additions); + rows +} + +fn flush(rows: &mut Vec, deletions: &mut Vec<&DiffLine>, additions: &mut Vec<&DiffLine>) { + let paired = deletions.len().max(additions.len()); + for index in 0..paired { + rows.push(SplitRow { + left: deletions.get(index).map(|line| side(line, line.old_number)), + right: additions.get(index).map(|line| side(line, line.new_number)), + }); + } + deletions.clear(); + additions.clear(); +} + +fn side(line: &DiffLine, number: Option) -> SideLine { + SideLine { + number, + origin: line.origin, + content: line.content.clone(), + } +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test -p gitr-ui --lib detail::diff::pairing` +Expected: PASS, 10 tests. + +- [ ] **Step 5: Register the module** + +Add `mod pairing;` to `crates/ui/src/detail/diff/mod.rs`. It is unused until Task 7, so add `#[allow(dead_code)]` above the `mod pairing;` line and delete that attribute in Task 7. + +- [ ] **Step 6: Lint, format, commit** + +```bash +cargo fmt --all +cargo clippy -p gitr-ui --all-targets -- -D warnings +git add crates/ui/src/detail/diff/ +git commit -m "feat(diff): pair deletions with additions for a side-by-side view" +``` + +--- + +### Task 3: Row palette + +Moves the four tint constants and their contrast reasoning out of `decorations.rs`, and adds the foreground colours the rows need. `decorations.rs` is still alive at the end of this task; Task 4 deletes it. + +**Files:** +- Create: `crates/ui/src/detail/diff/palette.rs` +- Modify: `crates/ui/src/detail/diff/mod.rs` — add `mod palette;` + +**Interfaces:** +- Consumes: `gpui_component::{ThemeColor, ThemeMode}`, `domain::LineOrigin`. +- Produces: `pub(super) struct LineColors { pub background: Option, pub foreground: Hsla }`, `pub(super) fn line_colors(origin: LineOrigin, mode: ThemeMode, theme: &ThemeColor) -> LineColors`. + +- [ ] **Step 1: Write the failing tests** + +Create `crates/ui/src/detail/diff/palette.rs` with this test module: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_context_line_has_no_background() { + let theme = ThemeColor::light(); + assert!(line_colors(LineOrigin::Context, ThemeMode::Light, &theme).background.is_none()); + } + + #[test] + fn an_addition_and_a_deletion_do_not_share_a_background() { + let theme = ThemeColor::light(); + let added = line_colors(LineOrigin::Addition, ThemeMode::Light, &theme).background; + let deleted = line_colors(LineOrigin::Deletion, ThemeMode::Light, &theme).background; + assert!(added.is_some() && deleted.is_some()); + assert_ne!(added, deleted); + } + + #[test] + fn dark_mode_does_not_reuse_the_light_pair() { + let theme = ThemeColor::dark(); + let light = line_colors(LineOrigin::Addition, ThemeMode::Light, &theme).background; + let dark = line_colors(LineOrigin::Addition, ThemeMode::Dark, &theme).background; + assert_ne!(light, dark); + } + + #[test] + fn every_band_is_distinguishable_from_the_background_it_sits_on() { + for (mode, theme) in [(ThemeMode::Light, ThemeColor::light()), (ThemeMode::Dark, ThemeColor::dark())] { + for origin in [LineOrigin::Addition, LineOrigin::Deletion] { + let band = line_colors(origin, mode, &theme).background.expect("a band"); + let distance = crate::theme_palette::rendered_distance(theme.background, band, theme.background); + assert!(distance > 0.0, "{origin:?} in {mode:?} must not vanish into the background"); + } + } + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test -p gitr-ui --lib detail::diff::palette` +Expected: FAIL to compile — `line_colors` is not defined. + +- [ ] **Step 3: Write the implementation** + +Prepend to `crates/ui/src/detail/diff/palette.rs`. The four constants and their reasoning come verbatim from `crates/ui/src/detail/decorations.rs:17-35` — copy the doc comments on the two dark constants across unchanged, they record contrast measurements that must not be lost: + +```rust +use domain::LineOrigin; +use gpui::{Hsla, rgb}; +use gpui_component::{ThemeColor, ThemeMode}; + +const LIGHT_ADDITION_BACKGROUND: u32 = 0xdafbe1; +const LIGHT_DELETION_BACKGROUND: u32 = 0xffebe9; +const DARK_ADDITION_BACKGROUND: u32 = 0x355a40; +const DARK_DELETION_BACKGROUND: u32 = 0x5f3c45; + +pub(super) struct LineColors { + pub background: Option, + pub foreground: Hsla, +} + +pub(super) fn line_colors(origin: LineOrigin, mode: ThemeMode, theme: &ThemeColor) -> LineColors { + let background = match (origin, mode.is_dark()) { + (LineOrigin::Context, _) => None, + (LineOrigin::Addition, false) => Some(rgb(LIGHT_ADDITION_BACKGROUND).into()), + (LineOrigin::Addition, true) => Some(rgb(DARK_ADDITION_BACKGROUND).into()), + (LineOrigin::Deletion, false) => Some(rgb(LIGHT_DELETION_BACKGROUND).into()), + (LineOrigin::Deletion, true) => Some(rgb(DARK_DELETION_BACKGROUND).into()), + }; + LineColors { background, foreground: theme.foreground } +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test -p gitr-ui --lib detail::diff::palette` +Expected: PASS, 4 tests. + +If `rendered_distance` is not `pub(crate)`, widen it in `crates/ui/src/theme_palette.rs` to `pub(crate)` rather than duplicating it. + +- [ ] **Step 5: Lint, format, commit** + +```bash +cargo fmt --all +cargo clippy -p gitr-ui --all-targets -- -D warnings +git add crates/ui/src/detail/diff/ crates/ui/src/theme_palette.rs +git commit -m "feat(diff): give rows their own palette, carrying the contrast reasoning over" +``` + +--- + +### Task 4: The selectable diff body + +The risk. One custom `Element` that paints every row and declares their text as runs of one selection participant. No windowing yet — Task 5 adds it — so this task must be verified on a small commit. + +**Files:** +- Modify: `Cargo.toml` — add `gpui-base` to `[workspace.dependencies]` +- Modify: `crates/ui/Cargo.toml` — add `gpui-base.workspace = true` +- Create: `crates/ui/src/detail/diff/body.rs` +- Modify: `crates/ui/src/detail/diff/mod.rs` — render `body`, drop the `Editor` +- Modify: `crates/ui/src/detail/mod.rs` — drop `pending_diff`, `diff_editor`, `diff_decorations` +- Delete: `crates/ui/src/detail/decorations.rs` +- Modify: `crates/ui/src/detail/format.rs` — delete `DiffLineRanges`, `unified_diff_text_with_line_ranges`, `write_file`, and the path helpers that only served them, plus their tests + +**Interfaces:** +- Consumes: `Row` and `rows` (Task 1), `LineColors` and `line_colors` (Task 3). +- Produces: `pub(super) struct DiffBody`, `pub(super) fn body(rows: Vec, selection: TextSelectionHandle, theme: ThemeColor, mode: ThemeMode) -> DiffBody`. Task 5 adds windowing inside it; Task 7 adds a split variant beside it. + +- [ ] **Step 1: Add the dependency** + +In `Cargo.toml`, after the `gpui-component-assets` line: + +```toml +gpui-base = { git = "https://github.com/longbridge/gpui-component" } +``` + +No `rev` — `gpui-component` is declared the same way, and Cargo unifies two git sources only when the reference matches exactly. In `crates/ui/Cargo.toml`, under `[dependencies]`, after `gpui-component.workspace = true`: + +```toml +gpui-base.workspace = true +``` + +- [ ] **Step 2: Verify the dependency unified rather than duplicated** + +Run: `cargo tree -p gitr-ui -i gpui-base 2>&1 | head -20` +Expected: exactly one `gpui-base v0.5.2` at source `#7acfc18…`. If two appear, stop — the reference does not match and every trait from one copy will fail to apply to the other. + +- [ ] **Step 3: Write the element** + +Create `crates/ui/src/detail/diff/body.rs`. This is adapted from the reference at +`~/.cargo/git/checkouts/gpui-component-95ce574d8a0da8b8/7acfc18/crates/base/examples/showcase/components/text_selection.rs`, +whose `PlainSelectableText` is the only worked example of a custom element joining the selection system. Read it before writing this. The two departures from it are that the participant declares many runs rather than one, and that each run is one row's code text. + +```rust +use std::ops::Range; + +use gpui::{ + App, Bounds, Element, ElementId, GlobalElementId, Hitbox, HitboxBehavior, InspectorElementId, + IntoElement, LayoutId, Pixels, Point, SharedString, StyledText, Window, fill, point, px, size, +}; +use gpui_base::{TextSelectionHandle, TextSelectionRegistration, TextSelectionRun}; +use gpui_component::{ThemeColor, ThemeMode}; + +use super::model::Row; +use super::palette::line_colors; + +const GUTTER_WIDTH: f32 = 44.; +const MARKER_WIDTH: f32 = 16.; +const ROW_HEIGHT: f32 = 18.; + +pub(super) struct DiffBody { + rows: Vec, + selection: TextSelectionHandle, + theme: ThemeColor, + mode: ThemeMode, + texts: Vec, +} + +pub(super) fn body( + rows: Vec, + selection: TextSelectionHandle, + theme: ThemeColor, + mode: ThemeMode, +) -> DiffBody { + let texts = rows.iter().map(|row| StyledText::new(row_text(row))).collect(); + DiffBody { rows, selection, theme, mode, texts } +} + +fn row_text(row: &Row) -> SharedString { + match row { + Row::FileHeader { path, stat } => format!("{path} {stat}").into(), + Row::HunkHeader { text } => text.clone().into(), + Row::Line { content, .. } => content.clone().into(), + Row::Placeholder { message } => (*message).into(), + } +} +``` + +The `Element` impl follows the reference's shape exactly — `type RequestLayoutState = ();`, `type PrepaintState = Hitbox;`, `id` and `source_location` returning `None`. In `prepaint`, lay out each `StyledText` at its row's bounds, insert one hitbox over the whole body, and register one participant: + +```rust +self.selection.register( + TextSelectionRegistration::new(hitbox.clone(), bounds) + .with_document_order(0) + .with_text_bounds(row_bounds.clone()), + window, + cx, +); +``` + +In `paint`, build one run per row, in document order, and project: + +```rust +let runs: Vec = self + .texts + .iter() + .enumerate() + .map(|(index, text)| { + TextSelectionRun::new(row_text(&self.rows[index]), text.layout().clone(), row_bounds[index]) + .with_document_order(index as u64) + }) + .collect(); +let projection = self.selection.update_runs(&runs, cx); +``` + +Then, per row: paint the tint quad across the full `bounds.size.width`, paint the two gutter numbers and the marker, paint the selection quads for `projection.ranges()[index]` if `Some`, and finally paint the row's `StyledText`. + +Only the code text becomes a run. The gutters and the marker are painted directly and are never registered, which is what keeps them out of the clipboard. + +**`TextLayout::bounds`, `line_height`, `len`, `position_for_index` and `index_for_position` panic if called before layout.** They are safe in `paint` and nowhere earlier. + +Copy `selection_quad_bounds` verbatim from the reference — it is a ready-made three-rectangle start/middle/end helper. + +- [ ] **Step 4: Render it, and tear out the editor** + +In `crates/ui/src/detail/diff/mod.rs`, replace the `Editor::new(...)` body of `render` with the new element, keeping the empty-patch early return unchanged. In `crates/ui/src/detail/mod.rs`, delete the `pending_diff`, `diff_editor` and `diff_decorations` fields, the flush block at the top of `Render::render`, and the `EditorState`/`TextDecorationCollection` imports. `DetailPanel` gains one `TextSelectionHandle`, built in `new`: + +```rust +let selection = TextSelectionHandle::new("", cx); +selection.refresh_window_on_change(window, cx).detach(); +``` + +Without that subscription the selection changes but nothing repaints. + +`set_detail` now derives rows directly — it needs no window, which is the whole reason `pending_diff` existed: + +```rust +pub fn set_detail(&mut self, detail: LoadState>, cx: &mut Context) { + self.detail = detail; + cx.notify(); +} +``` + +Delete `crates/ui/src/detail/decorations.rs` and its `mod decorations;` line. In `format.rs`, delete `DiffLineRanges`, `unified_diff_text_with_line_ranges`, `write_file`, `git_path`, `git_header_path`, `side_path`, and every test naming them. Keep `abbreviate`, `format_timestamp`, `escape_markdown` and `hunk_heading`. Update the module doc on `detail/mod.rs`, which describes the editor and the staging that no longer exist. + +- [ ] **Step 5: Stop the code from wrapping** + +The spec calls for horizontal scrolling rather than soft wrap, and nothing so far enforces +it: `StyledText` wraps to the bounds it is given. Lay each row's text out against a width +wide enough that it never wraps — the widest row's measured width, not the viewport's — and +put the body inside a horizontally scrollable parent in `diff/mod.rs`: + +```rust +div() + .id("detail-diff-scroll") + .size_full() + .overflow_x_scroll() + .restrict_scroll_to_axis() + .child(body(...)) +``` + +`restrict_scroll_to_axis` is not optional. It defaults to `false`, and with it unset a +scrollable-x element treats a vertical delta as horizontal whenever its own y overflow is +not `Scroll` — a vertical gesture over the diff would scroll it sideways and never scroll it +down. + +If a row wraps anyway, the fixed `ROW_HEIGHT` Task 5 relies on is wrong and the arithmetic +windowing breaks. Confirm no row wraps before moving on. + +- [ ] **Step 6: Build and lint** + +```bash +cargo fmt --all +cargo clippy --workspace --all-targets -- -D warnings +cargo test --workspace +``` +Expected: green. The tests deleted from `format.rs` are gone; nothing else should fail. + +- [ ] **Step 7: Verify in the running app — this task is not done without it** + +gitr is single-instance, so a running installed binary will swallow the launch and you will test the old build. Check first: + +```bash +ps -eo pid,command | grep "[g]itr" | grep -v cargo +``` + +If an instance is running, quit it, then: + +```bash +cargo run -p gitr_gui -- . +``` + +Select a commit, open the Diffs tab, and confirm all five: the code starts at the same column on every line; added and deleted lines carry a band that reaches the full width, including blank ones; both gutters show the file's own line numbers, not a running document count; a drag selects across row boundaries; and Cmd-C yields code with no line numbers and no `+`/`-`. + +- [ ] **Step 8: Commit** + +```bash +git add -A +git commit -m "feat(diff): render the diff as rows with selectable code" +``` + +--- + +### Task 5: Windowing + +Task 4 builds a `StyledText` per row for the whole patch. On a large commit that is thousands of laid-out lines per frame. This task limits the work to the rows on screen. + +**Files:** +- Modify: `crates/ui/src/detail/diff/body.rs` + +**Interfaces:** +- Consumes: `DiffBody` from Task 4. +- Produces: no new public names. `body()` keeps its signature. + +- [ ] **Step 1: Establish the baseline** + +Run the app on a repository with a large commit and note that scrolling is slow. `zed-industries/zed` has commits touching hundreds of files. Record what you saw — this is the before. + +- [ ] **Step 2: Compute the visible range in `prepaint`** + +Row height is fixed at `ROW_HEIGHT`, so the range is arithmetic, not measurement: + +```rust +let first = (scroll_offset.y / px(ROW_HEIGHT)).floor().max(0.) as usize; +let visible = (bounds.size.height / px(ROW_HEIGHT)).ceil() as usize + 1; +let range = first..(first + visible).min(self.rows.len()); +``` + +Lay out `StyledText` only for `range`, and store `range` on the element so `paint` uses the same one. + +- [ ] **Step 3: Keep document order absolute** + +Runs must carry their index in the whole patch, not in the visible window, or a selection dragged past the edge reorders on copy: + +```rust +.with_document_order(range.start as u64 + offset_in_window as u64) +``` + +- [ ] **Step 4: Report the scroll offset to the selection layer** + +```rust +TextSelectionRegistration::new(hitbox.clone(), bounds) + .with_scroll_offset(scroll_offset) +``` + +Without it, hit-testing maps window points into the wrong content position as soon as the body is scrolled. + +- [ ] **Step 5: Verify** + +Run the app again on the same large commit. Scrolling should be smooth. Then re-check the two selection behaviours from Task 4 Step 7 **while scrolled down**, and drag a selection from a visible row past the bottom edge to confirm document order survives. + +- [ ] **Step 6: Lint, format, commit** + +```bash +cargo fmt --all +cargo clippy --workspace --all-targets -- -D warnings +git add crates/ui/src/detail/diff/body.rs +git commit -m "feat(diff): lay out only the rows on screen" +``` + +--- + +### Task 6: The view-mode preference + +Pure and testable, and independent of Tasks 4 and 5 — it can be built in parallel with them. + +**Files:** +- Create: `crates/ui/src/diff_view_mode.rs` +- Modify: `crates/ui/src/lib.rs` — add `pub mod diff_view_mode;` +- Modify: `crates/ui/src/persistence.rs` + +**Interfaces:** +- Produces: `pub enum DiffViewMode { Unified, Split }` with `ALL`, `index`, `from_index`, `label`; `persistence::{save_diff_view_mode, load_diff_view_mode, save_diff_view_mode_to, load_diff_view_mode_from}`. + +- [ ] **Step 1: Read the pattern to copy** + +Run: `cat crates/ui/src/theme_preference.rs` and `grep -n "theme_preference" crates/ui/src/persistence.rs`. `DiffViewMode` mirrors `ThemePreference` — a small serde enum with a `Default`, and a `_to`/`_from` pair beside the `save`/`load` pair so the disk-free half stays testable. + +- [ ] **Step 2: Write the failing tests** + +In `crates/ui/src/diff_view_mode.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_default_is_unified() { + assert_eq!(DiffViewMode::default(), DiffViewMode::Unified); + } + + #[test] + fn every_mode_round_trips_through_its_index() { + for mode in DiffViewMode::ALL { + assert_eq!(DiffViewMode::from_index(mode.index()), mode); + } + } + + #[test] + fn an_out_of_range_index_falls_back_to_the_default() { + assert_eq!(DiffViewMode::from_index(99), DiffViewMode::default()); + } +} +``` + +And in `crates/ui/src/persistence.rs`'s test module: + +```rust +#[test] +fn a_diff_view_mode_round_trips_through_a_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("diff-view-preference.json"); + save_diff_view_mode_to(&path, &DiffViewMode::Split).expect("save"); + assert_eq!(load_diff_view_mode_from(&path).expect("load"), DiffViewMode::Split); +} +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `cargo test -p gitr-ui --lib diff_view_mode persistence` +Expected: FAIL to compile. + +- [ ] **Step 4: Write the implementation** + +```rust +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DiffViewMode { + #[default] + Unified, + Split, +} + +impl DiffViewMode { + pub const ALL: [DiffViewMode; 2] = [Self::Unified, Self::Split]; + + pub fn index(self) -> usize { + Self::ALL.iter().position(|mode| *mode == self).unwrap_or(0) + } + + pub fn from_index(index: usize) -> Self { + Self::ALL.get(index).copied().unwrap_or_default() + } + + pub fn label(self) -> &'static str { + match self { + Self::Unified => "Unified", + Self::Split => "Split", + } + } +} +``` + +In `persistence.rs`, add `const DIFF_VIEW_MODE_FILE: &str = "diff-view-preference.json";` beside the other file constants, and copy the four theme-preference functions, substituting the type and the constant. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo test -p gitr-ui --lib diff_view_mode persistence` +Expected: PASS. + +- [ ] **Step 6: Lint, format, commit** + +```bash +cargo fmt --all +cargo clippy -p gitr-ui --all-targets -- -D warnings +git add crates/ui/src/diff_view_mode.rs crates/ui/src/lib.rs crates/ui/src/persistence.rs +git commit -m "feat(diff): persist the chosen diff view mode" +``` + +--- + +### Task 7: Side-by-side view and the toggle + +**Files:** +- Create: `crates/ui/src/detail/diff/split.rs` +- Modify: `crates/ui/src/detail/diff/body.rs`, `crates/ui/src/detail/diff/mod.rs`, `crates/ui/src/detail/mod.rs` +- Modify: `crates/ui/src/detail/diff/pairing.rs` — remove the `#[allow(dead_code)]` added in Task 2 + +**Interfaces:** +- Consumes: `pair`, `SplitRow`, `SideLine` (Task 2); `DiffViewMode` (Task 6); `DiffBody` (Tasks 4-5). +- Produces: `pub(super) fn split_rows(patch: &Patch) -> Vec`. + +- [ ] **Step 1: Write the failing test for split row derivation** + +In `crates/ui/src/detail/diff/split.rs`, a test asserting that a two-file patch yields the rows of both files in order, with each file's header row present. Follow Task 1's fixtures. + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cargo test -p gitr-ui --lib detail::diff::split` +Expected: FAIL to compile. + +- [ ] **Step 3: Implement `split_rows`** + +Walk `patch.files`, emit the same `Row::FileHeader` and `Row::Placeholder` cases as Task 1, and for each hunk call `pairing::pair(&hunk.lines)`. + +- [ ] **Step 4: Run it to verify it passes** + +Run: `cargo test -p gitr-ui --lib detail::diff::split` + +- [ ] **Step 5: Paint two columns** + +Extend `DiffBody` to take an enum of either `Vec` or `Vec`. In the split case each row paints gutter, marker and code twice, at `bounds.size.width / 2.` — and declares **two** runs, left then right, so document order runs left-to-right within a row. + +- [ ] **Step 6: Add the toggle** + +In `crates/ui/src/detail/mod.rs`, add a second `TabBar::new("diff-view-mode").segmented().small()` beside the existing `detail-tabs` bar, rendered only when `selected_tab == DetailTab::Diff`. Its `on_click` sets the mode, calls `persistence::save_diff_view_mode` on a background executor, and `cx.notify()`. Load the saved mode in `DetailPanel::new`. + +- [ ] **Step 7: Verify in the running app** + +Confirm: the toggle switches views and survives a restart; a pure addition shows an empty left column rather than collapsing to one; a drag across the split view copies left-then-right within a row and top-to-bottom across rows. + +- [ ] **Step 8: Full check and commit** + +```bash +cargo fmt --all +cargo clippy --workspace --all-targets -- -D warnings +cargo test --workspace +git add -A +git commit -m "feat(diff): add a side-by-side view behind a persisted toggle" +``` + +--- + +## Notes for whoever executes this + +Tasks 1, 2, 3 and 6 are pure and carry real tests. Tasks 4, 5 and 7 touch gpui and are verified by running the app — this crate has no element-tree tests and this plan does not add the machinery for them. + +Task 4 is where this plan is most likely to be wrong. Its code is adapted from a reference example, not from something compiled while writing this. Read +`crates/base/examples/showcase/components/text_selection.rs` in the gpui-component checkout in full before starting it, and treat the snippets here as the shape rather than the letter. If the participant/run model turns out not to work as described, stop and re-open the spec's Virtualisation section rather than working around it in the element. diff --git a/docs/superpowers/specs/2026-08-29-github-style-diff-view-design.md b/docs/superpowers/specs/2026-08-29-github-style-diff-view-design.md new file mode 100644 index 0000000..4565ba9 --- /dev/null +++ b/docs/superpowers/specs/2026-08-29-github-style-diff-view-design.md @@ -0,0 +1,277 @@ +# GitHub-style diff view + +Date: 2026-08-29 +Status: implemented + +## The problem + +Reading a diff in gitr is hard. The reported cause is the `+`/`-` at the start of every line, +which shifts the code one column and clutters it. Investigation found a second cause the +report did not name, and it is the larger one. + +`TextDecoration` backgrounds hug the glyphs of a range rather than filling the row. gpui +paints a run's background as a quad from the first glyph to the last +(`gpui/src/text_system/line.rs:689-701`), one `line_height` tall and one glyph-advance +span wide. So the addition and deletion tints, which already carry GitHub's exact light +values, do not read as bands: they follow the ragged right edge of the text, and a blank +added or deleted line gets no background at all. The `+`/`-` markers are doing most of the +work of signalling a line's kind, because the colour only does half of it. + +Both causes are properties of the same decision: the diff is not a list of elements, it is +**one text document fed to a code editor** (`crates/ui/src/detail/diff.rs`). + +## Why the current architecture cannot deliver the target + +Three capabilities are absent from gpui-component 0.5.2 at the pinned revision +`7acfc184382d30864a688fdaa6c9ff719efc53ae`, and each one is independently fatal: + +- **Gutter content is fixed.** It is always `buffer_line + 1`, right-aligned, one column + (`crates/base/src/input/base/element.rs:2001`). There is no hook to substitute text and + no second column. A GitHub old/new pair is unreachable. +- **No full-width row background.** The only edge-to-edge row fill is the cursor's active + line, coloured from the theme (`element.rs:2107-2131`). Nothing is per-range or per-row. +- **No per-line element injection.** No inlay hints, line widgets, block decorations, or + row render hook anywhere in the input engine. + +The `+`/`-` are also load-bearing for the current colouring: `tree-sitter-diff` maps +additions to the theme's `string` scope and deletions to `keyword` by parsing those very +markers. Removing them from the text removes the foreground colouring with them. + +There is therefore no middle path that keeps the editor. The row rendering has to be built. + +## What that costs, and what it does not + +The module doc on `diff.rs` records three things bought by moving to the editor. Two are +recoverable and one was overstated: + +- **Virtualisation** — recoverable. `gpui_component::virtual_list` is public. The renderer + deleted at `5cc1caa` had no virtualisation at all, so this is an improvement over the + state we are returning to, not a regression. +- **Syntax highlighting** — recoverable and improvable. `gpui_component::highlighter::Highlighter` + is callable directly. Today the `diff` grammar colours by *role*; per file we can colour by + the file's actual language, which is what GitHub does. Deferred, see Out of scope. +- **Text selection** — not lost. `gpui-base` ships a window-level, cross-element document + selection system (`crates/base/src/text_selection.rs`), already mounted in this + application: `gpui_component::Root` renders `TextSelectionLayer` and binds copy + (`crates/ui/src/root.rs:551,592`), and `crates/gitr/src/main.rs:209` constructs that `Root`. + +The work is therefore not implementing selection. It is implementing a custom `Element` per +row type, because `register` and `update_runs` must be called from `prepaint` and `paint`. +A reference implementation exists at +`crates/base/examples/showcase/components/text_selection.rs` (359 lines). + +## Decisions + +**No soft wrap; horizontal scrolling instead.** This is what GitHub does. It is a product +choice, not a technical necessity — an earlier draft of this spec justified it by +virtualisation, which was wrong: `v_virtual_list` takes `Rc>>`, one size +per item, and accepts freely varying heights. What soft wrap really costs is measurement — +every row's wrapped height depends on the viewport width and has to be computed before +anything can be placed. The choice stands on fidelity and on that cost, and can be +revisited without invalidating the rest of this design. + +**The `+`/`-` marker stays, in a column of its own.** Sixteen pixels, between the gutters +and the code, as in the deleted renderer. It no longer shifts the code, it carries the +signal for anyone who reads the green and the red poorly, and being a separate element it +never reaches the clipboard. + +**The domain layer does not change.** `DiffLine` already stores `old_number`, `new_number`, +and a `content` with the marker stripped (`crates/domain/src/patch.rs:27`, parser at +`crates/vcs/src/process/patch_parser.rs:177-220`). Both line numbers are parsed today and +rendered nowhere. Nothing in `domain` or `vcs` needs touching. + +**`gpui-base` becomes a direct dependency**, declared without a `rev`, matching how +`gpui-component` is declared (`Cargo.toml:26`). It is already in `Cargo.lock` at the same +revision, so this unifies rather than adding a second copy — the failure mode `CLAUDE.md` +warns about does not apply here. + +## Module layout + +`crates/ui/src/detail/diff.rs` becomes a directory: + +| File | Owns | +|---|---| +| `diff/mod.rs` | Entry point: picks the layout for the current `DiffViewMode`. | +| `diff/model.rs` | `Row`: `FileHeader \| HunkHeader \| Line \| Placeholder`. Pure. | +| `diff/pairing.rs` | Left/right pairing for the split view. Pure. | +| `diff/body.rs` | The custom `Element`: visible-range windowing, hitbox, one selection participant, N runs, row painting, and the geometry of both views. | +| `diff/split.rs` | Side-by-side row geometry over `Vec`. | +| `diff/palette.rs` | The four tint constants, moved from `decorations.rs`. | +| `crates/ui/src/diff_view_mode.rs` | `DiffViewMode`, modelled on `theme_preference.rs`. | + +`model.rs` and `pairing.rs` hold the logic and carry the tests. `body.rs` holds the risk. +A separate `diff/unified.rs` was planned and not written: the unified view differs from the +split one only in its column count and the width of its gutters, so `body.rs` answers both +from `Rows` rather than arranging each in a module of its own. `split.rs` stays thin — it +arranges rows, it does not decide anything. + +## Data model + +Unified rendering consumes a flat `Vec` built once per patch: + +``` +Row::FileHeader { path, status, added, deleted } +Row::HunkHeader { text } +Row::Line { origin, old_number: Option, new_number: Option, content } +Row::Placeholder { message } +``` + +Split rendering consumes `Vec`, where each side is an `Option` — `None` +renders as a blank, unnumbered, untinted cell. + +Both are derived when the patch or the view mode changes, never per frame. + +## Pairing algorithm + +Within a hunk: accumulate consecutive deletions into `D` and consecutive additions into +`A`. When the run ends — at a context line or at the end of the hunk — emit `max(|D|,|A|)` +rows pairing `D[i]` with `A[i]`, padding the shorter side with `None`. A context line emits +one row carrying the same text on both sides. + +This is a pure function from `&[DiffLine]` to `Vec` and is where the bulk of the +tests go. Cases that must be covered: pure addition, pure deletion, equal-length +replacement, unequal-length replacement in both directions, a run at the very start of a +hunk, a run at the very end with no trailing context, and a hunk of context only. + +## Selection + +The body element registers one participant and declares that frame's visible rows as runs, +in `prepaint`/`paint`: + +- `TextSelectionRegistration::new(hitbox, viewport)` with `.with_document_order(n)` so a drag + across rows copies in document order, and `.with_scroll_offset(bounds.origin − + viewport.origin)`. `gpui-base` stores an endpoint as `position − bounds.origin − + scroll_offset` (`text_selection.rs:1336`) and resolves it back by adding both (`:806-812`), + so what matters is the sum. Reporting the element's own bounds and no offset gives the same + sum, and was what this was built with — but the registered *rectangle* is also the one + `AutoScroll::compute_delta` measures the pointer against (`:1452`), and the element's bounds + are the whole diff, not the part of it on screen, so the trigger zone sat off-screen and a + drag past the bottom edge never scrolled. Registering the viewport and reporting the + difference of the two origins keeps the sum identical by construction and puts the trigger + zone where the user can reach it. +- `TextSelectionRun::new(text, layout, bounds)` — **only for the code content**. The + gutters and the marker are neighbouring elements and are never registered, which is what + makes a copied diff come out as clean code with no line numbers and no markers. This is + strictly better than the editor, which copies its markers today. +- `TextSelectionContentKey` turns out not to be needed. It is a `u64` the participant + computes from an endpoint's content point through + `TextSelectionHandle::resolve_content_key_with`, which `gpui-base` stores on the endpoint + and hands straight back on `TextSelectionSnapshot::anchor().content_key()`. It is a + channel back to the participant and nothing more: it takes no part in hit-testing, in + `project_ranges`, or in copying. What actually makes an off-screen endpoint survive is + that the endpoint is stored as `position − bounds.origin` and this element's + `bounds.origin` already carries the scroll, so an endpoint above the viewport is a + negative `y` rather than a lost one — and a row index here is `y / ROW_HEIGHT`, which is + the identity a key would have carried anyway. + +The text has to go through `StyledText` rather than `div().child("…")`, because a run needs +a `TextLayout`. This is the one structural constraint the selection system imposes on the +row. + +## Virtualisation + +**Not `virtual_list`.** That list produces one element per row, and the selection API runs +the other way round: a participant declares all its runs in a single +`update_runs(&[TextSelectionRun]) -> TextSelectionProjection` call, and the projection +returns one `Option>` per run, in the order given. Per-row elements sharing +one handle would each overwrite the previous row's runs; a handle per row means one +`Entity` per visible line, rebuilt on every scroll. + +The diff body is therefore **one custom `Element`** that computes its own visible range and +paints the rows in it, registering a single participant and declaring that frame's visible +rows as N runs in one call. Virtualisation is kept; it is hand-rolled rather than borrowed. + +This is more code than delegating to a list, and it is the single largest piece of work in +this design. It is also not optional: it follows from the shape of the only selection API +available. + +## Toggle and persistence + +`DiffViewMode { Unified, Split }`, defaulting to `Unified`. Persisted to +`diff-view-preference.json` through the `save_to`/`load_from` plus `save`/`load` pair that +`persistence.rs` already uses for the theme, so the pure half stays testable without +touching the disk. Surfaced as a segmented control in the detail panel's existing tab row +(`detail/mod.rs:164-187`). + +## Edge cases + +- Binary file, rename with no content change, and an empty commit keep their three existing + placeholder messages. +- `\ No newline at end of file` is already swallowed by the parser. GitHub renders a marker + for it; v1 does not. +- Lines beyond 10 000 characters skip highlighting in gpui-component. With no soft wrap they + simply scroll. +- A file that is pure addition renders an entirely blank left column in split view. That is + correct and should not be special-cased into a single-column view. + +## Testing + +- `model.rs` and `pairing.rs`: pure unit tests, the bulk of the suite. +- The contrast reasoning in `decorations.rs:20-35` moves to `palette.rs` with its tests + intact. Those values were chosen against two bars — legibility of text on the band, and + visibility of the band against the theme background — and that reasoning must survive the + move. +- `format::unified_diff_text_with_line_ranges`, `DiffLineRanges`, `write_file` and the + private path helpers that exist only to serve them become dead. Delete them along with + their tests. The case guarded by + `line_ranges_are_read_from_the_marker_column_not_the_lines_own_content` does not + disappear; it becomes trivially correct once no marked text is reconstructed. +- No element-tree tests, consistent with the rest of the crate. +- Manual verification in the running app is required and must not be skipped: nothing here + proves a pixel. + +## Out of scope + +Deliberately excluded from v1, each worth its own change: + +- Syntax highlighting of code by the file's language. +- Word-level intra-line highlighting of what actually changed. +- Expandable context beyond the hunk. +- Per-file collapsing. + +## Risks and order of work + +The custom `Element` is the risk, not the algorithm. Build it first, against the unified +view alone, and prove three things before writing any split-view code: that a drag selects +across row boundaries, that a copy yields code without gutters or markers, and that both +survive scrolling. A bad surprise then arrives before the second view is written rather +than after. + +Second risk: the coordinate handling. `TextLayout::bounds`, `line_height`, `len`, +`position_for_index` and `index_for_position` all panic when called before the text has been +laid out, but not on the same cell: `len` and `line_height` need only the one the measure +closure fills (`text.rs:935-942`), while `bounds`, `position_for_index` and +`index_for_position` need the one prepaint fills as well (`:830-837`, `:864-871`, +`:930-932`). All five are safe from `paint` and from nowhere earlier — the distinction +matters only when deciding which rows may be skipped. This is where hand-rolled windowing +bites, and the +two obvious moves are both wrong. Narrowing layout while keeping a run per row panics on the +first scroll with a live selection, because `selection_range_for_run` reads `layout.len()` on +every run it is handed. Narrowing the runs to match avoids the panic and silently truncates a +copy to whatever rows are on screen. + +The way out is that the copy does not have to come from the projection. `update_runs` covers +the window and drives only the highlight; the copied text is derived from the selection's own +window points, which are scroll-invariant, so the row span is arithmetic and only the two rows +whose ends the selection cuts through need shaping at all — a row between them is whole by +construction, and answering `0..len` for it must short-circuit before the shaper is reached +rather than after, or the saving is only notional. A row already on screen keeps the +projection's range, so the highlight and the clipboard cannot disagree. What stays unwindowed +is the content-width measurement, which must consider every row or the horizontal scroll +extent moves as the view scrolls vertically. + +## Files touched + +- New: `crates/ui/src/detail/diff/` (six files), `crates/ui/src/diff_view_mode.rs`, + `docs/superpowers/specs/2026-08-29-github-style-diff-view-design.md`. +- Rewritten: `crates/ui/src/detail/mod.rs` — `pending_diff`, `diff_editor` and + `diff_decorations` all disappear, along with the flush block that pushed text into the + editor. +- Reduced: `crates/ui/src/detail/format.rs` — the text reconstruction goes. `hunk_heading` + stays, and so do `abbreviate`, `format_timestamp` and `escape_markdown`, which serve + `metadata.rs` and are unrelated to this change. Note that `file_header` and `diff_stat` + do **not** exist in this file today — they lived only in the renderer deleted at + `5cc1caa` and have to be written again, recoverable from + `git show 5cc1caa^:crates/ui/src/detail/format.rs`. +- Deleted: `crates/ui/src/detail/decorations.rs`, superseded by `diff/palette.rs`. +- Extended: `crates/ui/src/persistence.rs`, `Cargo.toml`.