Formatting support for named arguments anywhere. #1094
Conversation
I opted to add support for this while being minimally invasive to the
existing style and formatting. All existing code which does not have any
positional arguments after named ones retains its previous formatting.
For new argument lists that use positional arguments after named ones,
I try to mostly follow the existing style rules even though they can be
fairly complex. In particular, it will be pretty aggressive about
applying block-style formatting to function literals inside argument
lists even with named args anywhere. I think that's important to support
the main use case I know of for the feature which is trailing positional
closures like:
function(named: 1, another: 2, () {
block...
});
In argument lists using named args anywhere that don't have block
functions, it treats all arguments like named ones. That provides nice
simple formatting like:
function(
argument1,
named: argument2,
argument3,
another: argument4);
I think that does a good job of highlighting which arguments are named,
which is what we want.
Fix #1072.
| /// | ||
| /// Returns a list of (start, end] indexes if found, otherwise returns `null`. | ||
| static List<int>? _contiguousFunctions(List<Expression> arguments) { | ||
| int? functionsStart; |
There was a problem hiding this comment.
This code is essentially unchanged from line 66 on. The main difference is that the old code used to use arguments[0] is NamedExpression as a clever check to see if all arguments are named. That doesn't work with named args anywhere so now it iterates the list and checks them all.
| @@ -1,3 +1,7 @@ | |||
| # 2.2.2 | |||
There was a problem hiding this comment.
How do you normally choose when to bump the feature version? Support for a new language feature could justify it?
There was a problem hiding this comment.
I'm usually pretty conservative. This isn't a breaking change and doesn't really add any new "features" (in terms of the CLI arguments or library API). Also, it doesn't change the formatting of any existing code. That sounds like a point release to me, even though it's significant in that it adds support for new syntax. But since no code using that syntax exists in the wild, from the user's perspective, there's no visible change in dart_style at all.
| final List<Expression> _positional; | ||
|
|
||
| /// The named arguments, in order, and any positional arguments that follow | ||
| /// at least one named argument. | ||
| /// The named arguments, in order. If there are any named arguments that occur |
There was a problem hiding this comment.
I opted to add support for this while being minimally invasive to the
existing style and formatting. All existing code which does not have any
positional arguments after named ones retains its previous formatting.
For new argument lists that use positional arguments after named ones,
I try to mostly follow the existing style rules even though they can be
fairly complex. In particular, it will be pretty aggressive about
applying block-style formatting to function literals inside argument
lists even with named args anywhere. I think that's important to support
the main use case I know of for the feature which is trailing positional
closures like:
In argument lists using named args anywhere that don't have block
functions, it treats all arguments like named ones. That provides nice
simple formatting like:
I think that does a good job of highlighting which arguments are named,
which is what we want.
Fix #1072.