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
