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: + // + // + // + + 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 "", 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( '', '', + '', + '', ), 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$@