diff --git a/TASKS.md b/TASKS.md index bfd1707..2e74934 100644 --- a/TASKS.md +++ b/TASKS.md @@ -242,4 +242,5 @@ Per-component lines live in `tasks/`. Tags there: | T18 | 1 | FAIL | a missing folder ran in the app's own; combining marks dropped; Tab skipped the terminal; its wheel scrolled the page too; clear kept find matches; New bash reused the old shell; the dark script typed into the replay | | T18 | 2 | FAIL | a folder it could not enter still started the shell elsewhere; links read cluster cells as spaces | | T18 | 3 | PASS | | -| T19 | 1 | — | pending review | +| T19 | 1 | FAIL | a merge rewrote line endings and added a last newline; the split view numbered its right side from the old file; an empty side started its hunk a line late; the subject counter covered long subjects; clipped code rows had no line height | +| T19 | 2 | — | pending review | diff --git a/examples/gallery/pages/git/diffs.rs b/examples/gallery/pages/git/diffs.rs index 658f11f..b9c8708 100644 --- a/examples/gallery/pages/git/diffs.rs +++ b/examples/gallery/pages/git/diffs.rs @@ -6,7 +6,10 @@ use ely_gpui_component::{ }; use gpui::{App, IntoElement, ParentElement, SharedString, Styled, Window, div, px}; -use crate::ui::{keep, section, set}; +use crate::{ + probe::probe, + ui::{keep, section, set}, +}; const OLD: &str = r#"use std::collections::HashMap; @@ -84,7 +87,8 @@ pub fn viewer(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { "One file's changes, unified or side by side: each change in its context with old and new numbers, the words that changed washed, the stretches between folded until pressed. InlineDiff inside an editor is CodeEditor::set_diff.", cx, ) - .child( + .child(probe( + "git-diff", div() .w(px(840.)) .h(px(360.)) @@ -103,7 +107,7 @@ pub fn viewer(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { set(&unfold, next, cx) }), ), - ) + )) } const BASE: &str = diff --git a/examples/gallery/pages/git/mod.rs b/examples/gallery/pages/git/mod.rs index 1adc80f..ef0fcf3 100644 --- a/examples/gallery/pages/git/mod.rs +++ b/examples/gallery/pages/git/mod.rs @@ -16,7 +16,20 @@ pub const PAGE: Page = Page { script: SCRIPT, }; -const SCRIPT: &[Step] = &[Step::Rest]; +const SCRIPT: &[Step] = &[ + Step::Rest, + Step::DownAt("git-diff", 800.0, 21.0), + Step::UpAt("git-diff", 800.0, 21.0), + Step::Wait(300), + Step::Shot("split"), + Step::DownAt("git-commit", 40.0, 20.0), + Step::UpAt("git-commit", 40.0, 20.0), + Step::Wait(200), + Step::Key("cmd-a"), + Step::Type("Blend every accent toward white by lift, clamped in gamma"), + Step::Wait(300), + Step::Shot("subject"), +]; fn render(window: &mut Window, cx: &mut App) -> AnyElement { div() diff --git a/examples/gallery/pages/git/work.rs b/examples/gallery/pages/git/work.rs index 5f8c5ce..caa01b9 100644 --- a/examples/gallery/pages/git/work.rs +++ b/examples/gallery/pages/git/work.rs @@ -10,7 +10,10 @@ use ely_gpui_component::{ }; use gpui::{App, Entity, IntoElement, ParentElement, SharedString, Styled, Window, div, px}; -use crate::ui::{keep, row, section, set}; +use crate::{ + probe::probe, + ui::{keep, row, section, set}, +}; fn changed(path: &str, status: GitStatus, added: usize, removed: usize) -> Changed { Changed { @@ -113,7 +116,8 @@ pub fn changes(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { }), ), ) - .child( + .child(probe( + "git-commit", div().w(px(380.)).child( CommitInput::new("git-commit", &subject, &body, "main", count) .amend(now_amend) @@ -124,7 +128,7 @@ pub fn changes(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { set(&done, Some(format!("Committed {files} file{}.", if files == 1 { "" } else { "s" }).into()), cx); }), ), - ), + )), ) .children(told.map(Caption::new)) } diff --git a/src/git/blame.rs b/src/git/blame.rs index 86cbdd8..5288634 100644 --- a/src/git/blame.rs +++ b/src/git/blame.rs @@ -3,14 +3,14 @@ use std::{ops::Range, rc::Rc}; use gpui::{ AnyElement, App, ElementId, FontWeight, InteractiveElement, IntoElement, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, StyledText, Window, div, - prelude::*, + prelude::*, relative, }; use crate::{ data_display::Avatar, editor::code_colors, theme::{ActiveTheme, AvatarSize, TextSize}, - typography::Ellipsis, + typography::{Ellipsis, LEADING}, }; /// Who last changed lines: the commit, its author, when and its subject, and its age from 0, newest, to 1, oldest. @@ -190,6 +190,7 @@ impl RenderOnce for BlameView { .flex_1() .min_w_0() .overflow_hidden() + .line_height(relative(LEADING)) .flex() .flex_col() .children(span.clone().map(|line| { diff --git a/src/git/changes.rs b/src/git/changes.rs index cf32617..68688e3 100644 --- a/src/git/changes.rs +++ b/src/git/changes.rs @@ -333,9 +333,8 @@ impl RenderOnce for CommitInput { } }) .child( - div().relative().child(Input::new(&self.subject)).child( - tabular(div().absolute().top_2().right_2().text_color(count)) - .child(format!("{length}/{}", SUBJECT.0)), + Input::new(&self.subject).suffix( + tabular(div().text_color(count)).child(format!("{length}/{}", SUBJECT.0)), ), ) .child(Input::new(&self.body)) diff --git a/src/git/diff.rs b/src/git/diff.rs index c841f68..057b110 100644 --- a/src/git/diff.rs +++ b/src/git/diff.rs @@ -1,7 +1,7 @@ use std::ops::Range; use gpui::SharedString; -use similar::{ChangeTag, TextDiff}; +use similar::{ChangeTag, TextDiff, udiff::UnifiedHunkHeader}; /// Whether a diff line stayed, came in, or went. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -95,13 +95,7 @@ pub fn diff(old: &str, new: &str, context: usize) -> Vec { }); } } - let header = format!( - "@@ -{},{} +{},{} @@", - old_start + 1, - old_end - old_start, - new_start + 1, - new_end - new_start - ); + let header = UnifiedHunkHeader::new(&group).to_string(); out.push(Stretch::Hunk { header: header.into(), lines, @@ -157,6 +151,17 @@ pub fn pairs(lines: &[DiffLine]) -> Vec<(Option<&DiffLine>, Option<&DiffLine>)> out } +/// The numbers a side-by-side row shows: the old file's on the left, the new file's on the right. +pub(crate) fn side_numbers( + left: Option<&DiffLine>, + right: Option<&DiffLine>, +) -> (Option, Option) { + ( + left.and_then(|line| line.old), + right.and_then(|line| line.new), + ) +} + #[cfg(test)] mod tests { use super::*; @@ -214,4 +219,29 @@ mod tests { .collect(); assert_eq!(sides, [(true, true), (true, true), (false, true)]); } + + #[test] + fn each_side_numbers_its_own_file() { + let stretches = diff("a\nb\n", "x\na\nb\n", 3); + let [Stretch::Hunk { lines, .. }] = stretches.as_slice() else { + panic!("one hunk, not {stretches:?}"); + }; + let numbers: Vec<_> = pairs(lines) + .into_iter() + .map(|(left, right)| side_numbers(left, right)) + .collect(); + assert_eq!( + numbers, + [(None, Some(1)), (Some(1), Some(2)), (Some(2), Some(3))] + ); + } + + #[test] + fn an_empty_side_starts_before_its_range() { + let stretches = diff("", "a\n", 3); + let [Stretch::Hunk { header, .. }] = stretches.as_slice() else { + panic!("one hunk, not {stretches:?}"); + }; + assert_eq!(header.as_ref(), "@@ -0,0 +1 @@"); + } } diff --git a/src/git/merge.rs b/src/git/merge.rs index a2598d9..839142a 100644 --- a/src/git/merge.rs +++ b/src/git/merge.rs @@ -2,7 +2,7 @@ use std::rc::Rc; use gpui::{ AnyElement, App, Div, ElementId, FontWeight, Hsla, IntoElement, ParentElement, RenderOnce, - SharedString, Styled, Window, div, prelude::*, + SharedString, Styled, Window, div, prelude::*, relative, }; use super::merging::{Region, RegionKind, Take, conflicts, resolve, result}; @@ -10,7 +10,7 @@ use crate::{ buttons::{Button, ButtonVariant}, primitives::{Icon, IconName}, theme::{ActiveTheme, ControlSize, IconSize, Palette, Radius, TextSize}, - typography::format, + typography::{LEADING, format}, }; type OnTake = Rc; @@ -26,10 +26,14 @@ fn column(lines: &[String], rows: usize, wash: Option) -> Div { .px_2() .when_some(wash, |column, wash| column.bg(wash)) .children((0..rows).map(|ix| { + let line = lines + .get(ix) + .map_or("", |line| line.trim_end_matches(['\r', '\n'])); div() .whitespace_nowrap() .overflow_hidden() - .child(lines.get(ix).cloned().unwrap_or_default()) + .line_height(relative(LEADING)) + .child(line.to_string()) })) } diff --git a/src/git/merging.rs b/src/git/merging.rs index 1b44307..efee2cc 100644 --- a/src/git/merging.rs +++ b/src/git/merging.rs @@ -28,14 +28,34 @@ pub struct Region { pub theirs: Vec, } +/// Lines with their own endings, so a merge gives back each byte it keeps. fn lines(text: &str, range: std::ops::Range) -> Vec { - text.lines() + text.split_inclusive('\n') .skip(range.start) .take(range.len()) .map(str::to_string) .collect() } +/// The line ending the merged texts use: the first one found, or a newline. +fn ending(regions: &[Region]) -> &'static str { + let line = regions + .iter() + .flat_map(|region| region.base.iter().chain(®ion.ours).chain(®ion.theirs)) + .find(|line| line.ends_with('\n')); + match line { + Some(line) if line.ends_with("\r\n") => "\r\n", + _ => "\n", + } +} + +/// Ends the last line when it has no ending, so what follows starts a line. +fn break_line(out: &mut String, ending: &str) { + if !out.is_empty() && !out.ends_with('\n') { + out.push_str(ending); + } +} + /// `ours` and `theirs` merged against `base`, region by region. pub fn regions(base: &str, ours: &str, theirs: &str) -> Vec { TextMerge::from_lines(base, ours, theirs) @@ -59,41 +79,42 @@ pub fn regions(base: &str, ours: &str, theirs: &str) -> Vec { /// The merged text with each conflict as taken, and how many are still open; an open one keeps its markers. pub fn result(regions: &[Region], takes: &[Option]) -> (String, usize) { - let mut out: Vec = Vec::new(); + let ending = ending(regions); + let mut out = String::new(); let mut conflicts = 0; let mut open = 0; for region in regions { + let (ours, theirs) = (region.ours.concat(), region.theirs.concat()); match region.kind { - RegionKind::Unchanged | RegionKind::Ours | RegionKind::Both => { - out.extend(region.ours.iter().cloned()) - } - RegionKind::Theirs => out.extend(region.theirs.iter().cloned()), + RegionKind::Unchanged | RegionKind::Ours | RegionKind::Both => out.push_str(&ours), + RegionKind::Theirs => out.push_str(&theirs), RegionKind::Conflict => { match takes.get(conflicts).copied().flatten() { - Some(Take::Ours) => out.extend(region.ours.iter().cloned()), - Some(Take::Theirs) => out.extend(region.theirs.iter().cloned()), + Some(Take::Ours) => out.push_str(&ours), + Some(Take::Theirs) => out.push_str(&theirs), Some(Take::Both) => { - out.extend(region.ours.iter().cloned()); - out.extend(region.theirs.iter().cloned()); + out.push_str(&ours); + break_line(&mut out, ending); + out.push_str(&theirs); } None => { open += 1; - out.push("<<<<<<< ours".into()); - out.extend(region.ours.iter().cloned()); - out.push("=======".into()); - out.extend(region.theirs.iter().cloned()); - out.push(">>>>>>> theirs".into()); + for (marker, side) in [("<<<<<<< ours", &ours), ("=======", &theirs)] { + break_line(&mut out, ending); + out.push_str(marker); + out.push_str(ending); + out.push_str(side); + } + break_line(&mut out, ending); + out.push_str(">>>>>>> theirs"); + out.push_str(ending); } } conflicts += 1; } } } - let mut text = out.join("\n"); - if !out.is_empty() { - text.push('\n'); - } - (text, open) + (out, open) } /// A conflict in marked text: the lines of its markers, from zero. @@ -126,26 +147,24 @@ pub fn conflicts(text: &str) -> Vec { found } -/// `text` with conflict `ix` settled as taken. +/// `text` with conflict `ix` settled as taken; every other byte stays. pub fn resolve(text: &str, ix: usize, take: Take) -> String { let conflict = conflicts(text)[ix]; - let lines: Vec<&str> = text.lines().collect(); - let ours = &lines[conflict.start + 1..conflict.middle]; - let theirs = &lines[conflict.middle + 1..conflict.end]; - let kept: Vec<&str> = match take { - Take::Ours => ours.to_vec(), - Take::Theirs => theirs.to_vec(), - Take::Both => ours.iter().chain(theirs).copied().collect(), + let lines: Vec<&str> = text.split_inclusive('\n').collect(); + let ours = lines[conflict.start + 1..conflict.middle].concat(); + let theirs = lines[conflict.middle + 1..conflict.end].concat(); + let kept = match take { + Take::Ours => ours, + Take::Theirs => theirs, + Take::Both => ours + &theirs, }; - let mut out: Vec<&str> = lines[..conflict.start].to_vec(); - out.extend(kept); - out.extend(&lines[conflict.end + 1..]); - let mut settled = out.join("\n"); - if text.ends_with('\n') { - settled.push('\n'); - } log::info!("conflict {ix} took {take:?}"); - settled + [ + lines[..conflict.start].concat(), + kept, + lines[conflict.end + 1..].concat(), + ] + .concat() } #[cfg(test)] @@ -169,6 +188,22 @@ mod tests { assert_eq!(taken, "fn lift() {\n 0.75\n}\nfn keep() { log() }\n"); } + #[test] + fn merges_keep_line_endings_and_the_last_newline() { + assert_eq!(result(®ions("x", "x", "x"), &[]), ("x".into(), 0)); + let crlf = regions("a\r\nb\r\n", "a\r\nB\r\n", "a\r\nb\r\n"); + assert_eq!(result(&crlf, &[]).0, "a\r\nB\r\n"); + let held = regions("a\r\n", "b\r\n", "c\r\n"); + assert_eq!( + result(&held, &[]).0, + "<<<<<<< ours\r\nb\r\n=======\r\nc\r\n>>>>>>> theirs\r\n" + ); + let bare = regions("a", "b", "c"); + assert_eq!(result(&bare, &[Some(Take::Both)]).0, "b\nc"); + let marked = "a\r\n<<<<<<< HEAD\r\nours\r\n=======\r\ntheirs\r\n>>>>>>> topic\r\nz"; + assert_eq!(resolve(marked, 0, Take::Ours), "a\r\nours\r\nz"); + } + #[test] fn markers_resolve_to_one_side_or_both() { let marked = "a\n<<<<<<< HEAD\nours\n=======\ntheirs\n>>>>>>> topic\nz\n"; diff --git a/src/git/review.rs b/src/git/review.rs index 21f62ef..635fbf2 100644 --- a/src/git/review.rs +++ b/src/git/review.rs @@ -3,7 +3,7 @@ use std::rc::Rc; use gpui::{ AnyElement, App, ElementId, Entity, FontWeight, Hsla, InteractiveElement, IntoElement, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, Window, div, - prelude::*, + prelude::*, relative, }; use super::badges::DiffStat; @@ -13,7 +13,7 @@ use crate::{ forms::{Input, TextInput}, primitives::{Icon, IconName}, theme::{ActiveTheme, AvatarSize, ControlSize, IconSize, Palette, Radius, TextSize}, - typography::{Ellipsis, format}, + typography::{Ellipsis, LEADING, format}, }; type Run = Rc; @@ -359,6 +359,7 @@ impl RenderOnce for ReviewComment { .text_size(theme.text_size(TextSize::Xs)) .whitespace_nowrap() .overflow_hidden() + .line_height(relative(LEADING)) .child(format!("{} {}", self.line, self.code)), ) .children(notes) diff --git a/src/git/viewer.rs b/src/git/viewer.rs index c9fdea3..70a5fc7 100644 --- a/src/git/viewer.rs +++ b/src/git/viewer.rs @@ -3,18 +3,18 @@ use std::{ops::Range, rc::Rc}; use gpui::{ AnyElement, App, ElementId, FontWeight, HighlightStyle, Hsla, InteractiveElement, IntoElement, MouseButton, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, - StyledText, Window, div, prelude::*, uniform_list, + StyledText, Window, div, prelude::*, relative, uniform_list, }; use super::{ badges::DiffStat, - diff::{DiffLine, LineKind, Stretch, diff, pairs}, + diff::{DiffLine, LineKind, Stretch, diff, pairs, side_numbers}, }; use crate::{ buttons::SegmentedControl, editor::{code_colors, stack}, theme::{ActiveTheme, ControlSize, Palette, TextSize}, - typography::format, + typography::{LEADING, format}, }; /// How a diff lays out. @@ -210,7 +210,7 @@ impl RenderOnce for DiffViewer { .children(value.map(|value| value.to_string())) }; let side = - |line: Option<&DiffLine>, cx: &App| -> AnyElement { + |line: Option<&DiffLine>, shown: Option, cx: &App| -> AnyElement { let (wash, _) = line .map_or((Some(colors.hover.opacity(0.5)), colors.hover), |line| { washes(line.kind, &colors) @@ -222,7 +222,7 @@ impl RenderOnce for DiffViewer { .gap_2() .overflow_hidden() .when_some(wash, |side, wash| side.bg(wash)) - .child(number(line.and_then(|line| line.old.or(line.new)))) + .child(number(shown)) .children(line.map(|line| { div().whitespace_nowrap().child(code(line, &colors, cx)) })) @@ -230,7 +230,13 @@ impl RenderOnce for DiffViewer { }; range .map(|ix| { - let row = div().id(ix).w_full().flex().px_2().whitespace_nowrap(); + let row = div() + .id(ix) + .w_full() + .flex() + .px_2() + .whitespace_nowrap() + .line_height(relative(LEADING)); match &rows[ix] { Shown::Header(header) => row .bg(colors.hover) @@ -260,11 +266,13 @@ impl RenderOnce for DiffViewer { ) .into_any_element() } - Shown::Pair(left, right) => row - .gap_1() - .child(side(left.as_ref(), cx)) - .child(side(right.as_ref(), cx)) - .into_any_element(), + Shown::Pair(left, right) => { + let (old, new) = side_numbers(left.as_ref(), right.as_ref()); + row.gap_1() + .child(side(left.as_ref(), old, cx)) + .child(side(right.as_ref(), new, cx)) + .into_any_element() + } Shown::Fold(stretch, count) => { let (open, stretch) = (on_open.clone(), *stretch); row.id((id.clone(), format!("fold-{stretch}")))