Repository: dandavison/delta
Pull request #190 (2020-05-14 14:21:21)

Add option to show line numbers
Adds a CLI option, `--number`, that shows line numbers for the input and output diffs. The first column shows the line number for the original file, and the second column shows line number for the output file. A blank cell indicates that the line does not exist in one of the files (was removed or added, respectively).

Also provides the ability to customize the appearance of line numbers. Users can pass in a format string showing the position of the line numbers within the column, and can apply style elements to the numbers as well as the format string text, using new command line options:
- `--number-minus-style`
- `--number-plus-style`
- `--number-minus-format`
- `--number-plus-format`
- `--number-minus-format-style`
- `--number-plus-format-style`

![image](https://user-images.githubusercontent.com/5257313/81950427-c29fe000-95b8-11ea-842d-e34d53930882.png)

Addresses https://github.com/dandavison/delta/issues/130.

TODO: tests.

dandavison: Thanks very much for this @clnoll! This is a substantial contribution and it's a feature that's been requested multiple times. I'll start reviewing.
clnoll: Thanks @dandavison & @rygwdn! Great suggestions, making these changes.
clnoll: @dandavison @rygwdn I took all of the suggestions mentioned above, and added some additional command line options (colors for the line numbers and the dividers, and the ability to configure a string for the dividers).

FYI I'm seeing that it's a bit slower than `master` when benchmarking, so will take a closer look to see what I can do to speed it up.

Master:
```
Time (mean ± σ):      1.496 s ±  0.060 s    [User: 1.494 s, System: 0.042 s]
Range (min … max):    1.460 s …  1.661 s    10 runs
```

Feature branch, with `--numbers`
```
Time (mean ± σ):      1.807 s ±  0.078 s    [User: 1.786 s, System: 0.044 s]
Range (min … max):    1.757 s …  2.019 s    10 runs
```

Feature branch, without `--numbers`
```
Time (mean ± σ):      1.722 s ±  0.017 s    [User: 1.720 s, System: 0.043 s]
Range (min … max):    1.696 s …  1.748 s    10 runs
```
dandavison: I've been doing a lot of refactoring in parallel and so if it's OK with you I'd like to treat `style-strings` as the base branch for this branch to target. Seeing as you've already had to rebase once, I rebased this branch on top of `style-strings` as `line-numbers-rebased`. If that diff LGTY then please feel free to take it (the commit has your attribution as author email).

You'll note that it...doesn't actually compile :) But it's fairly superficial -- we need to re-express the color options in this branch as what are referred to as "styles" in `style-strings`. The function `paint_text_foreground` has gone away and one just calls `style.paint(...)` instead.
dandavison: > I've been doing a lot of refactoring in parallel and so if it's OK with you I'd like to treat `style-strings` as the base branch for this branch to target

The style-string refactoring is all in `master` now.

Patch:
diff --git a/src/cli.rs b/src/cli.rs
index 479eea8..0226832 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -127,6 +127,24 @@ within a style string):
    Specifying colors like this is useful if your terminal only supports 256 colors (i.e. doesn\'t
    support 24-bit color).
 
+LINE NUMBERS
+------------
+
+Options that have a name like --*-format allow you to specify a string to display for the line
+number columns. The string should specify the location of the line number using the placeholder
+%ln.
+
+For example, to display the line numbers divided by specific characters:
+
+    8 ⋮   9 │ Here is an output line
+    9 ⋮  10 │ Here is another output line
+   10 ⋮  11 │ Here is the line number
+
+you would use the following input:
+
+--number-minus-format '%ln ⋮'
+--number-plus-format '%ln │'
+
 If something isn't working correctly, or you have a feature request, please open an issue at
 https://github.com/dandavison/delta/issues.
 "
