Skip to content

Commit b134916

Browse files
bkonyibkonyi
committed
Address comments
1 parent 590b5c1 commit b134916

File tree

10 files changed

+215
-136
lines changed

10 files changed

+215
-136
lines changed

‎packages/devtools_app/lib/src/screens/debugger/codeview.dart

Lines changed: 199 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ class CodeView extends StatefulWidget {
6767
Key('debuggerCodeViewVerticalScrollbarKey');
6868

6969
static double get rowHeight => scaleByFontFactor(20.0);
70-
static double get assumedCharacterWidth => scaleByFontFactor(16.0);
7170

7271
final CodeViewController codeViewController;
7372
final DebuggerController? debuggerController;
@@ -116,6 +115,9 @@ class _CodeViewState extends State<CodeView>
116115
verticalController = LinkedScrollControllerGroup();
117116
gutterController = verticalController.addAndGet();
118117
textController = verticalController.addAndGet();
118+
if (widget.codeViewController.showProfileInformation.value) {
119+
profileController = verticalController.addAndGet();
120+
}
119121
horizontalController = ScrollController();
120122
_lastScriptRef = widget.scriptRef;
121123

@@ -238,14 +240,6 @@ class _CodeViewState extends State<CodeView>
238240
_lastScriptRef = scriptRef;
239241
}
240242

241-
void _onPressed(int line) {
242-
final onSelected = widget.onSelected;
243-
final script = scriptRef;
244-
if (onSelected != null && script != null) {
245-
onSelected(script, line);
246-
}
247-
}
248-
249243
@override
250244
Widget build(BuildContext context) {
251245
if (parsedScript == null) {
@@ -334,13 +328,6 @@ class _CodeViewState extends State<CodeView>
334328
}
335329
}
336330

337-
// Apply the log change-of-base formula, then add 16dp padding for every
338-
// digit in the maximum number of lines.
339-
final gutterWidth = CodeView.assumedCharacterWidth * 1.5 +
340-
CodeView.assumedCharacterWidth *
341-
(defaultEpsilon + math.log(math.max(lines.length, 100)) / math.ln10)
342-
.truncateToDouble();
343-
344331
_updateScrollPosition(animate: false);
345332

