From 2704e5d92d977abca0067382366e276236f60fc8 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 16 Dec 2024 14:19:31 -0800 Subject: [PATCH] Can send edit requests from the Property Editor (#8632) --- .../lib/src/service/editor/api_classes.dart | 3 +- .../lib/src/service/editor/editor_client.dart | 24 +++ .../property_editor_controller.dart | 36 ++++- .../property_editor/property_editor_view.dart | 138 ++++++++++++---- .../ide_shared/property_editor_test.dart | 153 +++++++++++++++++- 5 files changed, 312 insertions(+), 42 deletions(-) diff --git a/packages/devtools_app/lib/src/service/editor/api_classes.dart b/packages/devtools_app/lib/src/service/editor/api_classes.dart index 3a18e7f1851..437b2d58496 100644 --- a/packages/devtools_app/lib/src/service/editor/api_classes.dart +++ b/packages/devtools_app/lib/src/service/editor/api_classes.dart @@ -30,7 +30,8 @@ enum EditorMethod { enum LspMethod { editableArguments( methodName: 'experimental/dart/textDocument/editableArguments', - ); + ), + editArgument(methodName: 'experimental/dart/textDocument/editArgument'); const LspMethod({required this.methodName}); diff --git a/packages/devtools_app/lib/src/service/editor/editor_client.dart b/packages/devtools_app/lib/src/service/editor/editor_client.dart index 81b5a9e6942..b482eb1cf1a 100644 --- a/packages/devtools_app/lib/src/service/editor/editor_client.dart +++ b/packages/devtools_app/lib/src/service/editor/editor_client.dart @@ -6,10 +6,13 @@ import 'dart:async'; import 'package:devtools_app_shared/utils.dart'; import 'package:dtd/dtd.dart'; +import 'package:logging/logging.dart'; import '../../shared/analytics/constants.dart'; import 'api_classes.dart'; +final _log = Logger('editor_client'); + /// A client wrapper that connects to an editor over DTD. /// /// Changes made to the editor services/events should be considered carefully to @@ -246,6 +249,27 @@ class EditorClient extends DisposableController : null; } + /// Requests that the Analysis Server makes a code edit for an argument. + Future editArgument({ + required TextDocument textDocument, + required CursorPosition position, + required String name, + required T value, + }) async { + final response = await _callLspApi( + LspMethod.editArgument, + 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}'); + } + Future _call( EditorMethod method, { Map? params, 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 3e5109a6656..bd4701ebe0d 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 @@ -22,11 +22,6 @@ class PropertyEditorController extends DisposableController ValueListenable> get editableArgs => _editableArgs; final _editableArgs = ValueNotifier>([]); - @visibleForTesting - void updateEditableArgs(List args) { - _editableArgs.value = args; - } - void _init() { autoDisposeStreamSubscription( editorClient.activeLocationChangedStream.listen((event) async { @@ -45,8 +40,37 @@ class PropertyEditorController extends DisposableController position: cursorPosition, ); final args = result?.args ?? []; - updateEditableArgs(args); + _editableArgs.value = args; }), ); } + + Future editArgument({required String name, required T value}) async { + final document = _currentDocument; + final position = _currentCursorPosition; + if (document == null || position == null) return; + await editorClient.editArgument( + textDocument: document, + position: position, + name: name, + value: value, + ); + } + + @visibleForTesting + void initForTestsOnly({ + List? editableArgs, + TextDocument? document, + CursorPosition? cursorPosition, + }) { + if (editableArgs != null) { + _editableArgs.value = editableArgs; + } + if (document != null) { + _currentDocument = document; + } + if (cursorPosition != null) { + _currentCursorPosition = cursorPosition; + } + } } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart index 652fcbcba71..832c7369f53 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart @@ -50,7 +50,12 @@ class _PropertiesList extends StatelessWidget { ) : Column( children: [ - ...args.map((arg) => _EditablePropertyItem(argument: arg)), + ...args.map( + (arg) => _EditablePropertyItem( + argument: arg, + controller: controller, + ), + ), ].joinWith(const PaddedDivider.noPadding()), ); }, @@ -59,9 +64,13 @@ class _PropertiesList extends StatelessWidget { } class _EditablePropertyItem extends StatelessWidget { - const _EditablePropertyItem({required this.argument}); + const _EditablePropertyItem({ + required this.argument, + required this.controller, + }); final EditableArgument argument; + final PropertyEditorController controller; @override Widget build(BuildContext context) { @@ -72,7 +81,7 @@ class _EditablePropertyItem extends StatelessWidget { flex: 3, child: Padding( padding: const EdgeInsets.all(_PropertiesList.itemPadding), - child: _PropertyInput(argument: argument), + child: _PropertyInput(argument: argument, controller: controller), ), ), if (argument.isRequired || argument.isDefault) ...[ @@ -117,35 +126,45 @@ class _PropertyLabels extends StatelessWidget { } } -class _PropertyInput extends StatelessWidget { - const _PropertyInput({required this.argument}); +class _PropertyInput extends StatefulWidget { + const _PropertyInput({required this.argument, required this.controller}); final EditableArgument argument; + final PropertyEditorController controller; + + @override + State<_PropertyInput> createState() => _PropertyInputState(); +} + +class _PropertyInputState extends State<_PropertyInput> { + String get typeError => 'Please enter a ${widget.argument.type}.'; + + String currentValue = ''; @override Widget build(BuildContext context) { final decoration = InputDecoration( helperText: '', - errorText: argument.errorText, + errorText: widget.argument.errorText, isDense: true, - label: Text(argument.name), + label: Text(widget.argument.name), border: const OutlineInputBorder(), ); - switch (argument.type) { + switch (widget.argument.type) { case 'enum': case 'bool': final options = - argument.type == 'bool' + widget.argument.type == 'bool' ? ['true', 'false'] - : (argument.options ?? []); - options.add(argument.valueDisplay); - if (argument.isNullable) { + : (widget.argument.options ?? []); + options.add(widget.argument.valueDisplay); + if (widget.argument.isNullable) { options.add('null'); } return DropdownButtonFormField( - value: argument.valueDisplay, + value: widget.argument.valueDisplay, decoration: decoration, items: options.toSet().toList().map((option) { @@ -156,46 +175,103 @@ class _PropertyInput extends StatelessWidget { child: Text(option), ); }).toList(), - onChanged: (_) {}, + onChanged: (newValue) async { + await _editArgument(newValue); + }, ); case 'double': case 'int': case 'string': return TextFormField( - initialValue: argument.valueDisplay, - enabled: argument.isEditable, + initialValue: widget.argument.valueDisplay, + enabled: widget.argument.isEditable, autovalidateMode: AutovalidateMode.onUserInteraction, validator: _inputValidator, inputFormatters: [FilteringTextInputFormatter.singleLineFormatter], decoration: decoration, style: Theme.of(context).fixedFontStyle, // TODO(https://github.com/flutter/devtools/issues/8531) Handle onChanged. - onChanged: (_) {}, + onChanged: (newValue) { + currentValue = newValue; + }, + onEditingComplete: () async { + await _editArgument(currentValue); + }, + onTapOutside: (_) async { + await _editArgument(currentValue); + }, ); default: - return Text(argument.valueDisplay); + return Text(widget.argument.valueDisplay); } } - String? _inputValidator(String? inputValue) { - final isDouble = argument.type == 'double'; - final isInt = argument.type == 'int'; + Future _editArgument(String? valueAsString) async { + final argName = widget.argument.name; - // Only validate numeric types. - if (!isDouble && !isInt) { - return null; + // Can edit values to null. + if (widget.argument.isNullable && valueAsString == null || + (valueAsString == '' && widget.argument.type != 'string')) { + await widget.controller.editArgument(name: argName, value: null); + return; } - final validationMessage = - 'Please enter ${isInt ? 'an integer' : 'a double'}.'; - if (inputValue == null || inputValue == '') { - return validationMessage; + switch (widget.argument.type) { + case 'string': + case 'enum': + await widget.controller.editArgument( + name: argName, + value: valueAsString, + ); + break; + case 'bool': + await widget.controller.editArgument( + name: argName, + value: + valueAsString == 'true' || valueAsString == 'false' + ? valueAsString == 'true' + : valueAsString, // The boolean value might be an expression. + ); + break; + case 'double': + final numValue = _toNumber(valueAsString); + if (numValue != null) { + await widget.controller.editArgument( + name: argName, + value: numValue as double, + ); + } + break; + case 'int': + final numValue = _toNumber(valueAsString); + if (numValue != null) { + await widget.controller.editArgument( + name: argName, + value: numValue as int, + ); + } + break; } - final numValue = - isInt ? int.tryParse(inputValue) : double.tryParse(inputValue); + } + + String? _inputValidator(String? inputValue) { + final numValue = _toNumber(inputValue); if (numValue == null) { - return validationMessage; + return typeError; } return null; } + + Object? _toNumber(String? valueAsString) { + if (valueAsString == null || valueAsString == '') return null; + + final isDouble = widget.argument.type == 'double'; + final isInt = widget.argument.type == 'int'; + // Only try to convert numeric types. + if (!isDouble && !isInt) { + return null; + } + + return isInt ? int.tryParse(valueAsString) : double.tryParse(valueAsString); + } } 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 7e4ede3060b..12d64cc5c6d 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 @@ -134,7 +134,7 @@ void main() { await tester.pumpWidget(wrap(propertyEditor)); // Change the editable args. - controller.updateEditableArgs(result1.args); + controller.initForTestsOnly(editableArgs: result1.args); await tester.pumpAndSettle(); // Verify the inputs are expected. @@ -151,7 +151,7 @@ void main() { await tester.pumpWidget(wrap(propertyEditor)); // Change the editable args. - controller.updateEditableArgs(result2.args); + controller.initForTestsOnly(editableArgs: result2.args); await tester.pumpAndSettle(); // Verify the inputs are expected. @@ -167,7 +167,7 @@ void main() { await tester.pumpWidget(wrap(propertyEditor)); // Change the editable args. - controller.updateEditableArgs(result2.args); + controller.initForTestsOnly(editableArgs: result2.args); await tester.pumpAndSettle(); // Verify the input options are expected. @@ -185,7 +185,7 @@ void main() { await tester.pumpWidget(wrap(propertyEditor)); // Change the editable args. - controller.updateEditableArgs(result2.args); + controller.initForTestsOnly(editableArgs: result2.args); await tester.pumpAndSettle(); // Verify the input options are expected. @@ -208,6 +208,108 @@ void main() { ); }); }); + + group('editing arguments', () { + late Completer nextEditCompleter; + + setUp(() { + controller.initForTestsOnly( + document: textDocument1, + cursorPosition: activeCursorPosition1, + ); + + nextEditCompleter = Completer(); + when( + // ignore: discarded_futures, for mocking purposes. + mockEditorClient.editArgument( + textDocument: argThat(isNotNull, named: 'textDocument'), + position: argThat(isNotNull, named: 'position'), + name: argThat(isNotNull, named: 'name'), + value: argThat(isNotNull, named: 'value'), + ), + ).thenAnswer((realInvocation) { + final calledWithArgs = realInvocation.namedArguments; + final name = calledWithArgs[const Symbol('name')]; + final value = calledWithArgs[const Symbol('value')]; + nextEditCompleter.complete('$name: $value'); + return Future.value(); + }); + }); + + testWidgets('editing a string input (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: 'Brand New Title!', tester: tester); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('title: Brand New Title!')); + }); + }); + + testWidgets('editing a numeric input (height)', (tester) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result1.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Edit the height. + final heightInput = _findTextFormField('height'); + await _inputText(heightInput, text: '55.81', tester: tester); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('height: 55.81')); + }); + }); + + testWidgets('editing an enum input (align)', (tester) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result2.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Select the align: Alignment.topLeft option. + final alignInput = _findDropdownButtonFormField('align'); + await _selectDropdownMenuItem( + alignInput, + optionToSelect: 'Alignment.topLeft', + currentlySelected: 'Alignment.center', + tester: tester, + ); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('align: Alignment.topLeft')); + }); + }); + + testWidgets('editing a boolean input (softWrap)', (tester) async { + return await tester.runAsync(() async { + // Load the property editor. + controller.initForTestsOnly(editableArgs: result2.args); + await tester.pumpWidget(wrap(propertyEditor)); + + // Select the softWrap: false option. + final softWrapInput = _findDropdownButtonFormField('softWrap'); + await _selectDropdownMenuItem( + softWrapInput, + optionToSelect: 'false', + currentlySelected: 'true', + tester: tester, + ); + + // Verify the edit is expected. + final nextEdit = await nextEditCompleter.future; + expect(nextEdit, equals('softWrap: false')); + }); + }); + }); } final _findNoPropertiesMessage = find.text( @@ -249,6 +351,49 @@ Future _verifyDropdownMenuItems( } } +Future _selectDropdownMenuItem( + Finder dropdownButton, { + required String optionToSelect, + required String currentlySelected, + required WidgetTester tester, +}) async { + final optionToSelectFinder = find.descendant( + of: find.byType(DropdownMenuItem), + matching: find.text(optionToSelect), + ); + final currentlySelectedFinder = find.descendant( + of: find.byType(DropdownMenuItem), + matching: find.text(currentlySelected), + ); + + // Verify the option is not yet selected. + expect(currentlySelectedFinder, findsOneWidget); + expect(optionToSelectFinder, findsNothing); + + // Click button to open the options. + await tester.tap(dropdownButton); + await tester.pumpAndSettle(); + + // Click on the option. + expect(optionToSelectFinder, findsOneWidget); + await tester.tap(optionToSelectFinder); + await tester.pumpAndSettle(); + + // Verify the option is now selected. + expect(currentlySelectedFinder, findsNothing); + expect(optionToSelectFinder, findsOneWidget); +} + +Future _inputText( + Finder textFormField, { + required String text, + required WidgetTester tester, +}) async { + await tester.enterText(textFormField, text); + await tester.testTextInput.receiveAction(TextInputAction.done); + await tester.pump(); +} + // Location position 1 final activeCursorPosition1 = CursorPosition(character: 10, line: 20); final anchorCursorPosition1 = CursorPosition(character: 12, line: 7);