From 0d0b96cb7ae69152646c9446611fd23cdbf8b3c2 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:56:24 -0800 Subject: [PATCH] Don't show hovercards for primitive and null values (#8528) --- .../lib/src/screens/debugger/codeview.dart | 2 +- .../shared/console/widgets/description.dart | 3 +- .../devtools_app/lib/src/shared/ui/hover.dart | 19 +++++++++++ .../devtools_app/test/shared/hover_test.dart | 34 +++++++++++++++++++ 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/debugger/codeview.dart b/packages/devtools_app/lib/src/screens/debugger/codeview.dart index bf7c78f2aa4..bbaeca455e2 100644 --- a/packages/devtools_app/lib/src/screens/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/screens/debugger/codeview.dart @@ -1172,7 +1172,7 @@ class _LineItemState extends State final word = wordForHover(event.localPosition.dx, widget.lineContents); - if (word != '') { + if (word != '' && !isPrimitiveValueOrNull(word)) { try { final response = await evalService.evalAtCurrentFrame(word); final isolateRef = diff --git a/packages/devtools_app/lib/src/shared/console/widgets/description.dart b/packages/devtools_app/lib/src/shared/console/widgets/description.dart index bb5b5708b0e..b4ae8ed59bf 100644 --- a/packages/devtools_app/lib/src/shared/console/widgets/description.dart +++ b/packages/devtools_app/lib/src/shared/console/widgets/description.dart @@ -207,7 +207,8 @@ class DiagnosticsNodeDescription extends StatelessWidget { enabled: () => preferences.inspector.hoverEvalModeEnabled.value && - diagnosticLocal.objectGroupApi != null, + diagnosticLocal.objectGroupApi != null && + !isPrimitiveValueOrNull(description), asyncGenerateHoverCardData: ({ required event, required isHoverStale, diff --git a/packages/devtools_app/lib/src/shared/ui/hover.dart b/packages/devtools_app/lib/src/shared/ui/hover.dart index 29b353d69b9..49f86f778c9 100644 --- a/packages/devtools_app/lib/src/shared/ui/hover.dart +++ b/packages/devtools_app/lib/src/shared/ui/hover.dart @@ -66,6 +66,25 @@ String wordForHover(double dx, TextSpan line) { return word; } +bool isPrimitiveValueOrNull(String valueAsString) { + if (valueAsString.isEmpty) return false; + final isNull = valueAsString == 'null'; + final isBool = valueAsString == 'true' || valueAsString == 'false'; + final isInt = int.tryParse(valueAsString) != null; + final isDouble = double.tryParse(valueAsString) != null; + + bool isString = false; + if (valueAsString.length > 2) { + final firstChar = valueAsString[0]; + final lastChar = valueAsString[valueAsString.length - 1]; + isString = + [firstChar, lastChar].every((char) => char == '"') || + [firstChar, lastChar].every((char) => char == "'"); + } + + return isNull || isBool || isInt || isDouble || isString; +} + /// Returns the index in the Textspan's plainText for which the hover offset is /// located. int _hoverIndexFor(double dx, TextSpan line) { diff --git a/packages/devtools_app/test/shared/hover_test.dart b/packages/devtools_app/test/shared/hover_test.dart index f45d9ad169c..603cc8d3818 100644 --- a/packages/devtools_app/test/shared/hover_test.dart +++ b/packages/devtools_app/test/shared/hover_test.dart @@ -41,4 +41,38 @@ void main() { expect(wordForHover(250, _textSpan), 'foo.bar'); expect(wordForHover(300, _textSpan), 'foo.bar.baz'); }); + + group('isPrimitiveValueOrNull', () { + test('returns false for non-primitives values', () { + expect(isPrimitiveValueOrNull('myVariable'), isFalse); + expect(isPrimitiveValueOrNull('MyWidget'), isFalse); + expect(isPrimitiveValueOrNull('MyClass'), isFalse); + }); + + test('returns true for null', () { + expect(isPrimitiveValueOrNull('null'), isTrue); + }); + + test('returns true for ints', () { + expect(isPrimitiveValueOrNull('10'), isTrue); + expect(isPrimitiveValueOrNull('3'), isTrue); + expect(isPrimitiveValueOrNull('255'), isTrue); + }); + + test('returns true for doubles', () { + expect(isPrimitiveValueOrNull('.3'), isTrue); + expect(isPrimitiveValueOrNull('1.7'), isTrue); + expect(isPrimitiveValueOrNull('123.389'), isTrue); + }); + + test('returns true for bools', () { + expect(isPrimitiveValueOrNull('true'), isTrue); + expect(isPrimitiveValueOrNull('false'), isTrue); + }); + + test('returns true for strings', () { + expect(isPrimitiveValueOrNull('"Hello World!"'), isTrue); + expect(isPrimitiveValueOrNull("'Hello World!'"), isTrue); + }); + }); }