Skip to content

Reland ProgramExplorer #3448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/devtools_app/lib/devtools_app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export 'src/common_widgets.dart';
export 'src/connected_app.dart';
export 'src/console_service.dart';
export 'src/debugger/debugger_controller.dart';
export 'src/debugger/program_explorer_controller.dart';
export 'src/debugger/span_parser.dart';
export 'src/debugger/syntax_highlighter.dart';
export 'src/error_badge_manager.dart';
Expand Down
4 changes: 3 additions & 1 deletion packages/devtools_app/lib/src/common_widgets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ class AreaPaneHeader extends StatelessWidget implements PreferredSizeWidget {
this.needsLeftBorder = false,
this.leftActions = const [],
this.rightActions = const [],
this.leftPadding = defaultSpacing,
this.rightPadding = densePadding,
this.tall = false,
}) : super(key: key);
Expand All @@ -686,6 +687,7 @@ class AreaPaneHeader extends StatelessWidget implements PreferredSizeWidget {
final bool needsLeftBorder;
final List<Widget> leftActions;
final List<Widget> rightActions;
final double leftPadding;
final double rightPadding;
final bool tall;

Expand All @@ -704,7 +706,7 @@ class AreaPaneHeader extends StatelessWidget implements PreferredSizeWidget {
),
color: theme.titleSolidBackgroundColor,
),
padding: EdgeInsets.only(left: defaultSpacing, right: rightPadding),
padding: EdgeInsets.only(left: leftPadding, right: rightPadding),
alignment: Alignment.centerLeft,
child: Row(
children: [
Expand Down
53 changes: 43 additions & 10 deletions packages/devtools_app/lib/src/debugger/codeview.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class CodeView extends StatefulWidget {
const CodeView({
Key key,
this.controller,
this.initialPosition,
this.scriptRef,
this.parsedScript,
this.onSelected,
Expand All @@ -57,6 +58,7 @@ class CodeView extends StatefulWidget {
static double get assumedCharacterWidth => scaleByFontFactor(16.0);

final DebuggerController controller;
final ScriptLocation initialPosition;
final ScriptRef scriptRef;
final ParsedScript parsedScript;

Expand All @@ -80,6 +82,10 @@ class _CodeViewState extends State<CodeView>

ParsedScript get parsedScript => widget.parsedScript;

// Used to ensure we don't update the scroll position when expanding or
// collapsing the file explorer.
ScriptRef _lastScriptRef;

@override
void initState() {
super.initState();
Expand All @@ -88,6 +94,16 @@ class _CodeViewState extends State<CodeView>
gutterController = verticalController.addAndGet();
textController = verticalController.addAndGet();
horizontalController = ScrollController();
_lastScriptRef = widget.scriptRef;

if (widget.initialPosition != null) {
final location = widget.initialPosition.location;
// Lines are 1-indexed. Scrolling to line 1 required a scroll position of
// 0.
final lineIndex = location.line - 1;
final scrollPosition = lineIndex * CodeView.rowHeight;
verticalController.jumpTo(scrollPosition);
}

addAutoDisposeListener(
widget.controller.scriptLocation,
Expand Down Expand Up @@ -124,12 +140,8 @@ class _CodeViewState extends State<CodeView>
}

void _updateScrollPosition({bool animate = true}) {
if (widget.controller.scriptLocation.value?.scriptRef != scriptRef) {
return;
}

final location = widget.controller.scriptLocation.value?.location;
if (location?.line == null) {
if (widget.controller.scriptLocation.value?.scriptRef?.uri !=
scriptRef?.uri) {
return;
}

Expand All @@ -138,17 +150,37 @@ class _CodeViewState extends State<CodeView>
log('LinkedScrollControllerGroup has no attached controllers');
return;
}
final location = widget.controller.scriptLocation.value?.location;
if (location?.line == null) {
// Don't scroll to top if we're just rebuilding the code view for the
// same script.
if (_lastScriptRef?.uri != scriptRef?.uri) {
// Default to scrolling to the top of the script.
if (animate) {
verticalController.animateTo(
0,
duration: longDuration,
curve: defaultCurve,
);
} else {
verticalController.jumpTo(0);
}
_lastScriptRef = scriptRef;
}
return;
}

final position = verticalController.position;
final extent = position.extentInside;

// TODO(devoncarew): Adjust this so we don't scroll if we're already in the
// middle third of the screen.
if (parsedScript.lineCount * CodeView.rowHeight > extent) {
// Scroll to the middle of the screen.
final lineIndex = location.line - 1;
final scrollPosition =
lineIndex * CodeView.rowHeight - (extent - CodeView.rowHeight) / 2;
final scrollPosition = lineIndex * CodeView.rowHeight -
(widget.controller.shouldCenterScrollLocation
? ((extent - CodeView.rowHeight) / 2)
: 0);
if (animate) {
verticalController.animateTo(
scrollPosition,
Expand All @@ -159,6 +191,7 @@ class _CodeViewState extends State<CodeView>
verticalController.jumpTo(scrollPosition);
}
}
_lastScriptRef = scriptRef;
}

void _onPressed(int line) {
Expand Down Expand Up @@ -644,7 +677,7 @@ class _LinesState extends State<Lines> with AutoDisposeMixin {

if (isOutOfViewTop || isOutOfViewBottom) {
// Scroll this search token to the middle of the view.
final targetOffset = math.max(
final targetOffset = math.max<double>(
activeSearch.position.line * CodeView.rowHeight - widget.height / 2,
0.0,
);
Expand Down
6 changes: 3 additions & 3 deletions packages/devtools_app/lib/src/debugger/controls.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import '../common_widgets.dart';
import '../theme.dart';
import '../ui/label.dart';
import 'debugger_controller.dart';
import 'scripts.dart';

class DebuggingControls extends StatefulWidget {
const DebuggingControls({Key key}) : super(key: key);
Expand Down Expand Up @@ -118,13 +117,14 @@ class _DebuggingControlsState extends State<DebuggingControls>

Widget _librariesButton() {
return ValueListenableBuilder(
valueListenable: controller.librariesVisible,
valueListenable: controller.fileExplorerVisible,
builder: (context, visible, _) {
const libraryIcon = Icons.insert_chart;
return RoundedOutlinedBorder(
child: Container(
color: visible ? Theme.of(context).highlightColor : null,
child: DebuggerButton(
title: 'Libraries',
title: 'File Explorer',
icon: libraryIcon,
onPressed: controller.toggleLibrariesVisible,
),
Expand Down
55 changes: 34 additions & 21 deletions packages/devtools_app/lib/src/debugger/debugger_controller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import '../ui/search.dart';
import '../utils.dart';
import '../vm_service_wrapper.dart';
import 'debugger_model.dart';
import 'program_explorer_controller.dart';
import 'syntax_highlighter.dart';

// TODO(devoncarew): Add some delayed resume value notifiers (to be used to
Expand All @@ -33,12 +34,23 @@ class DebuggerController extends DisposableController
// `initialSwitchToIsolate` can be set to false for tests to skip the logic
// in `switchToIsolate`.
DebuggerController({this.initialSwitchToIsolate = true}) {
_programExplorerController = ProgramExplorerController(
debuggerController: this,
);
autoDispose(serviceManager.onConnectionAvailable
.listen(_handleConnectionAvailable));
_scriptHistoryListener = () {
_showScriptLocation(ScriptLocation(scriptsHistory.current.value));
};
scriptsHistory.current.addListener(_scriptHistoryListener);
addAutoDisposeListener(currentScriptRef, () {
if (!programExplorerController.initialized.value) {
programExplorerController.initialize();
}
if (currentScriptRef.value != null) {
programExplorerController.selectScriptNode(currentScriptRef.value);
}
});

if (_service != null) {
initialize();
Expand Down Expand Up @@ -119,6 +131,10 @@ class DebuggerController extends DisposableController
Map<LibraryRef, Future<Set<String>>>
libraryMemberAndImportsAutocompleteCache = {};

ProgramExplorerController get programExplorerController =>
_programExplorerController;
ProgramExplorerController _programExplorerController;

final ScriptCache _scriptCache = ScriptCache();

final ScriptsHistory scriptsHistory = ScriptsHistory();
Expand Down Expand Up @@ -160,8 +176,11 @@ class DebuggerController extends DisposableController
final _showFileOpener = ValueNotifier<bool>(false);

/// Jump to the given ScriptRef and optional SourcePosition.
void showScriptLocation(ScriptLocation scriptLocation) {
_showScriptLocation(scriptLocation);
void showScriptLocation(
ScriptLocation scriptLocation, {
bool centerLocation = true,
}) {
_showScriptLocation(scriptLocation, centerLocation: centerLocation);

// Update the scripts history (and make sure we don't react to the
// subsequent event).
Expand All @@ -172,7 +191,11 @@ class DebuggerController extends DisposableController

/// Show the given script location (without updating the script navigation
/// history).
void _showScriptLocation(ScriptLocation scriptLocation) {
void _showScriptLocation(
ScriptLocation scriptLocation, {
bool centerLocation = true,
}) {
_shouldCenterScrollLocation = centerLocation;
_currentScriptRef.value = scriptLocation?.scriptRef;

_parseCurrentScript();
Expand Down Expand Up @@ -281,6 +304,9 @@ class DebuggerController extends DisposableController
/// Return the sorted list of ScriptRefs active in the current isolate.
ValueListenable<List<ScriptRef>> get sortedScripts => _sortedScripts;

bool get shouldCenterScrollLocation => _shouldCenterScrollLocation;
bool _shouldCenterScrollLocation = true;

final _breakpoints = ValueNotifier<List<Breakpoint>>([]);

ValueListenable<List<Breakpoint>> get breakpoints => _breakpoints;
Expand All @@ -303,7 +329,7 @@ class DebuggerController extends DisposableController

final _librariesVisible = ValueNotifier(false);

ValueListenable<bool> get librariesVisible => _librariesVisible;
ValueListenable<bool> get fileExplorerVisible => _librariesVisible;

/// Make the 'Libraries' view on the right-hand side of the screen visible or
/// hidden.
Expand Down Expand Up @@ -607,7 +633,6 @@ class DebuggerController extends DisposableController

void _handleIsolateEvent(Event event) {
if (event.isolate.id != isolateRef?.id) return;

switch (event.kind) {
case EventKind.kIsolateReload:
_updateAfterIsolateReload(event);
Expand Down Expand Up @@ -862,25 +887,12 @@ class DebuggerController extends DisposableController
_populateScriptAndShowLocation(mainScriptRef);
}

SourcePosition calculatePosition(Script script, int tokenPos) {
final List<List<int>> table = script.tokenPosTable;
if (table == null) {
return null;
}

return SourcePosition(
line: script.getLineNumberFromTokenPos(tokenPos),
column: script.getColumnNumberFromTokenPos(tokenPos),
tokenPos: tokenPos,
);
}

Future<BreakpointAndSourcePosition> _createBreakpointWithLocation(
Breakpoint breakpoint) async {
if (breakpoint.resolved) {
final bp = BreakpointAndSourcePosition.create(breakpoint);
return getScript(bp.scriptRef).then((Script script) {
final pos = calculatePosition(script, bp.tokenPos);
final pos = SourcePosition.calculatePosition(script, bp.tokenPos);
return BreakpointAndSourcePosition.create(breakpoint, pos);
});
} else {
Expand All @@ -897,7 +909,8 @@ class DebuggerController extends DisposableController
}

final script = await getScript(location.script);
final position = calculatePosition(script, location.tokenPos);
final position =
SourcePosition.calculatePosition(script, location.tokenPos);
return StackFrameAndSourcePosition(frame, position: position);
}

Expand Down Expand Up @@ -1002,7 +1015,7 @@ class DebuggerController extends DisposableController
for (SourceReportRange range in report.ranges) {
if (range.possibleBreakpoints != null) {
for (int tokenPos in range.possibleBreakpoints) {
positions.add(calculatePosition(script, tokenPos));
positions.add(SourcePosition.calculatePosition(script, tokenPos));
}
}
}
Expand Down
21 changes: 19 additions & 2 deletions packages/devtools_app/lib/src/debugger/debugger_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,27 @@ class ScriptLocation {
String toString() => '${scriptRef.uri} $location';
}

/// A line, column, and an optional tokenPos.
class SourcePosition {
SourcePosition({@required this.line, @required this.column, this.tokenPos});
const SourcePosition({
@required this.line,
@required this.column,
this.file,
this.tokenPos,
});

factory SourcePosition.calculatePosition(Script script, int tokenPos) {
if (script.tokenPosTable == null) {
return null;
}

return SourcePosition(
line: script.getLineNumberFromTokenPos(tokenPos),
column: script.getColumnNumberFromTokenPos(tokenPos),
tokenPos: tokenPos,
);
}

final String file;
final int line;
final int column;
final int tokenPos;
Expand Down
Loading