346333
final contentBuilder = (context, ScriptRef? script) {
@@ -364,60 +351,24 @@ class _CodeViewState extends State<CodeView>
364351
final pausedFrame = frame == null
365352
? null
366353
: (frame.scriptRef == scriptRef ? frame : null);
367-
final currentParsedScript = parsedScript;
368-
final lineCount = widget.lineRange?.size ?? lines.length;
369-
final lineOffset = (widget.lineRange?.begin ?? 1) - 1;
370-
final sourceReport = currentParsedScript?.sourceReport ??
371-
const ProcessedSourceReport.empty();
372354
return Row(
373355
children: [
374-
DualValueListenableBuilder<
375-
List<BreakpointAndSourcePosition>, bool>(
376-
firstListenable:
377-
breakpointManager.breakpointsWithLocation,
378-
secondListenable:
379-
widget.codeViewController.showCodeCoverage,
380-
builder: (context, breakpoints, showCodeCoverage, _) {
381-
return Gutter(
382-
gutterWidth: gutterWidth,
383-
scrollController: gutterController,
384-
lineCount: lineCount,
385-
lineOffset: lineOffset,
386-
pausedFrame: pausedFrame,
387-
breakpoints: breakpoints
388-
.where((bp) => bp.scriptRef == scriptRef)
389-
.toList(),
390-
executableLines:
391-
currentParsedScript?.executableLines ??
392-
const <int>{},
393-
sourceReport: sourceReport,
394-
onPressed: _onPressed,
395-
// Disable dots for possible breakpoint locations.
396-
allowInteraction:
397-
!(widget.debuggerController?.isSystemIsolate ??
398-
false),
399-
showCodeCoverage: showCodeCoverage,
400-
);
401-
},
402-
),
403-
const SizedBox(width: denseSpacing),
404356
ValueListenableBuilder<bool>(
405357
valueListenable:
406358
widget.codeViewController.showProfileInformation,
407359
builder: (context, showProfileInformation, _) {
408-
if (!showProfileInformation) {
409-
return const SizedBox();
410-
}
411-
return Row(
412-
children: [
413-
ProfileInformationGutter(
414-
scrollController: profileController!,
415-
lineCount: lineCount,
416-
lineOffset: lineOffset,
417-
sourceReport: sourceReport,
418-
),
419-
const SizedBox(width: denseSpacing),
420-
],
360+
return Gutters(
361+
scriptRef: script,
362+
gutterController: gutterController,
363+
profileController: profileController,
364+
codeViewController: widget.codeViewController,
365+
debuggerController: widget.debuggerController,
366+
lines: lines,
367+
lineRange: widget.lineRange,
368+
onSelected: widget.onSelected,
369+
pausedFrame: pausedFrame,
370+
parsedScript: parsedScript,
371+
showProfileInformation: showProfileInformation,
421372
);
422373
},
423374
),
@@ -603,71 +554,98 @@ class ProfileInformationGutter extends StatelessWidget {
603554

604555
@override
605556
Widget build(BuildContext context) {
606-
final theme = Theme.of(context);
607-
final gutterWidth = CodeView.assumedCharacterWidth * 10 + denseSpacing;
608-
return Container(
609-
width: gutterWidth,
610-
decoration: BoxDecoration(
611-
border: Border(right: defaultBorderSide(theme)),
612-
color: Theme.of(context).titleSolidBackgroundColor,
557+
// Gutter width accounts for:
558+
// - a maximum of 16 characters of text (e.g., '100.00 %' x 2)
559+
// - Spacing for the vertical divider
560+
final gutterWidth = assumedMonospaceCharacterWidth * 16 + denseSpacing;
561+
return OutlineDecoration.onlyRight(
562+
child: Container(
563+
width: gutterWidth,
564+
decoration: BoxDecoration(
565+
color: Theme.of(context).titleSolidBackgroundColor,
566+
),
567+
child: Stack(
568+
children: [
569+
Column(
570+
children: [
571+
const _ProfileInformationGutterHeader(
572+
totalTimeTooltip: totalTimeTooltip,
573+
selfTimeTooltip: selfTimeTooltip,
574+
),
575+
Expanded(
576+
child: ListView.builder(
577+
controller: scrollController,
578+
itemExtent: CodeView.rowHeight,
579+
itemCount: lineCount,
580+
itemBuilder: (context, index) {
581+
final lineNum = lineOffset + index + 1;
582+
final data = sourceReport.profilerEntries[lineNum];
583+
if (data == null) {
584+
return const SizedBox();
585+
}
586+
return ProfileInformationGutterItem(
587+
lineNumber: lineNum,
588+
profilerData: data,
589+
);
590+
},
591+
),
592+
),
593+
],
594+
),
595+
const Center(
596+
child: VerticalDivider(),
597+
),
598+
],
599+
),
613600
),
614-
child: Stack(
601+
);
602+
}
603+
}
604+
605+
class _ProfileInformationGutterHeader extends StatelessWidget {
606+
const _ProfileInformationGutterHeader({
607+
required this.totalTimeTooltip,
608+
required this.selfTimeTooltip,
609+
});
610+
611+
final String totalTimeTooltip;
612+
final String selfTimeTooltip;
613+
614+
@override
615+
Widget build(BuildContext context) {
616+
return Container(
617+
height: CodeView.rowHeight,
618+
child: Column(
615619
children: [
616-
Column(
617-
children: [
618-
Container(
619-
height: CodeView.rowHeight,
620-
child: Row(
621-
crossAxisAlignment: CrossAxisAlignment.stretch,
622-
mainAxisAlignment: MainAxisAlignment.spaceAround,
623-
children: const [
624-
Expanded(
625-
child: DevToolsTooltip(
626-
message: totalTimeTooltip,
627-
child: Text(
628-
'Total %',
629-
textAlign: TextAlign.center,
630-
),
631-
),
632-
),
633-
SizedBox(width: denseSpacing),
634-
Expanded(
635-
child: DevToolsTooltip(
636-
message: selfTimeTooltip,
637-
child: Text(
638-
'Self %',
639-
textAlign: TextAlign.center,
640-
),
641-
),
620+
Expanded(
621+
child: Row(
622+
crossAxisAlignment: CrossAxisAlignment.stretch,
623+
mainAxisAlignment: MainAxisAlignment.spaceAround,
624+
children: [
625+
Expanded(
626+
child: DevToolsTooltip(
627+
message: totalTimeTooltip,
628+
child: const Text(
629+
'Total %',
630+
textAlign: TextAlign.center,
642631
),
643-
],
632+
),
644633
),
645-
),
646-
const Divider(
647-
height: 0,
648-
),
649-
Expanded(
650-
child: ListView.builder(
651-
controller: scrollController,
652-
itemExtent: CodeView.rowHeight,
653-
itemCount: lineCount,
654-
itemBuilder: (context, index) {
655-
final lineNum = lineOffset + index + 1;
656-
final data = sourceReport.profilerEntries[lineNum];
657-
if (data == null) {
658-
return const SizedBox();
659-
}
660-
return ProfileInformationGutterItem(
661-
lineNumber: lineNum,
662-
profilerData: data,
663-
);
664-
},
634+
const SizedBox(width: denseSpacing),
635+
Expanded(
636+
child: DevToolsTooltip(
637+
message: selfTimeTooltip,
638+
child: const Text(
639+
'Self %',
640+
textAlign: TextAlign.center,
641+
),
642+
),
665643
),
666-
),
667-
],
644+
],
645+
),
668646
),
669-
const Center(
670-
child: VerticalDivider(),
647+
const Divider(
648+
height: 0,
671649
),
672650
],
673651
),
@@ -735,14 +713,12 @@ class ProfilePercentageItem extends StatelessWidget {
735713
message: hoverText,
736714
child: Container(
737715
color: color,
738-
child: Container(
739-
margin: const EdgeInsets.symmetric(
740-
horizontal: densePadding,
741-
),
742-
child: Text(
743-
'${percentage.toStringAsFixed(2)} %',
744-
textAlign: TextAlign.end,
745-
),
716+
padding: const EdgeInsets.symmetric(
717+
horizontal: densePadding,
718+
),
719+
child: Text(
720+
'${percentage.toStringAsFixed(2)} %',
721+
textAlign: TextAlign.end,
746722
),
747723
),
748724
);
@@ -751,6 +727,98 @@ class ProfilePercentageItem extends StatelessWidget {
751727

752728
typedef IntCallback = void Function(int value);
753729

730+
class Gutters extends StatelessWidget {
731+
const Gutters({
732+
required this.scriptRef,
733+
this.debuggerController,
734+
required this.codeViewController,
735+
required this.lines,
736+
required this.lineRange,
737+
required this.gutterController,
738+
required this.showProfileInformation,
739+
required this.profileController,
740+
required this.parsedScript,
741+
this.pausedFrame,
742+
this.onSelected,
743+
});
744+
745+
final ScriptRef? scriptRef;
746+
final DebuggerController? debuggerController;
747+
final CodeViewController codeViewController;
748+
final ScrollController gutterController;
749+
final ScrollController? profileController;
750+
final StackFrameAndSourcePosition? pausedFrame;
751+
final List<TextSpan> lines;
752+
final LineRange? lineRange;
753+
final ParsedScript? parsedScript;
754+
final void Function(ScriptRef scriptRef, int line)? onSelected;
755+
final bool showProfileInformation;
756+
757+
@override
758+
Widget build(BuildContext context) {
759+
final lineCount = lineRange?.size ?? lines.length;
760+
final lineOffset = (lineRange?.begin ?? 1) - 1;
761+
final sourceReport =
762+
parsedScript?.sourceReport ?? const ProcessedSourceReport.empty();
763+
764+
// Apply the log change-of-base formula to get the max number of digits in
765+
// a line number. Add a character width space for:
766+
// - each character in the longest line number
767+
// - one for the breakpoint dot
768+
// - two for the paused arrow
769+
final gutterWidth = assumedMonospaceCharacterWidth * 4 +
770+
assumedMonospaceCharacterWidth *
771+
(defaultEpsilon + math.log(math.max(lines.length, 100)) / math.ln10)
772+
.truncateToDouble();
773+
774+
return Row(
775+
children: [
776+
DualValueListenableBuilder<List<BreakpointAndSourcePosition>, bool>(
777+
firstListenable: breakpointManager.breakpointsWithLocation,
778+
secondListenable: codeViewController.showCodeCoverage,
779+
builder: (context, breakpoints, showCodeCoverage, _) {
780+
return Gutter(
781+
gutterWidth: gutterWidth,
782+
scrollController: gutterController,
783+
lineCount: lineCount,
784+
lineOffset: lineOffset,
785+
pausedFrame: pausedFrame,
786+
breakpoints:
787+
breakpoints.where((bp) => bp.scriptRef == scriptRef).toList(),
788+
executableLines: parsedScript?.executableLines ?? const <int>{},
789+
sourceReport: sourceReport,
790+
onPressed: _onPressed,
791+
// Disable dots for possible breakpoint locations.
792+
allowInteraction: !(debuggerController?.isSystemIsolate ?? false),
793+
showCodeCoverage: showCodeCoverage,
794+
);
795+
},
796+
),
797+
const SizedBox(width: denseSpacing),
798+
!showProfileInformation
799+
? const SizedBox()
800+
: Padding(
801+
padding: const EdgeInsets.only(right: denseSpacing),
802+
child: ProfileInformationGutter(
803+
scrollController: profileController!,
804+
lineCount: lineCount,
805+
lineOffset: lineOffset,
806+
sourceReport: sourceReport,
807+
),
808+
),
809+
],
810+
);
811+
}
812+
813+
void _onPressed(int line) {
814+
final onSelectedLocal = onSelected!;
815+
final script = scriptRef;
816+
if (onSelected != null && script != null) {
817+
onSelectedLocal(script, line);
818+
}
819+
}
820+
}
821+
754822
class Gutter extends StatelessWidget {
755823
const Gutter({
756824
required this.gutterWidth,

0 commit comments

Comments
 (0)