Merge pull request #26 from dart-lang/tweak-highlight
Improve a span highlighting edge case
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2248a39..74ab342 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 1.5.1
+
+* Produce better source span highlights for multi-line spans that cover the
+ entire last line of the span, including the newline.
+
# 1.5.0
* Improve the output of `SourceSpan.highlight()` and `SourceSpan.message()`:
diff --git a/lib/src/file.dart b/lib/src/file.dart
index 6154e13..18c46e6 100644
--- a/lib/src/file.dart
+++ b/lib/src/file.dart
@@ -291,8 +291,29 @@
FileLocation get start => new FileLocation._(file, _start);
FileLocation get end => new FileLocation._(file, _end);
String get text => file.getText(_start, _end);
- String get context => file.getText(file.getOffset(start.line),
- end.line == file.lines - 1 ? null : file.getOffset(end.line + 1));
+
+ String get context {
+ var endLine = file.getLine(_end);
+ var endColumn = file.getColumn(_end);
+
+ int endOffset;
+ if (endColumn == 0 && endLine != 0) {
+ // If [end] is at the very beginning of the line, the span covers the
+ // previous newline, so we only want to include the previous line in the
+ // context.
+ endOffset = _end;
+ } else if (endLine == file.lines - 1) {
+ // If the span covers the last line of the file, the context should go all
+ // the way to the end of the file.
+ endOffset = file.length;
+ } else {
+ // Otherwise, the context should cover the full line on which [end]
+ // appears.
+ endOffset = file.getOffset(endLine + 1);
+ }
+
+ return file.getText(file.getOffset(file.getLine(_start)), endOffset);
+ }
_FileSpan(this.file, this._start, this._end) {
if (_end < _start) {
diff --git a/lib/src/highlighter.dart b/lib/src/highlighter.dart
index a05412a..97d696d 100644
--- a/lib/src/highlighter.dart
+++ b/lib/src/highlighter.dart
@@ -99,6 +99,22 @@
newSpan = new SourceSpanWithContext(start, end, text, context);
}
+ // Normalize [span] so that the end location is at the end of a line, rather
+ // than on the beginning of the next line.
+ if (newSpan.end.column == 0 && newSpan.end.line != newSpan.start.line) {
+ assert(newSpan.text.endsWith("\n"));
+
+ var text = newSpan.text.substring(0, newSpan.text.length - 1);
+ newSpan = new SourceSpanWithContext(
+ newSpan.start,
+ new SourceLocation(span.end.offset - 1,
+ sourceUrl: span.sourceUrl,
+ line: span.end.line - 1,
+ column: _lastColumn(text)),
+ text,
+ newSpan.context);
+ }
+
return new Highlighter._(newSpan, color);
}
diff --git a/pubspec.yaml b/pubspec.yaml
index ebd3a72..13fa648 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: source_span
-version: 1.5.0
+version: 1.5.1-dev
description: A library for identifying source spans and locations.
author: Dart Team <misc@dartlang.org>
diff --git a/test/highlight_test.dart b/test/highlight_test.dart
index 19f8ca8..e2536ba 100644
--- a/test/highlight_test.dart
+++ b/test/highlight_test.dart
@@ -158,6 +158,15 @@
});
test("highlights the full last line", () {
+ expect(file.span(4, 27).highlight(), equals("""
+ ,
+1 | foo bar baz
+ | ,-----^
+2 | \\ whiz bang boom
+ '"""));
+ });
+
+ test("highlights the full last line with no trailing newline", () {
expect(file.span(4, 26).highlight(), equals("""
,
1 | foo bar baz
@@ -176,7 +185,9 @@
'"""));
});
- test("highlights the full last line with no trailing newline", () {
+ test(
+ "highlights the full last line at the end of the file with no trailing"
+ " newline", () {
var file = new SourceFile.fromString("""
foo bar baz
whiz bang boom
@@ -305,21 +316,57 @@
});
});
- test("supports lines of preceding and following context", () {
- var span = new SourceSpanWithContext(
- new SourceLocation(5, line: 2, column: 5, sourceUrl: "foo.dart"),
- new SourceLocation(12, line: 2, column: 12, sourceUrl: "foo.dart"),
- "foo bar",
- "previous\nlines\n-----foo bar-----\nfollowing line\n");
+ group("supports lines of preceding and following context for a span", () {
+ test("within a single line", () {
+ var span = new SourceSpanWithContext(
+ new SourceLocation(20, line: 2, column: 5, sourceUrl: "foo.dart"),
+ new SourceLocation(27, line: 2, column: 12, sourceUrl: "foo.dart"),
+ "foo bar",
+ "previous\nlines\n-----foo bar-----\nfollowing line\n");
- expect(span.highlight(color: colors.YELLOW), equals("""
-${colors.BLUE} ,${colors.NONE}
-${colors.BLUE}1 |${colors.NONE} previous
-${colors.BLUE}2 |${colors.NONE} lines
-${colors.BLUE}3 |${colors.NONE} -----${colors.YELLOW}foo bar${colors.NONE}-----
-${colors.BLUE} |${colors.NONE} ${colors.YELLOW}^^^^^^^${colors.NONE}
-${colors.BLUE}4 |${colors.NONE} following line
-${colors.BLUE} '${colors.NONE}"""));
+ expect(span.highlight(), equals("""
+ ,
+1 | previous
+2 | lines
+3 | -----foo bar-----
+ | ^^^^^^^
+4 | following line
+ '"""));
+ });
+
+ test("covering a full line", () {
+ var span = new SourceSpanWithContext(
+ new SourceLocation(15, line: 2, column: 0, sourceUrl: "foo.dart"),
+ new SourceLocation(33, line: 3, column: 0, sourceUrl: "foo.dart"),
+ "-----foo bar-----\n",
+ "previous\nlines\n-----foo bar-----\nfollowing line\n");
+
+ expect(span.highlight(), equals("""
+ ,
+1 | previous
+2 | lines
+3 | -----foo bar-----
+ | ^^^^^^^^^^^^^^^^^
+4 | following line
+ '"""));
+ });
+
+ test("covering multiple full lines", () {
+ var span = new SourceSpanWithContext(
+ new SourceLocation(15, line: 2, column: 0, sourceUrl: "foo.dart"),
+ new SourceLocation(23, line: 4, column: 0, sourceUrl: "foo.dart"),
+ "foo\nbar\n",
+ "previous\nlines\nfoo\nbar\nfollowing line\n");
+
+ expect(span.highlight(), equals("""
+ ,
+1 | previous
+2 | lines
+3 | / foo
+4 | \\ bar
+5 | following line
+ '"""));
+ });
});
group("colors", () {
