Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15405154
D20181.id48212.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
16 KB
Referenced Files
None
Subscribers
None
D20181.id48212.diff
View Options
diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -11,7 +11,7 @@
'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => '261ee8cf',
'core.pkg.js' => '5ace8a1e',
- 'differential.pkg.css' => 'b8df73d4',
+ 'differential.pkg.css' => 'c3f15714',
'differential.pkg.js' => '67c9ea4c',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => '91192d85',
@@ -61,7 +61,7 @@
'rsrc/css/application/dashboard/dashboard.css' => '4267d6c6',
'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d',
'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
- 'rsrc/css/application/differential/changeset-view.css' => '73660575',
+ 'rsrc/css/application/differential/changeset-view.css' => '783a9206',
'rsrc/css/application/differential/core.css' => 'bdb93065',
'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b',
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
@@ -275,6 +275,8 @@
'rsrc/image/checker_dark.png' => '7fc8fa7b',
'rsrc/image/checker_light.png' => '3157a202',
'rsrc/image/checker_lighter.png' => 'c45928c1',
+ 'rsrc/image/chevron-in.png' => '1aa2f88f',
+ 'rsrc/image/chevron-out.png' => 'c815e272',
'rsrc/image/controls/checkbox-checked.png' => '1770d7a0',
'rsrc/image/controls/checkbox-unchecked.png' => 'e1deba0a',
'rsrc/image/d5d8e1.png' => '6764616e',
@@ -539,7 +541,7 @@
'conpherence-thread-manager' => 'aec8e38c',
'conpherence-transaction-css' => '3a3f5e7e',
'd3' => 'd67475f5',
- 'differential-changeset-view-css' => '73660575',
+ 'differential-changeset-view-css' => '783a9206',
'differential-core-view-css' => 'bdb93065',
'differential-revision-add-comment-css' => '7e5900d9',
'differential-revision-comment-css' => '7dbc8d1d',
@@ -1490,9 +1492,6 @@
'javelin-dom',
'javelin-uri',
),
- 73660575 => array(
- 'phui-inline-comment-view-css',
- ),
'73ecc1f8' => array(
'javelin-behavior',
'javelin-behavior-device',
@@ -1514,6 +1513,9 @@
'javelin-uri',
'javelin-request',
),
+ '783a9206' => array(
+ 'phui-inline-comment-view-css',
+ ),
'78bc5d94' => array(
'javelin-behavior',
'javelin-uri',
diff --git a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php
--- a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php
+++ b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php
@@ -199,8 +199,10 @@
'diff.background' => '#fff',
'new-background' => 'rgba(151, 234, 151, .3)',
'new-bright' => 'rgba(151, 234, 151, .6)',
+ 'new-background-strong' => 'rgba(151, 234, 151, 1)',
'old-background' => 'rgba(251, 175, 175, .3)',
'old-bright' => 'rgba(251, 175, 175, .7)',
+ 'old-background-strong' => 'rgba(251, 175, 175, 1)',
'move-background' => '#fdf5d4',
'copy-background' => '#f1c40f',
diff --git a/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace b/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace
--- a/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace
+++ b/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace
@@ -1,2 +1,2 @@
O 1 - -=[-Rocket-Ship>\n~
-N 1 + {( )}-=[-Rocket-Ship>\n~
+N 1 + {> )}-=[-Rocket-Ship>\n~
diff --git a/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace b/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace
--- a/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace
+++ b/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace
@@ -1,2 +1,2 @@
O 1 - -=[-Rocket-Ship>\n~
-N 1 + {( )}-=[-Rocket-Ship>\n~
+N 1 + {> )}-=[-Rocket-Ship>\n~
diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php
--- a/src/applications/differential/parser/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/DifferentialChangesetParser.php
@@ -8,6 +8,7 @@
protected $new = array();
protected $old = array();
protected $intra = array();
+ protected $depthOnlyLines = array();
protected $newRender = null;
protected $oldRender = null;
@@ -190,7 +191,7 @@
return $this;
}
- const CACHE_VERSION = 11;
+ const CACHE_VERSION = 12;
const CACHE_MAX_SIZE = 8e6;
const ATTR_GENERATED = 'attr:generated';
@@ -224,6 +225,15 @@
return $this;
}
+ public function setDepthOnlyLines(array $lines) {
+ $this->depthOnlyLines = $lines;
+ return $this;
+ }
+
+ public function getDepthOnlyLines() {
+ return $this->depthOnlyLines;
+ }
+
public function setVisibileLinesMask(array $mask) {
$this->visible = $mask;
return $this;
@@ -450,6 +460,7 @@
'new',
'old',
'intra',
+ 'depthOnlyLines',
'newRender',
'oldRender',
'specialAttributes',
@@ -754,6 +765,7 @@
$this->setOldLines($hunk_parser->getOldLines());
$this->setNewLines($hunk_parser->getNewLines());
$this->setIntraLineDiffs($hunk_parser->getIntraLineDiffs());
+ $this->setDepthOnlyLines($hunk_parser->getDepthOnlyLines());
$this->setVisibileLinesMask($hunk_parser->getVisibleLinesMask());
$this->hunkStartLines = $hunk_parser->getHunkStartLines(
$changeset->getHunks());
@@ -914,7 +926,8 @@
->setShowEditAndReplyLinks($this->getShowEditAndReplyLinks())
->setCanMarkDone($this->getCanMarkDone())
->setObjectOwnerPHID($this->getObjectOwnerPHID())
- ->setHighlightingDisabled($this->highlightingDisabled);
+ ->setHighlightingDisabled($this->highlightingDisabled)
+ ->setDepthOnlyLines($this->getDepthOnlyLines());
$shield = null;
if ($this->isTopLevel && !$this->comments) {
diff --git a/src/applications/differential/parser/DifferentialHunkParser.php b/src/applications/differential/parser/DifferentialHunkParser.php
--- a/src/applications/differential/parser/DifferentialHunkParser.php
+++ b/src/applications/differential/parser/DifferentialHunkParser.php
@@ -5,6 +5,7 @@
private $oldLines;
private $newLines;
private $intraLineDiffs;
+ private $depthOnlyLines;
private $visibleLinesMask;
private $whitespaceMode;
@@ -115,6 +116,14 @@
return $this;
}
+ public function setDepthOnlyLines(array $map) {
+ $this->depthOnlyLines = $map;
+ return $this;
+ }
+
+ public function getDepthOnlyLines() {
+ return $this->depthOnlyLines;
+ }
public function setWhitespaceMode($white_space_mode) {
$this->whitespaceMode = $white_space_mode;
@@ -334,6 +343,7 @@
$new = $this->getNewLines();
$diffs = array();
+ $depth_only = array();
foreach ($old as $key => $o) {
$n = $new[$key];
@@ -342,13 +352,75 @@
}
if ($o['type'] != $n['type']) {
- $diffs[$key] = ArcanistDiffUtils::generateIntralineDiff(
- $o['text'],
- $n['text']);
+ $o_segments = array();
+ $n_segments = array();
+ $tab_width = 2;
+
+ $o_text = $o['text'];
+ $n_text = $n['text'];
+
+ if ($o_text !== $n_text) {
+ $o_depth = $this->getIndentDepth($o_text, $tab_width);
+ $n_depth = $this->getIndentDepth($n_text, $tab_width);
+
+ if ($o_depth < $n_depth) {
+ $segment_type = '>';
+ $segment_width = $this->getCharacterCountForVisualWhitespace(
+ $n_text,
+ ($n_depth - $o_depth),
+ $tab_width);
+ if ($segment_width) {
+ $n_text = substr($n_text, $segment_width);
+ $n_segments[] = array(
+ $segment_type,
+ $segment_width,
+ );
+ }
+ } else if ($o_depth > $n_depth) {
+ $segment_type = '<';
+ $segment_width = $this->getCharacterCountForVisualWhitespace(
+ $o_text,
+ ($o_depth - $n_depth),
+ $tab_width);
+ if ($segment_width) {
+ $o_text = substr($o_text, $segment_width);
+ $o_segments[] = array(
+ $segment_type,
+ $segment_width,
+ );
+ }
+ }
+
+ // If there are no remaining changes to this line after we've marked
+ // off the indent depth changes, this line was only modified by
+ // changing the indent depth. Mark it for later so we can change how
+ // it is displayed.
+ if ($o_text === $n_text) {
+ $depth_only[$key] = $segment_type;
+ }
+ }
+
+ $intraline_segments = ArcanistDiffUtils::generateIntralineDiff(
+ $o_text,
+ $n_text);
+
+ foreach ($intraline_segments[0] as $o_segment) {
+ $o_segments[] = $o_segment;
+ }
+
+ foreach ($intraline_segments[1] as $n_segment) {
+ $n_segments[] = $n_segment;
+ }
+
+ $diffs[$key] = array(
+ $o_segments,
+ $n_segments,
+ );
}
}
$this->setIntraLineDiffs($diffs);
+ $this->setDepthOnlyLines($depth_only);
return $this;
}
@@ -671,4 +743,97 @@
return $offsets;
}
+
+ private function getIndentDepth($text, $tab_width) {
+ $len = strlen($text);
+
+ $depth = 0;
+ for ($ii = 0; $ii < $len; $ii++) {
+ $c = $text[$ii];
+
+ // If this is a space, increase the indent depth by 1.
+ if ($c == ' ') {
+ $depth++;
+ continue;
+ }
+
+ // If this is a tab, increase the indent depth to the next tabstop.
+
+ // For example, if the tab width is 4, these sequences both lead us to
+ // a visual width of 8, i.e. the cursor will be in the 8th column:
+ //
+ // <tab><tab>
+ // <space><tab><space><space><space><tab>
+
+ if ($c == "\t") {
+ $depth = ($depth + $tab_width);
+ $depth = $depth - ($depth % $tab_width);
+ continue;
+ }
+
+ break;
+ }
+
+ return $depth;
+ }
+
+ private function getCharacterCountForVisualWhitespace(
+ $text,
+ $depth,
+ $tab_width) {
+
+ // Here, we know the visual indent depth of a line has been increased by
+ // some amount (for example, 6 characters).
+
+ // We want to find the largest whitespace prefix of the string we can
+ // which still fits into that amount of visual space.
+
+ // In most cases, this is very easy. For example, if the string has been
+ // indented by two characters and the string begins with two spaces, that's
+ // a perfect match.
+
+ // However, if the string has been indented by 7 characters, the tab width
+ // is 8, and the string begins with "<space><space><tab>", we can only
+ // mark the two spaces as an indent change. These cases are unusual.
+
+ $character_depth = 0;
+ $visual_depth = 0;
+
+ $len = strlen($text);
+ for ($ii = 0; $ii < $len; $ii++) {
+ if ($visual_depth >= $depth) {
+ break;
+ }
+
+ $c = $text[$ii];
+
+ if ($c == ' ') {
+ $character_depth++;
+ $visual_depth++;
+ continue;
+ }
+
+ if ($c == "\t") {
+ // Figure out how many visual spaces we have until the next tabstop.
+ $tab_visual = ($visual_depth + $tab_width);
+ $tab_visual = $tab_visual - ($tab_visual % $tab_width);
+ $tab_visual = ($tab_visual - $visual_depth);
+
+ // If this tab would take us over the limit, we're all done.
+ $remaining_depth = ($depth - $visual_depth);
+ if ($remaining_depth < $tab_visual) {
+ break;
+ }
+
+ $character_depth++;
+ $visual_depth += $tab_visual;
+ continue;
+ }
+
+ break;
+ }
+
+ return $character_depth;
+ }
+
}
diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetRenderer.php
@@ -34,6 +34,7 @@
private $objectOwnerPHID;
private $highlightingDisabled;
private $scopeEngine;
+ private $depthOnlyLines;
private $oldFile = false;
private $newFile = false;
@@ -92,6 +93,15 @@
return $this->gaps;
}
+ public function setDepthOnlyLines(array $lines) {
+ $this->depthOnlyLines = $lines;
+ return $this;
+ }
+
+ public function getDepthOnlyLines() {
+ return $this->depthOnlyLines;
+ }
+
public function attachOldFile(PhabricatorFile $old = null) {
$this->oldFile = $old;
return $this;
diff --git a/src/applications/differential/render/DifferentialChangesetTestRenderer.php b/src/applications/differential/render/DifferentialChangesetTestRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetTestRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetTestRenderer.php
@@ -96,10 +96,14 @@
array(
'<span class="bright">',
'</span>',
+ '<span class="depth-out">',
+ '<span class="depth-in">',
),
array(
'{(',
')}',
+ '{<',
+ '{>',
),
$render);
diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
@@ -71,8 +71,8 @@
$mask = $this->getMask();
$scope_engine = $this->getScopeEngine();
-
$offset_map = null;
+ $depth_only = $this->getDepthOnlyLines();
for ($ii = $range_start; $ii < $range_start + $range_len; $ii++) {
if (empty($mask[$ii])) {
@@ -196,11 +196,29 @@
} else if (empty($old_lines[$ii])) {
$n_class = 'new new-full';
} else {
- $n_class = 'new';
+
+ // NOTE: At least for the moment, I'm intentionally clearing the
+ // line highlighting only on the right side of the diff when a
+ // line has only depth changes. When a block depth is decreased,
+ // this gives us a large color block on the left (to make it easy
+ // to see the depth change) but a clean diff on the right (to make
+ // it easy to pick out actual code changes).
+
+ if (isset($depth_only[$ii])) {
+ $n_class = '';
+ } else {
+ $n_class = 'new';
+ }
}
$n_classes = $n_class;
- if ($new_lines[$ii]['type'] == '\\' || !isset($copy_lines[$n_num])) {
+ $not_copied =
+ // If this line only changed depth, copy markers are pointless.
+ (!isset($copy_lines[$n_num])) ||
+ (isset($depth_only[$ii])) ||
+ ($new_lines[$ii]['type'] == '\\');
+
+ if ($not_copied) {
$n_copy = phutil_tag('td', array('class' => "copy {$n_class}"));
} else {
list($orig_file, $orig_line, $orig_type) = $copy_lines[$n_num];
diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css
--- a/webroot/rsrc/css/application/differential/changeset-view.css
+++ b/webroot/rsrc/css/application/differential/changeset-view.css
@@ -135,12 +135,33 @@
background: {$old-bright};
}
+
.differential-diff td.new span.bright,
.differential-diff td.new-full,
.prose-diff span.new {
background: {$new-bright};
}
+.differential-diff td span.depth-out,
+.differential-diff td span.depth-in {
+ padding: 2px 0;
+ background-size: 12px 12px;
+ background-repeat: no-repeat;
+ background-position: left center;
+}
+
+.differential-diff td span.depth-out {
+ background-image: url(/rsrc/image/chevron-out.png);
+ background-color: {$old-background-strong};
+}
+
+.differential-diff td span.depth-in {
+ background-position: 2px center;
+ background-image: url(/rsrc/image/chevron-in.png);
+ background-color: {$new-background-strong};
+}
+
+
.differential-diff td.copy {
min-width: 0.5%;
width: 0.5%;
diff --git a/webroot/rsrc/image/chevron-in.png b/webroot/rsrc/image/chevron-in.png
new file mode 100644
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000
GIT binary patch
literal 0
Hc$@<O00001
literal 0
Hc$@<O00001
diff --git a/webroot/rsrc/image/chevron-out.png b/webroot/rsrc/image/chevron-out.png
new file mode 100644
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000
GIT binary patch
literal 0
Hc$@<O00001
literal 0
Hc$@<O00001
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Mar 19, 10:28 AM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7388827
Default Alt Text
D20181.id48212.diff (16 KB)
Attached To
Mode
D20181: Render indent depth changes more clearly
Attached
Detach File
Event Timeline
Log In to Comment