From e08b4cbb2c830cfb1b981cf02147c8255e74ce93 Mon Sep 17 00:00:00 2001 From: vrana Date: Sun, 29 Apr 2012 21:35:43 -0700 Subject: [PATCH] Inform about moved code and prefer it over copied code Summary: Also reduce the memory usage a little bit (before increasing it again). I use the same CSS class as for the copied code. Test Plan: Parsed 100 diffs and checked about 10 of them - looks good. Reviewers: epriestley Reviewed By: epriestley CC: aran, Koolvin Differential Revision: https://secure.phabricator.com/D2339 --- scripts/differential/detect_copied_code.php | 48 +++++++++++++++++ src/__celerity_resource_map__.php | 32 ++++++------ .../changeset/DifferentialChangesetParser.php | 11 ++-- .../storage/diff/DifferentialDiff.php | 51 +++++++++++-------- .../differential/changeset-view.css | 4 ++ 5 files changed, 104 insertions(+), 42 deletions(-) create mode 100755 scripts/differential/detect_copied_code.php diff --git a/scripts/differential/detect_copied_code.php b/scripts/differential/detect_copied_code.php new file mode 100755 index 0000000000..2c62d061a5 --- /dev/null +++ b/scripts/differential/detect_copied_code.php @@ -0,0 +1,48 @@ +#!/usr/bin/env php + \n"; + exit(1); +} +list(, $from, $to) = $argv; + +for ($diff_id = $from; $diff_id <= $to; $diff_id++) { + echo "Processing $diff_id"; + $diff = id(new DifferentialDiff())->load($diff_id); + $diff->attachChangesets($diff->loadChangesets()); + $orig_copy = array(); + foreach ($diff->getChangesets() as $i => $changeset) { + $orig_copy[$i] = idx((array)$changeset->getMetadata(), 'copy:lines'); + $changeset->attachHunks($changeset->loadHunks()); + } + $diff->detectCopiedCode(); + foreach ($diff->getChangesets() as $i => $changeset) { + if (idx($changeset->getMetadata(), 'copy:lines') || $orig_copy[$i]) { + echo "."; + $changeset->save(); + } + } + echo "\n"; +} + +echo "Done.\n"; diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index a1e4230482..342129d7b0 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -531,7 +531,7 @@ celerity_register_resource_map(array( ), 'differential-changeset-view-css' => array( - 'uri' => '/res/5c7db62d/rsrc/css/application/differential/changeset-view.css', + 'uri' => '/res/c7edd492/rsrc/css/application/differential/changeset-view.css', 'type' => 'css', 'requires' => array( @@ -2491,7 +2491,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/0c96375e/core.pkg.js', 'type' => 'js', ), - '1715d060' => + 'acdf073e' => array( 'name' => 'differential.pkg.css', 'symbols' => @@ -2510,7 +2510,7 @@ celerity_register_resource_map(array( 11 => 'differential-local-commits-view-css', 12 => 'inline-comment-summary-css', ), - 'uri' => '/res/pkg/1715d060/differential.pkg.css', + 'uri' => '/res/pkg/acdf073e/differential.pkg.css', 'type' => 'css', ), 70509835 => @@ -2633,7 +2633,7 @@ celerity_register_resource_map(array( 'aphront-dialog-view-css' => '9c4e265b', 'aphront-error-view-css' => '9c4e265b', 'aphront-form-view-css' => '9c4e265b', - 'aphront-headsup-action-list-view-css' => '1715d060', + 'aphront-headsup-action-list-view-css' => 'acdf073e', 'aphront-headsup-view-css' => '9c4e265b', 'aphront-list-filter-view-css' => '9c4e265b', 'aphront-pager-view-css' => '9c4e265b', @@ -2643,19 +2643,19 @@ celerity_register_resource_map(array( 'aphront-tokenizer-control-css' => '9c4e265b', 'aphront-tooltip-css' => '9c4e265b', 'aphront-typeahead-control-css' => '9c4e265b', - 'differential-changeset-view-css' => '1715d060', - 'differential-core-view-css' => '1715d060', + 'differential-changeset-view-css' => 'acdf073e', + 'differential-core-view-css' => 'acdf073e', 'differential-inline-comment-editor' => '70509835', - 'differential-local-commits-view-css' => '1715d060', - 'differential-revision-add-comment-css' => '1715d060', - 'differential-revision-comment-css' => '1715d060', - 'differential-revision-comment-list-css' => '1715d060', - 'differential-revision-detail-css' => '1715d060', - 'differential-revision-history-css' => '1715d060', - 'differential-table-of-contents-css' => '1715d060', + 'differential-local-commits-view-css' => 'acdf073e', + 'differential-revision-add-comment-css' => 'acdf073e', + 'differential-revision-comment-css' => 'acdf073e', + 'differential-revision-comment-list-css' => 'acdf073e', + 'differential-revision-detail-css' => 'acdf073e', + 'differential-revision-history-css' => 'acdf073e', + 'differential-table-of-contents-css' => 'acdf073e', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'inline-comment-summary-css' => '1715d060', + 'inline-comment-summary-css' => 'acdf073e', 'javelin-behavior' => '8a5de8a3', 'javelin-behavior-aphront-basic-tokenizer' => '97f65640', 'javelin-behavior-aphront-drag-and-drop' => '70509835', @@ -2709,7 +2709,7 @@ celerity_register_resource_map(array( 'maniphest-task-summary-css' => '7839ae2d', 'maniphest-transaction-detail-css' => '7839ae2d', 'phabricator-app-buttons-css' => '9c4e265b', - 'phabricator-content-source-view-css' => '1715d060', + 'phabricator-content-source-view-css' => 'acdf073e', 'phabricator-core-buttons-css' => '9c4e265b', 'phabricator-core-css' => '9c4e265b', 'phabricator-directory-css' => '9c4e265b', @@ -2720,7 +2720,7 @@ celerity_register_resource_map(array( 'phabricator-keyboard-shortcut' => '0c96375e', 'phabricator-keyboard-shortcut-manager' => '0c96375e', 'phabricator-menu-item' => '0c96375e', - 'phabricator-object-selector-css' => '1715d060', + 'phabricator-object-selector-css' => 'acdf073e', 'phabricator-paste-file-upload' => '0c96375e', 'phabricator-prefab' => '0c96375e', 'phabricator-project-tag-css' => '7839ae2d', diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index e46fb6f049..f6db216d89 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -1410,18 +1410,19 @@ final class DifferentialChangesetParser { $n_text = $this->new[$ii]['text']; $n_attr = ' class="comment"'; } else if (isset($copy_lines[$n_num])) { - list($orig_file, $orig_line) = $copy_lines[$n_num]; + list($orig_file, $orig_line, $orig_type) = $copy_lines[$n_num]; + $title = ($orig_type == '-' ? 'Moved' : 'Copied').' from '; if ($orig_file == '') { - $title = "line {$orig_line}"; + $title .= "line {$orig_line}"; } else { - $title = + $title .= basename($orig_file). ":{$orig_line} in dir ". dirname('/'.$orig_file); } + $class = ($orig_type == '-' ? 'new-move' : 'new-copy'); $n_attr = - ' class="new new-copy"'. - ' title="Copied from '.phutil_escape_html($title).'"'; + ' class="'.$class.'" title="'.phutil_escape_html($title).'"'; } else if (empty($this->old[$ii])) { $n_attr = ' class="new new-full"'; } else { diff --git a/src/applications/differential/storage/diff/DifferentialDiff.php b/src/applications/differential/storage/diff/DifferentialDiff.php index 4ccac47a0a..d23701ed35 100644 --- a/src/applications/differential/storage/diff/DifferentialDiff.php +++ b/src/applications/differential/storage/diff/DifferentialDiff.php @@ -156,19 +156,25 @@ final class DifferentialDiff extends DifferentialDAO { return $diff; } - private function detectCopiedCode($min_width = 40, $min_lines = 3) { + public function detectCopiedCode($min_width = 40, $min_lines = 3) { $map = array(); $files = array(); + $types = array(); foreach ($this->changesets as $changeset) { $file = $changeset->getFilename(); foreach ($changeset->getHunks() as $hunk) { $line = $hunk->getOldOffset(); - foreach (explode("\n", $hunk->makeOldFile()) as $code) { - $files[$file][$line] = $code; - if (strlen($code) >= $min_width) { - $map[$code][] = array($file, $line); + foreach (explode("\n", $hunk->getChanges()) as $code) { + $type = (isset($code[0]) ? $code[0] : ''); + if ($type == '-' || $type == ' ') { + $code = (string)substr($code, 1); + $files[$file][$line] = $code; + $types[$file][$line] = $type; + if (strlen($code) >= $min_width) { + $map[$code][] = array($file, $line); + } + $line++; } - $line++; } } } @@ -179,11 +185,10 @@ final class DifferentialDiff extends DifferentialDAO { $added = $hunk->getAddedLines(); for (reset($added); list($line, $code) = each($added); next($added)) { if (isset($map[$code])) { // We found a long matching line. - $lengths = array(); - $max_offsets = array(); + $best_length = 0; foreach ($map[$code] as $val) { // Explore all candidates. list($file, $orig_line) = $val; - $lengths["$orig_line:$file"] = 1; + $length = 1; // Search also backwards for short lines. foreach (array(-1, 1) as $direction) { $offset = $direction; @@ -191,24 +196,28 @@ final class DifferentialDiff extends DifferentialDAO { isset($added[$line + $offset]) && idx($files[$file], $orig_line + $offset) === $added[$line + $offset]) { - $lengths["$orig_line:$file"]++; + $length++; $offset += $direction; } } - // ($offset - 1) contains number of forward matching lines. - $max_offsets["$orig_line:$file"] = $offset - 1; + if ($length > $best_length || + ($length == $best_length && // Prefer moves. + idx($types[$file], $orig_line) == '-')) { + $best_length = $length; + // ($offset - 1) contains number of forward matching lines. + $best_offset = $offset - 1; + $best_file = $file; + $best_line = $orig_line; + } } - $length = max($lengths); // Choose longest candidate. - $val = array_search($length, $lengths); - $offset = $max_offsets[$val]; - list($orig_line, $file) = explode(':', $val, 2); - $save_file = ($file == $changeset->getFilename() ? '' : $file); - for ($i = $length; $i--; ) { - $copies[$line + $offset - $i] = ($length < $min_lines + $file = ($best_file == $changeset->getFilename() ? '' : $best_file); + for ($i = $best_length; $i--; ) { + $type = idx($types[$best_file], $best_line + $best_offset - $i); + $copies[$line + $best_offset - $i] = ($best_length < $min_lines ? array() // Ignore short blocks. - : array($save_file, $orig_line + $offset - $i)); + : array($file, $best_line + $best_offset - $i, $type)); } - for ($i = 0; $i < $offset; $i++) { + for ($i = 0; $i < $best_offset; $i++) { next($added); } } diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index d20c98ab56..209ca0daf1 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -84,6 +84,10 @@ background: #ffffaa; } +.differential-diff td.new-move { + background: #ffddaa; +} + .differential-diff td.new-full, .differential-diff td.new span.bright { background: #aaffaa;