Use isolateManager as source of truth for isPaused. #5018
Conversation
packages/devtools_app/lib/src/screens/debugger/debugger_screen.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/debugger/debugger_screen.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/shared/console/widgets/evaluate.dart
Outdated
Show resolved
Hide resolved
|
|
||
| /// This setter should be invoked only by debugger. | ||
| void setPausedOnBreakpoint(bool value) => _isPaused.value = value; | ||
|
|
||
| ValueListenable<Frame?> get currentFrame => _currentFrame; |
There was a problem hiding this comment.
Is it ok for us to be removing the setPausedOnBreakpoint function in a refactor?
It sounds important
There was a problem hiding this comment.
the value here _isPaused not used any more, so setter does not make sense
|
|
||
| controlHeight = _isPaused ? defaultButtonHeight : 0.0; | ||
| addAutoDisposeListener( | ||
| serviceManager.isolateManager.mainIsolateState!.isPaused, () { |
There was a problem hiding this comment.
could isMainIsolatePaused just be a notifier too?
I feel like there is a bit of a disconnect by using isMainIsolatePaused as a proxy to state inside the isolateManager, and then needing to know that the two are connected if you ever want to monitor that the state has changed.
also will _isPaused update here, if the paused state changes?
There was a problem hiding this comment.
isMainIsolatePaused is a helper method. It's value is in shortening the code. If I make it notifier, the code will increase by .value in many places.
Yes, there are pros and cons here. But, whoever would want to listen to the value, will start with reading the method body and will understand how to monitor the state.
Added documentation to decrease possible confusion.
RELEASE_NOTE_EXCEPTION=[refactoring]