rtfpessoa / diff2html

Pretty diff to html javascript library (diff2html)
https://diff2html.xyz
MIT License
2.87k stars 278 forks source link

if diff has space in folderName/fileName in filePath then parsed filename is wrong #387

Closed future-pirate-king closed 3 years ago

future-pirate-king commented 3 years ago

Step -1: Before filling an issue check out troubleshooting section

Step 0: Describe your environment

Step 1: Describe the problem:

when user creates a folder or file with name some which has space in it then the parsed output of unified diff is giving wrong fileName

Steps to reproduce:

  1. use can use unified diff which has space in folder name or file name and check parsed output

diff example:

diff --git a/notebooks/Untitled Folder/untitled.txt b/notebooks/Untitled Folder/untitled.txt\nnew file mode 100644\nindex 0000000..e69de29\ndiff --git a/notebooks/new.txt b/notebooks/new1.txt\nsimilarity index 83%\nrename from notebooks/new.txt\nrename to notebooks/new1.txt\nindex 0dea293..fe4d7e2 100644\n--- a/notebooks/new.txt\n+++ b/notebooks/new1.txt\n@@ -1,2 +1,2 @@\n Text to test git backend functionality.\n-new li\n\\ No newline at end of file\n+new line\n\\ No newline at end of file\ndiff --git a/notebooks/untitled1.txt b/notebooks/untitled1.txt\nnew file mode 100644\nindex 0000000..3e5126c\n--- /dev/null\n+++ b/notebooks/untitled1.txt\n@@ -0,0 +1 @@\n+new\n\\ No newline at end of file\n

Observed Results:

Expected Results:

newName: "notebooks/Untitled Folder/untitled.txt" oldName: "notebooks/Untitled Folder/untitled.txt"

Relevant Code:

// TODO(you): code here to reproduce the problem
rtfpessoa commented 3 years ago

@future-pirate-king where did you get that diff from? I am asking because it is not a valid unified diff, since the lines with --- and +++ are missing in the first file.

future-pirate-king commented 3 years ago

so this is generated using these commands

  1. first we stage the changes using -> git add .
  2. then diff using -> git diff --cached

update: for diff example create a new folder with space in folder name and add a new empty file then execute the above commands to get the diff

rtfpessoa commented 3 years ago

That is indeed a very interesting corner case.

I think it is impossible to solve in a simple way. Since it is not a valid unified diff, due to the lines with --- and +++ and since the only header available does not have a clear separator between the file paths, it is very error prone to know the names using it.

Do you have any suggestions?

future-pirate-king commented 3 years ago

currently this line of code is parsing the filenames for this case https://github.com/rtfpessoa/diff2html/blob/73ab5ba05059dd476fbbb0b4b1f50a005aae38b7/src/diff-parser.ts#L279

if we change the regex to ^diff --git "?(a\/.+)"? "?(b\/.+)"?

seems to work. @rtfpessoa what do you think ?

rtfpessoa commented 3 years ago

That is a not going to work if the first path has a b/ in it. But probably it is not worse than it is right now.

future-pirate-king commented 3 years ago

yes @rtfpessoa , that was just an example. if we know the possible prefixes we can create regex like this ^diff --git "?([abiwco]\/.+)"? "?([abiwco]\/.+)"?

or

^diff --git "?([a-z]\/.+)"? "?([a-z]\/.+)"?

it will work for a/, b/ or any of the alphabets inside square brackets.

rtfpessoa commented 3 years ago

The problem is not the types of prefixes if if the paths have spaces and a path like this will parse incorrectly still: diff --git a/my/amazing b/path.js b/new/different/path.js

rtfpessoa commented 3 years ago

Anyway since it does not stay worse than what it is now I went ahead and merged the update. Let me know how it goes.

rtfpessoa commented 3 years ago

Released as version 3.4.7

karansaraswat19 commented 1 month ago

Hello @rtfpessoa I feel this issue is still there. I use the parse method and it appends \t at the end and destroys the value. Can you please check this? P.S: using 3.4.22 version

Screenshot 2024-07-16 at 3 18 58 PM
rtfpessoa commented 1 month ago

Can you provide an example?