Skip to content

Add handling added/deleted files in diff #50

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.

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

Merged

Conversation

sofiia-tesliuk
Copy link
Contributor

@sofiia-tesliuk sofiia-tesliuk commented Aug 10, 2020

Related issue: #49
closes #49

There are some changes from planned implementation:

  • empty NewName marked as empty string instead of nil
  • files aren't determined if they are from original source or new one. All filenames from such messages will be added to OrigName field.
    [2].OrigName: "source_b/file_3.txt and [2].NewName: "" will be relevant.
    Because in case diff file only contains messages about added/deleted files, it is impossible to determine which of the sources is the original one.

Then for this file example, the output below for ReadAllFiles() would be expected.
my_diff.txt

diff -u source_a/file_1.txt  source_b/file_1.txt
--- source_a/file_1.txt   2020-07-28 12:54:18.000000000 +0000
+++ source_b/file_1.txt  2020-07-28 12:54:18.000000000 +0000
@@ -2,3 +3,4 @@
 To be, or not to be, that is the question:
-Whether 'tis nobler in the mind to suffer
+The slings and arrows of outrageous fortune,
+Or to take arms against a sea of troubles
 And by opposing end them. To die—to sleep,
Only in source_a: file_2.txt
Only in source_b: file_3.txt
ReadAllFiles(myMultiFileDiffReader) -> {
    FileDiff{
        OrigName: “source_a/file_1.txt”, 
        OrigTime: ..., 
        NewName: “source_b/file_1.txt”, 
        NewTime: …,
        Entended: ..., 
        Hunks: …
    },
 
    FileDiff{
        OrigName: “source_a/file_2.txt”, 
        OrigTime: nil, 
        NewName: "", 
        NewTime: nil
        …
    }, 

    FileDiff{
        OrigName: “source_b/file_3.txt”, 
        OrigTime: nil, 
        NewName: "", 
        …
    }

}, nil

@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #50 into master will increase coverage by 1.17%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
+ Coverage   28.31%   29.48%   +1.17%     
==========================================
  Files           5        5              
  Lines        1155     1177      +22     
==========================================
+ Hits          327      347      +20     
- Misses        783      784       +1     
- Partials       45       46       +1     
Impacted Files Coverage Δ
diff/diff.go 86.48% <ø> (ø)
diff/print.go 45.67% <60.00%> (+0.94%) ⬆️
diff/parse.go 82.04% <100.00%> (+0.99%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f935979...29fa046. Read the comment docs.

Copy link

@schachmat schachmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Added some comments. Final decision on how to implement this is with the repo owners of course.

Copy link
Member

@keegancsmith keegancsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will want @eseliger to take a look as well.

@keegancsmith
Copy link
Member

BTW thank you so much for the great pull request!

@keegancsmith keegancsmith merged commit c3b8bb2 into sourcegraph:master Aug 18, 2020
@sofiia-tesliuk sofiia-tesliuk deleted the handling_added_deleted_files branch August 18, 2020 11:28
sofiia-tesliuk added a commit to sofiia-tesliuk/go-diff that referenced this pull request Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants