review comments

dart-lang · Nov 14, 2023 · 239cd10 · 239cd10
1 parent fd5490e
commit 239cd10
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 67 deletions.
diff --git 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
@@ -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
diff --git a/pkgs/dart_services/lib/src/shared/model.g.dart b/pkgs/dart_services/lib/src/shared/model.g.dart
diff --git 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
@@ -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