Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15443851
D19453.id46530.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
11 KB
Referenced Files
None
Subscribers
None
D19453.id46530.diff
View Options
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -5,6 +5,7 @@
private $revisionID;
private $changesetCount;
+ private $hiddenChangesets;
public function shouldAllowPublic() {
return true;
@@ -169,6 +170,7 @@
}
$handles = $this->loadViewerHandles($object_phids);
+ $warnings = array();
$request_uri = $request->getRequestURI();
@@ -203,11 +205,19 @@
pht('Expand All Files'))),
);
- $warning = id(new PHUIInfoView())
+ $warnings[] = id(new PHUIInfoView())
->setTitle(pht('Large Diff'))
->setSeverity(PHUIInfoView::SEVERITY_WARNING)
->appendChild($message);
+ $folded_changesets = $changesets;
+ } else {
+ $folded_changesets = array();
+ }
+
+ // Don't hide or fold changesets which have inline comments.
+ $hidden_changesets = $this->hiddenChangesets;
+ if ($hidden_changesets || $folded_changesets) {
$old = array_select_keys($changesets, $old_ids);
$new = array_select_keys($changesets, $new_ids);
@@ -222,16 +232,41 @@
$new,
$revision);
- $visible_changesets = array();
foreach ($inlines as $inline) {
$changeset_id = $inline->getChangesetID();
- if (isset($changesets[$changeset_id])) {
- $visible_changesets[$changeset_id] = $changesets[$changeset_id];
+ if (!isset($changesets[$changeset_id])) {
+ continue;
}
+
+ unset($hidden_changesets[$changeset_id]);
+ unset($folded_changesets[$changeset_id]);
}
- } else {
- $warning = null;
- $visible_changesets = $changesets;
+ }
+
+ // Update the set of hidden changesets, since we may have just un-hidden
+ // some of them.
+ if ($hidden_changesets) {
+ $warnings[] = id(new PHUIInfoView())
+ ->setTitle(pht('Showing Only Differences'))
+ ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
+ ->appendChild(
+ pht(
+ 'This revision modifies %s more file(s) that are hidden because '.
+ 'they were not modified between selected diffs and they have no '.
+ 'inline comments.',
+ phutil_count($hidden_changesets)));
+ }
+
+ // Compute the unfolded changesets. By default, everything is unfolded.
+ $unfolded_changesets = $changesets;
+ foreach ($folded_changesets as $changeset_id => $changeset) {
+ unset($unfolded_changesets[$changeset_id]);
+ }
+
+ // Throw away any hidden changesets.
+ foreach ($hidden_changesets as $changeset_id => $changeset) {
+ unset($changesets[$changeset_id]);
+ unset($unfolded_changesets[$changeset_id]);
}
$commit_hashes = mpull($diffs, 'getSourceControlBaseRevision');
@@ -267,7 +302,7 @@
if ($repository) {
$symbol_indexes = $this->buildSymbolIndexes(
$repository,
- $visible_changesets);
+ $unfolded_changesets);
} else {
$symbol_indexes = array();
}
@@ -328,7 +363,7 @@
} else {
$changeset_view = id(new DifferentialChangesetListView())
->setChangesets($changesets)
- ->setVisibleChangesets($visible_changesets)
+ ->setVisibleChangesets($unfolded_changesets)
->setStandaloneURI('/differential/changeset/')
->setRawFileURIs(
'/differential/changeset/?view=old',
@@ -405,7 +440,7 @@
$toc_view = $this->buildTableOfContents(
$changesets,
- $visible_changesets,
+ $unfolded_changesets,
$target->loadCoverageMap($viewer));
// Attach changesets to each reviewer so we can show which Owners package
@@ -520,7 +555,7 @@
$footer[] = array(
$anchor,
- $warning,
+ $warnings,
$tab_view,
$changeset_view,
);
@@ -807,6 +842,7 @@
DifferentialDiff $target,
DifferentialDiff $diff_vs = null,
PhabricatorRepository $repository = null) {
+ $viewer = $this->getViewer();
$load_diffs = array($target);
if ($diff_vs) {
@@ -814,7 +850,7 @@
}
$raw_changesets = id(new DifferentialChangesetQuery())
- ->setViewer($this->getRequest()->getUser())
+ ->setViewer($viewer)
->withDiffs($load_diffs)
->execute();
$changeset_groups = mgroup($raw_changesets, 'getDiffID');
@@ -822,17 +858,19 @@
$changesets = idx($changeset_groups, $target->getID(), array());
$changesets = mpull($changesets, null, 'getID');
- $refs = array();
- $vs_map = array();
+ $refs = array();
+ $vs_map = array();
$vs_changesets = array();
+ $must_compare = array();
if ($diff_vs) {
- $vs_id = $diff_vs->getID();
+ $vs_id = $diff_vs->getID();
$vs_changesets_path_map = array();
foreach (idx($changeset_groups, $vs_id, array()) as $changeset) {
$path = $changeset->getAbsoluteRepositoryPath($repository, $diff_vs);
$vs_changesets_path_map[$path] = $changeset;
$vs_changesets[$changeset->getID()] = $changeset;
}
+
foreach ($changesets as $key => $changeset) {
$path = $changeset->getAbsoluteRepositoryPath($repository, $target);
if (isset($vs_changesets_path_map[$path])) {
@@ -841,15 +879,20 @@
$refs[$changeset->getID()] =
$changeset->getID().'/'.$vs_changesets_path_map[$path]->getID();
unset($vs_changesets_path_map[$path]);
+
+ $must_compare[] = $changeset->getID();
+
} else {
$refs[$changeset->getID()] = $changeset->getID();
}
}
+
foreach ($vs_changesets_path_map as $path => $changeset) {
$changesets[$changeset->getID()] = $changeset;
- $vs_map[$changeset->getID()] = -1;
- $refs[$changeset->getID()] = $changeset->getID().'/-1';
+ $vs_map[$changeset->getID()] = -1;
+ $refs[$changeset->getID()] = $changeset->getID().'/-1';
}
+
} else {
foreach ($changesets as $changeset) {
$refs[$changeset->getID()] = $changeset->getID();
@@ -858,13 +901,25 @@
$changesets = msort($changesets, 'getSortKey');
+ // See T13137. When displaying the diff between two updates, hide any
+ // changesets which haven't actually changed.
+ $this->hiddenChangesets = array();
+ foreach ($must_compare as $changeset_id) {
+ $changeset = $changesets[$changeset_id];
+ $vs_changeset = $vs_changesets[$vs_map[$changeset_id]];
+
+ if ($changeset->hasSameEffectAs($vs_changeset)) {
+ $this->hiddenChangesets[$changeset_id] = $changesets[$changeset_id];
+ }
+ }
+
return array($changesets, $vs_map, $vs_changesets, $refs);
}
private function buildSymbolIndexes(
PhabricatorRepository $repository,
- array $visible_changesets) {
- assert_instances_of($visible_changesets, 'DifferentialChangeset');
+ array $unfolded_changesets) {
+ assert_instances_of($unfolded_changesets, 'DifferentialChangeset');
$engine = PhabricatorSyntaxHighlighter::newEngine();
@@ -889,7 +944,7 @@
$sources);
$indexed_langs = array_fill_keys($langs, true);
- foreach ($visible_changesets as $key => $changeset) {
+ foreach ($unfolded_changesets as $key => $changeset) {
$lang = $engine->getLanguageFromFilename($changeset->getFilename());
if (empty($indexed_langs) || isset($indexed_langs[$lang])) {
$symbol_indexes[$key] = array(
diff --git a/src/applications/differential/engine/DifferentialChangesetEngine.php b/src/applications/differential/engine/DifferentialChangesetEngine.php
--- a/src/applications/differential/engine/DifferentialChangesetEngine.php
+++ b/src/applications/differential/engine/DifferentialChangesetEngine.php
@@ -7,6 +7,7 @@
foreach ($changesets as $changeset) {
$this->detectGeneratedCode($changeset);
+ $this->computeHashes($changeset);
}
$this->detectCopiedCode($changesets);
@@ -59,6 +60,30 @@
}
+/* -( Content Hashes )----------------------------------------------------- */
+
+
+ private function computeHashes(DifferentialChangeset $changeset) {
+
+ $effect_key = DifferentialChangeset::METADATA_EFFECT_HASH;
+
+ $effect_hash = $this->newEffectHash($changeset);
+ if ($effect_hash !== null) {
+ $changeset->setChangesetMetadata($effect_key, $effect_hash);
+ }
+ }
+
+ private function newEffectHash(DifferentialChangeset $changeset) {
+
+ if ($changeset->getHunks()) {
+ $new_data = $changeset->makeNewFile();
+ return PhabricatorHash::digestForIndex($new_data);
+ }
+
+ return null;
+ }
+
+
/* -( Copied Code )-------------------------------------------------------- */
diff --git a/src/applications/differential/management/PhabricatorDifferentialRebuildChangesetsWorkflow.php b/src/applications/differential/management/PhabricatorDifferentialRebuildChangesetsWorkflow.php
--- a/src/applications/differential/management/PhabricatorDifferentialRebuildChangesetsWorkflow.php
+++ b/src/applications/differential/management/PhabricatorDifferentialRebuildChangesetsWorkflow.php
@@ -82,6 +82,10 @@
id(new DifferentialChangesetEngine())
->rebuildChangesets($changesets);
+ foreach ($changesets as $changeset) {
+ $changeset->save();
+ }
+
echo tsprintf(
"%s\n",
pht('Done.'));
diff --git a/src/applications/differential/storage/DifferentialChangeset.php b/src/applications/differential/storage/DifferentialChangeset.php
--- a/src/applications/differential/storage/DifferentialChangeset.php
+++ b/src/applications/differential/storage/DifferentialChangeset.php
@@ -26,6 +26,7 @@
const METADATA_TRUSTED_ATTRIBUTES = 'attributes.trusted';
const METADATA_UNTRUSTED_ATTRIBUTES = 'attributes.untrusted';
+ const METADATA_EFFECT_HASH = 'hash.effect';
const ATTRIBUTE_GENERATED = 'generated';
@@ -143,6 +144,48 @@
return $ret;
}
+ /**
+ * Test if this changeset and some other changeset put the affected file in
+ * the same state.
+ *
+ * @param DifferentialChangeset Changeset to compare against.
+ * @return bool True if the two changesets have the same effect.
+ */
+ public function hasSameEffectAs(DifferentialChangeset $other) {
+ if ($this->getFilename() !== $other->getFilename()) {
+ return false;
+ }
+
+ $hash_key = self::METADATA_EFFECT_HASH;
+
+ $u_hash = $this->getChangesetMetadata($hash_key);
+ if ($u_hash === null) {
+ return false;
+ }
+
+ $v_hash = $other->getChangesetMetadata($hash_key);
+ if ($v_hash === null) {
+ return false;
+ }
+
+ if ($u_hash !== $v_hash) {
+ return false;
+ }
+
+ // Make sure the final states for the file properties (like the "+x"
+ // executable bit) match one another.
+ $u_props = $this->getNewProperties();
+ $v_props = $other->getNewProperties();
+ ksort($u_props);
+ ksort($v_props);
+
+ if ($u_props !== $v_props) {
+ return false;
+ }
+
+ return true;
+ }
+
public function getSortKey() {
$sort_key = $this->getFilename();
// Sort files with ".h" in them first, so headers (.h, .hpp) come before
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Mar 28, 6:31 AM (1 w, 8 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7725003
Default Alt Text
D19453.id46530.diff (11 KB)
Attached To
Mode
D19453: When showing a diff-of-diffs, hide files which didn't get any more changes and have no inlines
Attached
Detach File
Event Timeline
Log In to Comment