Eval Console Autocomplete #3013
Conversation
| funcRef.name == clazz.name || funcRef.name.startsWith('${clazz.name}.'); | ||
|
|
||
| bool _isAccessible(String member, Class clazz) { | ||
| final frame = widget.controller.selectedStackFrame.value?.frame ?? |
There was a problem hiding this comment.
why is the ?? needed. Can we just always have the first frame be selected? Seems like that is the UI state in the debugger anyway.
There was a problem hiding this comment.
This is a bit of copied code from the evaluation logic. I'm not exactly sure why it's required but the added defensiveness seemed important.
There was a problem hiding this comment.
I would add it to the controller. Maybe "currentStackFrame".
| class _AutoCompleteController extends DisposableController | ||
| with SearchControllerMixin, AutoCompleteSearchControllerMixin {} |
There was a problem hiding this comment.
should this be public and in the search.dart file?
|
|
||
| Future<List<String>> _autoCompleteMembersFor( | ||
| ClassRef classRef, DebuggerController controller) async { | ||
| final result = <String>[]; |
There was a problem hiding this comment.
nit: make this return a Set as well as you are currently adding dupes.
| bool _validFunction(FuncRef funcRef, Class clazz) { | ||
| return !funcRef.isStatic && | ||
| !_isContructor(funcRef, clazz) && | ||
| funcRef.name != '=='; |
There was a problem hiding this comment.
need to detect other operators or you will suggest them as invalid autocompletes because the syntax is
foo+bar not foo.+bar
| final frame = controller.selectedStackFrame.value?.frame ?? | ||
| controller.stackFramesWithLocation.value.first.frame; | ||
| final currentScript = frame.location.script; | ||
| return !(member.startsWith('_') && |
There was a problem hiding this comment.
nit: I think
!member.startsWith('_') || currentScript.id == clazz.location?.script?.id
is a little easier to read.
| id: ''), | ||
| ], | ||
| functions: [ | ||
| FuncRef( |
There was a problem hiding this comment.
Add a couple constructors to make sure they are filtered out.
| } | ||
|
|
||
| Future<List<String>> _autoCompleteMembersFor( | ||
| ClassRef classRef, DebuggerController controller) async { |
| final _searchNotifier = ValueNotifier<String>(''); | ||
|
|
||
| /// Notify that the search has changed. | ||
| ValueListenable get searchNotifier => _searchNotifier; | ||
|
|
||
| bool isField = false; | ||
| bool get isField => search.endsWith('.'); |
There was a problem hiding this comment.
this seems specific to the debugger and not something that should be in a general Search mixin. Can we move this to the debugger?
| value: null, | ||
| declarationTokenPos: 0, | ||
| scopeStartTokenPos: 0, | ||
| scopeEndTokenPos: 0), |
There was a problem hiding this comment.
trailing comma for all these BoundVariable, FieldRef, Class, FuncRef, etc. instances
| @@ -0,0 +1,205 @@ | |||
| // Copyright 2021 The Chromium Authors. All rights reserved. | |||
There was a problem hiding this comment.
nit: can we rename this file to debugger_evalution_test.dart so it is clear that this test is testing functionality in the debugger
|
Merging knowing that we'll need to update the goldens one more time. Currently failing with a |
Bug fixes to the autocomplete logic:
New features:
Follow up work:
.in tracking modeAlthough not blocking, this feature requires dart-lang/webdev#1319 to be fully functional for Dart web applications.
Screen.Recording.2021-05-12.at.10.36.57.PM.mov