@@ -245,6 +263,52 @@ pub struct Opt {
     /// given.
     pub hunk_header_decoration_style: String,
 
+    /// Display line numbers next to the diff. The first column contains line
+    /// numbers in the previous version of the file, and the second column contains
+    /// line number in the new version of the file. A blank cell in the first or
+    /// second column indicates that the line does not exist in that file (it was
+    /// added or removed, respectively).
+    #[structopt(short = "n", long = "number")]
+    pub show_line_numbers: bool,
+
+    /// Style (foreground, background, attributes) for the left (minus) column of line numbers
+    /// (--number), if --number is set. See STYLES section.
+    /// Defaults to --hunk-style.
+    #[structopt(long = "number-minus-style", default_value = "auto")]
+    pub number_minus_style: String,
+
+    /// Style (foreground, background, attributes) for the right (plus) column of line numbers
+    /// (--number), if --number is set. See STYLES section.
+    /// Defaults to --hunk-style.
+    #[structopt(long = "number-plus-style", default_value = "auto")]
+    pub number_plus_style: String,
+
+    /// Format string for the left (minus) column of line numbers (--number), if --number is set.
+    /// Should include the placeholder %ln to indicate the position of the line number.
+    /// See the LINE NUMBERS section.
+    /// Defaults to '%ln⋮'
+    #[structopt(long = "number-minus-format", default_value = "%ln⋮")]
+    pub number_minus_format: String,
+
+    /// Format string for the right (plus) column of line numbers (--number), if --number is set.
+    /// Should include the placeholder %ln to indicate the position of the line number.
+    /// See the LINE NUMBERS section.
+    /// Defaults to '%ln│ '
+    #[structopt(long = "number-plus-format", default_value = "%ln│ ")]
+    pub number_plus_format: String,
+
+    /// Style (foreground, background, attributes) for the left (minus) line number format string
+    /// (--number), if --number is set. See STYLES section.
+    /// Defaults to --hunk-style.
+    #[structopt(long = "number-minus-format-style", default_value = "auto")]
+    pub number_minus_format_style: String,
+
+    /// Style (foreground, background, attributes) for the right (plus) line number format string
+    /// (--number), if --number is set. See STYLES section.
+    /// Defaults to --hunk-style.
+    #[structopt(long = "number-plus-format-style", default_value = "auto")]
+    pub number_plus_format_style: String,
+
     #[structopt(long = "color-only")]
     /// Do not alter the input in any way other than applying colors. Equivalent to
     /// `--keep-plus-minus-markers --width variable --tabs 0 --commit-decoration ''
diff --git a/src/config.rs b/src/config.rs
index 4c522ea..a248f29 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -41,12 +41,19 @@ pub struct Config<'a> {
     pub navigate: bool,
     pub null_style: Style,
     pub null_syntect_style: SyntectStyle,
+    pub number_minus_format: String,
+    pub number_minus_format_style: Style,
+    pub number_minus_style: Style,
+    pub number_plus_format: String,
+    pub number_plus_format_style: Style,
+    pub number_plus_style: Style,
     pub paging_mode: PagingMode,
     pub plus_emph_style: Style,
     pub plus_file: Option<PathBuf>,
     pub plus_line_marker: &'a str,
     pub plus_non_emph_style: Style,
     pub plus_style: Style,
+    pub show_line_numbers: bool,
     pub syntax_dummy_theme: SyntaxTheme,
     pub syntax_set: SyntaxSet,
     pub syntax_theme: Option<SyntaxTheme>,
@@ -120,6 +127,17 @@ pub fn get_config<'a>(
     let (commit_style, file_style, hunk_header_style) =
         make_commit_file_hunk_header_styles(&opt, true_color);
 
+    let (
+        number_minus_format_style,
+        number_minus_style,
+        number_plus_format_style,
+        number_plus_style,
+    ) = make_line_number_styles(
+        &opt,
+        hunk_header_style.decoration_ansi_term_style(),
+        true_color,
+    );
+
     let syntax_theme = if syntax_theme::is_no_syntax_highlighting_theme_name(&syntax_theme_name) {
         None
     } else {
@@ -164,12 +182,19 @@ pub fn get_config<'a>(
         navigate: opt.navigate,
         null_style: Style::new(),
         null_syntect_style: SyntectStyle::default(),
+        number_minus_format: opt.number_minus_format,
+        number_minus_format_style: number_minus_format_style,
+        number_minus_style: number_minus_style,
+        number_plus_format: opt.number_plus_format,
+        number_plus_format_style: number_plus_format_style,
+        number_plus_style: number_plus_style,
         paging_mode,
         plus_emph_style,
         plus_file: opt.plus_file.map(|s| s.clone()),
         plus_line_marker,
         plus_non_emph_style,
         plus_style,
+        show_line_numbers: opt.show_line_numbers,
         syntax_dummy_theme,
         syntax_set,
         syntax_theme,
@@ -264,6 +289,60 @@ fn make_hunk_styles<'a>(
     )
 }
 
