From 239cd100b35f7409c1ab6b75f884d00109cc9729 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 14 Nov 2023 13:52:39 -0800 Subject: [PATCH] review comments --- .../elements/analysis_results_controller.dart | 55 ++++--------------- pkgs/dart_pad/lib/embed.dart | 43 +++++++-------- pkgs/dart_services/lib/src/shared/model.dart | 3 + .../dart_services/lib/src/shared/model.g.dart | 2 + pkgs/dartpad_shared/analysis_options.yaml | 3 + pkgs/dartpad_shared/lib/model.dart | 3 + pkgs/dartpad_shared/lib/model.g.dart | 2 + 7 files changed, 44 insertions(+), 67 deletions(-) diff --git a/pkgs/dart_pad/lib/elements/analysis_results_controller.dart b/pkgs/dart_pad/lib/elements/analysis_results_controller.dart index a75adf8a3..a96e8a850 100644 --- a/pkgs/dart_pad/lib/elements/analysis_results_controller.dart +++ b/pkgs/dart_pad/lib/elements/analysis_results_controller.dart @@ -115,11 +115,6 @@ class AnalysisResultsController { ..classes.add('message')); } - // // TODO: This should likely be named contextMessages. - // for (final diagnostic in issue.diagnosticMessages) { - // columnElem.children.add(_createDiagnosticElement(diagnostic, issue)); - // } - elem.children.add(columnElem); final copyButton = MDCButton(ButtonElement(), isIcon: true); @@ -142,51 +137,17 @@ class AnalysisResultsController { elem.onClick.listen((_) { _onClickController.add(Location( - line: issue.line, - charStart: issue.charStart, - charLength: issue.charLength)); + line: issue.line, + charStart: issue.charStart, + charLength: issue.charLength, + // ignore: deprecated_member_use + inTestSource: issue.sourceName == 'test.dart', + )); }); return elem; } - // Element _createDiagnosticElement( - // DiagnosticMessage diagnosticMessage, AnalysisIssue parentIssue) { - // final message = diagnosticMessage.message; - - // final elem = DivElement()..classes.addAll(['message', 'clickable']); - // elem.text = message; - // elem.onClick.listen((event) { - // // Stop the mouse event so the outer issue mouse handler doesn't process - // // it. - // event.stopPropagation(); - - // _onClickController.add(Location( - // //TODO: @timmaffett multi files will need -> diagnosticMessage.sourceName, - // sourceName: parentIssue.sourceName, - // // For now if the source name is NOT main.dart then ASSUME that the - // // line number and charStart could have been adjust because of an - // // appended test, and use the information for the parentIssue instead. - // // (It would probably be safe to always do this, but by doing this - // // we DO NOT change any behavior except for when we have changed the - // // sourceName to `test.dart` (the only way the sourceName can currently - // // change until multi file source merged)) - // //TODO: @timmaffett For now we assume only 2 possibilities, 'main.dart' - // // or 'test.dart' (and in that case we changed line# and charStart). - // line: parentIssue.sourceName == 'main.dart' - // ? diagnosticMessage.line - // : parentIssue.line, - // charStart: parentIssue.sourceName == 'main.dart' - // ? diagnosticMessage.charStart - // : parentIssue.charStart, - // charLength: parentIssue.sourceName == 'main.dart' - // ? diagnosticMessage.charLength - // : parentIssue.charLength)); - // }); - - // return elem; - // } - void hideToggle() { toggle.setAttr('hidden'); } @@ -214,10 +175,14 @@ class Location { final int charStart; final int charLength; + /// Whether this is from an auxillary, synthetic test file. + final bool inTestSource; + Location({ required this.line, required this.charStart, required this.charLength, + this.inTestSource = false, }); } diff --git a/pkgs/dart_pad/lib/embed.dart b/pkgs/dart_pad/lib/embed.dart index fcffff64a..201e2b238 100644 --- a/pkgs/dart_pad/lib/embed.dart +++ b/pkgs/dart_pad/lib/embed.dart @@ -306,19 +306,19 @@ class Embed extends EditorUi { DElement(querySelector('#issues-toggle')!), snackbar, )..onItemClicked.listen((item) { - // if (item.sourceName == 'test.dart') { - // // must be test editor - // if (!_showTestCode) { - // _showTestCode = true; - // showTestCodeCheckmark.toggleClass('hide', !_showTestCode); - // tabController.setTabVisibility('test', _showTestCode); - // } - // tabController.selectTab('test'); - // _jumpToTest(item.line, item.charStart, item.charLength, focus: true); - // } else { - tabController.selectTab('dart'); - _jumpTo(item.line, item.charStart, item.charLength, focus: true); - // } + if (item.inTestSource) { + // must be test editor + if (!_showTestCode) { + _showTestCode = true; + showTestCodeCheckmark.toggleClass('hide', !_showTestCode); + tabController.setTabVisibility('test', _showTestCode); + } + tabController.selectTab('test'); + _jumpToTest(item.line, item.charStart, item.charLength, focus: true); + } else { + tabController.selectTab('dart'); + _jumpTo(item.line, item.charStart, item.charLength, focus: true); + } }); if (options.mode == EmbedMode.flutter || @@ -832,8 +832,6 @@ class Embed extends EditorUi { return issues .map((AnalysisIssue issue) { if (issue.line > dartSourceLineCount) { - // This is in the test source, do we adjust or hide it ? - // (We never hide errors). if (issue.kind != 'error' && !_showTestCode) { return null; } else { @@ -842,6 +840,7 @@ class Embed extends EditorUi { return AnalysisIssue( kind: issue.kind, message: issue.message, + sourceName: 'test.dart', correction: issue.correction, url: issue.url, charStart: issue.charStart - dartSourceCharCount, @@ -923,15 +922,15 @@ class Embed extends EditorUi { if (focus) context.focus(); } - // void _jumpToTest(int line, int charStart, int charLength, - // {bool focus = false}) { - // final doc = context.testDocument; + void _jumpToTest(int line, int charStart, int charLength, + {bool focus = false}) { + final doc = context.testDocument; - // doc.select( - // doc.posFromIndex(charStart), doc.posFromIndex(charStart + charLength)); + doc.select( + doc.posFromIndex(charStart), doc.posFromIndex(charStart + charLength)); - // if (focus) context.focus(); - // } + if (focus) context.focus(); + } @override void clearOutput() { diff --git a/pkgs/dart_services/lib/src/shared/model.dart b/pkgs/dart_services/lib/src/shared/model.dart index 61897cbf0..b5b36dc14 100644 --- a/pkgs/dart_services/lib/src/shared/model.dart +++ b/pkgs/dart_services/lib/src/shared/model.dart @@ -44,6 +44,8 @@ class AnalysisIssue { final String message; final String? correction; final String? url; + @Deprecated('Remove this once no longer referenced') + final String? sourceName; final int charStart; final int charLength; final int line; @@ -54,6 +56,7 @@ class AnalysisIssue { required this.message, this.correction, this.url, + this.sourceName, this.charStart = -1, this.charLength = 0, this.line = -1, diff --git a/pkgs/dart_services/lib/src/shared/model.g.dart b/pkgs/dart_services/lib/src/shared/model.g.dart index 642d49aa4..2646b8812 100644 --- a/pkgs/dart_services/lib/src/shared/model.g.dart +++ b/pkgs/dart_services/lib/src/shared/model.g.dart @@ -40,6 +40,7 @@ AnalysisIssue _$AnalysisIssueFromJson(Map json) => message: json['message'] as String, correction: json['correction'] as String?, url: json['url'] as String?, + sourceName: json['sourceName'] as String?, charStart: json['charStart'] as int? ?? -1, charLength: json['charLength'] as int? ?? 0, line: json['line'] as int? ?? -1, @@ -52,6 +53,7 @@ Map _$AnalysisIssueToJson(AnalysisIssue instance) => 'message': instance.message, 'correction': instance.correction, 'url': instance.url, + 'sourceName': instance.sourceName, 'charStart': instance.charStart, 'charLength': instance.charLength, 'line': instance.line, diff --git a/pkgs/dartpad_shared/analysis_options.yaml b/pkgs/dartpad_shared/analysis_options.yaml index c5bc1a8fc..9cfac8475 100644 --- a/pkgs/dartpad_shared/analysis_options.yaml +++ b/pkgs/dartpad_shared/analysis_options.yaml @@ -5,6 +5,9 @@ analyzer: strict-casts: true strict-inference: true strict-raw-types: true + errors: + # Remove this once we remove the AnalysisIssue.sourceName field. + deprecated_member_use_from_same_package: ignore linter: rules: diff --git a/pkgs/dartpad_shared/lib/model.dart b/pkgs/dartpad_shared/lib/model.dart index 61897cbf0..b5b36dc14 100644 --- a/pkgs/dartpad_shared/lib/model.dart +++ b/pkgs/dartpad_shared/lib/model.dart @@ -44,6 +44,8 @@ class AnalysisIssue { final String message; final String? correction; final String? url; + @Deprecated('Remove this once no longer referenced') + final String? sourceName; final int charStart; final int charLength; final int line; @@ -54,6 +56,7 @@ class AnalysisIssue { required this.message, this.correction, this.url, + this.sourceName, this.charStart = -1, this.charLength = 0, this.line = -1, diff --git a/pkgs/dartpad_shared/lib/model.g.dart b/pkgs/dartpad_shared/lib/model.g.dart index 642d49aa4..2646b8812 100644 --- a/pkgs/dartpad_shared/lib/model.g.dart +++ b/pkgs/dartpad_shared/lib/model.g.dart @@ -40,6 +40,7 @@ AnalysisIssue _$AnalysisIssueFromJson(Map json) => message: json['message'] as String, correction: json['correction'] as String?, url: json['url'] as String?, + sourceName: json['sourceName'] as String?, charStart: json['charStart'] as int? ?? -1, charLength: json['charLength'] as int? ?? 0, line: json['line'] as int? ?? -1, @@ -52,6 +53,7 @@ Map _$AnalysisIssueToJson(AnalysisIssue instance) => 'message': instance.message, 'correction': instance.correction, 'url': instance.url, + 'sourceName': instance.sourceName, 'charStart': instance.charStart, 'charLength': instance.charLength, 'line': instance.line,