Skip to content

Commit

Permalink
diff --check: explain why we do not care whether old side is binary
Browse files Browse the repository at this point in the history
All other codepaths refrain from running textual diff when either the old
or the new side is binary, but this function only checks the new side.  I
was almost going to change it to check both, but that would be a bad
change.  Explain why to prevent future mistakes.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Junio C Hamano committed Jun 27, 2008
1 parent c0f5c69 commit 5ff10dd
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -1544,8 +1544,9 @@ static void builtin_diffstat(const char *name_a, const char *name_b,

static void builtin_checkdiff(const char *name_a, const char *name_b,
const char *attr_path,
struct diff_filespec *one,
struct diff_filespec *two, struct diff_options *o)
struct diff_filespec *one,
struct diff_filespec *two,
struct diff_options *o)
{
mmfile_t mf1, mf2;
struct checkdiff_t data;
Expand All @@ -1564,6 +1565,12 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");

/*
* All the other codepaths check both sides, but not checking
* the "old" side here is deliberate. We are checking the newly
* introduced changes, and as long as the "new" side is text, we
* can and should check what it introduces.
*/
if (diff_filespec_is_binary(two))
goto free_and_return;
else {
Expand Down

0 comments on commit 5ff10dd

Please sign in to comment.