From 6288423de50f980a21f3ac8cf532a064b6d6cef6 Mon Sep 17 00:00:00 2001
From: Thomas Otto <th1000s@posteo.net>
Date: Tue, 26 Oct 2021 22:28:50 +0200
Subject: [PATCH 1/2] Simplify incrementing of line numbers

---
 src/features/line_numbers.rs |  9 +++++----
 src/features/side_by_side.rs | 24 ------------------------
 src/paint.rs                 |  5 +++++
 3 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/src/features/line_numbers.rs b/src/features/line_numbers.rs
index 56cb3daac..3afce6b1b 100644
--- a/src/features/line_numbers.rs
+++ b/src/features/line_numbers.rs
@@ -66,6 +66,7 @@ pub fn linenumbers_and_styles<'a>(
     line_numbers_data: &'a mut LineNumbersData,
     state: &State,
     config: &'a config::Config,
+    increment: bool,
 ) -> Option<(MinusPlus<Option<usize>>, MinusPlus<Style>)> {
     let nr_left = line_numbers_data.line_number[Left];
     let nr_right = line_numbers_data.line_number[Right];
@@ -76,18 +77,18 @@ pub fn linenumbers_and_styles<'a>(
     );
     let ((minus_number, plus_number), (minus_style, plus_style)) = match state {
         State::HunkMinus(_) => {
-            line_numbers_data.line_number[Left] += 1;
+            line_numbers_data.line_number[Left] += increment as usize;
             ((Some(nr_left), None), (minus_style, plus_style))
         }
         State::HunkMinusWrapped => ((None, None), (minus_style, plus_style)),
         State::HunkZero => {
-            line_numbers_data.line_number[Left] += 1;
-            line_numbers_data.line_number[Right] += 1;
+            line_numbers_data.line_number[Left] += increment as usize;
+            line_numbers_data.line_number[Right] += increment as usize;
             ((Some(nr_left), Some(nr_right)), (zero_style, zero_style))
         }
         State::HunkZeroWrapped => ((None, None), (zero_style, zero_style)),
         State::HunkPlus(_) => {
-            line_numbers_data.line_number[Right] += 1;
+            line_numbers_data.line_number[Right] += increment as usize;
             ((None, Some(nr_right)), (minus_style, plus_style))
         }
         State::HunkPlusWrapped => ((None, None), (minus_style, plus_style)),
diff --git a/src/features/side_by_side.rs b/src/features/side_by_side.rs
index 1485254c6..b052f97ce 100644
--- a/src/features/side_by_side.rs
+++ b/src/features/side_by_side.rs
@@ -197,15 +197,6 @@ pub fn paint_zero_lines_side_by_side<'a>(
                 config,
             );
             output_buffer.push_str(&panel_line);
-
-            if *panel_side == Left && state != State::HunkZeroWrapped {
-                // TODO: Avoid doing the superimpose_style_sections work twice.
-                // HACK: These are getting incremented twice, so knock them back down once.
-                if let Some(d) = line_numbers_data.as_mut() {
-                    d.line_number[Left] -= 1;
-                    d.line_number[Right] -= 1;
-                }
-            }
         }
         output_buffer.push('\n');
     }
@@ -386,21 +377,6 @@ fn paint_minus_or_plus_panel_line<'a>(
         config,
     );
 
-    // Knock back down spuriously incremented line numbers. See comment above.
-    match (state, &state_for_line_numbers_field) {
-        (s, t) if s == t => {}
-        (State::HunkPlus(_), State::HunkMinus(_)) => {
-            if let Some(d) = line_numbers_data.as_mut() {
-                d.line_number[Left] -= 1;
-            }
-        }
-        (State::HunkMinus(_), State::HunkPlus(_)) => {
-            if let Some(d) = line_numbers_data.as_mut() {
-                d.line_number[Right] -= 1;
-            }
-        }
-        _ => unreachable!(),
-    }
     (line, line_is_empty)
 }
 
