Panics · P19 Git: a lagging blame draws every line; resolve answers None for a conflict gone

src/git/blame.rs

@@ -24,8 +24,8 @@ pub struct Blame {
 }
 
 /// Consecutive lines from one change, as the change's index and the lines.
-pub(crate) fn runs(owners: &[usize]) -> Vec<(usize, Range<usize>)> {
-    let mut out: Vec<(usize, Range<usize>)> = Vec::new();
+pub(crate) fn runs(owners: &[Option<usize>]) -> Vec<(Option<usize>, Range<usize>)> {
+    let mut out: Vec<(Option<usize>, Range<usize>)> = Vec::new();
     for (line, owner) in owners.iter().enumerate() {
         match out.last_mut() {
             Some((last, lines)) if last == owner => lines.end = line + 1,
@@ -69,7 +69,7 @@ type OnCommit = Rc<dyn Fn(&SharedString, &mut Window, &mut App)>;
 pub struct BlameView {
     id: ElementId,
     code: SharedString,
-    owners: Vec<usize>,
+    owners: Vec<Option<usize>>,
     blames: Vec<Blame>,
     current: Option<usize>,
     on_commit: Option<OnCommit>,
@@ -86,15 +86,22 @@ impl BlameView {
         let code: SharedString = code.into();
         let owners: Vec<usize> = owners.into_iter().collect();
         let blames: Vec<Blame> = blames.into_iter().collect();
-        assert_eq!(
-            owners.len(),
-            code.lines().count(),
-            "every line has an owner"
-        );
-        assert!(
-            owners.iter().all(|owner| *owner < blames.len()),
-            "every owner is a blame"
-        );
+        let lines = code.lines().count();
+        if owners.len() != lines || owners.iter().any(|owner| *owner >= blames.len()) {
+            log::error!(
+                "blame view: {} owners for {lines} lines and {} blames; lines without one draw bare",
+                owners.len(),
+                blames.len()
+            );
+        }
+        let owners: Vec<Option<usize>> = (0..lines)
+            .map(|line| {
+                owners
+                    .get(line)
+                    .copied()
+                    .filter(|owner| *owner < blames.len())
+            })
+            .collect();
         Self {
             id: id.into(),
             code,
@@ -129,7 +136,16 @@ impl RenderOnce for BlameView {
         let blocks: Vec<AnyElement> = runs(&self.owners)
             .into_iter()
             .map(|(owner, span)| {
-                let blame = &self.blames[owner];
+                let blame = owner.map(|owner| self.blames[owner].clone());
+                let Some(found) = blame.clone() else {
+                    let bare = div()
+                        .flex_none()
+                        .w_64()
+                        .border_l_2()
+                        .border_color(colors.border);
+                    return (span, bare.into_any_element(), None);
+                };
+                let blame = found;
                 let fade = 1.0 - blame.age.clamp(0.0, 1.0) * 0.8;
                 let pick = self.on_commit.clone();
                 let commit = blame.commit.clone();
@@ -186,6 +202,9 @@ impl RenderOnce for BlameView {
                                     .child(Ellipsis::new(blame.subject.clone())),
                             ),
                     );
+                (span, gutter.into_any_element(), Some(blame))
+            })
+            .map(|(span, gutter, blame)| {
                 let code = div()
                     .flex_1()
                     .min_w_0()
@@ -210,9 +229,10 @@ impl RenderOnce for BlameView {
                                     .child((line + 1).to_string()),
                             )
                             .child(StyledText::new(text).with_highlights(styles))
-                            .when(self.current == Some(line), |row| {
-                                row.child(GitBlameAnnotation::new(blame.clone()))
-                            })
+                            .when_some(
+                                blame.clone().filter(|_| self.current == Some(line)),
+                                |row, blame| row.child(GitBlameAnnotation::new(blame)),
+                            )
                     }));
                 div()
                     .flex()
@@ -239,7 +259,16 @@ mod tests {
 
     #[test]
     fn lines_from_one_change_run_together() {
-        assert_eq!(runs(&[0, 0, 1, 0, 0, 0]), [(0, 0..2), (1, 2..3), (0, 3..6)]);
+        let owned = [Some(0), Some(0), Some(1), None, None, Some(0)];
+        assert_eq!(
+            runs(&owned),
+            [
+                (Some(0), 0..2),
+                (Some(1), 2..3),
+                (None, 3..5),
+                (Some(0), 5..6)
+            ]
+        );
         assert!(runs(&[]).is_empty());
     }
 }

src/git/merge.rs

@@ -295,7 +295,9 @@ impl RenderOnce for ConflictResolver {
         let on_take: Option<OnTake> = self.on_resolve.map(|on_resolve| {
             Rc::new(
                 move |ix: usize, take: Take, window: &mut Window, cx: &mut App| {
-                    on_resolve(resolve(&text, ix, take), window, cx)
+                    if let Some(settled) = resolve(&text, ix, take) {
+                        on_resolve(settled, window, cx)
+                    }
                 },
             ) as OnTake
         });

src/git/merging.rs

@@ -156,9 +156,12 @@ pub fn conflicts(text: &str) -> Vec<Conflict> {
     found
 }
 
-/// `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];
+/// `text` with conflict `ix` settled as taken, every other byte kept; none when `text` has no conflict `ix`.
+pub fn resolve(text: &str, ix: usize, take: Take) -> Option<String> {
+    let Some(conflict) = conflicts(text).get(ix).copied() else {
+        log::error!("no conflict {ix} to resolve in this text");
+        return None;
+    };
     let lines = text.tokenize_lines();
     let ours = lines[conflict.start + 1..conflict.middle].concat();
     let theirs = lines[conflict.middle + 1..conflict.end].concat();
@@ -168,12 +171,14 @@ pub fn resolve(text: &str, ix: usize, take: Take) -> String {
         Take::Both => ours + &theirs,
     };
     log::info!("conflict {ix} took {take:?}");
-    [
-        lines[..conflict.start].concat(),
-        kept,
-        lines[conflict.end + 1..].concat(),
-    ]
-    .concat()
+    Some(
+        [
+            lines[..conflict.start].concat(),
+            kept,
+            lines[conflict.end + 1..].concat(),
+        ]
+        .concat(),
+    )
 }
 
 #[cfg(test)]
@@ -210,7 +215,10 @@ mod tests {
         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");
+        assert_eq!(
+            resolve(marked, 0, Take::Ours).as_deref(),
+            Some("a\r\nours\r\nz")
+        );
     }
 
     #[test]
@@ -219,7 +227,8 @@ mod tests {
         assert_eq!(result(&changed, &[]).0, "a\rB\n");
         let held = result(&regions("a\r", "b\r", "c\r"), &[]).0;
         assert_eq!(held, "<<<<<<< ours\rb\r=======\rc\r>>>>>>> theirs\r");
-        assert_eq!(resolve(&held, 0, Take::Theirs), "c\r");
+        assert_eq!(resolve(&held, 0, Take::Theirs).as_deref(), Some("c\r"));
+        assert_eq!(resolve("plain\n", 0, Take::Ours), None, "no conflict left");
     }
 
     #[test]
@@ -239,8 +248,14 @@ mod tests {
                 end: 5
             }]
         );
-        assert_eq!(resolve(marked, 0, Take::Ours), "a\nours\nz\n");
-        assert_eq!(resolve(marked, 0, Take::Both), "a\nours\ntheirs\nz\n");
+        assert_eq!(
+            resolve(marked, 0, Take::Ours).as_deref(),
+            Some("a\nours\nz\n")
+        );
+        assert_eq!(
+            resolve(marked, 0, Take::Both).as_deref(),
+            Some("a\nours\ntheirs\nz\n")
+        );
         assert!(
             conflicts("a\n=======\nb\n").is_empty(),
             "a rule alone is no conflict"

src/git/tests.rs

@@ -50,3 +50,28 @@ fn a_fold_opens_from_the_keyboard(cx: &mut TestAppContext) {
     settle(cx);
     assert_eq!(view.read_with(cx, |folded, _| folded.opened.clone()), [0]);
 }
+
+/// A blame from before the file changed: an owner too few, and one past the blames.
+struct Behind;
+
+impl Render for Behind {
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        let blame = super::Blame {
+            commit: "a1".into(),
+            author: "Ann".into(),
+            when: "today".into(),
+            subject: "first".into(),
+            age: 0.0,
+        };
+        div().w(px(600.0)).child(
+            super::BlameView::new("blame", "one\ntwo\nthree\nfour", [0, 3, 0], [blame]).current(3),
+        )
+    }
+}
+
+#[gpui::test]
+fn a_blame_that_lags_its_code_draws_every_line(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    let (_, cx) = cx.add_window_view(|_, _| Behind);
+    cx.run_until_parked();
+}

tasks/panics.md

@@ -33,7 +33,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca
 - [x] P16 Finance — `src/finance`
 - [x] P17 Code Editor — `src/editor`
 - [x] P18 Terminal — `src/terminal`
-- [ ] P19 Git — `src/git`
+- [x] P19 Git — `src/git`
 - [ ] P20 Debug — `src/debug`
 - [ ] P21 Documents — `src/documents`
 - [ ] P22 Collaboration — `src/collab`
@@ -84,3 +84,4 @@ One line per item: what changed, what stayed and why, review rounds.
 - Noted for later items: `chat/params.rs` and `editor/settings.rs` pass `value.abs()` to `decimals`, which asserts a positive step.
 - P17: the editor keeps every owner offset on whole characters of its text (checked on intake, carried through edits by `editor::anchors`, cleared when the whole text changes); stale selections and edit batches are dropped; a painted row whose line the text lost takes no click, drag or IME query; the sticky header reads this frame; a glob too big to compile matches nothing; a missing font advance logs. Kept: owner contracts and construction. Review: 5 rounds, passed on the fifth.
 - P18: row paints and the font advance log gpui faults; a pty side caps at what ConPTY and alacritty's pixel sizes hold; a default shell past the list keeps the one before. Kept: constant regexes, captured matches, keyed rows, non-empty shells. Review: 2 rounds.
+- P19: a blame that lags its code draws every line, bare where no owner fits; `git::resolve` returns `Option` for a conflict gone from the text. Kept: diff groups, merge regions, similar's pinned resolutions. Review: 2 rounds.