From 379c20026dbbe61e1aa4a29b9e865afa770c9686 Mon Sep 17 00:00:00 2001
From: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>
Date: Fri, 24 Feb 2023 11:14:08 +0100
Subject: [PATCH] Get rid of Url.package_with_version

* Remove old Url.package
* Rename old Url.package_with_version into Url.package
* Update Url.with_version
  - None -> "/latest" (as in old Url.package_with_version)
  - Some "" -> "" (as in old Url.package)
  - Some x -> "/x" (as in old Url.package_with_version)
* Use new Url.with_version in new Url.package
* At call sites of old Url.package, add ~version:"" to get the
  same behaviour
* At call sites of old Url.package_with_version, replace by Url.package

A bit convoluted, but did not managed to observe a change of behaviour.
---
 src/global/url.ml                                      | 10 +++++-----
 .../components/package_breadcrumbs.eml                 |  2 +-
 .../pages/package_documentation_not_found.eml          |  2 +-
 src/ocamlorg_frontend/pages/package_overview.eml       |  8 ++++----
 src/ocamlorg_frontend/pages/packages.eml               |  2 +-
 .../pages/packages_autocomplete_fragment.eml           |  2 +-
 src/ocamlorg_frontend/pages/packages_search.eml        |  2 +-
 src/ocamlorg_frontend/pages/releases.eml               |  2 +-
 src/ocamlorg_web/lib/handler.ml                        |  2 +-
 src/ocamlorg_web/lib/router.ml                         | 10 ++++++----
 10 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/global/url.ml b/src/global/url.ml
index 87814bebdb..aadb5751ba 100644
--- a/src/global/url.ml
+++ b/src/global/url.ml
@@ -3,15 +3,15 @@ let packages = "/packages"
 let packages_search = "/packages/search"
 let packages_autocomplete_fragment = "/packages/autocomplete"
 let with_hash = Option.fold ~none:"/p" ~some:(( ^ ) "/u/")
-let package ?hash v = with_hash hash ^ "/" ^ v
 let package_docs v = "/p/" ^ v ^ "/doc"
-let with_version = Option.value ~default:"latest"
 
-let package_with_version ?version ?hash v =
-  with_hash hash ^ "/" ^ v ^ "/" ^ with_version version
+let with_version =
+  Option.fold ~none:"/latest" ~some:(fun v -> if v = "" then v else "/" ^ v)
+
+let package ?version ?hash v = with_hash hash ^ "/" ^ v ^ with_version version
 
 let package_doc ?hash ?version ?(page = "index.html") v =
-  with_hash hash ^ "/" ^ v ^ "/" ^ with_version version ^ "/doc/" ^ page
+  with_hash hash ^ "/" ^ v ^ with_version version ^ "/doc/" ^ page
 
 let community = "/community"
 let success_story v = "/success-stories/" ^ v
diff --git a/src/ocamlorg_frontend/components/package_breadcrumbs.eml b/src/ocamlorg_frontend/components/package_breadcrumbs.eml
index 9c99c6178f..6aaf2e7dee 100644
--- a/src/ocamlorg_frontend/components/package_breadcrumbs.eml
+++ b/src/ocamlorg_frontend/components/package_breadcrumbs.eml
@@ -51,7 +51,7 @@ let render_package_and_version
 =
   let version = if is_latest_url then None else Some package.version in
   <li>
-    <a class="font-semibold underline" href="<%s Url.package_with_version package.name ?version %>"><%s package.name %></a>
+    <a class="font-semibold underline" href="<%s Url.package package.name ?version %>"><%s package.name %></a>
   </li>
   <li>
     <select id="version" name="version" aria-label="version" onchange="location = this.value;"
diff --git a/src/ocamlorg_frontend/pages/package_documentation_not_found.eml b/src/ocamlorg_frontend/pages/package_documentation_not_found.eml
index 3c42be7a60..338dd173f1 100644
--- a/src/ocamlorg_frontend/pages/package_documentation_not_found.eml
+++ b/src/ocamlorg_frontend/pages/package_documentation_not_found.eml
@@ -18,7 +18,7 @@ Package_layout.render
         <p class="mt-1 text-base text-gray-500">We're sorry, for some reason we don't have the documentation for this package.</p>
       </div>
       <div class="mt-10 flex space-x-3 sm:border-l sm:border-transparent sm:pl-6">