diff --git a/src/paint.rs b/src/paint.rs
index bcf3e9418..2be015e35 100644
--- a/src/paint.rs
+++ b/src/paint.rs
@@ -532,10 +532,15 @@ impl<'a> Painter<'a> {
 
         let output_line_numbers = config.line_numbers && line_numbers_data.is_some();
         if output_line_numbers {
+            // Unified diff lines are printed in one go, but side-by-side lines
+            // are printed in two parts, so do not increment line numbers when the
+            // first (left) part is printed.
+            let increment = !matches!(side_by_side_panel, Some(side_by_side::Left));
             if let Some((line_numbers, styles)) = line_numbers::linenumbers_and_styles(
                 line_numbers_data.as_mut().unwrap(),
                 state,
                 config,
+                increment,
             ) {
                 ansi_strings.extend(line_numbers::format_and_paint_line_numbers(
                     line_numbers_data.as_ref().unwrap(),

From f3170dfcbf1fbdf4e2129a7b5a409d4515726928 Mon Sep 17 00:00:00 2001
From: Thomas Otto <th1000s@posteo.net>
Date: Tue, 26 Oct 2021 22:28:50 +0200
Subject: [PATCH 2/2] Fix side-by-side mode without line numbers

Even though this feature enables line numbers, these can be disabled via
the git config `delta.line-numbers=false`. So always enable line numbers,
even if they do not print numbers.

Construct more structs with ..Self::default()
---
 src/features/line_numbers.rs | 24 ++++++++++++++++----
 src/features/side_by_side.rs |  4 ++--
 src/format.rs                |  4 +---
 src/handlers/hunk_header.rs  |  2 ++
 src/paint.rs                 | 43 ++++++++++++++++++++++--------------
 5 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/src/features/line_numbers.rs b/src/features/line_numbers.rs
index 3afce6b1b..0cc4ebfb8 100644
--- a/src/features/line_numbers.rs
+++ b/src/features/line_numbers.rs
@@ -6,7 +6,7 @@ use regex::Regex;
 use crate::config;
 use crate::delta::State;
 use crate::features::hyperlinks;
-use crate::features::side_by_side::ansifill;
+use crate::features::side_by_side::ansifill::{self, ODD_PAD_CHAR};
 use crate::features::side_by_side::{Left, PanelSide, Right};
 use crate::features::OptionValueFunction;
 use crate::format::{self, Align, Placeholder};
@@ -175,9 +175,7 @@ impl<'a> LineNumbersData<'a> {
                     insert_center_space_on_odd_width,
                 ),
             ),
-            line_number: MinusPlus::new(0, 0),
-            hunk_max_line_number_width: 0,
-            plus_file: "".to_string(),
+            ..Self::default()
         }
     }
 
@@ -193,6 +191,24 @@ impl<'a> LineNumbersData<'a> {
         self.plus_file = plus_file;
     }
 
+    pub fn empty_for_sbs(use_full_width: ansifill::UseFullPanelWidth) -> LineNumbersData<'a> {
+        let insert_center_space_on_odd_width = use_full_width.pad_width();
+        Self {
+            format_data: if insert_center_space_on_odd_width {
+                let format_left = vec![format::FormatStringPlaceholderData::default()];
+                let format_right = vec![format::FormatStringPlaceholderData {
+                    prefix: format!("{}", ODD_PAD_CHAR).into(),
+                    prefix_len: 1,
+                    ..Default::default()
+                }];
+                MinusPlus::new(format_left, format_right)
+            } else {
+                MinusPlus::default()
+            },
+            ..Self::default()
+        }
+    }
+
     pub fn formatted_width(&self) -> SideBySideLineWidth {
         let format_data_width = |format_data: &format::FormatStringData<'a>| {
             // Provide each Placeholder with the max_line_number_width to calculate the
diff --git a/src/features/side_by_side.rs b/src/features/side_by_side.rs
index b052f97ce..ddc7f55d5 100644
--- a/src/features/side_by_side.rs
+++ b/src/features/side_by_side.rs
@@ -465,8 +465,8 @@ pub mod ansifill {
     /// The solution in this case is to add `ODD_PAD_CHAR` before the first line number in
     /// the right panel and increasing its width by one, thus using the full terminal width
     /// with the two panels.
-    /// This also means line numbers can not be disabled in side-by-side mode plus ANSI, as
-    /// this leaves no place for this fix.
+    /// This also means line numbers can not be disabled in side-by-side mode, but they may
+    /// not actually paint numbers.
     #[derive(Clone, Debug)]
     pub struct UseFullPanelWidth(pub bool);
     impl UseFullPanelWidth {
diff --git a/src/format.rs b/src/format.rs
index 5d86b40d4..bbc6e9f5a 100644
--- a/src/format.rs
+++ b/src/format.rs
@@ -147,11 +147,9 @@ pub fn parse_line_number_format<'a>(
         format_data.push(FormatStringPlaceholderData {
             prefix,
             prefix_len,
-            placeholder: None,
-            alignment_spec: None,
-            width: None,
             suffix: SmolStr::new(format_string),
             suffix_len: format_string.graphemes(true).count(),
+            ..Default::default()
         })
     }
 
diff --git a/src/handlers/hunk_header.rs b/src/handlers/hunk_header.rs
index b601ef541..a48ad834c 100644
--- a/src/handlers/hunk_header.rs
+++ b/src/handlers/hunk_header.rs
@@ -55,6 +55,8 @@ impl<'a> StateMachine<'a> {
         if self.config.line_numbers {
             self.painter
                 .line_numbers_data
+                .as_mut()
+                .unwrap()
                 .initialize_hunk(&line_numbers, self.plus_file.to_string());
         }
 
diff --git a/src/paint.rs b/src/paint.rs
index 2be015e35..7a8b8d2d2 100644
--- a/src/paint.rs
+++ b/src/paint.rs
@@ -26,7 +26,10 @@ pub struct Painter<'a> {
     pub highlighter: Option<HighlightLines<'a>>,
     pub config: &'a config::Config,
     pub output_buffer: String,
-    pub line_numbers_data: line_numbers::LineNumbersData<'a>,
+    // If config.line_numbers is true, then the following is always Some().
+    // In side-by-side mode it is always Some (but possibly an empty one), even
+    // if config.line_numbers is false. See `UseFullPanelWidth` as well.
+    pub line_numbers_data: Option<line_numbers::LineNumbersData<'a>>,
 }
 
 // How the background of a line is filled up to the end
@@ -59,17 +62,19 @@ impl<'a> Painter<'a> {
         let panel_width_fix = ansifill::UseFullPanelWidth::new(config);
 
         let line_numbers_data = if config.line_numbers {
-            line_numbers::LineNumbersData::from_format_strings(
+            Some(line_numbers::LineNumbersData::from_format_strings(
                 &config.line_numbers_format,
                 panel_width_fix,
-            )
+            ))
+        } else if config.side_by_side {
+            // If line numbers are disabled in side-by-side then the data is still used
+            // for width calculaction and to pad odd width to even, see `UseFullPanelWidth`
+            // for details.
+            Some(line_numbers::LineNumbersData::empty_for_sbs(
+                panel_width_fix,
+            ))
         } else {
-            // See `UseFullPanelWidth` for details why, but this can't happen at the time
-            // of writing because side-by-side automatically activates line numbers.
-            debug_assert!(
-                !config.side_by_side && config.line_fill_method == BgFillMethod::TryAnsiSequence
-            );
-            line_numbers::LineNumbersData::default()
+            None
         };
         Self {
             minus_lines: Vec::new(),
@@ -192,6 +197,10 @@ impl<'a> Painter<'a> {
                     .collect(),
             );
 
+            let line_numbers_data = self.line_numbers_data.as_mut().unwrap_or_else(|| {
+                delta_unreachable("side-by-side requires Some(line_numbers_data)")
+            });
+
             let bg_fill_left_right = MinusPlus::new(
                 // Using an ANSI sequence to fill the left panel would not work.
                 BgShouldFill::With(BgFillMethod::Spaces),
@@ -207,7 +216,7 @@ impl<'a> Painter<'a> {
                 if self.config.wrap_config.max_lines == 1 {
                     (false, MinusPlus::default(), MinusPlus::default())
                 } else {
-                    let line_width = available_line_width(self.config, &self.line_numbers_data);
+                    let line_width = available_line_width(self.config, line_numbers_data);
 
                     let lines = MinusPlus::new(&self.minus_lines, &self.plus_lines);
 
@@ -244,10 +253,12 @@ impl<'a> Painter<'a> {
                 line_alignment,
                 &mut self.output_buffer,
                 self.config,
-                &mut Some(&mut self.line_numbers_data),
+                &mut Some(line_numbers_data),
                 bg_fill_left_right,
             );
         } else {
+            // Unified mode:
+
             if !self.minus_lines.is_empty() {
                 Painter::paint_lines(
                     minus_line_syntax_style_sections,
@@ -255,7 +266,7 @@ impl<'a> Painter<'a> {
                     self.minus_lines.iter().map(|(_, state)| state),
                     &mut self.output_buffer,
                     self.config,
-                    &mut Some(&mut self.line_numbers_data),
+                    &mut self.line_numbers_data.as_mut(),
                     if self.config.keep_plus_minus_markers {
                         Some(self.config.minus_style.paint("-"))
                     } else {
@@ -272,7 +283,7 @@ impl<'a> Painter<'a> {
                     self.plus_lines.iter().map(|(_, state)| state),
                     &mut self.output_buffer,
                     self.config,
-                    &mut Some(&mut self.line_numbers_data),
+                    &mut self.line_numbers_data.as_mut(),
                     if self.config.keep_plus_minus_markers {
                         Some(self.config.plus_style.paint("+"))
                     } else {
@@ -311,7 +322,7 @@ impl<'a> Painter<'a> {
                 vec![diff_style_sections],
                 &mut self.output_buffer,
                 self.config,
-                &mut Some(&mut self.line_numbers_data),
+                &mut self.line_numbers_data.as_mut(),
                 painted_prefix,
                 BgShouldFill::With(BgFillMethod::Spaces),
             );
@@ -322,7 +333,7 @@ impl<'a> Painter<'a> {
                 [state].iter(),
                 &mut self.output_buffer,
                 self.config,
-                &mut Some(&mut self.line_numbers_data),
+                &mut self.line_numbers_data.as_mut(),
                 painted_prefix,
                 None,
                 BgShouldFill::With(BgFillMethod::Spaces),
@@ -530,7 +541,7 @@ impl<'a> Painter<'a> {
     ) -> (String, bool) {
         let mut ansi_strings = Vec::new();
 
-        let output_line_numbers = config.line_numbers && line_numbers_data.is_some();
+        let output_line_numbers = line_numbers_data.is_some();
         if output_line_numbers {
             // Unified diff lines are printed in one go, but side-by-side lines
             // are printed in two parts, so do not increment line numbers when the