From 96e3b6e3c578e2b5007572eaa6886eb3ed70a1ad Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 28 Jan 2019 19:01:23 -0500 Subject: [PATCH] Drop special support for point spans at the end of files (dart-lang/source_span#31) Drop special support for point spans at the end of files This broke point spans at the beginning of lines elsewhere in the file. Unfortunately, there's no way to disambiguate these currently. SourceSpanWithContext doesn't have any information about where the span appears in the context, so for point spans at column 0, it could be at the beginning of the line or after the trailing newline. We should eventually add something like SourceSpanWithContext.indexInContext, but that'll require a breaking release, since downstream users implement FileSpan as an interface. --- pkgs/source_span/CHANGELOG.md | 5 ++ pkgs/source_span/lib/src/file.dart | 4 +- pkgs/source_span/lib/src/highlighter.dart | 35 ++++------- pkgs/source_span/pubspec.yaml | 2 +- pkgs/source_span/test/file_test.dart | 8 +-- pkgs/source_span/test/highlight_test.dart | 71 +++++++++++++---------- 6 files changed, 61 insertions(+), 64 deletions(-) diff --git a/pkgs/source_span/CHANGELOG.md b/pkgs/source_span/CHANGELOG.md index 6e79fbc46..a07a0e6b4 100644 --- a/pkgs/source_span/CHANGELOG.md +++ b/pkgs/source_span/CHANGELOG.md @@ -1,3 +1,8 @@ +# 1.5.4 + +* `FileSpan.highlight()` now properly highlights point spans at the beginning of + lines. + # 1.5.3 * Fix an edge case where `FileSpan.highlight()` would put the highlight diff --git a/pkgs/source_span/lib/src/file.dart b/pkgs/source_span/lib/src/file.dart index ec4db63bc..27dae5db2 100644 --- a/pkgs/source_span/lib/src/file.dart +++ b/pkgs/source_span/lib/src/file.dart @@ -304,9 +304,9 @@ class _FileSpan extends SourceSpanMixin implements FileSpan { if (length == 0) { // ...unless this is a point span, in which case we want to include the - // next line (or the last line if this is the end of the file). + // next line (or the empty string if this is the end of the file). return endLine == file.lines - 1 - ? file.getText(file.getOffset(endLine - 1)) + ? "" : file.getText( file.getOffset(endLine), file.getOffset(endLine + 1)); } diff --git a/pkgs/source_span/lib/src/highlighter.dart b/pkgs/source_span/lib/src/highlighter.dart index 8100f1839..8a928b3f3 100644 --- a/pkgs/source_span/lib/src/highlighter.dart +++ b/pkgs/source_span/lib/src/highlighter.dart @@ -136,33 +136,18 @@ class Highlighter { /// than at the beginning of the next line. static SourceSpanWithContext _normalizeEndOfLine(SourceSpanWithContext span) { if (span.end.column != 0) return span; + if (span.end.line == span.start.line) return span; - if (span.length == 0) { - if (span.end.offset == 0) return span; + var text = span.text.substring(0, span.text.length - 1); - // If [span] is a point span with an empty context, there's no useful - // adjustment we can do. - if (span.context.isEmpty) return span; - - var location = new SourceLocation(span.end.offset - 1, - sourceUrl: span.sourceUrl, - line: span.end.line - 1, - column: _lastLineLength(span.context)); - return new SourceSpanWithContext(location, location, "", span.context); - } else { - if (span.end.line == span.start.line) return span; - - var text = span.text.substring(0, span.text.length - 1); - - return new SourceSpanWithContext( - span.start, - new SourceLocation(span.end.offset - 1, - sourceUrl: span.sourceUrl, - line: span.end.line - 1, - column: _lastLineLength(text)), - text, - span.context); - } + return new SourceSpanWithContext( + span.start, + new SourceLocation(span.end.offset - 1, + sourceUrl: span.sourceUrl, + line: span.end.line - 1, + column: _lastLineLength(text)), + text, + span.context); } /// Returns the length of the last line in [text], whether or not it ends in a diff --git a/pkgs/source_span/pubspec.yaml b/pkgs/source_span/pubspec.yaml index c4b4ccfdc..a47087561 100644 --- a/pkgs/source_span/pubspec.yaml +++ b/pkgs/source_span/pubspec.yaml @@ -1,5 +1,5 @@ name: source_span -version: 1.5.3 +version: 1.5.4 description: A library for identifying source spans and locations. author: Dart Team diff --git a/pkgs/source_span/test/file_test.dart b/pkgs/source_span/test/file_test.dart index 117458d10..e043ac3ae 100644 --- a/pkgs/source_span/test/file_test.dart +++ b/pkgs/source_span/test/file_test.dart @@ -276,13 +276,13 @@ zip zap zop""", url: "bar.dart").span(10, 11); expect(span.context, equals('whiz bang boom\n')); }); - group("contains the last line for a point span at the end of a file", () { - test("without a newline", () { + group("for a point span at the end of a file", () { + test("without a newline, contains the last line", () { var span = file.span(file.length, file.length); expect(span.context, equals('zip zap zop')); }); - test("with a newline", () { + test("with a newline, contains an empty line", () { file = new SourceFile.fromString(""" foo bar baz whiz bang boom @@ -290,7 +290,7 @@ zip zap zop """, url: "foo.dart"); var span = file.span(file.length, file.length); - expect(span.context, equals('zip zap zop\n')); + expect(span.context, isEmpty); }); }); }); diff --git a/pkgs/source_span/test/highlight_test.dart b/pkgs/source_span/test/highlight_test.dart index 7832e5106..af9cd1ba0 100644 --- a/pkgs/source_span/test/highlight_test.dart +++ b/pkgs/source_span/test/highlight_test.dart @@ -45,77 +45,84 @@ zip zap zop '""")); }); - test("works for a point span", () { - expect(file.location(4).pointSpan().highlight(), equals(""" + group("highlights a point span", () { + test("in the middle of a line", () { + expect(file.location(4).pointSpan().highlight(), equals(""" , 1 | foo bar baz | ^ '""")); - }); + }); - test("works for a point span at the beginning of the file", () { - expect(file.location(0).pointSpan().highlight(), equals(""" + test("at the beginning of the file", () { + expect(file.location(0).pointSpan().highlight(), equals(""" , 1 | foo bar baz | ^ '""")); - }); + }); + + test("at the beginning of a line", () { + expect(file.location(12).pointSpan().highlight(), equals(""" + , +2 | whiz bang boom + | ^ + '""")); + }); - test("works for a point span at the end of a line", () { - expect(file.location(11).pointSpan().highlight(), equals(""" + test("at the end of a line", () { + expect(file.location(11).pointSpan().highlight(), equals(""" , 1 | foo bar baz | ^ '""")); - }); + }); - test("works for a point span at the end of the file", () { - expect(file.location(38).pointSpan().highlight(), equals(""" + test("at the end of the file", () { + expect(file.location(38).pointSpan().highlight(), equals(""" , 3 | zip zap zop | ^ '""")); - }); + }); - test("works for a point span after the end of the file", () { - expect(file.location(39).pointSpan().highlight(), equals(""" + test("after the end of the file", () { + expect(file.location(39).pointSpan().highlight(), equals(""" , -3 | zip zap zop - | ^ +4 | + | ^ '""")); - }); + }); - test("works for a point span at the end of the file with no trailing newline", - () { - file = new SourceFile.fromString("zip zap zop"); - expect(file.location(10).pointSpan().highlight(), equals(""" + test("at the end of the file with no trailing newline", () { + file = new SourceFile.fromString("zip zap zop"); + expect(file.location(10).pointSpan().highlight(), equals(""" , 1 | zip zap zop | ^ '""")); - }); + }); - test( - "works for a point span after the end of the file with no trailing newline", - () { - file = new SourceFile.fromString("zip zap zop"); - expect(file.location(11).pointSpan().highlight(), equals(""" + test("after the end of the file with no trailing newline", () { + file = new SourceFile.fromString("zip zap zop"); + expect(file.location(11).pointSpan().highlight(), equals(""" , 1 | zip zap zop | ^ '""")); - }); + }); - test("works for a point span in an empty file", () { - expect(new SourceFile.fromString("").location(0).pointSpan().highlight(), - equals(""" + test("in an empty file", () { + expect(new SourceFile.fromString("").location(0).pointSpan().highlight(), + equals(""" , 1 | | ^ '""")); + }); }); - test("works for a single-line file without a newline", () { + test("highlights a single-line file without a newline", () { expect( new SourceFile.fromString("foo bar").span(0, 7).highlight(), equals(""" ,