Page MenuHomePhabricator
Files F14012458

D18716.id44936.diff
No OneTemporary
Actions

D18716.id44936.diff
View Options

diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -216,8 +216,11 @@
}
for ($ii = $start; $ii < $start + $new_impact; $ii++) {
+ // If the patch was at the end of the file and ends with a newline, we
+ // won't have an actual entry in the array for the last line, even though
+ // we want to show it in the diff.
$out[] = array(
- 'text' => $new_lines[$ii - 1],
+ 'text' => idx($new_lines, $ii - 1, ''),
'type' => '+',
'chevron' => ($ii == $start),
);
@@ -246,9 +249,17 @@
}
}
+ // If the line doesn't actually end in a newline, add one so the layout
+ // doesn't mess up. This can happen when the last line of the old file
+ // didn't have a newline at the end.
+ $text = $spec['text'];
+ if (!preg_match('/\n\z/', $spec['text'])) {
+ $text .= "\n";
+ }
+
$result[] = $this->renderLine(
idx($spec, 'number'),
- $spec['text'],
+ $text,
$chevron,
idx($spec, 'type'));
@@ -298,6 +309,15 @@
$offset += strlen($line);
}
+ // If the last line ends in a newline, add a virtual offset for the final
+ // line with no characters on it. This allows lint messages to target the
+ // last line of the file at character 1.
+ if ($lines) {
+ if (preg_match('/\n\z/', $line)) {
+ $line_map[$number] = $offset;
+ }
+ }
+
return $line_map;
}
diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -120,6 +120,28 @@
'original' => "\n",
'replacement' => '',
),
+
+ 'eofnewline' => array(
+ 'line' => 1,
+ 'char' => 7,
+ 'original' => '',
+ 'replacement' => "\n",
+ ),
+
+ 'eofmultilinechar' => array(
+ 'line' => 5,
+ 'char' => 3,
+ 'original' => '',
+ 'replacement' => "\nX\nY\n",
+ ),
+
+ 'eofmultilineline' => array(
+ 'line' => 6,
+ 'char' => 1,
+ 'original' => '',
+ 'replacement' => "\nX\nY\n",
+ ),
+
);
$defaults = array(
@@ -132,8 +154,6 @@
foreach ($map as $key => $test_case) {
$data = $this->readTestData("{$key}.txt");
- $data = preg_replace('/~+\s*$/m', '', $data);
-
$expect = $this->readTestData("{$key}.expect");
$test_case = $test_case + $defaults;
@@ -178,11 +198,6 @@
throw $ex;
}
- // Trim "~" off the ends of lines. This allows the "expect" file to test
- // for trailing whitespace without actually containing trailing
- // whitespace.
- $expect = preg_replace('/~+$/m', '', $expect);
-
$this->assertEqual(
$expect,
$actual,
@@ -194,7 +209,19 @@
private function readTestData($filename) {
$path = dirname(__FILE__).'/data/'.$filename;
- return Filesystem::readFile($path);
+ $data = Filesystem::readFile($path);
+
+ // If we find "~~~" at the end of the file, get rid of it and any whitespace
+ // afterwards. This allows specifying data files with trailing empty
+ // lines.
+ $data = preg_replace('/~~~\s*\z/', '', $data);
+
+ // Trim "~" off the ends of lines. This allows the "expect" file to test
+ // for trailing whitespace without actually containing trailing
+ // whitespace.
+ $data = preg_replace('/~$/m', '', $data);
+
+ return $data;
}
}
diff --git a/src/lint/renderer/__tests__/data/eofmultilinechar.expect b/src/lint/renderer/__tests__/data/eofmultilinechar.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/eofmultilinechar.expect
@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 3 3
+ 4 4
+ 5 5
+ >>> + ~
+ + X
+ + Y
diff --git a/src/lint/renderer/__tests__/data/eofmultilinechar.txt b/src/lint/renderer/__tests__/data/eofmultilinechar.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/eofmultilinechar.txt
@@ -0,0 +1,5 @@
+1
+2
+3
+4
+5
diff --git a/src/lint/renderer/__tests__/data/eofmultilineline.expect b/src/lint/renderer/__tests__/data/eofmultilineline.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/eofmultilineline.expect
@@ -0,0 +1,12 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ 3 3
+ 4 4
+ 5 5
+ >>> + ~
+ + X
+ + Y
diff --git a/src/lint/renderer/__tests__/data/eofmultilineline.txt b/src/lint/renderer/__tests__/data/eofmultilineline.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/eofmultilineline.txt
@@ -0,0 +1,5 @@
+1
+2
+3
+4
+5
diff --git a/src/lint/renderer/__tests__/data/eofnewline.expect b/src/lint/renderer/__tests__/data/eofnewline.expect
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/eofnewline.expect
@@ -0,0 +1,9 @@
+>>> Lint for path/to/example.c:
+
+
+ Warning (WARN123) Lint Warning
+ Consider this.
+
+ >>> - 1 abcdef
+ + abcdef
+ + ~
diff --git a/src/lint/renderer/__tests__/data/eofnewline.txt b/src/lint/renderer/__tests__/data/eofnewline.txt
new file mode 100644
--- /dev/null
+++ b/src/lint/renderer/__tests__/data/eofnewline.txt
@@ -0,0 +1 @@
+abcdef~~~
diff --git a/src/lint/renderer/__tests__/data/extrawhitespace.txt b/src/lint/renderer/__tests__/data/extrawhitespace.txt
--- a/src/lint/renderer/__tests__/data/extrawhitespace.txt
+++ b/src/lint/renderer/__tests__/data/extrawhitespace.txt
@@ -1,3 +1,3 @@
Adrift upon the sea.
-~
+~~~

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 2, 1:00 PM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6718969
Default Alt Text
D18716.id44936.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility