From 9092994e45c22be487798cbf8dba95e43a3c730b Mon Sep 17 00:00:00 2001 From: vrana Date: Mon, 30 Jul 2012 17:39:30 -0700 Subject: [PATCH] Make old and new files when checking for changes by commit Summary: We have some false positives on commit changes checker. I'm not sure if the reason is a difference between `git diff` and `svn diff` or something else but making this more robust doesn't harm anything. We couldn't make the files from the whole changeset because I want to ignore context bigger than `$num_lines` to reduce rebase noise. Test Plan: Ran the method on diff which had false positive previously. Ran the method on a diff with real change. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3107 --- ...torRepositoryCommitMessageParserWorker.php | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index da89a7a698..27eb70a93c 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -292,9 +292,27 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker if ($files[$file_phid]->loadFileData() != $corpus) { return $vs_diff; } - } else if ($changeset->makeChangesWithContext() != - $vs_changeset->makeChangesWithContext()) { - return $vs_diff; + } else { + $context = implode("\n", $changeset->makeChangesWithContext()); + $vs_context = implode("\n", $vs_changeset->makeChangesWithContext()); + + // We couldn't just compare $context and $vs_context because following + // diffs will be considered different: + // + // -(empty line) + // -echo 'test'; + // (empty line) + // + // (empty line) + // -echo "test"; + // -(empty line) + + $hunk = id(new DifferentialHunk())->setChanges($context); + $vs_hunk = id(new DifferentialHunk())->setChanges($vs_context); + if ($hunk->makeOldFile() != $vs_hunk->makeOldFile() || + $hunk->makeNewFile() != $vs_hunk->makeNewFile()) { + return $vs_diff; + } } }