From dc1106a9b9106e160b67a5da3b67578a7a62fc29 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Mon, 17 Nov 2014 15:12:07 -0800 Subject: [PATCH] Differential - stop showing the shield for "move away" operations Summary: The shield is just confusing. In one case it doesn't work, and in the other case it just shows you a copy of the file you can see just below except in red. Fixes T4599, T1211. Note T1211 proposed not showing the "move away" file **at all** but I think removing the shield fixes the source of confusion. The code here is a bit if / else if / else if... heavy but this is logically sound. Test Plan: made a diff where i moved a file then edited it in the new location. viewed diff, saw confusing shield, dropped caches, applied patch, viewed diff and saw no shield. made a diff where I moved a file and didn't edit in new location and saw similar shield disappearness. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T1211, T4599 Differential Revision: https://secure.phabricator.com/D10865 --- .../parser/DifferentialChangesetParser.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 2de2ee9140..9db301ab35 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -90,6 +90,7 @@ final class DifferentialChangesetParser { const ATTR_DELETED = 'attr:deleted'; const ATTR_UNCHANGED = 'attr:unchanged'; const ATTR_WHITELINES = 'attr:white'; + const ATTR_MOVEAWAY = 'attr:moveaway'; const LINES_CONTEXT = 8; @@ -438,6 +439,10 @@ final class DifferentialChangesetParser { return idx($this->specialAttributes, self::ATTR_WHITELINES, false); } + public function isMoveAway() { + return idx($this->specialAttributes, self::ATTR_MOVEAWAY, false); + } + private function applyIntraline(&$render, $intra, $corpus) { foreach ($render as $key => $text) { @@ -594,6 +599,7 @@ final class DifferentialChangesetParser { } } + $moveaway = false; $changetype = $this->changeset->getChangeType(); if ($changetype == DifferentialChangeType::TYPE_MOVE_AWAY) { // sometimes we show moved files as unchanged, sometimes deleted, @@ -601,12 +607,14 @@ final class DifferentialChangesetParser { // destination of the move. Rather than make a false claim, // omit the 'not changed' notice if this is the source of a move $unchanged = false; + $moveaway = true; } $this->setSpecialAttributes(array( self::ATTR_UNCHANGED => $unchanged, self::ATTR_DELETED => $hunk_parser->getIsDeleted(), self::ATTR_WHITELINES => !$hunk_parser->getHasTextChanges(), + self::ATTR_MOVEAWAY => $moveaway, )); $hunk_parser->generateIntraLineDiffs(); @@ -775,6 +783,8 @@ final class DifferentialChangesetParser { $shield = $renderer->renderShield( pht('The contents of this file were not changed.'), $type); + } else if ($this->isMoveAway()) { + $shield = null; } else if ($this->isWhitespaceOnly()) { $shield = $renderer->renderShield( pht('This file was changed only by adding or removing whitespace.'),