-
Notifications
You must be signed in to change notification settings - Fork 647
Automatically convert URLs in values into links in the dashboard #6176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is an impactful change with some risk. Post 9.0 feature. |
3ed51d8
to
cd1c9b4
Compare
641956c
to
e33c702
Compare
Please take another look. I came back to his and did some more testing and noticed that content wasn't properly HTML encoded. Required a change in order of operations when parsing logs and content. |
Please hold off on merging this for a day as I'm looking at #6230 today for 9.0 and will want to backport it. There's overlap here which would make that backport difficult/risky. |
e33c702
to
409cf35
Compare
// https?:// - http:// or https:// | ||
// [-A-Za-z0-9+&@#/%?=~_|$!:,.;]* - Any character in the list, matched zero or more times. | ||
// [A-Za-z0-9+&@#/%=~_|$] - Any character in the list, matched exactly once | ||
[GeneratedRegex("\\bhttps?://[-A-Za-z0-9+&@#/%?=~_|$!:,.;]*[A-Za-z0-9+&@#/%=~_|$]")] | ||
private static partial Regex GenerateUrlRegEx(); | ||
[GeneratedRegex("(?<=m|\\b)https?://[-A-Za-z0-9+&@#/%?=~_|$!:,.;]*[A-Za-z0-9+&@#/%=~_|$]")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we don't use captured groups here, so we could add RegexOptions.ExplicitCapture
to allocate less when matching, now that we've added a group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at this regex more and I removed the group. I added flags to ignore case and culture.
Description
Automatically convert URLs in values and properties into links:
GridValue is used in many places:
There is a limitation: it doesn't work together with highlighting. Highlighting is only supported over plain text, not HTML. That means links are disabled when you begin searching with a filter and you go back to the current experience.
Getting these two features to work together properly is quite tricky. Will require building up a model of the text and transforming it when applying different operations (adding in links, adding in highlights)
Addresses #6175
Fixes #4845
Checklist
<remarks />
and<code />
elements on your triple slash comments?