Fix panicking when diffing filenames with space enclosed dashes

[pt2121]

Fixes #139

[dandavison]

Hi @pt2121 thanks very much for this!

Did you see #120 where we are trying to handle the diff.noprefix flag? For a user with that flag set, the strings won't have a/ and b/ prefixes. Do you have any thoughts about handling that? Does it look like we need to instead start using the lines starting --- and +++ (what delta calls "file meta" lines) as the source of the file extension?

Some example input is (git --no-pager diff --no-prefix)

diff --git src/pa rse.rs src/pa rse.rs
index dcbea72..a642b6b 100644
--- src/pa rse.rs
+++ src/pa rse.rs

diff --git src/parse.rs src/pa rse.rs
similarity index 100%
rename from src/parse.rs
rename to src/pa rse.rs

diff --git src/pa rse.rs src/pa rse.rs                                                                                                                                                                          
new file mode 100644                                                                                                                                                                                            
index 0000000..dcbea72                                                                                                                                                                                          
--- /dev/null                                                                                                                                                                                                   
+++ src/pa rse.rs

[pt2121]

Yes, sure. Thanks for the pointer. I'll look into it.

[dandavison]

Thanks!

[pt2121]

Hi @dandavison,
I updated my code as you suggested. The diff.noprefix flag should be now handled.
Please let me know what you think. Thanks!

[dandavison]

Before I write anything else I want to make it clear that you've already done more than enough for your PR to be merged and it's very helpful, so thank you!

I don't have time right now to do further work on these edge cases myself so I'm just going to make some additional comments on the off chance that you want to double-check my thinking, since it's very helpful to have someone else familiar with this code.

Would you agree that this test case demonstrates a bug? I wonder if what we should do (in a different PR) is determine (programatically?) the cases where the sublime syntax definition is keyed off the entire filename (like Makefile, Dockerfile) and otherwise refuse to identify a file extension for random filenames that do not look like *.foo.

diff --git a/src/parse.rs b/src/parse.rs
index e1a0860..78cea85 100644
--- a/src/parse.rs
+++ b/src/parse.rs
@@ -123,6 +123,11 @@ mod tests {
             get_file_extension_from_file_meta_line_file_path("Makefile"),
             Some("Makefile")
         );
+        // TODO: The fact that the following test passes is perhaps a bug.
+        assert_eq!(
+            get_file_extension_from_file_meta_line_file_path("rs"),
+            Some("rs")
+        );
         assert_eq!(
             get_file_extension_from_file_meta_line_file_path("a/src/Makefile"),
             Some("Makefile")

[pt2121]

Oh, I see. Nice catch.
I'm not sure if there is a good way to differentiate them unless we have a list of all file types that don't have an extension e.g. Makefile, Dockerfile.

[dandavison]

I was hoping that the sublime syntax definitions might distinguish between genuine extensions, that involve a ., like foo.py, vs whole-file-name extensions like Makefile, Dockerfile. However, it looks like they don't. So I don't think this is delta's problem, seeing as bat also suffers from it:

[dandavison]

So I believe that there's another bug remaining (not your fault or responsibility!), in that unified diff is still broken for file names with spaces:

--- "src/pa r se.rs"	2020-05-03 16:40:13.851935846 -0400
+++ "src/pa rse.rs"	2020-05-03 16:41:49.798457052 -0400
@@ -231,4 +231,7 @@
         );
     }
 }
-vjhvjj
+
+pub fn test(s: &str) {
+    println!("hello {}", s);
+}

If I remove the double quotes and spaces from those filenames, then delta syntax-highlights the added function as Rust:

--- src/parse.rs	2020-05-03 16:40:13.851935846 -0400
+++ src/parse.rs	2020-05-03 16:41:49.798457052 -0400
@@ -231,4 +231,7 @@
         );
     }
 }
-vjhvjj
+
+pub fn test(s: &str) {
+    println!("hello {}", s);
+}

[dandavison]

Thanks a lot for this work. I'll do a final review/test and merge soon.

[pt2121]

Have you had a chance to look at this? Anything else I should address in this PR?

[dandavison]

Anything else I should address in this PR?

No, it looks great, thanks very much!

[pt2121]

No problem. I will try fixing the remaining bug when having time.