Skip to content

Commit e9a6d62

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Return EditableArguments in parameter order
Originally we put those with arguments first so that in the property editor they'd be at the top. However the results in the order changing if you add/remove them, and across widget instances. Returning them in source order keeps the consistent (and allows the "more important" ones to be at the top), though nothing prevents having a different sort (or an option to change it) on the client side. Change-Id: I4808f0cb2ae5fcbb6d5496aa736528e60ac7c935 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403582 Reviewed-by: Elliott Brooks <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent 5d5d223 commit e9a6d62

File tree

2 files changed

+23
-20
lines changed

2 files changed

+23
-20
lines changed

pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_editable_arguments.dart

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,23 +104,20 @@ class EditableArgumentsHandler
104104
) = invocation;
105105

106106
// Build the complete list of editable arguments.
107+
//
108+
// Arguments should be returned in the order of the parameters in the source
109+
// code. This keeps things consistent across different instances of the same
110+
// Widget class and prevents the order from changing as a user adds/removes
111+
// arguments.
112+
//
113+
// If an editor wants to sort provided arguments first (and keep these stable
114+
// across add/removes) it could still do so client-side, whereas if server
115+
// orders them that way, the opposite (using source-order) is not possible.
107116
var editableArguments = [
108-
// First arguments that exist in the order they were specified.
109-
for (var MapEntry(key: parameter, value: argument)
110-
in parameterArguments.entries)
117+
for (var parameter in parameters)
111118
_toEditableArgument(
112119
parameter,
113-
argument,
114-
numPositionals: numPositionals,
115-
numSuppliedPositionals: numSuppliedPositionals,
116-
),
117-
// Then the remaining parameters that don't have existing arguments.
118-
for (var parameter in parameters.where(
119-
(p) => !parameterArguments.containsKey(p),
120-
))
121-
_toEditableArgument(
122-
parameter,
123-
null,
120+
parameterArguments[parameter],
124121
positionalIndex: positionalParameterIndexes[parameter],
125122
numPositionals: numPositionals,
126123
numSuppliedPositionals: numSuppliedPositionals,

pkg/analysis_server/test/lsp/editable_arguments_test.dart

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,10 @@ class MyWidget extends StatelessWidget {
137137
expect(
138138
result,
139139
hasArgs(
140-
orderedEquals([
140+
unorderedEquals([
141141
isArg('aPositionalSupplied', hasArgument: true),
142-
isArg('aNamedSupplied', hasArgument: true),
143142
isArg('aPositionalNotSupplied', hasArgument: false),
143+
isArg('aNamedSupplied', hasArgument: true),
144144
isArg('aNamedNotSupplied', hasArgument: false),
145145
]),
146146
),
@@ -727,8 +727,14 @@ class MyWidget extends StatelessWidget {
727727
expect(result, hasArgNamed('a1'));
728728
}
729729

730-
/// Arguments should be returned in the order at the call site followed by
731-
/// by the unspecified parameters.
730+
/// Arguments should be returned in the order of the parameters in the source
731+
/// code. This keeps things consistent across different instances of the same
732+
/// Widget class and prevents the order from changing as a user adds/removes
733+
/// arguments.
734+
///
735+
/// If an editor wants to sort provided arguments first (and keep these stable
736+
/// across add/removes) it could still do so client-side, whereas if server
737+
/// orders them that way, the opposite (using source-order) is not possible.
732738
test_order() async {
733739
var result = await getEditableArgumentsFor('''
734740
class MyWidget extends StatelessWidget {
@@ -749,11 +755,11 @@ class MyWidget extends StatelessWidget {
749755
result,
750756
hasArgs(
751757
orderedEquals([
752-
isArg('b1'),
753-
isArg('a1'),
754758
isArg('c1'),
755759
isArg('c2'),
760+
isArg('a1'),
756761
isArg('a2'),
762+
isArg('b1'),
757763
isArg('b2'),
758764
]),
759765
),

0 commit comments

Comments
 (0)