Update subpath display

master_changes.md

@@ -78,6 +78,7 @@ users)
   * ◈ New option `opam pin --current` to fix a package in its current state (avoiding pending reinstallations or removals from the repository) [#4973 @AltGr - fix #4970]
   * [BUG] Fix using `--working-dir` with non pinned packages: it was not downloading sources as they were remove from package that need sources [#5082 @rjbou - fix #5060]
   * [NEW] Reactivate subpath and recursive pinning `--recursive` and `--subpath` [#4876 @rjbou]
+    * Change display from `git+https://url#hash (subpath)` to `/subpath in git+https://url#hash` [#5219 @rjbou]
   * scan: show subpaths [#4876 @rjbou]
   * [BUG] Fix windows path for subpath, by introducing their own type in `OpamFilename` [#4876 @rjbou]
   * [BUG] Fix repin of locked pins when there is no change in lock file [#5079 @rjbou - fix #4313]
@@ -482,3 +483,5 @@ users)
   * `OpamCompat`: add `Lazy` module and `Lazy.map` function [#5176 @dra27]
   * `OpamConsole.menu`: add `noninteractive` option to choose a different default when output is not a tty [#5156 @rjbou]
   * `OpamStd.Sys`: add `all_shells` list of all supported shells [#5217 @dra27]
+  * `OpamUrl`: add `to_string_w_subpath` to display subpath inside urls (before hash) [#5219 @rjbou]
+  * `OpamFilename.SubPath`: remove `pretty_string` in favor to `OpamUrl.to_string_w_subpath` [#5219 @rjbou]

src/client/opamAction.ml

@@ -619,10 +619,8 @@ let make_command st opam ?dir ?text_command (cmd, args) =
               nv.name
           in
           let rev = OpamProcess.Job.run (OpamRepository.revision src u) in
-          Printf.sprintf "pinned(%s%s%s)"
-            (OpamUrl.to_string u)
-            (OpamStd.Option.to_string OpamFilename.SubPath.pretty_string
-               (OpamFile.URL.subpath url))
+          Printf.sprintf "pinned(%s%s)"
+            (OpamUrl.to_string_w_subpath (OpamFile.URL.subpath url) u)
             (OpamStd.Option.to_string
                (fun r -> "#"^OpamPackage.Version.to_string r) rev)
       else

src/client/opamAuxCommands.ml

@@ -266,12 +266,10 @@ let autopin_aux st ?quiet ?(for_view=false) ?recurse ?subpath ?locked
   in
   log "autopin: %a"
     (slog @@ OpamStd.List.to_string (fun pin ->
-         Printf.sprintf "%s%s => %s%s"
+         Printf.sprintf "%s%s => %s"
            (OpamPackage.Name.to_string pin.pin_name)
            (if pin.pin.pin_locked = None then "" else "[locked]")
-           (OpamUrl.to_string pin.pin.pin_url)
-           (OpamStd.Option.to_string
-              OpamFilename.SubPath.pretty_string pin.pin.pin_subpath)))
+           (OpamUrl.to_string_w_subpath pin.pin.pin_subpath pin.pin.pin_url)))
     to_pin;
   let obsolete_pins =
     (* Packages not current but pinned to the same dirs *)

src/client/opamPinCommand.ml

@@ -17,16 +17,20 @@ open OpamStd.Option.Op
 let log fmt = OpamConsole.log "COMMAND" fmt
 let slog = OpamConsole.slog
 
-let string_of_pinned opam =
+let string_of_pinned ?(subpath_prefix=true) opam =
   let bold = OpamConsole.colorise `bold in
-  Printf.sprintf "pinned %s (version %s)"
+  let url, subpath =
+    match OpamFile.OPAM.url opam with
+    | None -> None, None
+    | Some u -> OpamFile.URL.(Some (url u), subpath u)
+  in
+  Printf.sprintf "%spinned %s (version %s)"
+    (if subpath_prefix && subpath <> None then "subpath-" else "")
     (OpamStd.Option.to_string ~none:(bold "locally")
-       (fun u -> Printf.sprintf "to %s%s"
-           (bold (OpamUrl.to_string (OpamFile.URL.url u)))
-           (OpamStd.Option.to_string (fun sp ->
-                " " ^ OpamFilename.SubPath.pretty_string sp)
-               (OpamFile.URL.subpath u)))
-       (OpamFile.OPAM.url opam))
+       (fun u -> Printf.sprintf "to %s"
+           (bold (OpamUrl.to_string_w_subpath subpath
+                    u)))
+       url)
     (bold (OpamPackage.Version.to_string (OpamFile.OPAM.version opam)))
 
 let read_opam_file_for_pinning ?locked ?(quiet=false) name f url =
@@ -396,11 +400,10 @@ let fetch_all_pins st ?working_dir pins =
         installed:%s\n\
         Continue anyway?"
        (OpamStd.Format.itemize (fun p ->
-            Printf.sprintf "%s:%s%s"
+            Printf.sprintf "%s:%s"
               (OpamPackage.Name.to_string p.pinned_name)
-              (OpamUrl.to_string p.pinned_url)
-              (OpamStd.Option.to_string
-                 OpamFilename.SubPath.pretty_string p.pinned_subpath))
+              (OpamUrl.to_string_w_subpath p.pinned_subpath
+                 p.pinned_url))
            (List.rev errored))
   then
     List.rev to_pin
@@ -451,12 +454,11 @@ and source_pin
     ?subpath ?locked
     target_url
   =
-  log "pin %a to %a %a%a"
+  log "pin %a to %a %a"
     (slog OpamPackage.Name.to_string) name
     (slog (OpamStd.Option.to_string OpamPackage.Version.to_string)) version
-    (slog (OpamStd.Option.to_string ~none:"none" OpamUrl.to_string)) target_url
-    (slog (OpamStd.Option.to_string OpamFilename.SubPath.pretty_string))
-    subpath;
+    (slog (OpamStd.Option.to_string ~none:"none"
+             (OpamUrl.to_string_w_subpath subpath))) target_url;
  (* let installed_version =
     try
       Some (OpamPackage.version
@@ -714,7 +716,8 @@ let unpin st names =
       | Some nv ->
         let pin_str =
           OpamStd.Option.to_string ~none:"pinned"
-            string_of_pinned (OpamSwitchState.opam_opt st nv)
+            (string_of_pinned ~subpath_prefix:false)
+            (OpamSwitchState.opam_opt st nv)
         in
         let st = unpin_one st nv in
         if OpamClientConfig.(!r.show) then
@@ -748,13 +751,8 @@ let list st ~short =
         match url with
         | Some url ->
           let u = OpamFile.URL.url url in
-          let subpath =
-            OpamStd.Option.to_string (fun sp ->
-                " " ^ OpamFilename.SubPath.pretty_string sp)
-              (OpamFile.URL.subpath url)
-          in
           OpamUrl.string_of_backend u.OpamUrl.backend,
-          OpamUrl.to_string u ^ subpath
+          OpamUrl.to_string_w_subpath (OpamFile.URL.subpath url) u
         | None -> "local definition", ""
       in
       let state, extra =

src/core/opamFilename.ml

@@ -510,7 +510,6 @@ module SubPath = struct
     |> OpamStd.String.remove_prefix ~prefix:"./"
     |> of_string
   let to_string = OpamSystem.forward_to_back
-  let pretty_string s = "("^s^")"
   let normalised_string s = s
 
   let (/) d s = d / to_string s

src/core/opamFilename.mli

@@ -333,9 +333,6 @@ module SubPath: sig
   (* Directory concatenation with an optional argument *)
   val (/?): Dir.t -> t option -> Dir.t
 
-  (* Pretty printing of subath, i.e. '(sub/path)' *)
-  val pretty_string: t -> string
-
   (* Subpath string with no file separator conversion *)
   val normalised_string: t -> string
 

src/core/opamUrl.ml

@@ -184,17 +184,27 @@ let parse_opt ?(quiet=false) ?backend ?handle_suffix ?from_file s =
 
 let of_string url = parse ~handle_suffix:false url
 
-let to_string url =
+let to_string_t ?subpath url =
   let hash = match url.hash with Some h -> "#" ^ h | None -> "" in
+  let subpath =
+    match subpath with
+    | Some sb ->
+      Printf.sprintf "directory /%s in "
+        (OpamFilename.SubPath.normalised_string sb)
+    | None -> ""
+  in
   match url.backend with
   | #version_control as vc ->
     let vc = string_of_backend vc in
     if url.transport = vc then (* Don't be redundant on e.g git:// protocols *)
-      Printf.sprintf "%s://%s%s" vc url.path hash
+      Printf.sprintf "%s%s://%s%s" subpath vc url.path hash
     else
-      Printf.sprintf "%s+%s://%s%s" vc url.transport url.path hash
+      Printf.sprintf "%s%s+%s://%s%s" subpath vc url.transport url.path hash
   | `rsync | `http ->
-    Printf.sprintf "%s://%s%s" url.transport url.path hash
+    Printf.sprintf "%s%s://%s%s" subpath url.transport url.path hash
+
+let to_string url = to_string_t url
+let to_string_w_subpath subpath = to_string_t ?subpath
 
 let base_url url =
   match url.transport with

src/core/opamUrl.mli

@@ -50,6 +50,10 @@ val parse_opt:
 
 include OpamStd.ABSTRACT with type t := t
 
+(* [to_string_w_subpath subpath url] Return string of [url] with [subpath]
+   integrated *)
+val to_string_w_subpath: OpamFilename.SubPath.t option -> t -> string
+
 (** Dummy filler url *)
 val empty: t
 

src/repository/opamRepository.ml

@@ -346,10 +346,7 @@ let pull_tree_t
           pull_from_mirrors label ?working_dir ?subpath cache_dir local_dirname
             checksums remote_urls
           @@| fun (url, res) ->
-          Printf.sprintf "%s%s"
-            (OpamUrl.to_string url)
-            (OpamStd.Option.to_string
-               OpamFilename.SubPath.pretty_string subpath),
+          (OpamUrl.to_string_w_subpath subpath url),
           res
         | _ ->
           pull_from_mirrors label ?working_dir cache_dir tmpdir

src/state/opamUpdate.ml

@@ -188,9 +188,11 @@ let fetch_dev_package url srcdir ?(working_dir=false) ?subpath nv =
   let remote_url = OpamFile.URL.url url in
   let mirrors = remote_url :: OpamFile.URL.mirrors url in
   let checksum = OpamFile.URL.checksum url in
-  log "updating %a%a" (slog OpamUrl.to_string) remote_url
+  log "updating %a" (slog (OpamUrl.to_string_w_subpath subpath)) remote_url;
+(*
     (slog (OpamStd.Option.to_string OpamFilename.SubPath.pretty_string))
     subpath;
+*)
   OpamRepository.pull_tree
     ~cache_dir:(OpamRepositoryPath.download_cache OpamStateConfig.(!r.root_dir))
     (OpamPackage.to_string nv) srcdir checksum ~working_dir ?subpath mirrors