Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2)

Unified Diff: lib/src/line_splitter.dart

Issue 996033002: Automatically harden splits containing too much nesting. Fix #108. (Closed) Base URL: https://github.com/dart-lang/dart_style.git@master
Patch Set: Update changelog. Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « CHANGELOG.md ('k') | test/regression/108.unit » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/src/line_splitter.dart
diff --git a/lib/src/line_splitter.dart b/lib/src/line_splitter.dart
index e0a8de13582cdb0a7b32aa39f15aa7ae332d5b8b..3291e280ed961f421dabec5166ba6213e2180fe5 100644
--- a/lib/src/line_splitter.dart
+++ b/lib/src/line_splitter.dart
@@ -103,9 +103,29 @@ class LineSplitter {
List<int> apply(StringBuffer buffer) {
if (debugFormatter) dumpLine(_chunks, _indent);
- _flattenNestingLevels();
+ var nestingDepth = _flattenNestingLevels();
+
+ // Hack. The formatter doesn't handle formatting very deeply nested code
+ // well. It can make performance spiral into a pit of sadness. Fortunately,
+ // we only tend to see expressions pathologically deeply nested in
+ // generated code that isn't read by humans much anyway. To avoid burning
+ // too much time on these, harden any splits containing more than a certain
+ // level of nesting.
+ //
+ // The number here was chosen empirically based on formatting the repo. It
+ // was picked to get the best performance while affecting the minimum amount
+ // of results.
+ // TODO(rnystrom): Do something smarter.
pquitslund 2015/03/11 00:09:50 Best comment I've read all year.
Bob Nystrom 2015/03/11 00:14:17 :D
+ if (nestingDepth > 9) {
+ for (var chunk in _chunks) {
+ if (chunk.param != null && nestingDepth - chunk.nesting > 9) {
+ chunk.harden();
+ }
+ }
+ }
var splits = _findBestSplits(new LinePrefix());
+
var selection = [null, null];
// Write each chunk and the split after it.
@@ -161,7 +181,11 @@ class LineSplitter {
/// Worse, if the splitter *does* consider these levels, it can dramatically
/// increase solving time. To avoid that, this renumbers all of the nesting
/// levels in the chunks to not have any of these unused gaps.
- void _flattenNestingLevels() {
+ ///
+ /// Returns the number of distinct nesting levels remaining after flattening.
+ /// This may be zero if the chunks have no nesting (i.e. just statement-level
+ /// indentation).
+ int _flattenNestingLevels() {
var nestingLevels = _chunks
.map((chunk) => chunk.nesting)
.where((nesting) => nesting != -1)
@@ -177,6 +201,8 @@ class LineSplitter {
for (var chunk in _chunks) {
chunk.nesting = nestingMap[chunk.nesting];
}
+
+ return nestingLevels.length;
}
/// Finds the best set of splits to apply to the remainder of the line
« no previous file with comments | « CHANGELOG.md ('k') | test/regression/108.unit » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698