From 93737c2b222340fd108cafb3da2139f34e76ac0f Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 28 Jan 2025 15:41:41 -0800 Subject: [PATCH 1/7] Property editor error handling --- .../lib/src/shared/editor/api_classes.dart | 50 +++++++++ .../lib/src/shared/editor/editor_client.dart | 49 ++++++--- .../property_editor_controller.dart | 9 +- .../property_editor_inputs.dart | 101 +++++++++++++++--- packages/devtools_app/pubspec.yaml | 1 + 5 files changed, 176 insertions(+), 34 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/editor/api_classes.dart b/packages/devtools_app/lib/src/shared/editor/api_classes.dart index 6efde9348fc..72cfeff63e9 100644 --- a/packages/devtools_app/lib/src/shared/editor/api_classes.dart +++ b/packages/devtools_app/lib/src/shared/editor/api_classes.dart @@ -442,6 +442,56 @@ class EditableArgumentsResult with Serializable { Map toJson() => {Field.arguments: args}; } +/// Errors that the Analysis Server returns for failed argument edits. +/// +/// These should be kept in sync with those listed in: +/// pkg/analysis_server/lib/src/lsp/constants.dart +enum EditArgumentError { + /// A request was made that requires use of workspace/applyEdit but the + /// current editor does not support it. + editsUnsupportedByEditor(-32016), + + /// An editArgument request tried to modify an invocation at a position where + /// there was no invocation. + editArgumentInvalidPosition(-32017), + + /// An editArgument request tried to modify a parameter that does not exist or + /// is not editable. + editArgumentInvalidParameter(-32018), + + /// An editArgument request tried to set an argument value that is not valid. + editArgumentInvalidValue(-32019); + + const EditArgumentError(this.code); + + final int code; + + static final _codeToErrorMap = EditArgumentError.values.fold( + {}, + (map, error) { + map[error.code] = error; + return map; + }, + ); + + static EditArgumentError? fromCode(int? code) { + if (code == null) return null; + return _codeToErrorMap[code]; + } +} + +/// Response to an edit argument request. +class EditArgumentResponse { + EditArgumentResponse({required this.success, this.errorMessage, errorCode}) + : _errorCode = errorCode; + + final bool success; + final String? errorMessage; + final int? _errorCode; + + EditArgumentError? get errorType => EditArgumentError.fromCode(_errorCode); +} + /// Information about a single editable argument of a widget. class EditableArgument with Serializable { EditableArgument({ diff --git a/packages/devtools_app/lib/src/shared/editor/editor_client.dart b/packages/devtools_app/lib/src/shared/editor/editor_client.dart index 65df1065b19..3d3198e264f 100644 --- a/packages/devtools_app/lib/src/shared/editor/editor_client.dart +++ b/packages/devtools_app/lib/src/shared/editor/editor_client.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:devtools_app_shared/utils.dart'; import 'package:dtd/dtd.dart'; import 'package:flutter/foundation.dart'; +import 'package:json_rpc_2/json_rpc_2.dart'; import 'package:logging/logging.dart'; import '../analytics/constants.dart'; @@ -274,26 +275,46 @@ class EditorClient extends DisposableController } /// Requests that the Analysis Server makes a code edit for an argument. - Future editArgument({ + Future editArgument({ required TextDocument textDocument, required CursorPosition position, required String name, required T value, }) async { final method = editArgumentMethodName.value; - if (method == null) return; - final response = await _callLspApi( - method, - params: { - 'type': 'Object', // This is required by DTD. - 'textDocument': textDocument.toJson(), - 'position': position.toJson(), - 'edit': {'name': name, 'newValue': value}, - }, - ); - // TODO(elliette): Handle response, currently the response from the Analysis - // Server is null. - _log.info('editArgument response: ${response.result}'); + if (method == null) { + return EditArgumentResponse( + success: false, + errorMessage: 'API is unavailable.', + ); + } + try { + await _callLspApi( + method, + params: { + 'type': 'Object', // This is required by DTD. + 'textDocument': textDocument.toJson(), + 'position': position.toJson(), + 'edit': {'name': name, 'newValue': value}, + }, + ); + return EditArgumentResponse(success: true); + } on RpcException catch (e) { + final errorMessage = e.message; + _log.severe(errorMessage); + return EditArgumentResponse( + success: false, + errorCode: e.code, + errorMessage: errorMessage, + ); + } catch (e) { + final errorMessage = 'Unknown error: $e'; + _log.severe(errorMessage); + return EditArgumentResponse( + success: false, + errorMessage: 'Unknown error: $e', + ); + } } Future _call( diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart index 1c991ea1568..0925099d686 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart @@ -49,11 +49,14 @@ class PropertyEditorController extends DisposableController ); } - Future editArgument({required String name, required T value}) async { + Future editArgument({ + required String name, + required T value, + }) async { final document = _currentDocument; final position = _currentCursorPosition; - if (document == null || position == null) return; - await editorClient.editArgument( + if (document == null || position == null) return null; + return editorClient.editArgument( textDocument: document, position: position, name: name, diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index 0a6fa25dde7..74301b4c570 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -6,6 +6,7 @@ import 'package:devtools_app_shared/ui.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import '../../../shared/editor/api_classes.dart'; import 'property_editor_controller.dart'; import 'property_editor_types.dart'; @@ -89,21 +90,37 @@ class StringInput extends StatelessWidget { } } -class _DropdownInput extends StatelessWidget with _PropertyInputMixin { - _DropdownInput({super.key, required this.property, required this.controller}); +class _DropdownInput extends StatefulWidget { + const _DropdownInput({ + super.key, + required this.property, + required this.controller, + }); final FiniteValuesProperty property; final PropertyEditorController controller; + @override + State<_DropdownInput> createState() => _DropdownInputState(); +} + +class _DropdownInputState extends State<_DropdownInput> + with _PropertyInputMixin<_DropdownInput, T> { @override Widget build(BuildContext context) { final theme = Theme.of(context); return DropdownButtonFormField( - value: property.valueDisplay, - decoration: decoration(property, theme: theme, padding: denseSpacing), + value: widget.property.valueDisplay, + autovalidateMode: AutovalidateMode.onUserInteraction, + validator: (text) => inputValidator(text, property: widget.property), + decoration: decoration( + widget.property, + theme: theme, + padding: denseSpacing, + ), isExpanded: true, items: - property.propertyOptions.map((option) { + widget.property.propertyOptions.map((option) { return DropdownMenuItem( value: option, child: Text(option, style: theme.fixedFontStyle), @@ -111,26 +128,31 @@ class _DropdownInput extends StatelessWidget with _PropertyInputMixin { }).toList(), onChanged: (newValue) async { await editProperty( - property, + widget.property, valueAsString: newValue, - controller: controller, + controller: widget.controller, ); }, ); } } -class _TextInput extends StatefulWidget with _PropertyInputMixin { - _TextInput({super.key, required this.property, required this.controller}); +class _TextInput extends StatefulWidget { + const _TextInput({ + super.key, + required this.property, + required this.controller, + }); final EditableProperty property; final PropertyEditorController controller; @override - State<_TextInput> createState() => _TextInputState(); + State<_TextInput> createState() => _TextInputState(); } -class _TextInputState extends State<_TextInput> { +class _TextInputState extends State<_TextInput> + with _PropertyInputMixin<_TextInput, T> { String currentValue = ''; double paddingDiffComparedToDropdown = 1.0; @@ -142,9 +164,9 @@ class _TextInputState extends State<_TextInput> { initialValue: widget.property.valueDisplay, enabled: widget.property.isEditable, autovalidateMode: AutovalidateMode.onUserInteraction, - validator: widget.property.inputValidator, + validator: (text) => inputValidator(text, property: widget.property), inputFormatters: [FilteringTextInputFormatter.singleLineFormatter], - decoration: widget.decoration( + decoration: decoration( widget.property, theme: theme, // Note: The text input has an extra pixel compared to the dropdown @@ -154,6 +176,7 @@ class _TextInputState extends State<_TextInput> { ), style: theme.fixedFontStyle, onChanged: (newValue) { + clearServerError(); setState(() { currentValue = newValue; }); @@ -166,7 +189,7 @@ class _TextInputState extends State<_TextInput> { } Future _editProperty() async { - await widget.editProperty( + await editProperty( widget.property, valueAsString: currentValue, controller: widget.controller, @@ -174,12 +197,15 @@ class _TextInputState extends State<_TextInput> { } } -mixin _PropertyInputMixin { +mixin _PropertyInputMixin on State { + String? _serverError; + Future editProperty( EditableProperty property, { required PropertyEditorController controller, required String? valueAsString, }) async { + clearServerError(); final argName = property.name; // Can edit values to null. @@ -188,8 +214,9 @@ mixin _PropertyInputMixin { return; } - final value = property.convertFromInputString(valueAsString) as T?; - await controller.editArgument(name: argName, value: value); + final value = property.convertFromInputString(valueAsString) as U?; + final response = await controller.editArgument(name: argName, value: value); + _maybeHandleServerError(response, property: property); } InputDecoration decoration( @@ -231,4 +258,44 @@ mixin _PropertyInputMixin { ), ); } + + String? inputValidator(String? input, {required EditableProperty property}) { + if (_serverError != null) return _serverError; + return property.inputValidator(input); + } + + void clearServerError() { + setState(() { + _serverError = null; + }); + } + + void _maybeHandleServerError( + EditArgumentResponse? errorResponse, { + required EditableProperty property, + }) { + if (errorResponse == null || errorResponse.success) return; + setState(() { + _serverError = _errorMessage(errorResponse.errorType, property: property); + }); + } + + String _errorMessage( + EditArgumentError? errorType, { + required EditableProperty property, + }) { + final propertyName = property.name; + switch (errorType) { + case EditArgumentError.editArgumentInvalidParameter: + return 'Invalid parameter $propertyName.'; + case EditArgumentError.editArgumentInvalidPosition: + return 'Invalid position for parameter $propertyName.'; + case EditArgumentError.editArgumentInvalidValue: + return 'Invalid value for parameter $propertyName.'; + case EditArgumentError.editsUnsupportedByEditor: + return 'IDE does not support property edits.'; + default: + return 'Encountered unknown error editing ${property.name}.'; + } + } } diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index f8d087d8e5a..8640bccbf98 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -37,6 +37,7 @@ dependencies: http: ^1.1.0 image: ^4.1.3 intl: ^0.19.0 + json_rpc_2: ^3.0.0 logging: ^1.1.1 meta: ^1.9.1 mime: ^2.0.0 From f58c474068b1bc7983b19c7fb76dbaf741e90022 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 28 Jan 2025 16:08:40 -0800 Subject: [PATCH 2/7] Add a test case --- .../property_editor_inputs.dart | 6 +- .../ide_shared/property_editor_test.dart | 66 ++++++++++++++++--- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index 74301b4c570..a9eb893984d 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -287,11 +287,11 @@ mixin _PropertyInputMixin on State { final propertyName = property.name; switch (errorType) { case EditArgumentError.editArgumentInvalidParameter: - return 'Invalid parameter $propertyName.'; + return 'Invalid parameter: $propertyName.'; case EditArgumentError.editArgumentInvalidPosition: - return 'Invalid position for parameter $propertyName.'; + return 'Invalid position for parameter: $propertyName.'; case EditArgumentError.editArgumentInvalidValue: - return 'Invalid value for parameter $propertyName.'; + return 'Invalid value for parameter: $propertyName.'; case EditArgumentError.editsUnsupportedByEditor: return 'IDE does not support property edits.'; default: diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart index 3cdd7034dec..ba61595245f 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart @@ -243,6 +243,9 @@ void main() { group('editing arguments', () { late Completer nextEditCompleter; + // A fake argument that the server can't support. + const fakeBogusArgument = 'fakeBogusArgument'; + setUp(() { controller.initForTestsOnly( document: textDocument1, @@ -262,10 +265,17 @@ void main() { final calledWithArgs = realInvocation.namedArguments; final name = calledWithArgs[const Symbol('name')]; final value = calledWithArgs[const Symbol('value')]; + final success = value != fakeBogusArgument; nextEditCompleter.complete( - '$name: $value (TYPE: ${value?.runtimeType ?? 'null'})', + '$name: $value (TYPE: ${value?.runtimeType ?? 'null'}, SUCCESS: $success)', + ); + + return Future.value( + EditArgumentResponse( + success: success, + errorCode: success ? null : -32019, + ), ); - return Future.value(); }); }); @@ -282,7 +292,10 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('title: Brand New Title! (TYPE: String)')); + expect( + nextEdit, + equals('title: Brand New Title! (TYPE: String, SUCCESS: true)'), + ); }); }); @@ -298,7 +311,7 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('title: null (TYPE: null)')); + expect(nextEdit, equals('title: null (TYPE: null, SUCCESS: true)')); }); }); @@ -320,12 +333,42 @@ void main() { nextEdit, equals( 'title: ' - ' (TYPE: String)', + ' (TYPE: String, SUCCESS: true)', ), ); }); }); + testWidgets('editing a string input to an invalid parameter (title)', ( + tester, + ) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result1.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Edit the title. + final titleInput = _findTextFormField('title'); + await _inputText(titleInput, text: fakeBogusArgument, tester: tester); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect( + nextEdit, + equals( + 'title: fakeBogusArgument' + ' (TYPE: String, SUCCESS: false)', + ), + ); + + await tester.pumpAndSettle(); + expect( + find.textContaining('Invalid value for parameter: title.'), + findsOneWidget, + ); + }); + }); + testWidgets('editing a numeric input (height)', (tester) async { return await tester.runAsync(() async { // Load the property editor. @@ -338,7 +381,7 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('height: 55.81 (TYPE: double)')); + expect(nextEdit, equals('height: 55.81 (TYPE: double, SUCCESS: true)')); }); }); @@ -354,7 +397,7 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('height: null (TYPE: null)')); + expect(nextEdit, equals('height: null (TYPE: null, SUCCESS: true)')); }); }); @@ -375,7 +418,10 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('align: Alignment.topLeft (TYPE: String)')); + expect( + nextEdit, + equals('align: Alignment.topLeft (TYPE: String, SUCCESS: true)'), + ); }); }); @@ -396,7 +442,7 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('align: null (TYPE: null)')); + expect(nextEdit, equals('align: null (TYPE: null, SUCCESS: true)')); }); }); @@ -417,7 +463,7 @@ void main() { // Verify the edit is expected. final nextEdit = await nextEditCompleter.future; - expect(nextEdit, equals('softWrap: false (TYPE: bool)')); + expect(nextEdit, equals('softWrap: false (TYPE: bool, SUCCESS: true)')); }); }); }); From c1394bfd9eddf9877485019e5afad7653a3f8d61 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 29 Jan 2025 10:22:00 -0800 Subject: [PATCH 3/7] Respond to PR comments --- .../lib/src/shared/editor/api_classes.dart | 20 +++++++++---- .../property_editor_inputs.dart | 29 +++---------------- packages/devtools_app/pubspec.yaml | 3 +- 3 files changed, 20 insertions(+), 32 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/editor/api_classes.dart b/packages/devtools_app/lib/src/shared/editor/api_classes.dart index 72cfeff63e9..8d66f94a554 100644 --- a/packages/devtools_app/lib/src/shared/editor/api_classes.dart +++ b/packages/devtools_app/lib/src/shared/editor/api_classes.dart @@ -449,22 +449,32 @@ class EditableArgumentsResult with Serializable { enum EditArgumentError { /// A request was made that requires use of workspace/applyEdit but the /// current editor does not support it. - editsUnsupportedByEditor(-32016), + editsUnsupportedByEditor(code: -32016, message: 'Invalid argument.'), /// An editArgument request tried to modify an invocation at a position where /// there was no invocation. - editArgumentInvalidPosition(-32017), + editArgumentInvalidPosition( + code: -32017, + message: 'Invalid position for argument.', + ), /// An editArgument request tried to modify a parameter that does not exist or /// is not editable. - editArgumentInvalidParameter(-32018), + editArgumentInvalidParameter( + code: -32018, + message: 'Invalid value for argument.', + ), /// An editArgument request tried to set an argument value that is not valid. - editArgumentInvalidValue(-32019); + editArgumentInvalidValue( + code: -32019, + message: 'IDE does not support property edits.', + ); - const EditArgumentError(this.code); + const EditArgumentError({required this.code, required this.message}); final int code; + final String message; static final _codeToErrorMap = EditArgumentError.values.fold( {}, diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index a9eb893984d..cdcd6a5f1b8 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -216,7 +216,7 @@ mixin _PropertyInputMixin on State { final value = property.convertFromInputString(valueAsString) as U?; final response = await controller.editArgument(name: argName, value: value); - _maybeHandleServerError(response, property: property); + _maybeHandleServerError(response); } InputDecoration decoration( @@ -270,32 +270,11 @@ mixin _PropertyInputMixin on State { }); } - void _maybeHandleServerError( - EditArgumentResponse? errorResponse, { - required EditableProperty property, - }) { + void _maybeHandleServerError(EditArgumentResponse? errorResponse) { if (errorResponse == null || errorResponse.success) return; setState(() { - _serverError = _errorMessage(errorResponse.errorType, property: property); + _serverError = + errorResponse.errorType?.message ?? 'Encountered unknown error.'; }); } - - String _errorMessage( - EditArgumentError? errorType, { - required EditableProperty property, - }) { - final propertyName = property.name; - switch (errorType) { - case EditArgumentError.editArgumentInvalidParameter: - return 'Invalid parameter: $propertyName.'; - case EditArgumentError.editArgumentInvalidPosition: - return 'Invalid position for parameter: $propertyName.'; - case EditArgumentError.editArgumentInvalidValue: - return 'Invalid value for parameter: $propertyName.'; - case EditArgumentError.editsUnsupportedByEditor: - return 'IDE does not support property edits.'; - default: - return 'Encountered unknown error editing ${property.name}.'; - } - } } diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 8640bccbf98..a8e13e64e35 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -37,7 +37,7 @@ dependencies: http: ^1.1.0 image: ^4.1.3 intl: ^0.19.0 - json_rpc_2: ^3.0.0 + json_rpc_2: ^3.0.2 logging: ^1.1.1 meta: ^1.9.1 mime: ^2.0.0 @@ -70,7 +70,6 @@ dev_dependencies: sdk: flutter integration_test: sdk: flutter - json_rpc_2: ^3.0.2 mockito: ^5.4.1 stager: ^1.0.1 stream_channel: ^2.1.1 From 57089dd38e9d52f9bc137bd74ed57380cb7d82cd Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 29 Jan 2025 10:30:40 -0800 Subject: [PATCH 4/7] Fix error codes and update test --- .../lib/src/shared/editor/api_classes.dart | 12 ++++++------ .../ide_shared/property_editor_test.dart | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/editor/api_classes.dart b/packages/devtools_app/lib/src/shared/editor/api_classes.dart index 8d66f94a554..928a09b5a3a 100644 --- a/packages/devtools_app/lib/src/shared/editor/api_classes.dart +++ b/packages/devtools_app/lib/src/shared/editor/api_classes.dart @@ -449,7 +449,10 @@ class EditableArgumentsResult with Serializable { enum EditArgumentError { /// A request was made that requires use of workspace/applyEdit but the /// current editor does not support it. - editsUnsupportedByEditor(code: -32016, message: 'Invalid argument.'), + editsUnsupportedByEditor( + code: -32016, + message: 'IDE does not support property edits.', + ), /// An editArgument request tried to modify an invocation at a position where /// there was no invocation. @@ -460,15 +463,12 @@ enum EditArgumentError { /// An editArgument request tried to modify a parameter that does not exist or /// is not editable. - editArgumentInvalidParameter( - code: -32018, - message: 'Invalid value for argument.', - ), + editArgumentInvalidParameter(code: -32018, message: 'Invalid parameter.'), /// An editArgument request tried to set an argument value that is not valid. editArgumentInvalidValue( code: -32019, - message: 'IDE does not support property edits.', + message: 'Invalid value for parameter.', ); const EditArgumentError({required this.code, required this.message}); diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart index ba61595245f..ff4d2f0b9da 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart @@ -363,7 +363,7 @@ void main() { await tester.pumpAndSettle(); expect( - find.textContaining('Invalid value for parameter: title.'), + find.textContaining('Invalid value for parameter.'), findsOneWidget, ); }); From ff72e62c48ae9c5efcc91cbd16701897d0161176 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 29 Jan 2025 10:46:59 -0800 Subject: [PATCH 5/7] Increase bundle size in test --- packages/devtools_app/benchmark/web_bundle_size_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/benchmark/web_bundle_size_test.dart b/packages/devtools_app/benchmark/web_bundle_size_test.dart index 9490e535a4b..3318c4cdc1f 100644 --- a/packages/devtools_app/benchmark/web_bundle_size_test.dart +++ b/packages/devtools_app/benchmark/web_bundle_size_test.dart @@ -12,7 +12,7 @@ import 'package:test/test.dart'; // Benchmark size in kB. const bundleSizeBenchmark = 5400; -const gzipBundleSizeBenchmark = 1550; +const gzipBundleSizeBenchmark = 1650; void main() { group('Web Compile', () { From 8bb823a51bbfc719d38c4c04b3db25326bd4d74c Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 29 Jan 2025 13:17:28 -0800 Subject: [PATCH 6/7] Add TODOs --- .../lib/src/shared/editor/api_classes.dart | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/shared/editor/api_classes.dart b/packages/devtools_app/lib/src/shared/editor/api_classes.dart index 928a09b5a3a..3b6c53d310d 100644 --- a/packages/devtools_app/lib/src/shared/editor/api_classes.dart +++ b/packages/devtools_app/lib/src/shared/editor/api_classes.dart @@ -27,6 +27,9 @@ enum EditorMethod { /// [pkg/analysis_server/lib/src/lsp/constants.dart][code link] /// /// [code link]: https://github.com/dart-lang/sdk/blob/ebfcd436da65802a2b20d415afe600b51e432305/pkg/analysis_server/lib/src/lsp/constants.dart#L136 +/// +/// TODO(https://github.com/flutter/devtools/issues/8824): Add tests that these +/// are in-sync with analysis_server. enum LspMethod { editableArguments(methodName: 'dart/textDocument/editableArguments'), editArgument(methodName: 'dart/textDocument/editArgument'); @@ -444,8 +447,13 @@ class EditableArgumentsResult with Serializable { /// Errors that the Analysis Server returns for failed argument edits. /// -/// These should be kept in sync with those listed in: -/// pkg/analysis_server/lib/src/lsp/constants.dart +/// These should be kept in sync with the error coes defined at +/// [pkg/analysis_server/lib/src/lsp/constants.dart][code link] +/// +/// [code link]: https://github.com/dart-lang/sdk/blob/35a10987e1652b7d49991ab2dc2ee7f521fe8d8f/pkg/analysis_server/lib/src/lsp/constants.dart#L300 +/// +/// TODO(https://github.com/flutter/devtools/issues/8824): Add tests that these +/// are in-sync with analysis_server. enum EditArgumentError { /// A request was made that requires use of workspace/applyEdit but the /// current editor does not support it. From bf86128f7a74d775823bd2dae8db6c5644320171 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 30 Jan 2025 14:47:09 -0800 Subject: [PATCH 7/7] Add property name to error --- .../property_editor/property_editor_inputs.dart | 9 ++++++--- .../standalone_ui/ide_shared/property_editor_test.dart | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index cdcd6a5f1b8..0c59c2f3efb 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -216,7 +216,7 @@ mixin _PropertyInputMixin on State { final value = property.convertFromInputString(valueAsString) as U?; final response = await controller.editArgument(name: argName, value: value); - _maybeHandleServerError(response); + _maybeHandleServerError(response, property: property); } InputDecoration decoration( @@ -270,11 +270,14 @@ mixin _PropertyInputMixin on State { }); } - void _maybeHandleServerError(EditArgumentResponse? errorResponse) { + void _maybeHandleServerError( + EditArgumentResponse? errorResponse, { + required EditableProperty property, + }) { if (errorResponse == null || errorResponse.success) return; setState(() { _serverError = - errorResponse.errorType?.message ?? 'Encountered unknown error.'; + '${errorResponse.errorType?.message ?? 'Encountered unknown error.'} (Property: ${property.name})'; }); } } diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart index ff4d2f0b9da..9bc2941d95e 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor_test.dart @@ -363,7 +363,7 @@ void main() { await tester.pumpAndSettle(); expect( - find.textContaining('Invalid value for parameter.'), + find.textContaining('Invalid value for parameter. (Property: title)'), findsOneWidget, ); });