Rename [Result] => [Rresult] and add another [Result] (refactor)

mbarbin · Oct 21, 2024 · 2a21526 · 2a21526
1 parent f4b6bc6
commit 2a21526
Show file tree

Hide file tree

Showing 21 changed files with 130 additions and 67 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -8,9 +8,10 @@
         "opam",
         "pathspec",
         "rmtree",
+        "rresult",
         "Stdenv",
         "subdir",
         "worktree",
         "worktrees"
     ]
-}
+}
diff --git a/CHANGES.md b/CHANGES.md
@@ -2,12 +2,13 @@
 
 ### Added
 
-- Add new `vcs-base` package meant to extend `vcs` with base-style functionality.
+- Add new `vcs-base` package meant to extend `vcs` with base-style functionality (#31, @mbarbin).
 - Add `Vcs.find_enclosing_repo_root` helper (#28, @mbarbin).
 - Add `Vcs.read_dir` helper (#28, @mbarbin).
 
 ### Changed
 
+- Rename what was `Vcs.Result` to `Vcs.Rresult` and introduce `Vcs.Result` whose type is simpler (#PR, @mbarbin).
 - Moved `ocaml-vcs more-tests` commands at top-level (#28, @mbarbin).
 
 ### Deprecated

diff --git a/README.md b/README.md
@@ -39,7 +39,7 @@ We extend our gratitude to the following individuals and teams, whose contributi
 
 - Vincent Simonet and contributors for [headache](https://github.com/Frama-C/headache), which we use to manage the copyright headers at the beginning of our files.
 
-- The [Rresult](https://erratique.ch/software/rresult/doc/Rresult/index.html#usage) developers: Their usage design guidelines have been a reference in the design of `Vcs`'s error handling, the `Vcs.Result` module in particular.
+- The [Rresult](https://erratique.ch/software/rresult/doc/Rresult/index.html#usage) developers: Their usage design guidelines have been a reference in the design of `Vcs`'s error handling, the `Vcs.Rresult` module in particular.
 
 We look forward to continuing to learn from and collaborate with the broader open source community.
 

diff --git a/example/hello_blocking.ml b/example/hello_blocking.ml
@@ -68,7 +68,7 @@ let%expect_test "hello commit" =
         ~f:Result.return
     with
     | Ok _ -> assert false
-    | Error (`Vcs err) -> print_s (Vcs.Err.sexp_of_t err)
+    | Error err -> print_s (Vcs.Err.sexp_of_t err)
   in
   [%expect
     {|
@@ -88,10 +88,10 @@ let%expect_test "hello commit" =
       Vcs.Result.git vcs ~repo_root ~args:[ "rev-parse"; "INVALID-REF" ] ~f:(fun output ->
         if output.exit_code = 0
         then assert false [@coverage off]
-        else Error (`Vcs (Vcs.Err.create_s [%sexp "Hello invalid exit code"])))
+        else Error (Vcs.Err.create_s [%sexp "Hello invalid exit code"]))
     with
     | Ok _ -> assert false
-    | Error (`Vcs err) ->
+    | Error err ->
       print_s
         (Vcs_test_helpers.redact_sexp
            (Vcs.Err.sexp_of_t err)

diff --git a/example/hello_error.ml b/example/hello_error.ml
@@ -71,7 +71,7 @@ let%expect_test "hello error" =
   let () =
     match Vcs.Result.init vcs ~path:invalid_path with
     | Ok _ -> assert false
-    | Error (`Vcs err) -> print_s (redact_sexp [%sexp (err : Vcs.Err.t)])
+    | Error err -> print_s (redact_sexp [%sexp (err : Vcs.Err.t)])
   in
   [%expect
     {|

diff --git a/example/hello_git_cli.ml b/example/hello_git_cli.ml
@@ -150,10 +150,10 @@ let%expect_test "hello cli" =
       Vcs.Result.git vcs ~repo_root ~args:[ "rev-parse"; "INVALID-REF" ] ~f:(fun output ->
         if output.exit_code = 0
         then assert false [@coverage off]
-        else Error (`Vcs (Vcs.Err.create_s [%sexp "Hello invalid exit code"])))
+        else Error (Vcs.Err.create_s [%sexp "Hello invalid exit code"]))
     with
     | Ok _ -> assert false
-    | Error (`Vcs err) ->
+    | Error err ->
       print_s
         (Vcs_test_helpers.redact_sexp
            [%sexp (err : Vcs.Err.t)]

diff --git a/lib/vcs/src/git.ml b/lib/vcs/src/git.ml
@@ -79,4 +79,5 @@ let exit0 output = Or_error.exit0 output |> err_exn
 let exit0_and_stdout output = Or_error.exit0_and_stdout output |> err_exn
 let exit_code output ~accept = Or_error.exit_code output ~accept |> err_exn
 
-module Result = Non_raising.Make (Vcs_result0)
+module Rresult = Non_raising.Make (Vcs_rresult0)
+module Result = Non_raising.Make (Err.Private.Non_raising_M)
diff --git a/lib/vcs/src/git.mli b/lib/vcs/src/git.mli
@@ -33,8 +33,8 @@ end
 
 (** This is the interface commonly used by raising and non-raising helper
     modules, such as {!module:Vcs.Git}, {!module:Vcs.Git.Or_error},
-    {!module:Vcs.Git.Result}, and custom ones built with
-    {!module:Vcs.Git.Non_raising.Make}. [S] is parametrized by the result
+    {!module:Vcs.Git.Result}, {!module:Vcs.Git.Rresult}, and custom ones built
+    with {!module:Vcs.Git.Non_raising.Make}. [S] is parametrized by the result
     type returned by the helpers. *)
 module type S = Vcs_interface.Process_S
 
@@ -48,16 +48,17 @@ module type S = Vcs_interface.Process_S
 include S with type 'a result := 'a
 
 module Non_raising : sig
-  (** A functor to build non raising helpers based on a custom result type.
+  (** A functor to build non raising helpers based on a custom error type.
 
-      In addition to {!module:Vcs.Git.Or_error} and {!module:Vcs.Git.Result}, we
-      provide this functor to create a [Git.S] interface based on a custom
-      result type of your choice. *)
+      In addition to {!module:Vcs.Git.Or_error}, {!module:Vcs.Git.Result} and
+      {!module:Vcs.Git.Rresult} we provide this functor to create a [Git.S]
+      interface based on a custom error type of your choice. *)
 
   module type M = Vcs_interface.Error_S
 
   module Make (M : M) : S with type 'a result := ('a, M.t) Result.t
 end
 
 module Or_error : S with type 'a result := ('a, Vcs_or_error0.t) Result.t
-module Result : S with type 'a result := ('a, Vcs_result0.t) Result.t
+module Rresult : S with type 'a result := ('a, Vcs_rresult0.t) Result.t
+module Result : S with type 'a result := ('a, Err.t) Result.t
diff --git a/lib/vcs/src/vcs.ml b/lib/vcs/src/vcs.ml
@@ -45,6 +45,7 @@ module Remote_name = Remote_name
 module Repo_name = Repo_name
 module Repo_root = Repo_root
 module Result = Vcs_result
+module Rresult = Vcs_rresult
 module Rev = Rev
 module Tag_name = Tag_name
 module Trait = Trait

diff --git a/lib/vcs/src/vcs.mli b/lib/vcs/src/vcs.mli
@@ -324,6 +324,7 @@ module For_test = For_test
 
 module Or_error = Vcs_or_error
 module Result = Vcs_result
+module Rresult = Vcs_rresult
 module Non_raising = Non_raising
 
 module Private : sig

diff --git a/lib/vcs/src/vcs_or_error.ml b/lib/vcs/src/vcs_or_error.ml
@@ -19,7 +19,7 @@
 (*  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.       *)
 (*******************************************************************************)
 
-type err = Vcs_or_error0.t
-type 'a result = ('a, err) Result.t
+type err = Vcs_or_error0.t [@@deriving sexp_of]
+type 'a t = ('a, err) Result.t [@@deriving sexp_of]
 
 include Non_raising.Make (Vcs_or_error0)
diff --git a/lib/vcs/src/vcs_or_error.mli b/lib/vcs/src/vcs_or_error.mli
@@ -21,11 +21,11 @@
 
 (** An [Vcs] API based on [Base.Or_error]. *)
 
-type err = Error.t
-type 'a result = 'a Or_error.t
+type err = Error.t [@@deriving sexp_of]
+type 'a t = 'a Or_error.t [@@deriving sexp_of]
 
 (** {1 Non raising API}
 
     The individual functions are documented the {!module:Vcs} module. *)
 
-include Non_raising.S with type 'a t := 'a Vcs0.t and type 'a result := 'a result
+include Non_raising.S with type 'a t := 'a Vcs0.t and type 'a result := 'a t
diff --git a/lib/vcs/src/vcs_result.ml b/lib/vcs/src/vcs_result.ml
@@ -19,18 +19,9 @@
 (*  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.       *)
 (*******************************************************************************)
 
-type err = Vcs_result0.t
-type 'a result = ('a, err) Result.t
+open! Import
 
-include Non_raising.Make (Vcs_result0)
+type err = Err.t [@@deriving sexp_of]
+type 'a t = ('a, err) Result.t [@@deriving sexp_of]
 
-let pp_error fmt (`Vcs err) = Stdlib.Format.pp_print_string fmt (Err.to_string_hum err)
-
-let open_error = function
-  | Ok _ as r -> r
-  | Error (`Vcs _) as r -> r
-;;
-
-let error_to_msg (r : 'a result) =
-  Result.map_error r ~f:(fun (`Vcs err) -> `Msg (Err.to_string_hum err))
-;;
+include Non_raising.Make (Err.Private.Non_raising_M)
diff --git a/lib/vcs/src/vcs_result.mli b/lib/vcs/src/vcs_result.mli
@@ -19,24 +19,13 @@
 (*_  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.       *)
 (*_******************************************************************************)
 
-(** An [Vcs] API in the style of
-    {{:https://erratique.ch/software/rresult/doc/Rresult/index.html#usage} Rresult}. *)
+(** An [Vcs] API based on [Result] and [Vcs.Err]. *)
 
-type err = [ `Vcs of Err.t ]
-type 'a result = ('a, err) Result.t
-
-(** {1 Utils}
-
-    This part exposes the functions prescribed by the
-    {{:https://erratique.ch/software/rresult/doc/Rresult/index.html#usage} Rresult}
-    usage design guidelines. *)
-
-val pp_error : Stdlib.Format.formatter -> [ `Vcs of Err.t ] -> unit
-val open_error : 'a result -> ('a, [> `Vcs of Err.t ]) Result.t
-val error_to_msg : 'a result -> ('a, [ `Msg of string ]) Result.t
+type err = Err.t [@@deriving sexp_of]
+type 'a t = ('a, err) Result.t [@@deriving sexp_of]
 
 (** {1 Non raising API}
 
     The individual functions are documented the {!module:Vcs} module. *)
 
-include Non_raising.S with type 'a t := 'a Vcs0.t and type 'a result := 'a result
+include Non_raising.S with type 'a t := 'a Vcs0.t and type 'a result := 'a t
diff --git a/lib/vcs/src/vcs_rresult.ml b/lib/vcs/src/vcs_rresult.ml
@@ -0,0 +1,39 @@
+(*******************************************************************************)
+(*  Vcs - a Versatile OCaml Library for Git Operations                         *)
+(*  Copyright (C) 2024 Mathieu Barbin <[email protected]>               *)
+(*                                                                             *)
+(*  This file is part of Vcs.                                                  *)
+(*                                                                             *)
+(*  Vcs is free software; you can redistribute it and/or modify it under       *)
+(*  the terms of the GNU Lesser General Public License as published by the     *)
+(*  Free Software Foundation either version 3 of the License, or any later     *)
+(*  version, with the LGPL-3.0 Linking Exception.                              *)
+(*                                                                             *)
+(*  Vcs is distributed in the hope that it will be useful, but WITHOUT ANY     *)
+(*  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS  *)
+(*  FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License and    *)
+(*  the file `NOTICE.md` at the root of this repository for more details.      *)
+(*                                                                             *)
+(*  You should have received a copy of the GNU Lesser General Public License   *)
+(*  and the LGPL-3.0 Linking Exception along with this library. If not, see    *)
+(*  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.       *)
+(*******************************************************************************)
+
+open! Import
+
+type err = Vcs_rresult0.t [@@deriving sexp_of]
+type 'a t = ('a, err) Result.t [@@deriving sexp_of]
+type 'a result = 'a t
+
+include Non_raising.Make (Vcs_rresult0)
+
+let pp_error fmt (`Vcs err) = Stdlib.Format.pp_print_string fmt (Err.to_string_hum err)
+
+let open_error = function
+  | Ok _ as r -> r
+  | Error (`Vcs _) as r -> r
+;;
+
+let error_to_msg (r : 'a result) =
+  Result.map_error r ~f:(fun (`Vcs err) -> `Msg (Err.to_string_hum err))
+;;
diff --git a/lib/vcs/src/vcs_rresult.mli b/lib/vcs/src/vcs_rresult.mli
@@ -0,0 +1,43 @@
+(*_******************************************************************************)
+(*_  Vcs - a Versatile OCaml Library for Git Operations                         *)
+(*_  Copyright (C) 2024 Mathieu Barbin <[email protected]>               *)
+(*_                                                                             *)
+(*_  This file is part of Vcs.                                                  *)
+(*_                                                                             *)
+(*_  Vcs is free software; you can redistribute it and/or modify it under       *)
+(*_  the terms of the GNU Lesser General Public License as published by the     *)
+(*_  Free Software Foundation either version 3 of the License, or any later     *)
+(*_  version, with the LGPL-3.0 Linking Exception.                              *)
+(*_                                                                             *)
+(*_  Vcs is distributed in the hope that it will be useful, but WITHOUT ANY     *)
+(*_  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS  *)
+(*_  FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License and    *)
+(*_  the file `NOTICE.md` at the root of this repository for more details.      *)
+(*_                                                                             *)
+(*_  You should have received a copy of the GNU Lesser General Public License   *)
+(*_  and the LGPL-3.0 Linking Exception along with this library. If not, see    *)
+(*_  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.       *)
+(*_******************************************************************************)
+
+(** An [Vcs] API in the style of
+    {{:https://erratique.ch/software/rresult/doc/Rresult/index.html#usage} Rresult}. *)
+
+type err = [ `Vcs of Err.t ] [@@deriving sexp_of]
+type 'a t = ('a, err) Result.t [@@deriving sexp_of]
+type 'a result = 'a t
+
+(** {1 Utils}
+
+    This part exposes the functions prescribed by the
+    {{:https://erratique.ch/software/rresult/doc/Rresult/index.html#usage} Rresult}
+    usage design guidelines. *)
+
+val pp_error : Stdlib.Format.formatter -> [ `Vcs of Err.t ] -> unit
+val open_error : 'a result -> ('a, [> `Vcs of Err.t ]) Result.t
+val error_to_msg : 'a result -> ('a, [ `Msg of string ]) Result.t
+
+(** {1 Non raising API}
+
+    The individual functions are documented the {!module:Vcs} module. *)
+
+include Non_raising.S with type 'a t := 'a Vcs0.t and type 'a result := 'a result
diff --git a/lib/vcs/src/vcs_result0.ml → lib/vcs/src/vcs_rresult0.ml b/lib/vcs/src/vcs_result0.ml → lib/vcs/src/vcs_rresult0.ml
diff --git a/lib/vcs/src/vcs_result0.mli → lib/vcs/src/vcs_rresult0.mli b/lib/vcs/src/vcs_result0.mli → lib/vcs/src/vcs_rresult0.mli
diff --git a/lib/vcs/test/test__git.ml b/lib/vcs/test/test__git.ml
@@ -113,13 +113,9 @@ let%expect_test "exit_code" =
 (* [Vcs.Git.Result] *)
 
 let%expect_test "exit0" =
-  let test output =
-    match Vcs.Git.Result.exit0 output with
-    | Ok () -> print_s [%sexp Ok]
-    | Error (`Vcs err) -> print_s [%sexp Error (err : Vcs.Err.t)]
-  in
+  let test output = print_s [%sexp (Vcs.Git.Result.exit0 output : unit Vcs.Result.t)] in
   test { exit_code = 0; stdout = ""; stderr = "" };
-  [%expect {| Ok |}];
+  [%expect {| (Ok ()) |}];
   (* The error does not contain the stdout or stderr, as this is already handled
      by the code that interprets the result of the user function supplied to
      [Vcs.Result.git]. *)
@@ -130,9 +126,7 @@ let%expect_test "exit0" =
 
 let%expect_test "exit0_and_stdout" =
   let test output =
-    match Vcs.Git.Result.exit0_and_stdout output with
-    | Ok stdout -> print_s [%sexp Ok (stdout : string)]
-    | Error (`Vcs err) -> print_s [%sexp Error (err : Vcs.Err.t)]
+    print_s [%sexp (Vcs.Git.Result.exit0_and_stdout output : string Vcs.Result.t)]
   in
   test { exit_code = 0; stdout = "stdout"; stderr = "" };
   [%expect {| (Ok stdout) |}];
@@ -144,9 +138,10 @@ let%expect_test "exit0_and_stdout" =
 
 let%expect_test "exit_code" =
   let test output =
-    match Vcs.Git.Result.exit_code output ~accept:[ 0, "ok"; 42, "other" ] with
-    | Ok result -> print_s [%sexp Ok (result : string)]
-    | Error (`Vcs err) -> print_s [%sexp Error (err : Vcs.Err.t)]
+    print_s
+      [%sexp
+        (Vcs.Git.Result.exit_code output ~accept:[ 0, "ok"; 42, "other" ]
+         : string Vcs.Result.t)]
   in
   test { exit_code = 0; stdout = ""; stderr = "" };
   [%expect {| (Ok ok) |}];

diff --git a/lib/vcs/test/test__result.ml → lib/vcs/test/test__rresult.ml b/lib/vcs/test/test__result.ml → lib/vcs/test/test__rresult.ml
@@ -20,14 +20,14 @@
 (*******************************************************************************)
 
 let%expect_test "pp_error" =
-  Vcs.Result.pp_error Stdlib.Format.std_formatter (`Vcs (Vcs.Err.create_s [%sexp Hello]));
+  Vcs.Rresult.pp_error Stdlib.Format.std_formatter (`Vcs (Vcs.Err.create_s [%sexp Hello]));
   [%expect {| Hello |}];
   ()
 ;;
 
 let%expect_test "error_to_msg" =
   let test r =
-    print_s [%sexp (Vcs.Result.error_to_msg r : (unit, [ `Msg of string ]) Result.t)]
+    print_s [%sexp (Vcs.Rresult.error_to_msg r : (unit, [ `Msg of string ]) Result.t)]
   in
   test (Ok ());
   [%expect {| (Ok ()) |}];
@@ -57,10 +57,10 @@ let%expect_test "open_error" =
       | Ok _ as r -> r
       | Error (`My_int_error _) as r -> r [@coverage off]
     in
-    let ok = (Ok () : unit Vcs.Result.result) in
-    let%bind.Result () = Vcs.Result.open_error ok in
+    let ok = (Ok () : unit Vcs.Rresult.t) in
+    let%bind.Result () = Vcs.Rresult.open_error ok in
     let error = Error (`Vcs (Vcs.Err.create_s [%sexp Vcs_error])) in
-    let%bind.Result () = Vcs.Result.open_error error in
+    let%bind.Result () = Vcs.Rresult.open_error error in
     (Result.return () [@coverage off])
   in
   print_s [%sexp (result : (unit, [ `My_int_error of int | `Vcs of Vcs.Err.t ]) Result.t)];

diff --git a/lib/vcs/test/test__result.mli → lib/vcs/test/test__rresult.mli b/lib/vcs/test/test__result.mli → lib/vcs/test/test__rresult.mli