Skip to content

UnifiedDiffReader.parseUnifiedDiff(): Using unified diff with no prefix in file names leads to cut file names in some cases. #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
IgnacioGarcia198 opened this issue Jan 11, 2022 · 1 comment
Closed

UnifiedDiffReader.parseUnifiedDiff(): Using unified diff with no prefix in file names leads to cut file names in some cases. #141

IgnacioGarcia198 opened this issue Jan 11, 2022 · 1 comment

Comments

@IgnacioGarcia198
Copy link

IgnacioGarcia198 commented Jan 11, 2022 •
edited
Loading

Describe the bug
When using an unified diff with no a/, b/ prefix in file names as input for UnifiedDiffReader.parseUnifiedDiff(), extracted file names are cut when they start with one of a, b, old new. This seems to be due to this piece of code:

return line.substring(4).replaceFirst("^(a|b|old|new)(\\/)?", "")
                .trim();

which is located at UnifiedDiffReader::356 in a method called extractFileName.

To Reproduce
Steps to reproduce the behavior:

  1. Example data: Use an unified diff that contains at least one file whose name starts with "a", "b", "old" or "new".
    For example:
--- a.txt
+++ a1.txt
@@ -8,7 +8,7 @@
     <Setting>
         <Setting a>
             <setting b>
-                <value>23</value>
+                <value>24</value>
             </setting b>
             <setting c>
                 <value>1</value>
  1. simple program snippet
val diff = """
        --- a.txt
        +++ a1.txt
        @@ -8,7 +8,7 @@
             <Setting>
                 <Setting a>
                     <setting b>
        -                <value>23</value>
        +                <value>24</value>
                     </setting b>
                     <setting c>
                         <value>1</value>
    """.trimIndent()
val unifiedDiff = UnifiedDiffReader.parseUnifiedDiff(diff.byteInputStream())
unifiedDiff.files.forEach { unifiedDiffFile ->
                println(unifiedDiffFile.fromFile)
                println(unifiedDiffFile.toFile)
}
  1. See error

This snipet prints:

.txt
1.txt

Expected behavior
The snippet should print:

a.txt
a1.txt

System

  • Java version 11
  • Version 4.11
@wumpz
Copy link
Collaborator

wumpz commented Mar 10, 2022

OK. In the original spec I saw a/ and b/ as starting "virtual" directories. Seeing your example the assumption they are always there was obviously wrong.

@wumpz wumpz pinned this issue Mar 10, 2022
helbaroudy added a commit to helbaroudy/java-diff-utils that referenced this issue Mar 15, 2022
@helbaroudy
Add test to reproduce issue java-diff-utils#141
6fd75cf
helbaroudy added a commit to helbaroudy/java-diff-utils that referenced this issue Mar 15, 2022
@helbaroudy
Fix assumption on test to match expected behaviour expressed on java-…
66e5653 
…diff-utils#141
helbaroudy added a commit to helbaroudy/java-diff-utils that referenced this issue Mar 15, 2022
@helbaroudy
Make "/" after "virtual" directories mandatory
a6bb83a 
Now the regex only matches with "a/", "b/", "new/" and "old/", previously the slash was taken as optional.
fixes java-diff-utils#141
helbaroudy added a commit to helbaroudy/java-diff-utils that referenced this issue Mar 15, 2022
@helbaroudy
Add test to reproduce issue java-diff-utils#141
e17a006
helbaroudy added a commit to helbaroudy/java-diff-utils that referenced this issue Mar 15, 2022
@helbaroudy
Fix assumption on test to match expected behaviour expressed on java-…
d17877f 
…diff-utils#141
helbaroudy added a commit to helbaroudy/java-diff-utils that referenced this issue Mar 15, 2022
@helbaroudy
Make "/" after "virtual" directories mandatory
49722a1 
Now the regex only matches with "a/", "b/", "new/" and "old/", previously the slash was taken as optional.
fixes java-diff-utils#141
@helbaroudy helbaroudy mentioned this issue Mar 15, 2022
Merged
@wumpz wumpz closed this as completed in ea9942d Apr 9, 2022
@wumpz wumpz unpinned this issue May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@wumpz @IgnacioGarcia198