Reduce use of [Error] in [Err] (refactor)

lib/vcs/src/err.ml

@@ -22,24 +22,45 @@
 open! Import
 
 type t =
-  { steps : Info.t list
-  ; error : Error.t
+  { steps : Sexp.t list
+  ; error : Sexp.t
   }
 [@@deriving sexp_of]
 
-let sexp_of_t ({ steps; error } as t) =
-  if List.is_empty steps then Error.sexp_of_t error else sexp_of_t t
-;;
-
+let sexp_of_t ({ steps; error } as t) = if List.is_empty steps then error else sexp_of_t t
 let to_string_hum t = t |> sexp_of_t |> Sexp.to_string_hum
-let create_s sexp = { steps = []; error = Error.create_s sexp }
+let create_s sexp = { steps = []; error = sexp }
+let error_string str = create_s (Sexp.Atom str)
+let of_exn exn = create_s (sexp_of_exn exn)
+let add_context t ~step = { steps = step :: t.steps; error = t.error }
+let init error ~step = { steps = [ step ]; error }
+
+module Private = struct
+  module Non_raising_M = struct
+    type nonrec t = t
+
+    let sexp_of_t = sexp_of_t
+    let to_err t = t
+    let of_err t = t
+  end
+
+  module View = struct
+    type nonrec t = t =
+      { steps : Sexp.t list
+      ; error : Sexp.t
+      }
+  end
+
+  let view t = t
+end
 
-let to_error t =
-  match t.steps with
-  | [] -> t.error
-  | _ :: _ -> Error.create_s (sexp_of_t t)
-;;
+module Vcs_base = struct
+  let to_error t =
+    Error.create_s
+      (match Private.view t with
+       | { steps = []; error } -> error
+       | { steps = _ :: _; error = _ } -> sexp_of_t t)
+  ;;
 
-let of_error error = { steps = []; error }
-let add_context t ~step = { steps = Info.create_s step :: t.steps; error = t.error }
-let init error ~step = { steps = [ Info.create_s step ]; error }
+  let of_error error = create_s (Error.sexp_of_t error)
+end

lib/vcs/src/err.mli

@@ -36,18 +36,40 @@ val sexp_of_t : t -> Sexp.t
 (** [to_string_hum t] is a convenience wrapper around [t |> sexp_of_t |> Sexp.to_string_hum]. *)
 val to_string_hum : t -> string
 
+val error_string : string -> t
 val create_s : Sexp.t -> t
-
-(** Inject [t] into [Base.Error.t]. This is useful if you'd like to use [Vcs]
-    inside the [Or_error] monad. *)
-val to_error : t -> Error.t
-
-(** Create an error with no initial step. *)
-val of_error : Error.t -> t
+val of_exn : exn -> t
 
 (** Add a step of context into the stack trace contained by the error. *)
 val add_context : t -> step:Sexp.t -> t
 
-(** This is useful if you are starting from an [Error.t] initially with an
+(** This is useful if you are starting from an [Sexp.t] initially with an
     initial step. *)
-val init : Error.t -> step:Sexp.t -> t
+val init : Sexp.t -> step:Sexp.t -> t
+
+module Private : sig
+  module Non_raising_M : sig
+    type nonrec t = t [@@deriving sexp_of]
+
+    val to_err : t -> t
+    val of_err : t -> t
+  end
+
+  module View : sig
+    type t =
+      { steps : Sexp.t list
+      ; error : Sexp.t
+      }
+  end
+
+  val view : t -> View.t
+end
+
+module Vcs_base : sig
+  (** Inject [t] into [Base.Error.t]. This is useful if you'd like to use [Vcs]
+      inside the [Or_error] monad. *)
+  val to_error : t -> Error.t
+
+  (** Create an error with no initial step. *)
+  val of_error : Error.t -> t
+end

lib/vcs/src/git.ml

@@ -61,7 +61,7 @@ module Non_raising = struct
   module Make (M : M) : S with type 'a result := ('a, M.t) Result.t = struct
     let map_result = function
       | Ok x -> Ok x
-      | Error error -> Error (M.of_err (Err.of_error error))
+      | Error error -> Error (M.of_err (Err.Vcs_base.of_error error))
     ;;
 
     let exit0 output = Or_error.exit0 output |> map_result
@@ -72,7 +72,7 @@ end
 
 let err_exn = function
   | Ok x -> x
-  | Error err -> raise (Exn0.E (Err.of_error err))
+  | Error err -> raise (Exn0.E (Err.Vcs_base.of_error err))
 ;;
 
 let exit0 output = Or_error.exit0 output |> err_exn

lib/vcs/src/non_raising.ml

@@ -99,14 +99,15 @@ module Make (M : M) :
   let git ?env ?run_in_subdir vcs ~repo_root ~args ~f =
     match
       Vcs0.Private.git ?env ?run_in_subdir vcs ~repo_root ~args ~f:(fun output ->
-        f output |> Result.map_error ~f:(fun err_m -> Err.to_error (M.to_err err_m)))
+        f output
+        |> Result.map_error ~f:(fun err_m -> Err.Vcs_base.to_error (M.to_err err_m)))
     with
     | Ok t -> Ok t
     | Error error ->
       Error
         (M.of_err
            (Err.init
-              error
+              (Error.sexp_of_t error)
               ~step:
                 (Vcs0.Private.make_git_err_step ?env ?run_in_subdir ~repo_root ~args ())))
   ;;

