Better indentation of adjacent strings in argument lists. (#800)
See the CHANGELOG diff for details. The relevant examples are:
1.2.4:
function(
"adjacent"
"string");
function(
"adjacent"
"string", // Not great.
);
function(
"adjacent"
"string",
"another");
function(
"adjacent"
"string",
"another",
);
1.2.5:
function(
"adjacent"
"string"); // Regression, but consistent with trailing comma.
function(
"adjacent"
"string",
);
function(
"adjacent"
"string", // Uintentional, but an improvement.
"another");
function(
"adjacent"
"string", // Unchanged.
"another",
);
Now:
function(
"adjacent"
"string"); // No indent, since not needed to distinguish args.
function(
"adjacent"
"string", // Consistent with non-trailing.
);
function(
"adjacent"
"string", // Intentional, distinguishes from next string.
"another");
function(
"adjacent"
"string", // Consistent with non-trailing.
"another",
);diff --git a/CHANGELOG.md b/CHANGELOG.md
index fae032d..1178530 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,20 @@
# 1.2.6
* Properly format trailing commas in assertions.
+* Improve indentation of adjacent strings. This fixes a regression introduced
+ in 1.2.5 and hopefully makes adjacent strings generally look better.
+
+ Adjacent strings in argument lists now format the same regardless of whether
+ the argument list contains a trailing comma. The rule is that if the
+ argument list contains no other strings, then the adjacent strings do not
+ get extra indentation. This keeps them lined up when doing so is unlikely to
+ be confused as showing separate independent string arguments.
+
+ Previously, adjacent strings were never indented in argument lists without a
+ trailing comma and always in argument lists that did. With this change,
+ adjacent strings are still always indented in collection literals because
+ readers are likely to interpret a series of unindented lines there as showing
+ separate collection elements.
# 1.2.5
diff --git a/lib/src/source_visitor.dart b/lib/src/source_visitor.dart
index c3f1525..d31f6b8 100644
--- a/lib/src/source_visitor.dart
+++ b/lib/src/source_visitor.dart
@@ -186,9 +186,72 @@
}
visitAdjacentStrings(AdjacentStrings node) {
+ // We generally want to indent adjacent strings because it can be confusing
+ // otherwise when they appear in a list of expressions, like:
+ //
+ // [
+ // "one",
+ // "two"
+ // "three",
+ // "four"
+ // ]
+ //
+ // Especially when these stings are longer, it can be hard to tell that
+ // "three" is a continuation of the previous argument.
+ //
+ // However, the indentation is distracting in argument lists that don't
+ // suffer from this ambiguity:
+ //
+ // test(
+ // "A very long test description..."
+ // "this indentation looks bad.", () { ... });
+ //
+ // To balance these, we omit the indentation when an adjacent string
+ // expression is the only string in an argument list.
+ var shouldNest = true;
+
+ isString(AstNode node) => node is StringLiteral || node is AdjacentStrings;
+
+ var parent = node.parent;
+ if (parent is ArgumentList) {
+ shouldNest = false;
+
+ for (var argument in parent.arguments) {
+ if (argument == node) continue;
+ if (isString(argument)) {
+ shouldNest = true;
+ break;
+ }
+ }
+ } else if (parent is Assertion) {
+ // Treat asserts like argument lists.
+ shouldNest = false;
+ if (parent.condition != node && isString(parent.condition)) {
+ shouldNest = true;
+ }
+
+ if (parent.message != node && isString(parent.message)) {
+ shouldNest = true;
+ }
+ } else if (parent is VariableDeclaration ||
+ parent is AssignmentExpression &&
+ parent.rightHandSide == node &&
+ parent.parent is ExpressionStatement) {
+ // Don't add extra indentation in a variable initializer or assignment:
+ //
+ // var variable =
+ // "no extra"
+ // "indent";
+ shouldNest = false;
+ } else if (parent is NamedExpression) {
+ shouldNest = false;
+ }
+
builder.startSpan();
builder.startRule();
+ if (shouldNest) builder.nestExpression();
visitNodes(node.strings, between: splitOrNewline);
+ if (shouldNest) builder.unnest();
builder.endRule();
builder.endSpan();
}
diff --git a/test/regression/0100/0185.stmt b/test/regression/0100/0185.stmt
index aac3da2..29a2b7a 100644
--- a/test/regression/0100/0185.stmt
+++ b/test/regression/0100/0185.stmt
@@ -9,6 +9,6 @@
const NO_DART_SCRIPT_AND_EXPERIMENTAL = const MessageTemplate(
const MessageId('polymer', 6),
'The experimental bootstrap feature doesn\'t support script tags on '
- 'the main document (for now).',
+ 'the main document (for now).',
'Script tags with experimental bootstrap',
'This experimental feature is no longer supported.');
\ No newline at end of file
diff --git a/test/regression/0200/0211.unit b/test/regression/0200/0211.unit
index 01766d8..aad9674 100644
--- a/test/regression/0200/0211.unit
+++ b/test/regression/0200/0211.unit
@@ -24,7 +24,7 @@
JS(
'void',
'Object.defineProperty(#, #, '
- '{value: #, enumerable: false, writable: true, configurable: true})',
+ '{value: #, enumerable: false, writable: true, configurable: true})',
obj,
property,
value);
diff --git a/test/regression/0400/0467.unit b/test/regression/0400/0467.unit
index 57df4c3..29ef4f3 100644
--- a/test/regression/0400/0467.unit
+++ b/test/regression/0400/0467.unit
@@ -19,8 +19,7 @@
void showNodesAndEntryPoints([_, __]) {
addAll(
'nodesAndEntryPoints',
- new PolymerDom(this).children.map((child) =>
- '${child.outerHtml} ------> '
+ new PolymerDom(this).children.map((child) => '${child.outerHtml} ------> '
'${(new PolymerDom(child).getDestinationInsertionPoints()[0] as Element).outerHtml}'));
}
}
\ No newline at end of file
diff --git a/test/regression/0700/0705.stmt b/test/regression/0700/0705.stmt
new file mode 100644
index 0000000..38f2fd2
--- /dev/null
+++ b/test/regression/0700/0705.stmt
@@ -0,0 +1,10 @@
+>>>
+final _appUrl = Platform.isIOS
+ ? 'https://itunes.apple.com/us/app/google-adwords/id1037457231'
+ : 'https://play.google.com/store/apps/details?id=com.google.android.apps.'
+ 'adwords';
+<<<
+final _appUrl = Platform.isIOS
+ ? 'https://itunes.apple.com/us/app/google-adwords/id1037457231'
+ : 'https://play.google.com/store/apps/details?id=com.google.android.apps.'
+ 'adwords';
\ No newline at end of file
diff --git a/test/regression/0700/0799.unit b/test/regression/0700/0799.unit
new file mode 100644
index 0000000..acd54a4
--- /dev/null
+++ b/test/regression/0700/0799.unit
@@ -0,0 +1,14 @@
+>>>
+@ShouldThrow(
+ 'Could not generate `toJson` code for `watch`.\n'
+ 'None of the provided `TypeHelper` instances support the defined type.',
+ configurations: ['default'],
+)
+class C {}
+<<<
+@ShouldThrow(
+ 'Could not generate `toJson` code for `watch`.\n'
+ 'None of the provided `TypeHelper` instances support the defined type.',
+ configurations: ['default'],
+)
+class C {}
\ No newline at end of file
diff --git a/test/regression/other/flutter.unit b/test/regression/other/flutter.unit
new file mode 100644
index 0000000..98dbde8
--- /dev/null
+++ b/test/regression/other/flutter.unit
@@ -0,0 +1,18 @@
+>>>
+const Vendor _trevor = Vendor(
+ name: 'Trevor’s shop',
+ avatarAsset: 'people/square/trevor.png',
+ avatarAssetPackage: _kGalleryAssetsPackage,
+ description:
+ 'Trevor makes great stuff for awesome people like you. Super cool and extra '
+ 'awesome all of his shop’s goods are handmade with love. Custom orders are '
+ 'available upon request if you need something extra special.');
+<<<
+const Vendor _trevor = Vendor(
+ name: 'Trevor’s shop',
+ avatarAsset: 'people/square/trevor.png',
+ avatarAssetPackage: _kGalleryAssetsPackage,
+ description:
+ 'Trevor makes great stuff for awesome people like you. Super cool and extra '
+ 'awesome all of his shop’s goods are handmade with love. Custom orders are '
+ 'available upon request if you need something extra special.');
\ No newline at end of file
diff --git a/test/whitespace/adjacent_strings.stmt b/test/whitespace/adjacent_strings.stmt
new file mode 100644
index 0000000..40d5da1
--- /dev/null
+++ b/test/whitespace/adjacent_strings.stmt
@@ -0,0 +1,232 @@
+40 columns |
+>>> do not indent adjacent strings if other args are not strings
+function(
+ notString,
+ "adjacent"
+ "string");
+<<<
+function(
+ notString,
+ "adjacent"
+ "string");
+>>> do not indent adjacent strings if other args are not strings
+function(
+ notString,
+ "adjacent"
+ "string",);
+<<<
+function(
+ notString,
+ "adjacent"
+ "string",
+);
+>>> do indent adjacent strings if other arg is string
+function(
+ "string",
+ notString,
+ "adjacent"
+ "string");
+<<<
+function(
+ "string",
+ notString,
+ "adjacent"
+ "string");
+>>> do indent adjacent strings if other arg is string
+function(
+ "string",
+ notString,
+ "adjacent"
+ "string",);
+<<<
+function(
+ "string",
+ notString,
+ "adjacent"
+ "string",
+);
+>>> do indent adjacent strings if other arg is string interpolation
+function(
+ "${str}${ing}",
+ notString,
+ "adjacent"
+ "string");
+<<<
+function(
+ "${str}${ing}",
+ notString,
+ "adjacent"
+ "string");
+>>> do indent adjacent strings if other arg is adjacent string
+function(
+ "adjacent"
+ "string",
+ notString,
+ "adjacent"
+ "string");
+<<<
+function(
+ "adjacent"
+ "string",
+ notString,
+ "adjacent"
+ "string");
+>>> always indent adjacent strings in list
+var list = [
+ "adjacent"
+ "string",
+ "another"
+ "adjacent"
+ "string"
+];
+<<<
+var list = [
+ "adjacent"
+ "string",
+ "another"
+ "adjacent"
+ "string"
+];
+>>> always indent adjacent strings in map key
+var map = {
+ "adjacent"
+ "string": value,
+ "another"
+ "adjacent"
+ "string": value
+};
+<<<
+var map = {
+ "adjacent"
+ "string": value,
+ "another"
+ "adjacent"
+ "string": value
+};
+>>> always indent adjacent strings in map value
+var map = {
+ key: "adjacent"
+ "string",
+ key: "another"
+ "adjacent"
+ "string"
+};
+<<<
+var map = {
+ key: "adjacent"
+ "string",
+ key: "another"
+ "adjacent"
+ "string"
+};
+>>> always indent adjacent strings in set
+var set = {
+ "adjacent"
+ "string",
+ "another"
+ "adjacent"
+ "string"
+};
+<<<
+var set = {
+ "adjacent"
+ "string",
+ "another"
+ "adjacent"
+ "string"
+};
+>>> don't indent in => body
+main() => "adjacent"
+"string";
+<<<
+main() => "adjacent"
+ "string";
+>>> indent in then branch of ?:
+var string = c ? "adjacent"
+"string" : "other";
+<<<
+var string = c
+ ? "adjacent"
+ "string"
+ : "other";
+>>> indent in else branch of ?:
+var string = c ? "other" : "adjacent"
+"string";
+<<<
+var string = c
+ ? "other"
+ : "adjacent"
+ "string";
+>>> don't indent in initializer
+var longVariableName = "very long adjacent"
+"string";
+<<<
+var longVariableName =
+ "very long adjacent"
+ "string";
+>>> don't indent assignment in statement position
+long.receiver.expression = "very long adjacent"
+"string";
+<<<
+long.receiver.expression =
+ "very long adjacent"
+ "string";
+>>> string interpolation counts as a string
+function(
+ "adjacent"
+ "string",
+ "${str}${ing}");
+<<<
+function(
+ "adjacent"
+ "string",
+ "${str}${ing}");
+>>> another adjacent string counts as a string
+function(
+ "adjacent"
+ "string",
+ "another"
+ "adjacent");
+<<<
+function(
+ "adjacent"
+ "string",
+ "another"
+ "adjacent");
+>>> do not indent adjacent strings in assert if other args are not strings
+assert(
+ condition,
+ "adjacent"
+ "string");
+<<<
+assert(
+ condition,
+ "adjacent"
+ "string");
+>>> don't need extra indentation inside named arguments
+function(named: "adjacent"
+"string",
+another: "adjacent"
+"string"
+"more");
+<<<
+function(
+ named: "adjacent"
+ "string",
+ another: "adjacent"
+ "string"
+ "more");
+>>> don't need extra indentation inside named arguments
+function(named: "adjacent"
+"string",
+another: "adjacent"
+"string"
+"more",);
+<<<
+function(
+ named: "adjacent"
+ "string",
+ another: "adjacent"
+ "string"
+ "more",
+);
\ No newline at end of file
