mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 23:02:41 +01:00
Restore ANSI highlighting of lint sections to lint output
Summary: Fixes T9846. This restores the last missing feature, ANSI highlighting of diff sections. Test Plan: Added a mode so we can actually test this stuff, activated that mode, wrote unit tests. Did a bunch of actual lint locally too and looked at it, all seemed sane. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9846 Differential Revision: https://secure.phabricator.com/D18512
This commit is contained in:
parent
213ed3ff15
commit
ad8214456a
7 changed files with 71 additions and 8 deletions
|
@ -6,12 +6,22 @@
|
||||||
final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
|
final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
|
||||||
|
|
||||||
private $showAutofixPatches = false;
|
private $showAutofixPatches = false;
|
||||||
|
private $testableMode;
|
||||||
|
|
||||||
public function setShowAutofixPatches($show_autofix_patches) {
|
public function setShowAutofixPatches($show_autofix_patches) {
|
||||||
$this->showAutofixPatches = $show_autofix_patches;
|
$this->showAutofixPatches = $show_autofix_patches;
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setTestableMode($testable_mode) {
|
||||||
|
$this->testableMode = $testable_mode;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getTestableMode() {
|
||||||
|
return $this->testableMode;
|
||||||
|
}
|
||||||
|
|
||||||
public function renderLintResult(ArcanistLintResult $result) {
|
public function renderLintResult(ArcanistLintResult $result) {
|
||||||
$messages = $result->getMessages();
|
$messages = $result->getMessages();
|
||||||
$path = $result->getPath();
|
$path = $result->getPath();
|
||||||
|
@ -90,6 +100,7 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
|
||||||
|
|
||||||
$old = $data;
|
$old = $data;
|
||||||
$old_lines = phutil_split_lines($old);
|
$old_lines = phutil_split_lines($old);
|
||||||
|
$old_impact = substr_count($original, "\n") + 1;
|
||||||
$start = $line;
|
$start = $line;
|
||||||
|
|
||||||
if ($message->isPatchable()) {
|
if ($message->isPatchable()) {
|
||||||
|
@ -105,9 +116,26 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
|
||||||
// Figure out how many "-" and "+" lines we have by counting the newlines
|
// Figure out how many "-" and "+" lines we have by counting the newlines
|
||||||
// for the relevant patches. This may overestimate things if we are adding
|
// for the relevant patches. This may overestimate things if we are adding
|
||||||
// or removing entire lines, but we'll adjust things below.
|
// or removing entire lines, but we'll adjust things below.
|
||||||
$old_impact = substr_count($original, "\n") + 1;
|
|
||||||
$new_impact = substr_count($replacement, "\n") + 1;
|
$new_impact = substr_count($replacement, "\n") + 1;
|
||||||
|
|
||||||
|
|
||||||
|
// If this is a change on a single line, we'll try to highlight the
|
||||||
|
// changed character range to make it easier to pick out.
|
||||||
|
if ($old_impact === 1 && $new_impact === 1) {
|
||||||
|
$old_lines[$start - 1] = substr_replace(
|
||||||
|
$old_lines[$start - 1],
|
||||||
|
$this->highlightText($original),
|
||||||
|
$char - 1,
|
||||||
|
strlen($original));
|
||||||
|
|
||||||
|
$new_lines[$start - 1] = substr_replace(
|
||||||
|
$new_lines[$start - 1],
|
||||||
|
$this->highlightText($replacement),
|
||||||
|
$char - 1,
|
||||||
|
strlen($replacement));
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
// If lines at the beginning of the changed line range are actually the
|
// If lines at the beginning of the changed line range are actually the
|
||||||
// same, shrink the range. This happens when a patch just adds a line.
|
// same, shrink the range. This happens when a patch just adds a line.
|
||||||
do {
|
do {
|
||||||
|
@ -154,6 +182,18 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
|
||||||
} while (true);
|
} while (true);
|
||||||
|
|
||||||
} else {
|
} else {
|
||||||
|
|
||||||
|
// If we have "original" text and it is contained on a single line,
|
||||||
|
// highlight the affected area. If we don't have any text, we'll mark
|
||||||
|
// the character with a caret (below, in rendering) instead.
|
||||||
|
if ($old_impact == 1 && strlen($original)) {
|
||||||
|
$old_lines[$start - 1] = substr_replace(
|
||||||
|
$old_lines[$start - 1],
|
||||||
|
$this->highlightText($original),
|
||||||
|
$char - 1,
|
||||||
|
strlen($original));
|
||||||
|
}
|
||||||
|
|
||||||
$old_impact = 0;
|
$old_impact = 0;
|
||||||
$new_impact = 0;
|
$new_impact = 0;
|
||||||
}
|
}
|
||||||
|
@ -263,4 +303,12 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
|
||||||
return $line_map;
|
return $line_map;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function highlightText($text) {
|
||||||
|
if ($this->getTestableMode()) {
|
||||||
|
return '>'.$text.'<';
|
||||||
|
} else {
|
||||||
|
return (string)tsprintf('##%s##', $text);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -61,6 +61,12 @@ final class ArcanistConsoleLintRendererTestCase
|
||||||
'name' => 'Fruit Misinformation',
|
'name' => 'Fruit Misinformation',
|
||||||
'description' => 'Arguably untrue.',
|
'description' => 'Arguably untrue.',
|
||||||
),
|
),
|
||||||
|
|
||||||
|
'original' => array(
|
||||||
|
'line' => 1,
|
||||||
|
'char' => 4,
|
||||||
|
'original' => 'should of',
|
||||||
|
),
|
||||||
);
|
);
|
||||||
|
|
||||||
$defaults = array(
|
$defaults = array(
|
||||||
|
@ -105,7 +111,8 @@ final class ArcanistConsoleLintRendererTestCase
|
||||||
->setData($data)
|
->setData($data)
|
||||||
->addMessage($message);
|
->addMessage($message);
|
||||||
|
|
||||||
$renderer = new ArcanistConsoleLintRenderer();
|
$renderer = id(new ArcanistConsoleLintRenderer())
|
||||||
|
->setTestableMode(true);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
PhutilConsoleFormatter::disableANSI(true);
|
PhutilConsoleFormatter::disableANSI(true);
|
||||||
|
|
|
@ -4,5 +4,5 @@
|
||||||
Warning (WARN123) Lint Warning
|
Warning (WARN123) Lint Warning
|
||||||
Consider this.
|
Consider this.
|
||||||
|
|
||||||
>>> - 1 adjudicated
|
>>> - 1 adjudi>cat<ed
|
||||||
+ adjudidoged
|
+ adjudi>dog<ed
|
||||||
|
|
7
src/lint/renderer/__tests__/data/original.expect
Normal file
7
src/lint/renderer/__tests__/data/original.expect
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
>>> Lint for path/to/example.c:
|
||||||
|
|
||||||
|
|
||||||
|
Warning (WARN123) Lint Warning
|
||||||
|
Consider this.
|
||||||
|
|
||||||
|
>>> 1 He >should of< known.
|
1
src/lint/renderer/__tests__/data/original.txt
Normal file
1
src/lint/renderer/__tests__/data/original.txt
Normal file
|
@ -0,0 +1 @@
|
||||||
|
He should of known.
|
|
@ -4,5 +4,5 @@
|
||||||
Warning (WARN123) Lint Warning
|
Warning (WARN123) Lint Warning
|
||||||
Consider this.
|
Consider this.
|
||||||
|
|
||||||
>>> - 1 tantawount
|
>>> - 1 tanta>w<ount
|
||||||
+ tantamount
|
+ tanta>m<ount
|
||||||
|
|
|
@ -4,7 +4,7 @@
|
||||||
Warning (WARN123) Lint Warning
|
Warning (WARN123) Lint Warning
|
||||||
Consider this.
|
Consider this.
|
||||||
|
|
||||||
>>> - 1 a
|
>>> - 1 >a<
|
||||||
+ z
|
+ >z<
|
||||||
2 b
|
2 b
|
||||||
3 c
|
3 c
|
||||||
|
|
Loading…
Reference in a new issue