[result feed] Allow pasted links to GitHub issues in our orgs
Allow comments to be created by pasting a link to a GitHub issue
in the dart-lang, google, or flutter organizations' repos.
These are converted to GitHub-style short links. The short links
can also be entered directly in the comment.
Change-Id: Ide24b2a310db514d802a1009af55131f4ba39f90
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/143742
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/results_feed/lib/src/components/try_results_component.html b/results_feed/lib/src/components/try_results_component.html
index b53854b..182864b 100644
--- a/results_feed/lib/src/components/try_results_component.html
+++ b/results_feed/lib/src/components/try_results_component.html
@@ -26,8 +26,10 @@
</simple-html>
</div>
<div *ngIf="approving">
- Comment: (type #4567 for GitHub SDK issue 4567,
- or co19 #123 for a co19 issue. Use \n for newline.)<br>
+ Comment: Use GitHub short links like #456 (dart-lang sdk issue 456),
+ flutter/engine#93, or google/quiver-dart#45. You can also paste
+ URLs of GitHub issues in dart-lang, flutter, or google directly.
+ <br>
<material-input
multiline
style="width: 80%"
diff --git a/results_feed/lib/src/formatting.dart b/results_feed/lib/src/formatting.dart
index 3e6f2a0..b4dc6a9 100644
--- a/results_feed/lib/src/formatting.dart
+++ b/results_feed/lib/src/formatting.dart
@@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
+import 'dart:convert';
+
import 'package:intl/intl.dart';
int _year = DateTime.now().year;
@@ -26,3 +28,35 @@
// Checkmark followed by a space. Used for approved results.
String checkmark = '\u2714 ';
+
+// Format comments and change Github short issue links to HTML links.
+const organizations = ['dart-lang', 'google', 'flutter'];
+final organization = organizations.join('|');
+const repo = '[\\w-]+';
+const issue = '\\d+';
+
+final pastedIssueUrlMatcher =
+ RegExp('\\bhttps://github.com/($organization)/($repo)/issues/($issue)\\b');
+// We allow short links of the form "organization/repo#number",
+// "repo#number", and "#number". Organization and repo default
+// to dart-lang and sdk.
+final shortLinkMatcher = RegExp('(\\b(($organization)/)?($repo))?#($issue)');
+
+String formatComment(String comment) => comment == null
+ ? null
+ : HtmlEscape(HtmlEscapeMode.element)
+ .convert(comment)
+ .replaceAll('\n', '<br>')
+ .replaceAllMapped(pastedIssueUrlMatcher,
+ (match) => ' ${match[1]}/${match[2]}#${match[3]} ')
+ .replaceAllMapped(
+ shortLinkMatcher,
+ (match) => '<a target="_blank" rel="noopener" '
+ 'href="https://${[
+ 'github.com',
+ match[3] ?? 'dart-lang',
+ match[4] ?? 'sdk',
+ 'issues',
+ match[5]
+ ].join('/')}">'
+ '${match[0]}</a>');
diff --git a/results_feed/lib/src/model/comment.dart b/results_feed/lib/src/model/comment.dart
index dd7a675..ad26e2b 100644
--- a/results_feed/lib/src/model/comment.dart
+++ b/results_feed/lib/src/model/comment.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:firebase/firestore.dart' show DocumentSnapshot;
+import '../formatting.dart';
class Comment implements Comparable {
final String id;
@@ -38,7 +39,7 @@
this.patchset)
: results = List<String>.from(_results),
tryResults = List<String>.from(_tryResults) {
- commentHtml = createCommentHtml();
+ commentHtml = formatComment(comment);
}
Comment.fromDocument(DocumentSnapshot document)
@@ -56,30 +57,12 @@
pinnedIndex = document.get('pinned_index'),
review = document.get('review'),
patchset = document.get('patchset') {
- commentHtml = createCommentHtml();
+ commentHtml = formatComment(comment);
}
String approvedText() =>
(approved == null) ? '' : approved ? 'approved' : 'disapproved';
- static const repositories = ['sdk', 'co19'];
- // Matches a repository or nothing, followed by #[digits][word break].
- static final issueMatcher =
- RegExp('(${repositories.join("|")})?\\s*#(\\d+)\\b');
-
- String createCommentHtml() {
- final result = comment
- ?.replaceAll('<', '<')
- ?.replaceAll('\\n', '<br>')
- ?.replaceAllMapped(
- issueMatcher,
- (match) =>
- '<a target="_blank" rel="noopener" href="https://github.com/'
- 'dart-lang/${match[1] ?? "sdk"}/issues/${match[2]}">'
- '${match[0]}</a>');
- return result;
- }
-
@override
int compareTo(Object other) => created.compareTo((other as Comment).created);
}
diff --git a/results_feed/test/formatting_test.dart b/results_feed/test/formatting_test.dart
index e4af604..7f8a30e 100644
--- a/results_feed/test/formatting_test.dart
+++ b/results_feed/test/formatting_test.dart
@@ -21,4 +21,131 @@
expect(formattedDate(DateTime.parse('2018-11-13T15:07:55.123')),
'15:07 Tue Nov 13, 2018');
});
+
+ test('Format comment', () {
+ const unformatted = 'This comment has\n'
+ 'newlines and\n'
+ 'escaped newlines and '
+ 'a < sign and an '
+ '& ampersand';
+ const formatted = 'This comment has<br>'
+ 'newlines and<br>'
+ 'escaped newlines and '
+ 'a < sign and an '
+ '& ampersand';
+ expect(formatComment(unformatted), formatted);
+ });
+
+ test('Format comment with GitHub short issue links', () {
+ const unformatted = '#012 This comment has\n'
+ 'a short issue link #1234 and \n'
+ 'one after a newline \n'
+ '#987 and a link with a repo dart_style#567 '
+ 'and one with the dart-lang org dart-lang/dart_ci#890 '
+ 'and one with the google org google/a-repo#000 '
+ 'and one with the flutter org flutter/engine#92text afterward '
+ 'but the following are invalid and should\n'
+ 'not be changed to html:\n'
+ 'one with no number # '
+ 'one across a newline sdk#\n34 '
+ 'one with an org and repo but no number google/sdk#deadbeef '
+ 'The following get converted, but with the default org:\n'
+ 'one from a different org private/dart_clone#358 '
+ 'one with an org but no repo dart-lang/#1234 '
+ 'one with a slash but no org /dart-pad#567';
+ const formatted = '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/012">#012</a> '
+ 'This comment has<br>'
+ 'a short issue link '
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/1234">#1234</a> '
+ 'and <br>one after a newline <br>'
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/987">#987</a> and '
+ 'a link with a repo '
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/dart_style/issues/567">'
+ 'dart_style#567</a> '
+ 'and one with the dart-lang org '
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/dart_ci/issues/890">'
+ 'dart-lang/dart_ci#890</a> '
+ 'and one with the google org '
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/google/a-repo/issues/000">'
+ 'google/a-repo#000</a> '
+ 'and one with the flutter org '
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/flutter/engine/issues/92">'
+ 'flutter/engine#92</a>text afterward '
+ 'but the following are invalid and should<br>'
+ 'not be changed to html:<br>'
+ 'one with no number # '
+ 'one across a newline sdk#<br>34 '
+ 'one with an org and repo but no number google/sdk#deadbeef '
+ 'The following get converted, but with the default org:<br>'
+ 'one from a different org private/'
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/dart_clone/issues/358">'
+ 'dart_clone#358</a> '
+ 'one with an org but no repo dart-lang/'
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/1234">'
+ '#1234</a> '
+ 'one with a slash but no org /'
+ '<a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/dart-pad/issues/567">'
+ 'dart-pad#567</a>';
+ expect(formatComment(unformatted), formatted);
+ });
+
+ test('Format comment with pasted GitHub issue URL', () {
+ const unformatted = 'This comment has a pasted issue URL\n'
+ 'https://github.com/dart-lang/sdk/issues/4132\n'
+ 'and one from a different repo '
+ '\nhttps://github.com/dart-lang/dart_ci/issues/4321\n'
+ 'and one from flutter '
+ '\nhttps://github.com/flutter/engine/issues/3241 '
+ 'here are links with text around them '
+ 'link https://github.com/dart-lang/sdk/issues/4132 here'
+ 'link:https://github.com/dart-lang/sdk/issues/4132<here>'
+ 'These links should not be converted:\n'
+ 'myhttps://github.com/dart-lang/sdk/issues/4132 '
+ 'link https://github.com/dart-lang/sdk/issues/4132foo here'
+ 'link https://github.com/dart-clone/sdk/issues/4132 here'
+ 'link https://github.com/google//issues/4132 here'
+ 'link https://github.com/dart-lang/sdk/issues/ here'
+ 'link https://github.com/dart-lang/sdk/issues/new here'
+ 'link https://gitfoohub.com/dart-lang/sdk/issues/4132 here'
+ 'link https://github.com/dart-lang/s.dk/issues/1234 here';
+ const formatted = 'This comment has a pasted issue URL<br>'
+ ' <a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/4132">'
+ 'dart-lang/sdk#4132</a> <br>'
+ 'and one from a different repo '
+ '<br> <a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/dart_ci/issues/4321">'
+ 'dart-lang/dart_ci#4321</a> <br>'
+ 'and one from flutter '
+ '<br> <a target="_blank" rel="noopener" '
+ 'href="https://github.com/flutter/engine/issues/3241">'
+ 'flutter/engine#3241</a> '
+ 'here are links with text around them '
+ 'link <a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/4132">'
+ 'dart-lang/sdk#4132</a> here'
+ 'link: <a target="_blank" rel="noopener" '
+ 'href="https://github.com/dart-lang/sdk/issues/4132">'
+ 'dart-lang/sdk#4132</a> <here>'
+ 'These links should not be converted:<br>'
+ 'myhttps://github.com/dart-lang/sdk/issues/4132 '
+ 'link https://github.com/dart-lang/sdk/issues/4132foo here'
+ 'link https://github.com/dart-clone/sdk/issues/4132 here'
+ 'link https://github.com/google//issues/4132 here'
+ 'link https://github.com/dart-lang/sdk/issues/ here'
+ 'link https://github.com/dart-lang/sdk/issues/new here'
+ 'link https://gitfoohub.com/dart-lang/sdk/issues/4132 here'
+ 'link https://github.com/dart-lang/s.dk/issues/1234 here';
+ expect(formatComment(unformatted), formatted);
+ });
}
diff --git a/results_feed/web/main.dart b/results_feed/web/main.dart
index d706a9f..5fa424d 100644
--- a/results_feed/web/main.dart
+++ b/results_feed/web/main.dart
@@ -9,11 +9,13 @@
import 'package:dart_results_feed/src/services/firestore_service.dart';
import 'package:dart_results_feed/src/components/routing_wrapper_component.template.dart'
as ng;
+import 'package:dart_results_feed/src/formatting.dart' as formatting;
import 'main.template.dart' as self;
-// Allow links from comments to GitHub issues in the dart-lang organization.
+// Allow links from comments to GitHub issues in allowed organizations.
List<Uri> getUriWhitelist() => List.unmodifiable(<Uri>[
- Uri.https('github.com', 'dart-lang/'),
+ for (final organization in formatting.organizations)
+ Uri.https('github.com', '$organization/')
]);
// Use for local testing
