From 9991c6ea1ba518f736694a6568abca9b966b37b0 Mon Sep 17 00:00:00 2001 From: Robert Nystrom Date: Wed, 21 Feb 2024 11:54:21 -0800 Subject: [PATCH 1/2] Add more tests for type arguments in object patterns and remove a TODO. I'm honestly not sure what the TODO was even trying to say. But I think the formatter behaves like we want here so there's nothing TODO as far as I can tell. But this corner did seem a little under tested, so added some more. --- test/pattern/object.stmt | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/test/pattern/object.stmt b/test/pattern/object.stmt index 6e79f89d..271e3c7f 100644 --- a/test/pattern/object.stmt +++ b/test/pattern/object.stmt @@ -165,10 +165,9 @@ if (obj case Foo( )) { ; } ->>> Split in type argument. +>>> Split in type argument with no field subpatterns. if (obj case LongClassName()) {;} <<< -### TODO(tall): It formats like this if there's no elements. Similarly with lists, maps etc. if (obj case LongClassName< First, @@ -176,7 +175,15 @@ if (obj >()) { ; } ->>> Split in type argument and body. +>>> Prefer splitting in field subpatterns instead of type arguments. +if (obj case Foo(first: 1)) {;} +<<< +if (obj case Foo( + first: 1, +)) { + ; +} +>>> Split in type argument but not field subpatterns. if (obj case LongClassName(first: 1, second: 2, third: 3)) {;} <<< if (obj case LongClassName< @@ -186,3 +193,18 @@ if (obj case LongClassName< >(first: 1, second: 2, third: 3)) { ; } +>>> Split in type arguments and field subpatterns. +if (obj case LongClassName(first: 1, second: 2, third: 3, fourth: 4)) {;} +<<< +if (obj case LongClassName< + First, + Second, + Third +>( + first: 1, + second: 2, + third: 3, + fourth: 4, +)) { + ; +} \ No newline at end of file From 76444135aae104c0e7a8127bf9ae2c8506ec7687 Mon Sep 17 00:00:00 2001 From: Robert Nystrom Date: Wed, 21 Feb 2024 15:03:09 -0800 Subject: [PATCH 2/2] Fix indentation collapsing. A block indent shouldn't be able to partially eat an expression-sized collapsible indentation. The only time we want to actually collapse is when we have two equivalent expression-sized indentations. --- lib/src/back_end/code_writer.dart | 20 ++++++++++++-------- test/pattern/object.stmt | 4 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/src/back_end/code_writer.dart b/lib/src/back_end/code_writer.dart index 4be9b1a9..751af83c 100644 --- a/lib/src/back_end/code_writer.dart +++ b/lib/src/back_end/code_writer.dart @@ -153,15 +153,19 @@ class CodeWriter { var parentIndent = _indentStack.last.indent; var parentCollapse = _indentStack.last.collapsible; - if (canCollapse) { - // Increase the indent and the collapsible indent. - _indentStack.add(_Indent(parentIndent + indent, parentCollapse + indent)); - } else if (parentCollapse > indent) { - // All new indent is collapsed with the existing collapsible indent. - _indentStack.add(_Indent(parentIndent, parentCollapse - indent)); + if (parentCollapse == indent) { + // We're indenting by the same existing collapsible amount, so collapse + // this new indentation with that existing one. + _indentStack.add(_Indent(parentIndent, 0)); + } else if (canCollapse) { + // We should never get multiple levels of nested collapsible indentation. + assert(parentCollapse == 0); + + // Increase the indentation and note that it can be collapsed with + // further indentation. + _indentStack.add(_Indent(parentIndent + indent, indent)); } else { - // Use up the collapsible indent (if any) and then indent by the rest. - indent -= parentCollapse; + // Regular indentation, so just increase the indent. _indentStack.add(_Indent(parentIndent + indent, 0)); } } diff --git a/test/pattern/object.stmt b/test/pattern/object.stmt index 271e3c7f..4eb504fc 100644 --- a/test/pattern/object.stmt +++ b/test/pattern/object.stmt @@ -170,8 +170,8 @@ if (obj case LongClassName()) {;} <<< if (obj case LongClassName< - First, - Second + First, + Second >()) { ; }