[openstreetmap/openstreetmap-website] Add URL prettification for plain texts (PR #6518)

[Tom Hughes]

@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/3454725787 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20251112/4bd636f5/attachment.htm>