@tomhughes commented on this pull request.
> end.html_safe
end
private
+ def expand_link_shorthands(text)
+ linkify_detection_rules = Array.wrap(Settings.linkify&.detection_rules)
+ linkify_detection_rules.each do |rule|
+ next unless rule.path_template && rule.patterns.is_a?(Array)
+
+ rule.patterns.each do |pattern|
+ expanded_path = "#{Settings.server_protocol}://#{rule.host ||
Settings.server_url}/#{rule.path_template}"
+
text.gsub!(Regexp.new("(?<before>\\s|^|>)#{pattern}(?<after>\\s|$|<)",
Regexp::IGNORECASE), "\\k<before>#{expanded_path}\\k<after>")
+ end
+ end
+ [[Settings.linkify_hosts, Settings.linkify_hosts_replacement],
[Settings.linkify_wiki_hosts, Settings.linkify_wiki_hosts_replacement]].each do
|hosts, replacement|
+ text.gsub!(/(\s)#{Regexp.escape(replacement)}/,
"\\1#{Settings.server_protocol}://#{hosts[0]}") if replacement && hosts&.any?
+ end
Can you explain the purpose of this? It comes from a commit that says "Fix
copy-pasting breaking links in linkify" but what exactly is breaking the links
and why does this fix it?
The PR talks about it rewriting the URL to use a canonical name but wasn't the
short name (which you're rewriting from here) supposed to be the canonical name?
> @@ -139,6 +139,35 @@ fossgis_valhalla_url:
> "https://valhalla1.openstreetmap.de/route";
# Endpoints for Wikimedia integration
wikidata_api_url: "https://www.wikidata.org/w/api.php";
wikimedia_commons_url: "https://commons.wikimedia.org/wiki/";
+linkify:
+ detection_rules:
+ - patterns: ["node/(?<id>\\d+)", "node (?<id>\\d{5,})", "n(?<id>\\d+)"]
+ path_template: "node/\\k<id>"
+ - patterns: ["way/(?<id>\\d+)", "way (?<id>\\d{5,})", "w(?<id>\\d+)"]
+ path_template: "way/\\k<id>"
+ - patterns: ["relation/(?<id>\\d+)", "relation (?<id>\\d{5,})",
"r(?<id>\\d+)"]
+ path_template: "relation/\\k<id>"
+ - patterns: ["changeset/(?<id>\\d+)", "changeset (?<id>\\d{5,})", "cs
?(?<id>\\d{5,})"]
Why do we allow a space after `cs` but not after object object types like `n`
or `w`?
> end.html_safe
end
private
+ def expand_link_shorthands(text)
+ linkify_detection_rules = Array.wrap(Settings.linkify&.detection_rules)
+ linkify_detection_rules.each do |rule|
+ next unless rule.path_template && rule.patterns.is_a?(Array)
+
+ rule.patterns.each do |pattern|
+ expanded_path = "#{Settings.server_protocol}://#{rule.host ||
Settings.server_url}/#{rule.path_template}"
It makes sense to combine `Settings.server_protocol` and `Settings.server_url`
but there's no logical reason it's right combine it with `rule.host`? Should
the rules have a base URL rather than just a host?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6518#pullrequestreview-3454725787
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6518/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