+fn make_line_number_styles<'a>(
+    opt: &'a cli::Opt,
+    default_style: Option<ansi_term::Style>,
+    true_color: bool,
+) -> (Style, Style, Style, Style) {
+    let (default_foreground, default_background) = match default_style {
+        Some(default_style) => (default_style.foreground, default_style.background),
+        None => (None, None),
+    };
+
+    let number_minus_format_style = Style::from_str(
+        &opt.number_minus_format_style,
+        default_foreground,
+        default_background,
+        None,
+        true_color,
+        false,
+    );
+
+    let number_minus_style = Style::from_str(
+        &opt.number_minus_style,
+        default_foreground,
+        default_background,
+        None,
+        true_color,
+        false,
+    );
+
+    let number_plus_format_style = Style::from_str(
+        &opt.number_plus_format_style,
+        default_foreground,
+        default_background,
+        None,
+        true_color,
+        false,
+    );
+
+    let number_plus_style = Style::from_str(
+        &opt.number_plus_style,
+        default_foreground,
+        default_background,
+        None,
+        true_color,
+        false,
+    );
+
+    (
+        number_minus_format_style,
+        number_minus_style,
+        number_plus_format_style,
+        number_plus_style,
+    )
+}
+
 fn make_commit_file_hunk_header_styles(opt: &cli::Opt, true_color: bool) -> (Style, Style, Style) {
     (
         Style::from_str_with_handling_of_special_decoration_attributes_and_respecting_deprecated_foreground_color_arg(
diff --git a/src/delta.rs b/src/delta.rs
index bab2667..b498bd0 100644
--- a/src/delta.rs
+++ b/src/delta.rs
@@ -369,7 +369,9 @@ fn handle_hunk_header_line(
             draw::write_no_decoration
         }
     };
-    let (raw_code_fragment, line_number) = parse::parse_hunk_metadata(&line);
+    let (raw_code_fragment, line_numbers) = parse::parse_hunk_metadata(&line);
+    painter.minus_line_number = line_numbers[0];
+    painter.plus_line_number = line_numbers[line_numbers.len() - 1];
     if config.hunk_header_style.is_raw {
         writeln!(painter.writer)?;
         draw_fn(
@@ -397,6 +399,7 @@ fn handle_hunk_header_line(
             Painter::paint_lines(
                 syntax_style_sections,
                 vec![vec![(config.hunk_header_style, &lines[0])]],
+                vec![None],
                 &mut painter.output_buffer,
                 config,
                 "",
@@ -418,10 +421,14 @@ fn handle_hunk_header_line(
             };
         }
     };
-    match config.hunk_header_style.decoration_ansi_term_style() {
-        Some(style) => writeln!(painter.writer, "{}", style.paint(line_number))?,
-        None => writeln!(painter.writer, "{}", line_number)?,
-    };
+
+    if !config.show_line_numbers {
+        let line_number = &format!("{}", painter.plus_line_number);
+        match config.hunk_header_style.decoration_ansi_term_style() {
+            Some(style) => writeln!(painter.writer, "{}", style.paint(line_number))?,
+            None => writeln!(painter.writer, "{}", line_number)?,
+        }
+    }
     Ok(())
 }
 
@@ -474,6 +481,10 @@ fn handle_hunk_line(
             Painter::paint_lines(
                 syntax_style_sections,
                 vec![diff_style_sections],
+                vec![Some((
+                    Some(painter.minus_line_number),
+                    Some(painter.plus_line_number),
+                ))],
                 &mut painter.output_buffer,
                 config,
                 prefix,
@@ -481,6 +492,8 @@ fn handle_hunk_line(
                 config.zero_style,
                 None,
             );
+            painter.minus_line_number += 1;
+            painter.plus_line_number += 1;
             state
         }
         _ => {
diff --git a/src/paint.rs b/src/paint.rs
index bfd029e..b18a439 100644
--- a/src/paint.rs
+++ b/src/paint.rs
@@ -1,3 +1,5 @@
+use lazy_static::lazy_static;
+use regex::Regex;
 use std::io::Write;
 
 use ansi_term;
@@ -22,6 +24,8 @@ pub struct Painter<'a> {
     pub highlighter: HighlightLines<'a>,
     pub config: &'a config::Config<'a>,
     pub output_buffer: String,
+    pub minus_line_number: usize,
+    pub plus_line_number: usize,
 }
 
 impl<'a> Painter<'a> {
@@ -37,6 +41,8 @@ impl<'a> Painter<'a> {
             highlighter: dummy_highlighter,
             writer,
             config,
+            minus_line_number: 0,
+            plus_line_number: 0,
         }
     }
 
@@ -71,11 +77,23 @@ impl<'a> Painter<'a> {
         );
         let (minus_line_diff_style_sections, plus_line_diff_style_sections) =
             Self::get_diff_style_sections(&self.minus_lines, &self.plus_lines, self.config);
+
+        let mut minus_line_numbers = Vec::new();
+        let mut plus_line_numbers = Vec::new();
+        for _line in &self.minus_lines {
+            minus_line_numbers.push(Some((Some(self.minus_line_number), None)));
+            self.minus_line_number += 1;
+        }
+        for _line in &self.plus_lines {
+            plus_line_numbers.push(Some((None, Some(self.plus_line_number))));
+            self.plus_line_number += 1;
+        }
         // TODO: lines and style sections contain identical line text
         if !self.minus_lines.is_empty() {
             Painter::paint_lines(
                 minus_line_syntax_style_sections,
                 minus_line_diff_style_sections,
+                minus_line_numbers,
                 &mut self.output_buffer,
                 self.config,
                 self.config.minus_line_marker,
@@ -88,6 +106,7 @@ impl<'a> Painter<'a> {
             Painter::paint_lines(
                 plus_line_syntax_style_sections,
                 plus_line_diff_style_sections,
+                plus_line_numbers,
                 &mut self.output_buffer,
                 self.config,
                 self.config.plus_line_marker,
@@ -105,6 +124,7 @@ impl<'a> Painter<'a> {
     pub fn paint_lines(
         syntax_style_sections: Vec<Vec<(SyntectStyle, &str)>>,
         diff_style_sections: Vec<Vec<(Style, &str)>>,
+        line_number_sections: Vec<Option<(Option<usize>, Option<usize>)>>,
         output_buffer: &mut String,
         config: &config::Config,
         prefix: &str,
@@ -120,8 +140,11 @@ impl<'a> Painter<'a> {
         // 2. We must ensure that we fill rightwards with the appropriate
         //    non-emph background color. In that case we don't use the last
         //    style of the line, because this might be emph.
-        for (syntax_sections, diff_sections) in
-            syntax_style_sections.iter().zip(diff_style_sections.iter())
+
+        for ((syntax_sections, diff_sections), line_numbers) in syntax_style_sections
+            .iter()
+            .zip(diff_style_sections.iter())
+            .zip(line_number_sections.iter())
         {
             let non_emph_style = if style_sections_contain_more_than_one_style(diff_sections) {
                 non_emph_style // line contains an emph section
@@ -130,6 +153,45 @@ impl<'a> Painter<'a> {
             };
             let mut ansi_strings = Vec::new();
             let mut handled_prefix = false;
+            if config.show_line_numbers && line_numbers.is_some() {
+                let (minus, plus) = line_numbers.unwrap();
+                let (minus_before, minus_number, minus_after) =
+                    get_line_number_components(minus, &config.number_minus_format);
+                let (plus_before, plus_number, plus_after) =
+                    get_line_number_components(plus, &config.number_plus_format);
+
+                ansi_strings.push(
+                    config
+                        .number_minus_format_style
+                        .ansi_term_style
+                        .paint(minus_before),
+                );
+                ansi_strings.push(
+                    config
+                        .number_minus_style
+                        .ansi_term_style
+                        .paint(minus_number),
+                );
+                ansi_strings.push(
+                    config
+                        .number_minus_format_style
+                        .ansi_term_style
+                        .paint(minus_after),
+                );
+                ansi_strings.push(
+                    config
+                        .number_plus_format_style
+                        .ansi_term_style
+                        .paint(plus_before),
+                );
+                ansi_strings.push(config.number_plus_style.ansi_term_style.paint(plus_number));
+                ansi_strings.push(
+                    config
+                        .number_plus_format_style
+                        .ansi_term_style
+                        .paint(plus_after),
+                );
+            }
             for (section_style, mut text) in superimpose_style_sections(
                 syntax_sections,
                 diff_sections,
@@ -499,3 +561,40 @@ mod superimpose_style_sections {
         }
     }
 }
+
+lazy_static! {
+    static ref LINE_NUMBER_REGEXP: Regex =
+        Regex::new(r"(?P<before>.*)(?P<ln>%ln)(?P<after>.*)").unwrap();
+}
+
+fn format_line_number(line_number: Option<usize>) -> String {
+    match line_number {
+        Some(x) => format!("{:^4}", x),
+        None => format!("    "),
+    }
+}
+
+fn get_line_number_components(
+    number: Option<usize>,
+    number_format: &str,
+) -> (String, String, String) {
+    let _caps = LINE_NUMBER_REGEXP.captures(number_format);
+
+    let caps = match _caps {
+        Some(_) => _caps.unwrap(),
+        None => return (number_format.to_string(), "".to_string(), "".to_string()),
+    };
+
+    let before = caps.name("before").unwrap().as_str();
+    let ln = caps.name("ln");
+    let after = caps.name("after").unwrap().as_str();
+    let display_number = match ln {
+        Some(_) => number,
+        None => None,
+    };
+    (
+        before.to_string(),
+        format_line_number(display_number),
+        after.to_string(),
+    )
+}
diff --git a/src/parse.rs b/src/parse.rs
index 7bccd65..5cc1123 100644
--- a/src/parse.rs
+++ b/src/parse.rs
@@ -1,3 +1,5 @@
+use lazy_static::lazy_static;
+use regex::Regex;
 use std::path::Path;
 
 use crate::config::Config;
@@ -87,17 +89,36 @@ pub fn get_file_change_description_from_file_paths(
     }
 }
 
+lazy_static! {
+    static ref HUNK_METADATA_REGEXP: Regex =
+        Regex::new(r"@+ (?P<lns>([-+]\d+(?:,\d+)? ){2,4})@+(?P<cf>.*\s?)").unwrap();
+}
+
+lazy_static! {
+    static ref LINE_NUMBER_REGEXP: Regex = Regex::new(r"[-+]").unwrap();
+}
+
+fn _make_line_number_vec(line: &str) -> Vec<usize> {
+    let mut numbers = Vec::<usize>::new();
+
+    for s in LINE_NUMBER_REGEXP.split(line) {
+        let number = s.split(',').nth(0).unwrap().split_whitespace().nth(0);
+        match number {
+            Some(number) => numbers.push(number.parse::<usize>().unwrap()),
+            None => continue,
+        }
+    }
+    return numbers;
+}
+
 /// Given input like
 /// "@@ -74,15 +74,14 @@ pub fn delta("
-/// Return " pub fn delta("
-pub fn parse_hunk_metadata(line: &str) -> (&str, &str) {
-    let mut iter = line.split("@@").skip(1);
-    let line_number = iter
-        .next()
-        .and_then(|s| s.split('+').nth(1).and_then(|s| s.split(',').next()))
-        .unwrap_or("");
-    let code_fragment = iter.next().unwrap_or("");
-    (code_fragment, line_number)
+/// Return " pub fn delta(" and a vector of line numbers
+pub fn parse_hunk_metadata(line: &str) -> (&str, Vec<usize>) {
+    let caps = HUNK_METADATA_REGEXP.captures(line).unwrap();
+    let line_numbers = _make_line_number_vec(caps.name("lns").unwrap().as_str());
+    let code_fragment = caps.name("cf").unwrap().as_str();
+    return (code_fragment, line_numbers);
 }
 
 /// Attempt to parse input as a file path and return extension as a &str.
@@ -249,9 +270,42 @@ mod tests {
 
     #[test]
     fn test_parse_hunk_metadata() {
-        assert_eq!(
-            parse_hunk_metadata("@@ -74,15 +75,14 @@ pub fn delta(\n"),
-            (" pub fn delta(\n", "75")
-        );
+        let parsed = parse_hunk_metadata("@@ -74,15 +75,14 @@ pub fn delta(\n");
+        let code_fragment = parsed.0;
+        let line_numbers = parsed.1;
+        assert_eq!(code_fragment, " pub fn delta(\n",);
+        assert_eq!(line_numbers[0], 74,);
+        assert_eq!(line_numbers[1], 75,);
+    }
+
+    #[test]
+    fn test_parse_hunk_metadata_added_file() {
+        let parsed = parse_hunk_metadata("@@ -1,22 +0,0 @@");
+        let code_fragment = parsed.0;
+        let line_numbers = parsed.1;
+        assert_eq!(code_fragment, "",);
+        assert_eq!(line_numbers[0], 1,);
+        assert_eq!(line_numbers[1], 0,);
+    }
+
+    #[test]
+    fn test_parse_hunk_metadata_deleted_file() {
+        let parsed = parse_hunk_metadata("@@ -0,0 +1,3 @@");
+        let code_fragment = parsed.0;
+        let line_numbers = parsed.1;
+        assert_eq!(code_fragment, "",);
+        assert_eq!(line_numbers[0], 0,);
+        assert_eq!(line_numbers[1], 1,);
+    }
+
+    #[test]
+    fn test_parse_hunk_metadata_merge() {
+        let parsed = parse_hunk_metadata("@@@ -293,11 -358,15 +358,16 @@@ dependencies =");
+        let code_fragment = parsed.0;
+        let line_numbers = parsed.1;
+        assert_eq!(code_fragment, " dependencies =",);
+        assert_eq!(line_numbers[0], 293,);
+        assert_eq!(line_numbers[1], 358,);
+        assert_eq!(line_numbers[2], 358,);
     }
 }
diff --git a/src/rewrite.rs b/src/rewrite.rs
index ab75508..c7db5fa 100644
--- a/src/rewrite.rs
+++ b/src/rewrite.rs
@@ -49,7 +49,8 @@ fn _rewrite_options_to_honor_git_config(
             ("dark", dark),
             ("navigate", navigate),
             ("color-only", color_only),
-            ("keep-plus-minus-markers", keep_plus_minus_markers)
+            ("keep-plus-minus-markers", keep_plus_minus_markers),
+            ("number", show_line_numbers)
         ],
         opt,
         git_config,
@@ -76,6 +77,12 @@ fn _rewrite_options_to_honor_git_config(
             ("minus-emph-style", minus_emph_style),
             ("minus-non-emph-style", minus_non_emph_style),
             ("minus-style", minus_style),
+            ("number-minus-format", number_minus_format),
+            ("number-minus-format-style", number_minus_format_style),
+            ("number-minus-style", number_minus_style),
+            ("number-plus-format", number_plus_format),
+            ("number-plus-format-style", number_plus_format_style),
+            ("number-plus-style", number_plus_style),
             ("paging-mode", paging_mode),
             ("plus-emph-style", plus_emph_style),
             ("plus-non-emph-style", plus_non_emph_style),


Test patch:
diff --git a/src/tests/ansi_test_utils.rs b/src/tests/ansi_test_utils.rs
index 5beb3f4..7d0ccf3 100644
--- a/src/tests/ansi_test_utils.rs
+++ b/src/tests/ansi_test_utils.rs
@@ -107,6 +107,7 @@ pub mod ansi_test_utils {
         paint::Painter::paint_lines(
             vec![syntax_style_sections],
             vec![vec![(syntax_highlighted_style, lines[0])]],
+            vec![None],
             &mut output_buffer,
             config,
             "",