Fix filenames with spaces (dandavison#643)

* Fix parsing of file path from diff line

Fixes dandavison#625

* Parse diff line only if needed

* Add test of diff of filenames with spaces

* Strip trailing tab inserted by git when file path contains space

git appears to add a trailing tab character when the file name contains a space:

$ git diff --no-index b.file 01g\ -\ Text | cat -A
diff·--git·a/b.file·b/01g·-·Text␊
index·e69de29..d00491f·100644␊
---·a/b.file␊
+++·b/01g·-·Text├──┤␊
@@·-0,0·+1·@@␊
+1␊

src/delta.rs

@@ -64,7 +64,7 @@ struct StateMachine<'a> {
     plus_file: String,
     minus_file_event: parse::FileEvent,
     plus_file_event: parse::FileEvent,
-    file_paths_from_diff_line: (Option<String>, Option<String>),
+    diff_line: String,
     painter: Painter<'a>,
     config: &'a Config,
 
@@ -95,7 +95,7 @@ impl<'a> StateMachine<'a> {
             plus_file: "".to_string(),
             minus_file_event: parse::FileEvent::NoEvent,
             plus_file_event: parse::FileEvent::NoEvent,
-            file_paths_from_diff_line: (None, None),
+            diff_line: "".to_string(),
             current_file_pair: None,
             handled_file_meta_header_line_file_pair: None,
             painter: Painter::new(writer, config),
@@ -259,7 +259,7 @@ impl<'a> StateMachine<'a> {
         self.painter.paint_buffered_minus_and_plus_lines();
         self.state = State::FileMeta;
         self.handled_file_meta_header_line_file_pair = None;
-        self.file_paths_from_diff_line = parse::get_file_paths_from_diff_line(&self.line);
+        self.diff_line = self.line.clone();
         Ok(false)
     }
 
@@ -278,9 +278,10 @@ impl<'a> StateMachine<'a> {
         // In the case of ModeChange only, the file path is taken from the diff
         // --git line (since that is the only place the file path occurs);
         // otherwise it is taken from the --- / +++ line.
-        self.minus_file = match (&file_event, &self.file_paths_from_diff_line) {
-            (parse::FileEvent::ModeChange(_), (Some(file), _)) => file.clone(),
-            _ => path_or_mode,
+        self.minus_file = if let parse::FileEvent::ModeChange(_) = &file_event {
+            parse::get_repeated_file_path_from_diff_line(&self.diff_line).unwrap_or(path_or_mode)
+        } else {
+            path_or_mode
         };
         self.minus_file_event = file_event;
 
@@ -325,9 +326,10 @@ impl<'a> StateMachine<'a> {
         // In the case of ModeChange only, the file path is taken from the diff
         // --git line (since that is the only place the file path occurs);
         // otherwise it is taken from the --- / +++ line.
-        self.plus_file = match (&file_event, &self.file_paths_from_diff_line) {
-            (parse::FileEvent::ModeChange(_), (_, Some(file))) => file.clone(),
-            _ => path_or_mode,
+        self.plus_file = if let parse::FileEvent::ModeChange(_) = &file_event {
+            parse::get_repeated_file_path_from_diff_line(&self.diff_line).unwrap_or(path_or_mode)
+        } else {
+            path_or_mode
         };
         self.plus_file_event = file_event;
         self.painter

src/parse.rs

@@ -2,6 +2,7 @@ use lazy_static::lazy_static;
 use regex::Regex;
 use std::borrow::Cow;
 use std::path::Path;
+use unicode_segmentation::UnicodeSegmentation;
 
 use crate::config::Config;
 use crate::features;
@@ -73,18 +74,32 @@ pub fn parse_file_meta_line(
     (path_or_mode, file_event)
 }
 
-/// Given input like "diff --git a/src/main.rs b/src/main.rs"
-/// return (Some("src/main.rs"), Some("src/main.rs"))
-pub fn get_file_paths_from_diff_line(line: &str) -> (Option<String>, Option<String>) {
-    let mut iter = line.split(' ').skip(2);
-    (
-        iter.next().map(|s| _parse_file_path(&s[2..], true)),
-        iter.next().map(|s| _parse_file_path(&s[2..], true)),
-    )
+/// Given input like "diff --git a/src/my file.rs b/src/my file.rs"
+/// return Some("src/my file.rs")
+pub fn get_repeated_file_path_from_diff_line(line: &str) -> Option<String> {
+    if let Some(line) = line.strip_prefix("diff --git ") {
+        let line: Vec<&str> = line.graphemes(true).collect();
+        let midpoint = line.len() / 2;
+        if line[midpoint] == " " {
+            let first_path = _parse_file_path(&line[..midpoint].join(""), true);
+            let second_path = _parse_file_path(&line[midpoint + 1..].join(""), true);
+            if first_path == second_path {
+                return Some(first_path);
+            }
+        }
+    }
+    None
 }
 
 fn _parse_file_path(s: &str, git_diff_name: bool) -> String {
-    match s {
+    // It appears that, if the file name contains a space, git appends a tab
+    // character in the diff metadata lines, e.g.
+    // $ git diff --no-index "a b" "c d" | cat -A
+    // diff·--git·a/a·b·b/c·d␊
+    // index·d00491f..0cfbf08·100644␊
+    // ---·a/a·b├──┤␊
+    // +++·b/c·d├──┤␊
+    match s.strip_suffix("\t").unwrap_or(s) {
         path if path == "/dev/null" => "/dev/null",
         path if git_diff_name && DIFF_PREFIXES.iter().any(|s| path.starts_with(s)) => &path[2..],
         path if git_diff_name => &path,
@@ -257,6 +272,30 @@ fn get_extension(s: &str) -> Option<&str> {
 mod tests {
     use super::*;
 
+    #[test]
+    fn test_get_repeated_file_path_from_diff_line() {
+        assert_eq!(
+            get_repeated_file_path_from_diff_line("diff --git a/src/main.rs b/src/main.rs"),
+            Some("src/main.rs".to_string())
+        );
+        assert_eq!(
+            get_repeated_file_path_from_diff_line("diff --git a/a b/a"),
+            Some("a".to_string())
+        );
+        assert_eq!(
+            get_repeated_file_path_from_diff_line("diff --git a/a b b/a b"),
+            Some("a b".to_string())
+        );
+        assert_eq!(
+            get_repeated_file_path_from_diff_line("diff --git a/a b/aa"),
+            None
+        );
+        assert_eq!(
+            get_repeated_file_path_from_diff_line("diff --git a/.config/Code - Insiders/User/settings.json b/.config/Code - Insiders/User/settings.json"),
+            Some(".config/Code - Insiders/User/settings.json".to_string())
+        );
+    }
+
     #[test]
     fn test_get_file_extension_from_marker_line() {
         assert_eq!(

src/tests/test_example_diffs.rs

@@ -1608,6 +1608,15 @@ src/align.rs:71: impl<'a> Alignment<'a> { │
         assert!(output.contains(r"https://invent.kde.org/utilities/konsole/-/commit/94907c0f136f46dc46ffae2dc92dca9af7eb7c2e"));
     }
 
+    #[test]
+    fn test_filenames_with_spaces() {
+        let config = integration_test_utils::make_config_from_args(&[]);
+        let output =
+            integration_test_utils::run_delta(GIT_DIFF_NO_INDEX_FILENAMES_WITH_SPACES, &config);
+        let output = strip_ansi_codes(&output);
+        assert!(output.contains("a b ⟶   c d\n"));
+    }
+
     const GIT_DIFF_SINGLE_HUNK: &str = "\
 commit 94907c0f136f46dc46ffae2dc92dca9af7eb7c2e
 Author: Dan Davison <dandavison7@gmail.com>
@@ -2222,5 +2231,15 @@ new mode 100755
 diff --git a/src/delta.rs b/src/delta.rs
 old mode 100755
 new mode 100644
+";
+
+    const GIT_DIFF_NO_INDEX_FILENAMES_WITH_SPACES: &str = "
+diff --git a/a b b/c d
+index d00491f..0cfbf08 100644
+--- a/a b	
++++ b/c d	
+@@ -1 +1 @@
+-1
++2
 ";
 }