Make relative-paths work with binary files

.gitignore

@@ -5,3 +5,7 @@ performance/index.html
 src/junk.rs
 **/*.rs.bk
 /target
+
+# insta: unreviewed inline data, and separate snapshot files
+*.pending-snap
+**/snapshots/*.snap.new

Cargo.toml

@@ -68,7 +68,7 @@ default-features = false
 features = []
 
 [dev-dependencies]
-insta = { version = "1.*", features = ["colors"] }
+insta = { version = "1.*", features = ["colors", "filters"] }
 
 [profile.test]
 opt-level = 2

src/handlers/diff_header.rs

@@ -84,12 +84,11 @@ impl<'a> StateMachine<'a> {
             return Ok(false);
         }
 
-        let (path_or_mode, file_event) =
+        let (mut path_or_mode, file_event) =
             parse_diff_header_line(&self.line, self.source == Source::GitDiff);
 
-        self.minus_file = utils::path::relativize_path_maybe(&path_or_mode, self.config)
-            .map(|p| p.to_string_lossy().into_owned())
-            .unwrap_or(path_or_mode);
+        utils::path::relativize_path_maybe(&mut path_or_mode, self.config);
+        self.minus_file = path_or_mode;
         self.minus_file_event = file_event;
 
         if self.source == Source::DiffUnified {
@@ -121,12 +120,11 @@ impl<'a> StateMachine<'a> {
             return Ok(false);
         }
         let mut handled_line = false;
-        let (path_or_mode, file_event) =
+        let (mut path_or_mode, file_event) =
             parse_diff_header_line(&self.line, self.source == Source::GitDiff);
 
-        self.plus_file = utils::path::relativize_path_maybe(&path_or_mode, self.config)
-            .map(|p| p.to_string_lossy().into_owned())
-            .unwrap_or(path_or_mode);
+        utils::path::relativize_path_maybe(&mut path_or_mode, self.config);
+        self.plus_file = path_or_mode;
         self.plus_file_event = file_event;
         self.painter
             .set_syntax(get_filename_from_diff_header_line_file_path(
@@ -470,6 +468,8 @@ pub fn get_file_change_description_from_file_paths(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::tests::integration_test_utils::{make_config_from_args, DeltaTest};
+    use insta::assert_snapshot;
 
     #[test]
     fn test_get_filename_from_marker_line() {
@@ -639,4 +639,43 @@ mod tests {
             Some(".config/Code - Insiders/User/settings.json".to_string())
         );
     }
+
+    pub const BIN_AND_TXT_FILE_ADDED: &str = "\
+diff --git a/BIN b/BIN
+new file mode 100644
+index 0000000..a5d0c46
+Binary files /dev/null and b/BIN differ
+diff --git a/TXT b/TXT
+new file mode 100644
+index 0000000..323fae0
+--- /dev/null
++++ b/TXT
+@@ -0,0 +1 @@
++plain text";
+
+    #[test]
+    fn test_diff_header_relative_paths() {
+        // rustfmt ignores the assert macro arguments, so do the setup outside
+        let mut cfg = make_config_from_args(&["--relative-paths", "-s"]);
+        cfg.cwd_relative_to_repo_root = Some("src/utils/".into());
+        let result = DeltaTest::with_config(&cfg)
+            .with_input(BIN_AND_TXT_FILE_ADDED)
+            .output;
+        // convert windows '..\' to unix '../' paths
+        insta::with_settings!({filters => vec![(r"\.\.\\", "../")]}, {
+            assert_snapshot!(result, @r###"
+
+            added: ../../BIN (binary file)
+            ───────────────────────────────────────────
+
+            added: ../../TXT
+            ───────────────────────────────────────────
+
+            ───┐
+            1: │
+            ───┘
+            │    │                │  1 │plain text
+            "###)
+        });
+    }
 }

src/handlers/diff_header_misc.rs

@@ -1,4 +1,5 @@
 use crate::delta::{DiffType, Source, State, StateMachine};
+use crate::utils::path::relativize_path_maybe;
 
 impl<'a> StateMachine<'a> {
     #[inline]
@@ -29,9 +30,11 @@ impl<'a> StateMachine<'a> {
             }
 
             if self.minus_file != "/dev/null" {
+                relativize_path_maybe(&mut self.minus_file, self.config);
                 self.minus_file.push_str(" (binary file)");
             }
             if self.plus_file != "/dev/null" {
+                relativize_path_maybe(&mut self.plus_file, self.config);
                 self.plus_file.push_str(" (binary file)");
             }
             return Ok(true);

src/tests/test_example_diffs.rs

@@ -8,6 +8,7 @@ mod tests {
     use crate::tests::ansi_test_utils::ansi_test_utils;
     use crate::tests::integration_test_utils;
     use crate::tests::integration_test_utils::DeltaTest;
+    use insta::assert_snapshot;
 
     #[test]
     fn test_added_file() {
@@ -126,7 +127,6 @@ mod tests {
             "bash",
         ]);
         let output = integration_test_utils::run_delta(MODIFIED_BASH_AND_CSHARP_FILES, &config);
-        eprintln!("{}", &output);
         ansi_test_utils::assert_line_has_syntax_highlighted_substring(
             &output,
             19,
@@ -311,37 +311,55 @@ index 0123456..1234567 100644
 
     #[test]
     fn test_binary_files_differ() {
-        let config =
-            integration_test_utils::make_config_from_args(&["--file-modified-label", "modified:"]);
-        let output = integration_test_utils::run_delta(BINARY_FILES_DIFFER, &config);
-        let output = strip_ansi_codes(&output);
-        assert!(output.contains("\nmodified: foo (binary file)\n"));
+        let output = DeltaTest::with_args(&["--file-modified-label", "modified:"])
+            .with_input(BINARY_FILES_DIFFER)
+            .skip_header();
+
+        assert_snapshot!(output, @r###"
+
+        modified: foo (binary file)
+        ───────────────────────────────────────────
+        "###);
     }
 
     #[test]
     fn test_binary_file_added() {
-        let config = integration_test_utils::make_config_from_args(&[]);
-        let output = integration_test_utils::run_delta(BINARY_FILE_ADDED, &config);
-        let output = strip_ansi_codes(&output);
-        assert!(output.contains("\nadded: foo (binary file)\n"));
+        let output = DeltaTest::with_args(&[])
+            .with_input(BINARY_FILE_ADDED)
+            .skip_header();
+        assert_snapshot!(output, @r###"
+
+        added: foo (binary file)
+        ───────────────────────────────────────────
+        "###);
     }
 
     #[test]
     fn test_binary_file_removed() {
-        let config = integration_test_utils::make_config_from_args(&[]);
-        let output = integration_test_utils::run_delta(BINARY_FILE_REMOVED, &config);
-        let output = strip_ansi_codes(&output);
-        assert!(output.contains("\nremoved: foo (binary file)\n"));
+        let output = DeltaTest::with_args(&[])
+            .with_input(BINARY_FILE_REMOVED)
+            .skip_header();
+        assert_snapshot!(output, @r###"
+
+        removed: foo (binary file)
+        ───────────────────────────────────────────
+        "###);
     }
 
     #[test]
     fn test_binary_files_differ_after_other() {
-        let config =
-            integration_test_utils::make_config_from_args(&["--file-modified-label", "modified:"]);
-        let output = integration_test_utils::run_delta(BINARY_FILES_DIFFER_AFTER_OTHER, &config);
-        let output = strip_ansi_codes(&output);
-        assert!(output.contains("\nrenamed: foo ⟶   bar\n"));
-        assert!(output.contains("\nmodified: qux (binary file)\n"));
+        let output = DeltaTest::with_args(&["--file-modified-label", "modified:"])
+            .with_input(BINARY_FILES_DIFFER_AFTER_OTHER)
+            .output;
+        assert_snapshot!(output, @r###"
+
+
+        renamed: foo ⟶   bar
+        ───────────────────────────────────────────
+
+        modified: qux (binary file)
+        ───────────────────────────────────────────
+        "###);
     }
 
     #[test]

src/utils/path.rs

@@ -25,16 +25,24 @@ pub fn absolute_path(relative_path: &str, config: &Config) -> Option<PathBuf> {
     .map(normalize_path)
 }
 
-/// Relativize path if delta config demands that and paths are not already relativized by git.
-pub fn relativize_path_maybe(path: &str, config: &Config) -> Option<PathBuf> {
-    if config.relative_paths && !calling_process().paths_in_input_are_relative_to_cwd() {
-        if let Some(base) = config.cwd_relative_to_repo_root.as_deref() {
-            pathdiff::diff_paths(path, base)
-        } else {
-            None
+/// Relativize `path` if delta `config` demands that and paths are not already relativized by git.
+pub fn relativize_path_maybe(path: &mut String, config: &Config) {
+    let mut inner_relativize = || -> Option<()> {
+        let base = config.cwd_relative_to_repo_root.as_deref()?;
+        let relative_path = pathdiff::diff_paths(&path, base)?;
+        if relative_path.is_relative() {
+            #[cfg(target_os = "windows")]
+            // '/dev/null' is converted to '\dev\null' and considered relative. Work
+            // around that by leaving all paths like that untouched:
+            if relative_path.starts_with(Path::new(r"\")) {
+                return None;
+            }
+            *path = relative_path.to_string_lossy().into_owned();
         }
-    } else {
-        None
+        Some(())
+    };
+    if config.relative_paths && !calling_process().paths_in_input_are_relative_to_cwd() {
+        let _ = inner_relativize();
     }
 }