mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 14:52:40 +01:00
Make the ArcanistBundle algorithm do what "diff -u" does when hunks are arguably mergeable
Summary: Ref T13187. See PHI838. If two hunks are separated by 7 lines of context, we can render them as either: ```lang=diff + Hunk A Context 1 Context 2 Context 3 Context 4 Context 5 Context 6 Context 7 + Hunk B ``` ...or: ```lang=diff + Hunk A Context 1 Context 2 Context 3 @@ +1,2 -3,4 @@ Context 5 Context 6 Context 7 + Hunk B ``` Since we get the same number of output lines either way and the first one is more human-readable, we picked that one. However, `diff -u` does the second one. Since human-readability is probably less important than compatibility, change the behavior to be more similar to `diff -u`. Test Plan: Added unit tests for the edge cases with default parameters (6 context lines, 7 context lines) and made them pass. Reviewers: amckinley Maniphest Tasks: T13187 Differential Revision: https://secure.phabricator.com/D19603
This commit is contained in:
parent
d9a4293ae7
commit
e1e93271e6
8 changed files with 131 additions and 1 deletions
|
@ -560,7 +560,16 @@ final class ArcanistBundle extends Phobject {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($jj - $last_change > (($context + 1) * 2)) {
|
// If the context value is "3" and there are 7 unchanged lines
|
||||||
|
// between the two changes, we could either generate one or two hunks
|
||||||
|
// and end up with the same number of output lines. If we generate
|
||||||
|
// one hunk, the middle line will be a line of source. If we generate
|
||||||
|
// two hunks, the middle line will be an "@@ -1,2 +3,4 @@" header.
|
||||||
|
|
||||||
|
// We choose to generate two hunks because this is the behavior of
|
||||||
|
// "diff -u". See PHI838.
|
||||||
|
|
||||||
|
if ($jj - $last_change >= ($context * 2 + 1)) {
|
||||||
// We definitely aren't going to merge this with the next hunk, so
|
// We definitely aren't going to merge this with the next hunk, so
|
||||||
// break out of the loop. We'll end the hunk at $break_here.
|
// break out of the loop. We'll end the hunk at $break_here.
|
||||||
break;
|
break;
|
||||||
|
|
|
@ -642,6 +642,24 @@ EODIFF;
|
||||||
'disjoint-hunks.new')->toUnifiedDiff());
|
'disjoint-hunks.new')->toUnifiedDiff());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testMergeHunks() {
|
||||||
|
// Hunks should merge if represented by sufficiently few unchanged
|
||||||
|
// lines.
|
||||||
|
$this->assertEqual(
|
||||||
|
$this->loadResource('merge-hunks.diff'),
|
||||||
|
$this->loadOneChangeBundle(
|
||||||
|
'merge-hunks.old',
|
||||||
|
'merge-hunks.new')->toUnifiedDiff());
|
||||||
|
|
||||||
|
// Hunks should not merge if they are separated by too many unchanged
|
||||||
|
// lines.
|
||||||
|
$this->assertEqual(
|
||||||
|
$this->loadResource('no-merge-hunks.diff'),
|
||||||
|
$this->loadOneChangeBundle(
|
||||||
|
'no-merge-hunks.old',
|
||||||
|
'no-merge-hunks.new')->toUnifiedDiff());
|
||||||
|
}
|
||||||
|
|
||||||
public function testNonlocalTrailingNewline() {
|
public function testNonlocalTrailingNewline() {
|
||||||
// Diffs without changes near the end of the file should not generate a
|
// Diffs without changes near the end of the file should not generate a
|
||||||
// bogus, change-free hunk if the file has no trailing newline.
|
// bogus, change-free hunk if the file has no trailing newline.
|
||||||
|
|
19
src/parser/__tests__/bundle/merge-hunks.diff
Normal file
19
src/parser/__tests__/bundle/merge-hunks.diff
Normal file
|
@ -0,0 +1,19 @@
|
||||||
|
Index: file
|
||||||
|
===================================================================
|
||||||
|
--- file
|
||||||
|
+++ file
|
||||||
|
@@ -2,12 +2,14 @@
|
||||||
|
B
|
||||||
|
C
|
||||||
|
D
|
||||||
|
+Chocolate
|
||||||
|
E
|
||||||
|
F
|
||||||
|
G
|
||||||
|
H
|
||||||
|
I
|
||||||
|
J
|
||||||
|
+Caramel
|
||||||
|
K
|
||||||
|
L
|
||||||
|
M
|
17
src/parser/__tests__/bundle/merge-hunks.new
Normal file
17
src/parser/__tests__/bundle/merge-hunks.new
Normal file
|
@ -0,0 +1,17 @@
|
||||||
|
A
|
||||||
|
B
|
||||||
|
C
|
||||||
|
D
|
||||||
|
Chocolate
|
||||||
|
E
|
||||||
|
F
|
||||||
|
G
|
||||||
|
H
|
||||||
|
I
|
||||||
|
J
|
||||||
|
Caramel
|
||||||
|
K
|
||||||
|
L
|
||||||
|
M
|
||||||
|
N
|
||||||
|
O
|
15
src/parser/__tests__/bundle/merge-hunks.old
Normal file
15
src/parser/__tests__/bundle/merge-hunks.old
Normal file
|
@ -0,0 +1,15 @@
|
||||||
|
A
|
||||||
|
B
|
||||||
|
C
|
||||||
|
D
|
||||||
|
E
|
||||||
|
F
|
||||||
|
G
|
||||||
|
H
|
||||||
|
I
|
||||||
|
J
|
||||||
|
K
|
||||||
|
L
|
||||||
|
M
|
||||||
|
N
|
||||||
|
O
|
20
src/parser/__tests__/bundle/no-merge-hunks.diff
Normal file
20
src/parser/__tests__/bundle/no-merge-hunks.diff
Normal file
|
@ -0,0 +1,20 @@
|
||||||
|
Index: file
|
||||||
|
===================================================================
|
||||||
|
--- file
|
||||||
|
+++ file
|
||||||
|
@@ -2,6 +2,7 @@
|
||||||
|
B
|
||||||
|
C
|
||||||
|
D
|
||||||
|
+Chocolate
|
||||||
|
E
|
||||||
|
F
|
||||||
|
G
|
||||||
|
@@ -9,6 +10,7 @@
|
||||||
|
I
|
||||||
|
J
|
||||||
|
K
|
||||||
|
+Caramel
|
||||||
|
L
|
||||||
|
M
|
||||||
|
N
|
17
src/parser/__tests__/bundle/no-merge-hunks.new
Normal file
17
src/parser/__tests__/bundle/no-merge-hunks.new
Normal file
|
@ -0,0 +1,17 @@
|
||||||
|
A
|
||||||
|
B
|
||||||
|
C
|
||||||
|
D
|
||||||
|
Chocolate
|
||||||
|
E
|
||||||
|
F
|
||||||
|
G
|
||||||
|
H
|
||||||
|
I
|
||||||
|
J
|
||||||
|
K
|
||||||
|
Caramel
|
||||||
|
L
|
||||||
|
M
|
||||||
|
N
|
||||||
|
O
|
15
src/parser/__tests__/bundle/no-merge-hunks.old
Normal file
15
src/parser/__tests__/bundle/no-merge-hunks.old
Normal file
|
@ -0,0 +1,15 @@
|
||||||
|
A
|
||||||
|
B
|
||||||
|
C
|
||||||
|
D
|
||||||
|
E
|
||||||
|
F
|
||||||
|
G
|
||||||
|
H
|
||||||
|
I
|
||||||
|
J
|
||||||
|
K
|
||||||
|
L
|
||||||
|
M
|
||||||
|
N
|
||||||
|
O
|
Loading…
Reference in a new issue