Fix linkify to only link to proper URLs #5920
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
okurz
previously requested changes
Sep 11, 2024
perlpunk
force-pushed
the
fix-linkify
branch
2 times, most recently
from
28f5b98
to
e36fd22
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5920 +/- ##
=======================================
Coverage 98.73% 98.73%
=======================================
Files 395 395
Lines 38764 38770 +6
=======================================
+ Hits 38275 38281 +6
Misses 489 489 ☔ View full report in Codecov by Sentry. |
Martchus
reviewed
Sep 11, 2024
perlpunk
force-pushed
the
fix-linkify
branch
3 times, most recently
from
5f3cabc
to
c9721ce
Compare
|
I added a test |
Martchus
approved these changes
Sep 11, 2024
This doesn't check for really well formed URLs, but it limits the matching to
only allowed characters in URIs.
It makes up for escapeForHtml() being already called, so it matches `&`
separately.
Tested with:
# "> should be excluded
"https://github.com/os-autoinst/openQA.git"
<https://github.com/os-autoinst/os-autoinst.git>
# ) should be included
https://de.wikipedia.org/wiki/Queen_(Band)
# query strings should be included
https://github.com/os-autoinst/openQA?foo=1%20&bar=*2
Single quotes actually can be part of URIs so that's something we should adapt
in our code (and use double quotes, or I remember back in usenet times we used
`<http://...>` whenever referring to a URL in plain text)
Issue: https://progress.opensuse.org/issues/166676
perlpunk
force-pushed
the
fix-linkify
branch
from
c9721ce
to
61be703
Compare
b10n1k
approved these changes
Sep 12, 2024
b10n1k
added a commit
to b10n1k/os-autoinst
that referenced
this pull request
Sep 12, 2024
Solution provided on os-autoinst/openQA#5920 and this is a adaptable change to that commit as unescaped single quotes are valid parts of URIs https://progress.opensuse.org/issues/166676 [0] https://github.com/IonicaBizau/anser/tree/master?tab=readme-ov-file#anserlinkifytxt Signed-off-by: ybonatakis <[email protected]>
b10n1k
added a commit
to b10n1k/os-autoinst
that referenced
this pull request
Sep 12, 2024
Solution provided on os-autoinst/openQA#5920 and this is a adaptable change to that commit as unescaped single quotes are valid parts of URIs https://progress.opensuse.org/issues/166676 Signed-off-by: ybonatakis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This doesn't check for really well formed URLs, but it limits the matching to only allowed characters in URIs.
It makes up for escapeForHtml() being already called, so it matches
&separately.Tested with:
Single quotes actually can be part of URIs so that's something we should adapt
in our code (and use double quotes, or I remember back in usenet times we used
<http://...>whenever referring to a URL in plain text).We could use this as a workaround until an according PR to https://github.com/IonicaBizau/anser has been made.
There is a related issue: IonicaBizau/anser#63
Alternative to
Issue: https://progress.opensuse.org/issues/166676