From ff4e965c90fc61b130f11844fc372f97eeb46b16 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 4 Dec 2013 14:39:07 -0800 Subject: [PATCH] Don't try to download diffs-of-diffs Summary: Ref T1715. When the user clicks "Download Raw Diff" in Differential, we try to build a diff of exactly what they're seeing. However: - This doesn't work if any of the changes have multiple hunks, and fixing it seems hard. - I suspect this diff is never actually useful anyway? And probably kind of confusing in the best case. You can't really apply it to anyhting, since you'd have to apply another diff first. Instead, just build the right-side diff, which should align well with user expectation and doesn't suffer from the multi-hunk bug. Some day, we could maybe add some of the fancy options in T1715. See: Test Plan: Downloaded a multi-hunk diff, got the original back and applied it cleanly. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1715 Differential Revision: https://secure.phabricator.com/D7694 --- .../DifferentialRevisionViewController.php | 33 +------------------ 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 460d24f529..e418669ada 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -863,43 +863,12 @@ final class DifferentialRevisionViewController extends DifferentialController { $viewer = $this->getRequest()->getUser(); - $engine = new PhabricatorDifferenceEngine(); - $generated_changesets = array(); foreach ($changesets as $changeset) { $changeset->attachHunks($changeset->loadHunks()); - $right = $changeset->makeNewFile(); - $choice = $changeset; - $vs = idx($vs_map, $changeset->getID()); - if ($vs == -1) { - $left = $right; - $right = $changeset->makeOldFile(); - } else if ($vs) { - $choice = $vs_changeset = $vs_changesets[$vs]; - $vs_changeset->attachHunks($vs_changeset->loadHunks()); - $left = $vs_changeset->makeNewFile(); - } else { - $left = $changeset->makeOldFile(); - } - - $synthetic = $engine->generateChangesetFromFileContent( - $left, - $right); - - if (!$synthetic->getAffectedLineCount()) { - $filetype = $choice->getFileType(); - if ($filetype == DifferentialChangeType::FILE_TEXT || - $filetype == DifferentialChangeType::FILE_SYMLINK) { - continue; - } - } - - $choice->attachHunks($synthetic->getHunks()); - - $generated_changesets[] = $choice; } $diff = new DifferentialDiff(); - $diff->attachChangesets($generated_changesets); + $diff->attachChangesets($changesets); $raw_changes = $diff->buildChangesList(); $changes = array(); foreach ($raw_changes as $changedict) {