Skip to content

Commit 1edb875

Browse files
FBNealepriestley
FBNeal
authored and
epriestley
committed
Adding support for 'adds' and 'removes' in diff content.
Summary: Does what it says on the label. We already had 'Any changed file content', now we have 'Any added file content' and 'Any removed file content'. - There is a bit of copied/pasted code here: I'm open to suggestions on how to refactor it so it's less redundant. - The wording seems a little awkward, and as @epriestley mentioned in T3829, moved code will be detected less than ideally. Test Plan: Created Herald Rules, verified via dry run that they were triggered in appropriate situations. Reviewers: epriestley Reviewed By: epriestley CC: Korvin, aran Maniphest Tasks: T3829 Differential Revision: https://secure.phabricator.com/D7214
1 parent a6c4117 commit 1edb875

File tree

5 files changed

+105
-9
lines changed

5 files changed

+105
-9
lines changed

‎src/applications/differential/storage/DifferentialHunk.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ public function getAddedLines() {
1313
return $this->makeContent($include = '+');
1414
}
1515

16+
public function getRemovedLines() {
17+
return $this->makeContent($include = '-');
18+
}
19+
1620
public function makeNewFile() {
1721
return implode('', $this->makeContent($include = ' +'));
1822
}

‎src/applications/herald/adapter/HeraldAdapter.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ abstract class HeraldAdapter {
1515
const FIELD_TAGS = 'tags';
1616
const FIELD_DIFF_FILE = 'diff-file';
1717
const FIELD_DIFF_CONTENT = 'diff-content';
18+
const FIELD_DIFF_ADDED_CONTENT = 'diff-added-content';
19+
const FIELD_DIFF_REMOVED_CONTENT = 'diff-removed-content';
1820
const FIELD_REPOSITORY = 'repository';
1921
const FIELD_RULE = 'rule';
2022
const FIELD_AFFECTED_PACKAGE = 'affected-package';
@@ -127,6 +129,8 @@ public function getFieldNameMap() {
127129
self::FIELD_TAGS => pht('Tags'),
128130
self::FIELD_DIFF_FILE => pht('Any changed filename'),
129131
self::FIELD_DIFF_CONTENT => pht('Any changed file content'),
132+
self::FIELD_DIFF_ADDED_CONTENT => pht('Any added file content'),
133+
self::FIELD_DIFF_REMOVED_CONTENT => pht('Any removed file content'),
130134
self::FIELD_REPOSITORY => pht('Repository'),
131135
self::FIELD_RULE => pht('Another Herald rule'),
132136
self::FIELD_AFFECTED_PACKAGE => pht('Any affected package'),
@@ -197,6 +201,8 @@ public function getConditionsForField($field) {
197201
self::CONDITION_REGEXP,
198202
);
199203
case self::FIELD_DIFF_CONTENT:
204+
case self::FIELD_DIFF_ADDED_CONTENT:
205+
case self::FIELD_DIFF_REMOVED_CONTENT:
200206
return array(
201207
self::CONDITION_CONTAINS,
202208
self::CONDITION_REGEXP,

‎src/applications/herald/adapter/HeraldCommitAdapter.php

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ public function getFields() {
6161
self::FIELD_REPOSITORY,
6262
self::FIELD_DIFF_FILE,
6363
self::FIELD_DIFF_CONTENT,
64+
self::FIELD_DIFF_ADDED_CONTENT,
65+
self::FIELD_DIFF_REMOVED_CONTENT,
6466
self::FIELD_RULE,
6567
self::FIELD_AFFECTED_PACKAGE,
6668
self::FIELD_AFFECTED_PACKAGE_OWNER,
@@ -255,6 +257,17 @@ private function loadCommitDiff() {
255257
return $diff;
256258
}
257259

260+
private function loadChangesets() {
261+
try {
262+
$diff = $this->loadCommitDiff();
263+
} catch (Exception $ex) {
264+
return array(
265+
'<<< Failed to load diff, this may mean the change was '.
266+
'unimaginably enormous. >>>');
267+
}
268+
return $diff->getChangesets();
269+
}
270+
258271
public function getHeraldField($field) {
259272
$data = $this->commitData;
260273
switch ($field) {
@@ -271,16 +284,9 @@ public function getHeraldField($field) {
271284
case self::FIELD_REPOSITORY:
272285
return $this->repository->getPHID();
273286
case self::FIELD_DIFF_CONTENT:
274-
try {
275-
$diff = $this->loadCommitDiff();
276-
} catch (Exception $ex) {
277-
return array(
278-
'<<< Failed to load diff, this may mean the change was '.
279-
'unimaginably enormous. >>>');
280-
}
281287
$dict = array();
282288
$lines = array();
283-
$changes = $diff->getChangesets();
289+
$changes = $this->loadChangesets();
284290
foreach ($changes as $change) {
285291
$lines = array();
286292
foreach ($change->getHunks() as $hunk) {
@@ -289,6 +295,30 @@ public function getHeraldField($field) {
289295
$dict[$change->getFilename()] = implode("\n", $lines);
290296
}
291297
return $dict;
298+
case self::FIELD_DIFF_ADDED_CONTENT:
299+
$dict = array();
300+
$lines = array();
301+
$changes = $this->loadChangesets();
302+
foreach ($changes as $change) {
303+
$lines = array();
304+
foreach ($change->getHunks() as $hunk) {
305+
$lines[] = implode('', $hunk->getAddedLines());
306+
}
307+
$dict[$change->getFilename()] = implode("\n", $lines);
308+
}
309+
return $dict;
310+
case self::FIELD_DIFF_REMOVED_CONTENT:
311+
$dict = array();
312+
$lines = array();
313+
$changes = $this->loadChangesets();
314+
foreach ($changes as $change) {
315+
$lines = array();
316+
foreach ($change->getHunks() as $hunk) {
317+
$lines[] = implode('', $hunk->getRemovedLines());
318+
}
319+
$dict[$change->getFilename()] = implode("\n", $lines);
320+
}
321+
return $dict;
292322
case self::FIELD_AFFECTED_PACKAGE:
293323
$packages = $this->loadAffectedPackages();
294324
return mpull($packages, 'getPHID');

‎src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public function getFields() {
4444
self::FIELD_REPOSITORY,
4545
self::FIELD_DIFF_FILE,
4646
self::FIELD_DIFF_CONTENT,
47+
self::FIELD_DIFF_ADDED_CONTENT,
48+
self::FIELD_DIFF_REMOVED_CONTENT,
4749
self::FIELD_RULE,
4850
self::FIELD_AFFECTED_PACKAGE,
4951
self::FIELD_AFFECTED_PACKAGE_OWNER,
@@ -194,6 +196,56 @@ protected function loadContentDictionary() {
194196
return $dict;
195197
}
196198

199+
protected function loadAddedContentDictionary() {
200+
$changesets = $this->loadChangesets();
201+
202+
$hunks = array();
203+
if ($changesets) {
204+
$hunks = id(new DifferentialHunk())->loadAllWhere(
205+
'changesetID in (%Ld)',
206+
mpull($changesets, 'getID'));
207+
}
208+
209+
$dict = array();
210+
$hunks = mgroup($hunks, 'getChangesetID');
211+
$changesets = mpull($changesets, null, 'getID');
212+
foreach ($changesets as $id => $changeset) {
213+
$path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
214+
$content = array();
215+
foreach (idx($hunks, $id, array()) as $hunk) {
216+
$content[] = implode('', $hunk->getAddedLines());
217+
}
218+
$dict[$path] = implode("\n", $content);
219+
}
220+
221+
return $dict;
222+
}
223+
224+
protected function loadRemovedContentDictionary() {
225+
$changesets = $this->loadChangesets();
226+
227+
$hunks = array();
228+
if ($changesets) {
229+
$hunks = id(new DifferentialHunk())->loadAllWhere(
230+
'changesetID in (%Ld)',
231+
mpull($changesets, 'getID'));
232+
}
233+
234+
$dict = array();
235+
$hunks = mgroup($hunks, 'getChangesetID');
236+
$changesets = mpull($changesets, null, 'getID');
237+
foreach ($changesets as $id => $changeset) {
238+
$path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
239+
$content = array();
240+
foreach (idx($hunks, $id, array()) as $hunk) {
241+
$content[] = implode('', $hunk->getRemovedLines());
242+
}
243+
$dict[$path] = implode("\n", $content);
244+
}
245+
246+
return $dict;
247+
}
248+
197249
public function loadAffectedPackages() {
198250
if ($this->affectedPackages === null) {
199251
$this->affectedPackages = array();
@@ -243,6 +295,10 @@ public function getHeraldField($field) {
243295
return $repository->getPHID();
244296
case self::FIELD_DIFF_CONTENT:
245297
return $this->loadContentDictionary();
298+
case self::FIELD_DIFF_ADDED_CONTENT:
299+
return $this->loadAddedContentDictionary();
300+
case self::FIELD_DIFF_REMOVED_CONTENT:
301+
return $this->loadRemovedContentDictionary();
246302
case self::FIELD_AFFECTED_PACKAGE:
247303
$packages = $this->loadAffectedPackages();
248304
return mpull($packages, 'getPHID');

‎src/applications/herald/storage/HeraldRule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ final class HeraldRule extends HeraldDAO
1313
protected $repetitionPolicy;
1414
protected $ruleType;
1515

16-
protected $configVersion = 11;
16+
protected $configVersion = 12;
1717

1818
private $ruleApplied = self::ATTACHABLE; // phids for which this rule has been applied
1919
private $validAuthor = self::ATTACHABLE;

0 commit comments

Comments
 (0)