Permalink
Browse files

New, simpler and faster line splitter.

Instead of dynamic programming and memoization, this uses a more
straightforward search space exploration. It avoids the combinatorial
explosion by only trying splits inside long lines.

Performance is noticeably better across the board. It slightly
regresses on very nasty lines that do not end up fitting. In
return for that, it produces optimal output (i.e. without
pre-emption bugs) in many more cases, and faster output in cases
where the old splitter would bog down.

It is also less code and easier to maintain.

Fix #360. Fix #380.

R=kevmoo@google.com

Review URL: https://chromiumcodereview.appspot.com//1255643002 .
  • Loading branch information...
munificent committed Jul 23, 2015
1 parent 12328f1 commit 05712e0e0400de95e0ba6f9729410b3401fbc320
View
@@ -1,4 +1,5 @@
.idea
.pub
packages
.packages
build
View
@@ -2,6 +2,7 @@
* Force multi-line comments to the next line (#241).
* Better splitting in metadata annotations in parameter lists (#247).
* New optimized line splitter (#360, #380).
* Allow splitting after argument name (#368).
* Parsing a statement fails if there is unconsumed input (#372).
* Allow splitting on `as` and `is` expressions (#384).
View
@@ -36,7 +36,7 @@ void main(List<String> args) {
parser.addFlag("follow-links",
negatable: false,
help: "Follow links to files and directories.\n"
"If unset, links will be ignored.");
"If unset, links will be ignored.");
parser.addFlag("transform",
abbr: "t",
negatable: false,
View
@@ -2,34 +2,34 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:dart_style/dart_style.dart';
library dart_style.example.format;
import 'dart:io';
import 'dart:mirrors';
import 'package:path/path.dart' as p;
import 'package:dart_style/dart_style.dart';
import 'package:dart_style/src/debug.dart' as debug;
void main(List<String> args) {
// Enable debugging so you can see some of the formatter's internal state.
// Normal users do not do this.
debug.traceChunkBuilder = true;
//debug.traceLineWriter = true;
//debug.traceSplitter = true;
debug.traceLineWriter = true;
debug.traceSplitter = true;
debug.useAnsiColors = true;
formatStmt("""
init({@Option(
help: 'The git Uri containing the jefe.yaml.',
abbr: 'g') String gitUri, @Option(
help: 'The directory to install into',
abbr: 'd') String installDirectory: '.', @Flag(
help: 'Skips the checkout of the develop branch',
abbr: 's') bool skipCheckout: false}) async {}
""", 80);
runTest("regression/0000/0068.stmt", 14);
formatStmt("hello(world);");
}
void formatStmt(String source, [int pageWidth = 40]) {
void formatStmt(String source, [int pageWidth = 80]) {
runFormatter(source, pageWidth, isCompilationUnit: false);
}
void formatUnit(String source, [int pageWidth = 40]) {
void formatUnit(String source, [int pageWidth = 80]) {
runFormatter(source, pageWidth, isCompilationUnit: true);
}
@@ -61,3 +61,109 @@ void drawRuler(String label, int width) {
var padding = " " * (width - label.length - 1);
print("$label:$padding|");
}
/// Runs the formatter test starting on [line] at [path] inside the "test"
/// directory.
void runTest(String path, int line) {
var indentPattern = new RegExp(r"^\(indent (\d+)\)\s*");
// Locate the "test" directory. Use mirrors so that this works with the test
// package, which loads this suite into an isolate.
var testDir = p.join(
p.dirname(currentMirrorSystem()
.findLibrary(#dart_style.example.format)
.uri
.path),
"../test");
var lines = new File(p.join(testDir, path)).readAsLinesSync();
// The first line may have a "|" to indicate the page width.
var pageWidth = 80;
if (lines[0].endsWith("|")) {
pageWidth = lines[0].indexOf("|");
lines = lines.skip(1).toList();
}
var i = 0;
while (i < lines.length) {
var description = lines[i++].replaceAll(">>>", "").trim();
// Let the test specify a leading indentation. This is handy for
// regression tests which often come from a chunk of nested code.
var leadingIndent = 0;
var indentMatch = indentPattern.firstMatch(description);
if (indentMatch != null) {
leadingIndent = int.parse(indentMatch[1]);
description = description.substring(indentMatch.end);
}
if (description == "") {
description = "line ${i + 1}";
} else {
description = "line ${i + 1}: $description";
}
var startLine = i + 1;
var input = "";
while (!lines[i].startsWith("<<<")) {
input += lines[i++] + "\n";
}
var expectedOutput = "";
while (++i < lines.length && !lines[i].startsWith(">>>")) {
expectedOutput += lines[i] + "\n";
}
if (line != startLine) continue;
var isCompilationUnit = p.extension(path) == ".unit";
var inputCode =
_extractSelection(input, isCompilationUnit: isCompilationUnit);
var expected =
_extractSelection(expectedOutput, isCompilationUnit: isCompilationUnit);
var formatter =
new DartFormatter(pageWidth: pageWidth, indent: leadingIndent);
var actual = formatter.formatSource(inputCode);
// The test files always put a newline at the end of the expectation.
// Statements from the formatter (correctly) don't have that, so add
// one to line up with the expected result.
var actualText = actual.text;
if (!isCompilationUnit) actualText += "\n";
print("$path $description");
drawRuler("before", pageWidth);
print(input);
if (actualText == expected.text) {
drawRuler("result", pageWidth);
print(actualText);
} else {
print("FAIL");
drawRuler("expected", pageWidth);
print(expected.text);
drawRuler("actual", pageWidth);
print(actualText);
}
}
}
/// Given a source string that contains ‹ and › to indicate a selection, returns
/// a [SourceCode] with the text (with the selection markers removed) and the
/// correct selection range.
SourceCode _extractSelection(String source, {bool isCompilationUnit: false}) {
var start = source.indexOf("‹");
source = source.replaceAll("‹", "");
var end = source.indexOf("›");
source = source.replaceAll("›", "");
return new SourceCode(source,
isCompilationUnit: isCompilationUnit,
selectionStart: start == -1 ? null : start,
selectionLength: end == -1 ? null : end - start);
}
View
@@ -5,7 +5,7 @@
library dart_style.src.chunk;
import 'fast_hash.dart';
import 'nesting.dart';
import 'nesting_level.dart';
import 'rule/rule.dart';
/// Tracks where a selection start or end point may appear in some piece of
@@ -131,7 +131,15 @@ class Chunk extends Selection {
///
/// Used for multi-line strings and commented out code.
bool get flushLeft => _flushLeft;
bool _flushLeft;
bool _flushLeft = false;
/// If `true`, then the line after this chunk and its contained block should
/// be flush left.
bool get flushLeftAfter {
if (blockChunks.isEmpty) return _flushLeft;
return blockChunks.last.flushLeftAfter;
}
/// Whether this chunk should append an extra space if it does not split.
///
@@ -293,12 +301,6 @@ class Cost {
/// Splitting before a type argument or type parameter.
static const typeArgument = 4;
/// The cost of a single character that goes past the page limit.
///
/// This cost is high to ensure any solution that fits in the page is
/// preferred over one that does not.
static const overflowChar = 1000;
}
/// The in-progress state for a [Span] that has been started but has not yet
Oops, something went wrong.

0 comments on commit 05712e0

Please sign in to comment.