Don't split inside string interpolation. (#709) * Don't split inside string interpolation. When it happens, it's usually a sign that you should split the string yourself to avoid it. But it still looks terrible. And in multi-line strings, you may *want* to have lines longer than the page width, in which case the splitting is really bad. * Reword doc comment.
diff --git a/CHANGELOG.md b/CHANGELOG.md index b2aa4ea..686a406 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md
@@ -1,3 +1,7 @@ +# 1.1.2 + +* Don't split inside string interpolations. + # 1.1.1 * Format expressions in string interpolations (#226).
diff --git a/bin/format.dart b/bin/format.dart index 23d0979..e0de12d 100644 --- a/bin/format.dart +++ b/bin/format.dart
@@ -15,7 +15,7 @@ import 'package:dart_style/src/style_fix.dart'; // Note: The following line of code is modified by tool/grind.dart. -const version = "1.1.1"; +const version = "1.1.2"; void main(List<String> args) { var parser = new ArgParser(allowTrailingOptions: true);
diff --git a/lib/src/chunk_builder.dart b/lib/src/chunk_builder.dart index 34a480d..4bdd166 100644 --- a/lib/src/chunk_builder.dart +++ b/lib/src/chunk_builder.dart
@@ -93,6 +93,17 @@ /// block. bool _firstFlushLeft = false; + /// The number of calls to [preventSplit()] that have not been ended by a + /// call to [endPreventSplit()]. + /// + /// Splitting is completely disabled inside string interpolation. We do want + /// to fix the whitespace inside interpolation, though, so we still format + /// them. This tracks whether we're inside an interpolation. We can't use a + /// simple bool because interpolation can nest. + /// + /// When this is non-zero, splits are ignored. + int _preventSplitNesting = 0; + /// Whether there is pending whitespace that depends on the number of /// newlines in the source. /// @@ -501,6 +512,10 @@ /// /// Nested blocks are handled using their own independent [LineWriter]. ChunkBuilder startBlock(Chunk argumentChunk) { + // If we are not allowed to split at all, don't create a new block. Instead, + // the block contents will end up in the current chunk. + if (_preventSplitNesting > 0) return this; + var chunk = _chunks.last; chunk.makeBlock(argumentChunk); @@ -523,6 +538,11 @@ /// /// Returns the previous writer for the surrounding block. ChunkBuilder endBlock(Rule ignoredSplit, {bool forceSplit}) { + // If we are not allowed to split at all, we didn't create a new block and + // thus didn't create a new ChunkBuilder, so there is no builder to pop. + // We are still in the current one. + if (_preventSplitNesting > 0) return this; + _divideChunks(); // If we don't already know if the block is going to split, see if it @@ -602,6 +622,15 @@ selectionLength: selectionLength); } + void preventSplit() { + _preventSplitNesting++; + } + + void endPreventSplit() { + _preventSplitNesting--; + assert(_preventSplitNesting >= 0, "Mismatched calls."); + } + /// Writes the current pending [Whitespace] to the output, if any. /// /// This should only be called after source lines have been preserved to turn @@ -738,6 +767,21 @@ /// to be at column zero. Otherwise, it uses the normal indentation and /// nesting behavior. void _writeHardSplit({bool isDouble, bool flushLeft, bool nest: false}) { + // If we are not allowed to split at all, simply write a space. Instead of: + // + // foo("${() { + // a; + // b; + // }}"); + // + // produces: + // + // foo("${() { a; b; }}"); + if (_preventSplitNesting > 0) { + _writeText(" "); + return; + } + // A hard split overrides any other whitespace. _pendingWhitespace = null; _writeSplit(new Rule.hard(), @@ -749,7 +793,16 @@ /// Returns the chunk. Chunk _writeSplit(Rule rule, {bool flushLeft, bool isDouble, bool nest, bool space}) { - if (nest == null) nest = true; + nest ??= true; + space ??= false; + + // If we are not allowed to split at all, don't. Returning null for the + // chunk is safe since the rule that uses the chunk will itself get + // discarded because no chunk references it. + if (_preventSplitNesting > 0) { + if (space) write(" "); + return null; + } if (_chunks.isEmpty) { if (flushLeft != null) _firstFlushLeft = flushLeft;
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart index a05f5c8..da05270 100644 --- a/lib/src/source_visitor.dart +++ b/lib/src/source_visitor.dart
@@ -1559,11 +1559,13 @@ } visitInterpolationExpression(InterpolationExpression node) { + builder.preventSplit(); token(node.leftBracket); builder.startSpan(); visit(node.expression); builder.endSpan(); token(node.rightBracket); + builder.endPreventSplit(); } visitInterpolationString(InterpolationString node) {
diff --git a/pubspec.yaml b/pubspec.yaml index 0810110..634c852 100644 --- a/pubspec.yaml +++ b/pubspec.yaml
@@ -1,6 +1,6 @@ name: dart_style # Note: See tool/grind.dart for how to bump the version. -version: 1.1.1 +version: 1.1.2 author: Dart Team <misc@dartlang.org> description: Opinionated, automatic Dart source code formatter. homepage: https://github.com/dart-lang/dart_style
diff --git a/test/regression/0000/0057.stmt b/test/regression/0000/0057.stmt index be00d6b..77294f8 100644 --- a/test/regression/0000/0057.stmt +++ b/test/regression/0000/0057.stmt
@@ -5,8 +5,7 @@ name: 'embeddedPlural2', desc: 'An embedded plural', args: [n]); <<< embeddedPlural2(n) => Intl.message( - "${Intl.plural(n, - zero: 'none', one: 'one', other: 'some')} plus some text.", + "${Intl.plural(n, zero: 'none', one: 'one', other: 'some')} plus some text.", name: 'embeddedPlural2', desc: 'An embedded plural', args: [n]); \ No newline at end of file
diff --git a/test/regression/0100/0189.stmt b/test/regression/0100/0189.stmt index 4acf089..243f837 100644 --- a/test/regression/0100/0189.stmt +++ b/test/regression/0100/0189.stmt
@@ -11,9 +11,8 @@ nestedSelect(currency, amount) => Intl.select( currency, { - "CDN": """${Intl.plural(amount, - one: '$amount Canadian dollar', - other: '$amount Canadian dollars')}""", + "CDN": + """${Intl.plural(amount, one: '$amount Canadian dollar', other: '$amount Canadian dollars')}""", "other": "Whatever", }, name: "nestedSelect",
diff --git a/test/regression/0400/0467.unit b/test/regression/0400/0467.unit index 680067c..57df4c3 100644 --- a/test/regression/0400/0467.unit +++ b/test/regression/0400/0467.unit
@@ -21,8 +21,6 @@ 'nodesAndEntryPoints', new PolymerDom(this).children.map((child) => '${child.outerHtml} ------> ' - '${(new PolymerDom(child).getDestinationInsertionPoints()[0] - as Element) - .outerHtml}')); + '${(new PolymerDom(child).getDestinationInsertionPoints()[0] as Element).outerHtml}')); } } \ No newline at end of file
diff --git a/test/regression/0500/0500.unit b/test/regression/0500/0500.unit index 82988db..4e1a62f 100644 --- a/test/regression/0500/0500.unit +++ b/test/regression/0500/0500.unit
@@ -26,10 +26,9 @@ for (int i = 0; i < names.length; ++i) { env.initialize(names[i], getter: (TopLevelBinding binding, ExprCont ek, ExprCont k) { - binding.getter = - (TopLevelBinding _, ExprCont ek0, ExprCont k0) => ek0( - "Reading static variable '${binding - .name}' during its initialization"); + binding.getter = (TopLevelBinding _, ExprCont ek0, + ExprCont k0) => + ek0("Reading static variable '${binding.name}' during its initialization"); initializers[i](env, ek, (v) { binding.getter = (TopLevelBinding _, ExprCont ek1, ExprCont k1) => k1(v);
diff --git a/test/splitting/strings.stmt b/test/splitting/strings.stmt index f6d5529..d3bdaab 100644 --- a/test/splitting/strings.stmt +++ b/test/splitting/strings.stmt
@@ -138,17 +138,31 @@ "many", "elements" ]); ->>> interpolation in multi-line does not force unneeded splits +>>> interpolation is not split even when line is too long +someMethod("some text that is pretty long ${ interpolate + +a + thing } more text"); +<<< +someMethod( + "some text that is pretty long ${interpolate + a + thing} more text"); +>>> multi-line string interpolation does not split someMethod("foo", """ some text that is pretty long - some more text that is pretty long - ${ inpolate + a + thing } - more text + some more text that is pretty long ${ interpolate + a + thing } more text """); <<< someMethod("foo", """ some text that is pretty long - some more text that is pretty long - ${inpolate + a + thing} - more text -"""); \ No newline at end of file + some more text that is pretty long ${interpolate + a + thing} more text +"""); +>>> nested interpolation is not split +someMethod("some text that is ${pretty + 'long ${ interpolate + +a + thing } more'} text", "another arg"); +<<< +someMethod( + "some text that is ${pretty + 'long ${interpolate + a + thing} more'} text", + "another arg"); +>>> hard splits are not split in interpolation +someMethod("before ${(){statement();statement();statement();}} after"); +<<< +someMethod( + "before ${() { statement(); statement(); statement(); }} after"); \ No newline at end of file