-        <a href="<%s Url.package_with_version package.name ~version:(if is_latest_url then "latest" else package.version) %>"
+        <a href="<%s Url.package package.name ~version:(if is_latest_url then "latest" else package.version) %>"
           class="inline-flex items-center px-4 py-2 border border-transparent text-sm font-medium rounded-md shadow-sm text-white bg-orange-600 hover:bg-orange-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-orange-500">
           Go To Package Overview
         </a>
diff --git a/src/ocamlorg_frontend/pages/package_overview.eml b/src/ocamlorg_frontend/pages/package_overview.eml
index 030297df49..4661f7907d 100644
--- a/src/ocamlorg_frontend/pages/package_overview.eml
+++ b/src/ocamlorg_frontend/pages/package_overview.eml
@@ -27,7 +27,7 @@ Package_layout.render
 ~is_latest_url
 ~title:(if is_latest_url then (Printf.sprintf "%s %s · OCaml Package" package.name "latest") else (Printf.sprintf "%s %s · OCaml Package" package.name package.version))
 ~description:(if is_latest_url then (Printf.sprintf "%s %s: %s" package.name "latest" package.description) else (Printf.sprintf "%s %s: %s" package.name package.version package.description))
-~canonical:(Url.package_with_version package.name ~version:package.version)
+~canonical:(Url.package package.name ~version:package.version)
 ~package
 ~path:Overview @@
 <div class="flex lg:space-x-4 xl:space-x-12 flex-col lg:flex-row justify-between">
