T19 round 2 fixes

- regions read their lines through TextMerge's accessors; conflicts and resolve
  split with similar's tokenize_lines, so a lone return ends a line everywhere
- the merged ending and line breaks know a lone return
- taking both sides separates them only when both hold text
- merge views split rows the same way

TASKS.md

@@ -243,4 +243,5 @@ Per-component lines live in `tasks/`. Tags there:
 | 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 | 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 |
+| T19 | 2 | FAIL | a lone return split lines for similar but not for regions, so a change was lost; taking both sides added a newline to an empty side |
+| T19 | 3 | — | pending review |

src/git/merge.rs

@@ -5,6 +5,8 @@ use gpui::{
     SharedString, Styled, Window, div, prelude::*, relative,
 };
 
+use similar::DiffableStr;
+
 use super::merging::{Region, RegionKind, Take, conflicts, resolve, result};
 use crate::{
     buttons::{Button, ButtonVariant},
@@ -238,7 +240,8 @@ impl RenderOnce for ThreeWayMerge {
                     .font_family(theme.mono_family.clone())
                     .text_size(theme.text_size(TextSize::Xs))
                     .text_color(colors.fg)
-                    .children(merged.lines().map(|line| {
+                    .children(merged.tokenize_lines().into_iter().map(|line| {
+                        let line = line.trim_end_matches(['\r', '\n']);
                         let marker = ["<<<<<<<", "=======", ">>>>>>>"]
                             .iter()
                             .any(|mark| line.starts_with(mark));
@@ -298,7 +301,8 @@ impl RenderOnce for ConflictResolver {
         });
         let (current, incoming) = (colors.success.opacity(0.1), colors.info.opacity(0.1));
         let mut rows: Vec<AnyElement> = Vec::new();
-        for (ix, line) in self.text.lines().enumerate() {
+        for (ix, line) in self.text.tokenize_lines().into_iter().enumerate() {
+            let line = line.trim_end_matches(['\r', '\n']);
             if let Some(at) = found.iter().position(|conflict| conflict.start == ix) {
                 rows.push(
                     takes(

src/git/merging.rs

@@ -1,4 +1,4 @@
-use similar::{MergeResolution, TextMerge};
+use similar::{DiffableStr, MergeResolution, TextMerge};
 
 /// Which side a conflict takes.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -28,12 +28,17 @@ pub struct Region {
     pub theirs: Vec<String>,
 }
 
-/// Lines with their own endings, so a merge gives back each byte it keeps.
+/// A region's lines as similar split them, each with its ending.
-fn lines(text: &str, range: std::ops::Range<usize>) -> Vec<String> {
+fn lines<'a>(
-    text.split_inclusive('\n')
+    range: std::ops::Range<usize>,
-        .skip(range.start)
+    line: impl Fn(usize) -> Option<&'a str>,
-        .take(range.len())
+) -> Vec<String> {
-        .map(str::to_string)
+    range
+        .map(|ix| {
+            line(ix)
+                .expect("a region's lines are in its text")
+                .to_string()
+        })
         .collect()
 }
 
@@ -42,23 +47,25 @@ fn ending(regions: &[Region]) -> &'static str {
     let line = regions
         .iter()
         .flat_map(|region| region.base.iter().chain(&region.ours).chain(&region.theirs))
-        .find(|line| line.ends_with('\n'));
+        .find(|line| line.as_str().ends_with_newline());
     match line {
         Some(line) if line.ends_with("\r\n") => "\r\n",
+        Some(line) if line.ends_with('\r') => "\r",
         _ => "\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') {
+    if !out.is_empty() && !out.as_str().ends_with_newline() {
         out.push_str(ending);
     }
 }
 
 /// `ours` and `theirs` merged against `base`, region by region.
 pub fn regions(base: &str, ours: &str, theirs: &str) -> Vec<Region> {
-    TextMerge::from_lines(base, ours, theirs)
+    let merge = TextMerge::from_lines(base, ours, theirs);
+    merge
         .regions()
         .iter()
         .map(|region| Region {
@@ -70,9 +77,9 @@ pub fn regions(base: &str, ours: &str, theirs: &str) -> Vec<Region> {
                 MergeResolution::Conflict => RegionKind::Conflict,
                 other => unreachable!("a merge resolution this crate does not know: {other:?}"),
             },
-            base: lines(base, region.base_range()),
+            base: lines(region.base_range(), |ix| merge.base_line(ix)),
-            ours: lines(ours, region.ours_range()),
+            ours: lines(region.ours_range(), |ix| merge.ours_line(ix)),
-            theirs: lines(theirs, region.theirs_range()),
+            theirs: lines(region.theirs_range(), |ix| merge.theirs_line(ix)),
         })
         .collect()
 }
@@ -94,7 +101,9 @@ pub fn result(regions: &[Region], takes: &[Option<Take>]) -> (String, usize) {
                     Some(Take::Theirs) => out.push_str(&theirs),
                     Some(Take::Both) => {
                         out.push_str(&ours);
-                        break_line(&mut out, ending);
+                        if !theirs.is_empty() {
+                            break_line(&mut out, ending);
+                        }
                         out.push_str(&theirs);
                     }
                     None => {
@@ -129,7 +138,7 @@ pub struct Conflict {
 pub fn conflicts(text: &str) -> Vec<Conflict> {
     let mut found = Vec::new();
     let (mut start, mut middle) = (None, None);
-    for (ix, line) in text.lines().enumerate() {
+    for (ix, line) in text.tokenize_lines().into_iter().enumerate() {
         if line.starts_with("<<<<<<<") {
             (start, middle) = (Some(ix), None);
         } else if line.starts_with("=======") && start.is_some() {
@@ -150,7 +159,7 @@ pub fn conflicts(text: &str) -> Vec<Conflict> {
 /// `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.split_inclusive('\n').collect();
+    let lines = text.tokenize_lines();
     let ours = lines[conflict.start + 1..conflict.middle].concat();
     let theirs = lines[conflict.middle + 1..conflict.end].concat();
     let kept = match take {
@@ -204,6 +213,21 @@ mod tests {
         assert_eq!(resolve(marked, 0, Take::Ours), "a\r\nours\r\nz");
     }
 
+    #[test]
+    fn a_bare_return_ends_a_line_as_it_does_for_similar() {
+        let changed = regions("a\rb\n", "a\rb\n", "a\rB\n");
+        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");
+    }
+
+    #[test]
+    fn taking_both_adds_no_line_to_an_empty_side() {
+        assert_eq!(result(&regions("a", "b", ""), &[Some(Take::Both)]).0, "b");
+        assert_eq!(result(&regions("a", "", "c"), &[Some(Take::Both)]).0, "c");
+    }
+
     #[test]
     fn markers_resolve_to_one_side_or_both() {
         let marked = "a\n<<<<<<< HEAD\nours\n=======\ntheirs\n>>>>>>> topic\nz\n";