Skip to content

Commit e08b4cb

Browse files
author
vrana
committed
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
1 parent 087cc08 commit e08b4cb

File tree

5 files changed

+104
-42
lines changed

5 files changed

+104
-42
lines changed
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
/*
5+
* Copyright 2012 Facebook, Inc.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
$root = dirname(dirname(dirname(__FILE__)));
21+
require_once $root.'/scripts/__init_script__.php';
22+
23+
if ($argc != 3 || !is_numeric($argv[1]) || !is_numeric($argv[2])) {
24+
echo "Usage: {$argv[0]} <diff_id_from> <diff_id_to>\n";
25+
exit(1);
26+
}
27+
list(, $from, $to) = $argv;
28+
29+
for ($diff_id = $from; $diff_id <= $to; $diff_id++) {
30+
echo "Processing $diff_id";
31+
$diff = id(new DifferentialDiff())->load($diff_id);
32+
$diff->attachChangesets($diff->loadChangesets());
33+
$orig_copy = array();
34+
foreach ($diff->getChangesets() as $i => $changeset) {
35+
$orig_copy[$i] = idx((array)$changeset->getMetadata(), 'copy:lines');
36+
$changeset->attachHunks($changeset->loadHunks());
37+
}
38+
$diff->detectCopiedCode();
39+
foreach ($diff->getChangesets() as $i => $changeset) {
40+
if (idx($changeset->getMetadata(), 'copy:lines') || $orig_copy[$i]) {
41+
echo ".";
42+
$changeset->save();
43+
}
44+
}
45+
echo "\n";
46+
}
47+
48+
echo "Done.\n";

src/__celerity_resource_map__.php

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@
531531
),
532532
'differential-changeset-view-css' =>
533533
array(
534-
'uri' => '/res/5c7db62d/rsrc/css/application/differential/changeset-view.css',
534+
'uri' => '/res/c7edd492/rsrc/css/application/differential/changeset-view.css',
535535
'type' => 'css',
536536
'requires' =>
537537
array(
@@ -2491,7 +2491,7 @@
24912491
'uri' => '/res/pkg/0c96375e/core.pkg.js',
24922492
'type' => 'js',
24932493
),
2494-
'1715d060' =>
2494+
'acdf073e' =>
24952495
array(
24962496
'name' => 'differential.pkg.css',
24972497
'symbols' =>
@@ -2510,7 +2510,7 @@
25102510
11 => 'differential-local-commits-view-css',
25112511
12 => 'inline-comment-summary-css',
25122512
),
2513-
'uri' => '/res/pkg/1715d060/differential.pkg.css',
2513+
'uri' => '/res/pkg/acdf073e/differential.pkg.css',
25142514
'type' => 'css',
25152515
),
25162516
70509835 =>
@@ -2633,7 +2633,7 @@
26332633
'aphront-dialog-view-css' => '9c4e265b',
26342634
'aphront-error-view-css' => '9c4e265b',
26352635
'aphront-form-view-css' => '9c4e265b',
2636-
'aphront-headsup-action-list-view-css' => '1715d060',
2636+
'aphront-headsup-action-list-view-css' => 'acdf073e',
26372637
'aphront-headsup-view-css' => '9c4e265b',
26382638
'aphront-list-filter-view-css' => '9c4e265b',
26392639
'aphront-pager-view-css' => '9c4e265b',
@@ -2643,19 +2643,19 @@
26432643
'aphront-tokenizer-control-css' => '9c4e265b',
26442644
'aphront-tooltip-css' => '9c4e265b',
26452645
'aphront-typeahead-control-css' => '9c4e265b',
2646-
'differential-changeset-view-css' => '1715d060',
2647-
'differential-core-view-css' => '1715d060',
2646+
'differential-changeset-view-css' => 'acdf073e',
2647+
'differential-core-view-css' => 'acdf073e',
26482648
'differential-inline-comment-editor' => '70509835',
2649-
'differential-local-commits-view-css' => '1715d060',
2650-
'differential-revision-add-comment-css' => '1715d060',
2651-
'differential-revision-comment-css' => '1715d060',
2652-
'differential-revision-comment-list-css' => '1715d060',
2653-
'differential-revision-detail-css' => '1715d060',
2654-
'differential-revision-history-css' => '1715d060',
2655-
'differential-table-of-contents-css' => '1715d060',
2649+
'differential-local-commits-view-css' => 'acdf073e',
2650+
'differential-revision-add-comment-css' => 'acdf073e',
2651+
'differential-revision-comment-css' => 'acdf073e',
2652+
'differential-revision-comment-list-css' => 'acdf073e',
2653+
'differential-revision-detail-css' => 'acdf073e',
2654+
'differential-revision-history-css' => 'acdf073e',
2655+
'differential-table-of-contents-css' => 'acdf073e',
26562656
'diffusion-commit-view-css' => 'c8ce2d88',
26572657
'diffusion-icons-css' => 'c8ce2d88',
2658-
'inline-comment-summary-css' => '1715d060',
2658+
'inline-comment-summary-css' => 'acdf073e',
26592659
'javelin-behavior' => '8a5de8a3',
26602660
'javelin-behavior-aphront-basic-tokenizer' => '97f65640',
26612661
'javelin-behavior-aphront-drag-and-drop' => '70509835',
@@ -2709,7 +2709,7 @@
27092709
'maniphest-task-summary-css' => '7839ae2d',
27102710
'maniphest-transaction-detail-css' => '7839ae2d',
27112711
'phabricator-app-buttons-css' => '9c4e265b',
2712-
'phabricator-content-source-view-css' => '1715d060',
2712+
'phabricator-content-source-view-css' => 'acdf073e',
27132713
'phabricator-core-buttons-css' => '9c4e265b',
27142714
'phabricator-core-css' => '9c4e265b',
27152715
'phabricator-directory-css' => '9c4e265b',
@@ -2720,7 +2720,7 @@
27202720
'phabricator-keyboard-shortcut' => '0c96375e',
27212721
'phabricator-keyboard-shortcut-manager' => '0c96375e',
27222722
'phabricator-menu-item' => '0c96375e',
2723-
'phabricator-object-selector-css' => '1715d060',
2723+
'phabricator-object-selector-css' => 'acdf073e',
27242724
'phabricator-paste-file-upload' => '0c96375e',
27252725
'phabricator-prefab' => '0c96375e',
27262726
'phabricator-project-tag-css' => '7839ae2d',

src/applications/differential/parser/changeset/DifferentialChangesetParser.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,18 +1410,19 @@ protected function renderTextChange(
14101410
$n_text = $this->new[$ii]['text'];
14111411
$n_attr = ' class="comment"';
14121412
} else if (isset($copy_lines[$n_num])) {
1413-
list($orig_file, $orig_line) = $copy_lines[$n_num];
1413+
list($orig_file, $orig_line, $orig_type) = $copy_lines[$n_num];
1414+
$title = ($orig_type == '-' ? 'Moved' : 'Copied').' from ';
14141415
if ($orig_file == '') {
1415-
$title = "line {$orig_line}";
1416+
$title .= "line {$orig_line}";
14161417
} else {
1417-
$title =
1418+
$title .=
14181419
basename($orig_file).
14191420
":{$orig_line} in dir ".
14201421
dirname('/'.$orig_file);
14211422
}
1423+
$class = ($orig_type == '-' ? 'new-move' : 'new-copy');
14221424
$n_attr =
1423-
' class="new new-copy"'.
1424-
' title="Copied from '.phutil_escape_html($title).'"';
1425+
' class="'.$class.'" title="'.phutil_escape_html($title).'"';
14251426
} else if (empty($this->old[$ii])) {
14261427
$n_attr = ' class="new new-full"';
14271428
} else {

src/applications/differential/storage/diff/DifferentialDiff.php

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -156,19 +156,25 @@ public static function newFromRawChanges(array $changes) {
156156
return $diff;
157157
}
158158

159-
private function detectCopiedCode($min_width = 40, $min_lines = 3) {
159+
public function detectCopiedCode($min_width = 40, $min_lines = 3) {
160160
$map = array();
161161
$files = array();
162+
$types = array();
162163
foreach ($this->changesets as $changeset) {
163164
$file = $changeset->getFilename();
164165
foreach ($changeset->getHunks() as $hunk) {
165166
$line = $hunk->getOldOffset();
166-
foreach (explode("\n", $hunk->makeOldFile()) as $code) {
167-
$files[$file][$line] = $code;
168-
if (strlen($code) >= $min_width) {
169-
$map[$code][] = array($file, $line);
167+
foreach (explode("\n", $hunk->getChanges()) as $code) {
168+
$type = (isset($code[0]) ? $code[0] : '');
169+
if ($type == '-' || $type == ' ') {
170+
$code = (string)substr($code, 1);
171+
$files[$file][$line] = $code;
172+
$types[$file][$line] = $type;
173+
if (strlen($code) >= $min_width) {
174+
$map[$code][] = array($file, $line);
175+
}
176+
$line++;
170177
}
171-
$line++;
172178
}
173179
}
174180
}
@@ -179,36 +185,39 @@ private function detectCopiedCode($min_width = 40, $min_lines = 3) {
179185
$added = $hunk->getAddedLines();
180186
for (reset($added); list($line, $code) = each($added); next($added)) {
181187
if (isset($map[$code])) { // We found a long matching line.
182-
$lengths = array();
183-
$max_offsets = array();
188+
$best_length = 0;
184189
foreach ($map[$code] as $val) { // Explore all candidates.
185190
list($file, $orig_line) = $val;
186-
$lengths["$orig_line:$file"] = 1;
191+
$length = 1;
187192
// Search also backwards for short lines.
188193
foreach (array(-1, 1) as $direction) {
189194
$offset = $direction;
190195
while (!isset($copies[$line + $offset]) &&
191196
isset($added[$line + $offset]) &&
192197
idx($files[$file], $orig_line + $offset) ===
193198
$added[$line + $offset]) {
194-
$lengths["$orig_line:$file"]++;
199+
$length++;
195200
$offset += $direction;
196201
}
197202
}
198-
// ($offset - 1) contains number of forward matching lines.
199-
$max_offsets["$orig_line:$file"] = $offset - 1;
203+
if ($length > $best_length ||
204+
($length == $best_length && // Prefer moves.
205+
idx($types[$file], $orig_line) == '-')) {
206+
$best_length = $length;
207+
// ($offset - 1) contains number of forward matching lines.
208+
$best_offset = $offset - 1;
209+
$best_file = $file;
210+
$best_line = $orig_line;
211+
}
200212
}
201-
$length = max($lengths); // Choose longest candidate.
202-
$val = array_search($length, $lengths);
203-
$offset = $max_offsets[$val];
204-
list($orig_line, $file) = explode(':', $val, 2);
205-
$save_file = ($file == $changeset->getFilename() ? '' : $file);
206-
for ($i = $length; $i--; ) {
207-
$copies[$line + $offset - $i] = ($length < $min_lines
213+
$file = ($best_file == $changeset->getFilename() ? '' : $best_file);
214+
for ($i = $best_length; $i--; ) {
215+
$type = idx($types[$best_file], $best_line + $best_offset - $i);
216+
$copies[$line + $best_offset - $i] = ($best_length < $min_lines
208217
? array() // Ignore short blocks.
209-
: array($save_file, $orig_line + $offset - $i));
218+
: array($file, $best_line + $best_offset - $i, $type));
210219
}
211-
for ($i = 0; $i < $offset; $i++) {
220+
for ($i = 0; $i < $best_offset; $i++) {
212221
next($added);
213222
}
214223
}

webroot/rsrc/css/application/differential/changeset-view.css

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@
8484
background: #ffffaa;
8585
}
8686

87+
.differential-diff td.new-move {
88+
background: #ffddaa;
89+
}
90+
8791
.differential-diff td.new-full,
8892
.differential-diff td.new span.bright {
8993
background: #aaffaa;

0 commit comments

Comments
 (0)