@@ -175,7 +175,7 @@ Package_layout.render
             <%s if List.length dependencies = 0 then "None" else "" %>
             <% dependencies |> List.iter (fun (name, cstr) -> %>
             <div class="flex items-center space-x-3">
-                <a href="<%s Url.package name %>" class="text-primary-600 hover:underline">
+                <a href="<%s Url.package ~version:"" name %>" class="text-primary-600 hover:underline">
                     <%s name %>
                 </a>
                 <% match cstr with None -> () | Some cstr -> %>
@@ -192,7 +192,7 @@ Package_layout.render
             <%s if List.length rev_dependencies = 0 then "None" else "" %>
             <% rev_dependencies |> List.iter (fun (name, cstr, version) -> %>
             <div class="flex items-center space-x-3">
-                <a href="<%s Url.package_with_version name ~version %>" class="text-primary-600 hover:underline">
+                <a href="<%s Url.package name ~version %>" class="text-primary-600 hover:underline">
                     <%s name %>
                 </a>
                 <% match cstr with None -> () | Some cstr -> %>
@@ -209,7 +209,7 @@ Package_layout.render
             <%s if List.length conflicts = 0 then "None" else "" %>
             <% conflicts |> List.iter (fun (name, cstr) -> %>
             <div class="flex items-center space-x-3">
-                <a href="<%s Url.package name %>" class="text-primary-600 hover:underline">
+                <a href="<%s Url.package ~version:"" name %>" class="text-primary-600 hover:underline">
                     <%s name %>
                 </a>
                 <% match cstr with None -> () | Some cstr -> %>
diff --git a/src/ocamlorg_frontend/pages/packages.eml b/src/ocamlorg_frontend/pages/packages.eml
index 734b30add5..7421578c76 100644
--- a/src/ocamlorg_frontend/pages/packages.eml
+++ b/src/ocamlorg_frontend/pages/packages.eml
@@ -1,6 +1,6 @@
 open Package_intf
 
-let href_package pkg = Url.package_with_version pkg.name ~version:pkg.version
+let href_package pkg = Url.package pkg.name ~version:pkg.version
 
 let render (stats : packages_stats option) (featured_packages : package list) =
 Layout.render
diff --git a/src/ocamlorg_frontend/pages/packages_autocomplete_fragment.eml b/src/ocamlorg_frontend/pages/packages_autocomplete_fragment.eml
index c4167a927e..bab0282ce9 100644
--- a/src/ocamlorg_frontend/pages/packages_autocomplete_fragment.eml
+++ b/src/ocamlorg_frontend/pages/packages_autocomplete_fragment.eml
@@ -14,7 +14,7 @@ let render
   <% packages |> List.iter (fun (package : Package_intf.package) -> %>
     <li class="flex flex-row">
       <a
-        href="<%s Url.package_with_version package.name %>"
+        href="<%s Url.package package.name %>"
         class="flex-grow px-2 py-2 leading-6 font-semibold hover:bg-primary-100 text-primary-600 inline-block"
       >
         <%s! Search.highlight_search_terms ~class_:"bg-background-light-blue text-gray-800 font-normal" ~search package.name %>
diff --git a/src/ocamlorg_frontend/pages/packages_search.eml b/src/ocamlorg_frontend/pages/packages_search.eml
index f12a747567..dd7ae929af 100644
--- a/src/ocamlorg_frontend/pages/packages_search.eml
+++ b/src/ocamlorg_frontend/pages/packages_search.eml
@@ -50,7 +50,7 @@ let render ~total ~search (packages : Package_intf.package list) = Layout.render
         <% packages |> List.iter (fun (package : Package_intf.package) -> %>
         <div class="border-gray-200 border-t mt-4 pt-4 mb-2 space-y-2">
           <a
-            href="<%s Url.package_with_version package.name %>"
+            href="<%s Url.package package.name %>"
             class="text-lg font-semibold text-primary-600 inline-block"
           >
             <%s package.name %>
diff --git a/src/ocamlorg_frontend/pages/releases.eml b/src/ocamlorg_frontend/pages/releases.eml
index e5afc0fd4e..d5fa63be09 100644
--- a/src/ocamlorg_frontend/pages/releases.eml
+++ b/src/ocamlorg_frontend/pages/releases.eml
@@ -60,7 +60,7 @@ Layout.render
                                 <a href="<%s Url.release release.version %>" class="text-primary-600 font-medium block">
                                     Release Notes
                                 </a>
-                                <a href="<%s Url.package_with_version "ocaml" ~version:release.version %>" class="text-primary-600 font-medium block">
+                                <a href="<%s Url.package "ocaml" ~version:release.version %>" class="text-primary-600 font-medium block">
                                     OCaml Package
                                 </a>
                                 <a href="<%s Url.manual_with_version release.version %>" class="text-primary-600 font-medium block">
diff --git a/src/ocamlorg_web/lib/handler.ml b/src/ocamlorg_web/lib/handler.ml
index 92c72e39ed..d9ad8c1090 100644
--- a/src/ocamlorg_web/lib/handler.ml
+++ b/src/ocamlorg_web/lib/handler.ml
@@ -360,7 +360,7 @@ let packages_autocomplete_fragment t req =
 
 let package _t req =
   let package = Dream.param req "name" in
-  let target = Url.package_with_version package in
+  let target = Url.package package in
   Dream.redirect req target
 
 (** Redirect any URL with suffix /p/PACKAGE/docs to the latest documentation for
diff --git a/src/ocamlorg_web/lib/router.ml b/src/ocamlorg_web/lib/router.ml
index d8555f666c..c5ad36e525 100644
--- a/src/ocamlorg_web/lib/router.ml
+++ b/src/ocamlorg_web/lib/router.ml
@@ -53,14 +53,16 @@ let package_route t =
       Dream.get Url.packages_search (Handler.packages_search t);
       Dream.get Url.packages_autocomplete_fragment
         (Handler.packages_autocomplete_fragment t);
-      Dream.get (Url.package ":name") (Handler.package t);
+      Dream.get (Url.package ~version:"" ":name") (Handler.package t);
       Dream.get (Url.package_docs ":name") (Handler.package_docs t);
-      Dream.get (Url.package ~hash:":hash" ":name") (Handler.package t);
       Dream.get
-        (Url.package_with_version ":name" ~version:":version")
+        (Url.package ~hash:":hash" ~version:"" ":name")
+        (Handler.package t);
+      Dream.get
+        (Url.package ":name" ~version:":version")
         ((Handler.package_versioned t) Handler.Package);
       Dream.get
-        (Url.package_with_version ~hash:":hash" ":name" ~version:":version")
+        (Url.package ~hash:":hash" ":name" ~version:":version")
         ((Handler.package_versioned t) Handler.Universe);
       Dream.get
         (Url.package_doc ":name" ~version:":version" ~page:"**")