Merge branch 'johan/diff3-base-minus'

Fix handling of conflict sections with - prefixes in the base section.

README.md

@@ -4,6 +4,8 @@ Riff is a wrapper around `diff` that highlights which parts of lines have change
 
 ![Screenshot of riff in action](screenshot.png 'git show')
 
+`riff` also [helpfully highlights conflicts and merge commits](#more-features).
+
 Much like `git`, Riff sends its output to a pager, trying these in order:
 
 1. Whatever is specified in the `$PAGER` environment variable
@@ -104,6 +106,9 @@ editing](https://jonasbn.github.io/til/vscode/integrate_with_cli.html).
 
 If you put example input and output in the `testdata` directory, then `cargo test` will verify that they match.
 
+On mismatches, you can run `testdata-examples.sh` to compare current output to
+the expected output for all examples, and optionally update expectations.
+
 Invoke `ci.sh` to run the same thing as CI.
 
 Invoke `benchmark.py` to get numbers for how fast your current source code is

src/conflicts_highlighter.rs

@@ -31,6 +31,9 @@ pub(crate) struct ConflictsHighlighter {
     /// Always ends with a newline.
     base: String,
 
+    /// Prefixes of the base section, one per line.
+    base_line_prefixes: vec::Vec<String>,
+
     /// `=======`, followed by `c2`
     c2_header: String,
 
@@ -90,17 +93,17 @@ impl LinesHighlighter for ConflictsHighlighter {
         // All header lines handled, this is a content line
         //
 
-        let destination = if !self.c2_header.is_empty() {
-            &mut self.c2
+        let (prefix_destination, destination) = if !self.c2_header.is_empty() {
+            (None, &mut self.c2)
         } else if !self.base_header.is_empty() {
-            &mut self.base
+            (Some(&mut self.base_line_prefixes), &mut self.base)
         } else {
-            &mut self.c1
+            (None, &mut self.c1)
         };
 
         let prefixes = if self.c1_header.starts_with("++") {
             // Possible content line prefixes when doing "git diff"
-            vec!["+ ", "++", " +"]
+            vec!["+ ", "++", " +", " -", "- "]
         } else {
             vec![""]
         };
@@ -110,6 +113,11 @@ impl LinesHighlighter for ConflictsHighlighter {
                 // Handle the context line
                 destination.push_str(line);
                 destination.push('\n');
+
+                if let Some(prefix_destination) = prefix_destination {
+                    prefix_destination.push(prefix.to_string());
+                }
+
                 return Ok(Response {
                     line_accepted: LineAcceptance::AcceptedWantMore,
                     highlighted: vec![],
@@ -149,6 +157,7 @@ impl ConflictsHighlighter {
             footer: String::new(),
             c1: String::new(),
             base: String::new(),
+            base_line_prefixes: Vec::new(),
             c2: String::new(),
         });
     }
@@ -240,18 +249,18 @@ impl ConflictsHighlighter {
     ///   vs C1 or vs C2.
     /// * In section C2, we highlight additions compared to base
     fn render_diff3(&self, thread_pool: &ThreadPool) -> StringFuture {
-        let (header_prefix, c1_prefix, base_prefix, c2_prefix, reset) =
-            if self.c1_header.starts_with("++") {
-                (INVERSE_VIDEO, " +", "++", "+ ", NORMAL)
-            } else {
-                (INVERSE_VIDEO, "", "", "", "")
-            };
+        let (header_prefix, c1_prefix, c2_prefix, reset) = if self.c1_header.starts_with("++") {
+            (INVERSE_VIDEO, " +", "+ ", NORMAL)
+        } else {
+            (INVERSE_VIDEO, "", "", "")
+        };
 
         assert!(!self.base.is_empty());
         let c1_header = self.c1_header.clone();
         let c1 = self.c1.clone();
         let base_header = self.base_header.clone();
         let base = self.base.clone();
+        let base_line_prefixes = self.base_line_prefixes.clone();
         let c2_header = self.c2_header.clone();
         let c2 = self.c2.clone();
         let footer = self.footer.clone();
@@ -296,8 +305,11 @@ impl ConflictsHighlighter {
                     }
                 }
 
-                let highlighted_base =
-                    token_collector::render(&LINE_STYLE_CONFLICT_BASE, base_prefix, &base_tokens);
+                let highlighted_base = token_collector::render_multiprefix(
+                    &LINE_STYLE_CONFLICT_BASE,
+                    &base_line_prefixes,
+                    &base_tokens,
+                );
 
                 let mut rendered = String::new();
                 rendered.push_str(header_prefix);

src/token_collector.rs

@@ -122,7 +122,12 @@ impl StyledToken {
 }
 
 #[must_use]
-fn render_row(line_style: &LineStyle, prefix: &str, row: &[StyledToken]) -> String {
+fn render_row(
+    line_style: &LineStyle,
+    prefix: &str,
+    row: &[StyledToken],
+    force_faint: bool,
+) -> String {
     let mut rendered = String::new();
 
     let mut current_style = ANSI_STYLE_NORMAL;
@@ -134,7 +139,7 @@ fn render_row(line_style: &LineStyle, prefix: &str, row: &[StyledToken]) -> Stri
 
     // Render tokens
     for token in row {
-        let new_style = match token.style {
+        let mut new_style = match token.style {
             Style::Context => ANSI_STYLE_NORMAL,
             Style::Lowlighted => ANSI_STYLE_NORMAL.with_weight(Weight::Faint),
             Style::Bright => ANSI_STYLE_NORMAL.with_weight(Weight::Bold),
@@ -144,6 +149,10 @@ fn render_row(line_style: &LineStyle, prefix: &str, row: &[StyledToken]) -> Stri
             Style::Error => ANSI_STYLE_NORMAL.with_color(Red).with_inverse(true),
         };
 
+        if force_faint {
+            new_style = new_style.with_weight(Weight::Faint);
+        }
+
         rendered.push_str(&new_style.from(&current_style));
         current_style = new_style;
         rendered.push_str(&token.token);
@@ -163,7 +172,8 @@ pub fn render(line_style: &LineStyle, prefix: &str, tokens: &[StyledToken]) -> S
     let mut current_row_start = 0;
     for (i, token) in tokens.iter().enumerate() {
         if token.token == "\n" {
-            let rendered_row = &render_row(line_style, prefix, &tokens[current_row_start..i]);
+            let rendered_row =
+                &render_row(line_style, prefix, &tokens[current_row_start..i], false);
             rendered.push_str(rendered_row);
             rendered.push('\n');
             current_row_start = i + 1;
@@ -172,7 +182,57 @@ pub fn render(line_style: &LineStyle, prefix: &str, tokens: &[StyledToken]) -> S
     }
 
     if current_row_start < tokens.len() {
-        let rendered_row = &render_row(line_style, prefix, &tokens[current_row_start..]);
+        let rendered_row = &render_row(line_style, prefix, &tokens[current_row_start..], false);
+        rendered.push_str(rendered_row);
+    }
+
+    return rendered;
+}
+
+/// Render all the tokens into a (most of the time multiline) string. Each line
+/// is prefixed by a prefix from `line_prefixes`.
+#[must_use]
+pub fn render_multiprefix(
+    line_style: &LineStyle,
+    line_prefixes: &[String],
+    tokens: &[StyledToken],
+) -> String {
+    let mut rendered = String::new();
+
+    let mut current_row_start = 0;
+    let mut line_number_zero_based = 0;
+    for (i, token) in tokens.iter().enumerate() {
+        if token.token != "\n" {
+            continue;
+        }
+
+        let prefix = &line_prefixes[line_number_zero_based];
+        let force_faint = prefix.chars().any(|c| c == '-');
+
+        let rendered_row = &render_row(
+            line_style,
+            prefix,
+            &tokens[current_row_start..i],
+            force_faint,
+        );
+        rendered.push_str(rendered_row);
+        rendered.push('\n');
+        current_row_start = i + 1;
+
+        line_number_zero_based += 1;
+    }
+
+    if current_row_start < tokens.len() {
+        // Render the last row
+        let prefix = &line_prefixes[line_number_zero_based];
+        let force_faint = prefix.chars().any(|c| c == '-');
+
+        let rendered_row = &render_row(
+            line_style,
+            prefix,
+            &tokens[current_row_start..],
+            force_faint,
+        );
         rendered.push_str(rendered_row);
     }
 

testdata-examples.sh

@@ -0,0 +1,63 @@
+#!/bin/bash
+
+# This script will figure out all testdata example mismatches.
+#
+# For each mismatch, it will create a /tmp/before.sh shell script that will show
+# the .riff-output flavor of the testdata example in moar.
+#
+# It will also show the actual output.
+#
+# The idea is that you should run /tmp/before.sh in another tab, and switch
+# between tabs to see the differences.
+#
+# After showing the current output, it will ask whether or not to update the
+# .riff-output file.
+
+set -e -o pipefail
+
+WORKFILE=$(mktemp)
+trap 'rm -f "$WORKFILE"' EXIT
+
+echo
+read -r -p "Run /tmp/before.sh in another tab to compare the output. Press Enter to continue."
+
+for EXPECTED in testdata/*.riff-output; do
+    INPUT="${EXPECTED%.riff-output}.diff"
+    if [ ! -f "$INPUT" ]; then
+        INPUT="${EXPECTED%.riff-output}"
+        if [ ! -f "$INPUT" ]; then
+            echo "No input file for $EXPECTED"
+            exit 1
+        fi
+    fi
+
+    echo
+    echo "$INPUT"
+
+    # Create /tmp/before.sh
+    cat <<EOF >/tmp/before.sh
+#!/bin/bash -e
+
+moar $EXPECTED
+EOF
+    chmod +x /tmp/before.sh
+
+    # Capture the actual output
+    cargo run -- --color=on <"$INPUT" >"$WORKFILE" || true
+
+    # Is the output different?
+    if diff -u "$EXPECTED" "$WORKFILE" >/dev/null; then
+        echo "Already up to date, never mind: $EXPECTED"
+        continue
+    fi
+
+    moar "$WORKFILE"
+
+    echo
+    echo -n "Update $EXPECTED? [y/N] "
+    read -r
+    if [ "$REPLY" = "y" ]; then
+        cp "$WORKFILE" "$EXPECTED"
+    fi
+    echo
+done

testdata/conflict-with-context.riff-output

@@ -71,9 +71,9 @@
 ++	p.scrollPosition = *firstHitPosition
 ++}
 ++
-++// NOTE: When we search, we do that by looping over the *input lines*, not
-++// the screen lines. That's why we're using a line number rather than a
-++// scrollPosition for searching.
++ // NOTE: When we search, we do that by looping over the *input lines*, not
++ // the screen lines. That's why we're using a line number rather than a
++ // scrollPosition for searching.
 ++=======
 + // NOTE: When we search, we do that by looping over the *input lines*, not
 + // the screen lines. That's why we're using a line number rather than a