lib/vcs/src/vcs0.ml

@@ -28,7 +28,7 @@ let ( let* ) = Stdlib.Result.bind
 
 let of_result ~step = function
   | Ok r -> r
-  | Error error -> raise (Exn0.E (Err.init error ~step:(force step)))
+  | Error error -> raise (Exn0.E (Err.init (Error.sexp_of_t error) ~step:(force step)))
 ;;
 
 let load_file (Provider.T { t; handler }) ~path =

lib/vcs/src/vcs_or_error0.ml

@@ -21,5 +21,5 @@
 
 type t = Error.t [@@deriving sexp_of]
 
-let of_err = Err.to_error
-let to_err = Err.of_error
+let of_err = Err.Vcs_base.to_error
+let to_err = Err.Vcs_base.of_error

lib/vcs/test/test__err.ml

@@ -28,23 +28,22 @@ let%expect_test "to_string_hum" =
 let%expect_test "sexp_of_t" =
   print_s [%sexp (Vcs.Err.create_s [%sexp Hello] : Vcs.Err.t)];
   [%expect {| Hello |}];
-  print_s
-    [%sexp (Vcs.Err.init (Error.create_s [%sexp Hello]) ~step:[%sexp Step] : Vcs.Err.t)];
+  print_s [%sexp (Vcs.Err.init [%sexp Hello] ~step:[%sexp Step] : Vcs.Err.t)];
   [%expect {| ((steps (Step)) (error Hello)) |}];
   ()
 ;;
 
 let%expect_test "to_error" =
-  let test err = print_s [%sexp (Vcs.Err.to_error err : Error.t)] in
+  let test err = print_s [%sexp (Vcs.Err.Vcs_base.to_error err : Error.t)] in
   test (Vcs.Err.create_s [%sexp Hello]);
   [%expect {| Hello |}];
-  test (Vcs.Err.init (Error.create_s [%sexp Hello]) ~step:[%sexp Step]);
+  test (Vcs.Err.init [%sexp Hello] ~step:[%sexp Step]);
   [%expect {| ((steps (Step)) (error Hello)) |}];
   ()
 ;;
 
 let%expect_test "of_error" =
-  let test err = print_s [%sexp (Vcs.Err.of_error err : Vcs.Err.t)] in
+  let test err = print_s [%sexp (Vcs.Err.Vcs_base.of_error err : Vcs.Err.t)] in
   test (Error.create_s [%sexp Hello]);
   [%expect {| Hello |}];
   ()
@@ -64,8 +63,7 @@ let%expect_test "add_context" =
 ;;
 
 let%expect_test "init" =
-  print_s
-    [%sexp (Vcs.Err.init (Error.create_s [%sexp Hello]) ~step:[%sexp Step] : Vcs.Err.t)];
+  print_s [%sexp (Vcs.Err.init [%sexp Hello] ~step:[%sexp Step] : Vcs.Err.t)];
   [%expect {| ((steps (Step)) (error Hello)) |}];
   ()
 ;;