refactor give plain names to the error-preserving lookups

is_directory_result, file_exists_result, is_git_directory_result and repository_layout_result carried a suffix naming their return container, which the type already states. The suffix existed only to leave the plain name free for repository_layout, the wrapper that discards errors into None — so the four careful functions were marked and the lossy one was not. Invert it: the error-preserving functions take the plain names, and the wrapper becomes repository_layout_or_none. Naming the qualifier on the lossy path puts it on the behaviour a reader needs warning about, since collapsing Error to None turns an unreadable directory into one that merely is not a repository. The interface now exposes both, so callers choose deliberately; the discovery tests, which only assert on layout shape, use the option form.

Commit
92eff9d4d0a1659ff3db9271e409c677a29a3013
Author
Claude Sonnet 4 <claude@anthropic.invalid>
Author date
Committer
Marius Peter <dev@marius-peter.com>
Committer date
Changed files
lib/resolvers.ml
index 70fb3dad..1378bc14 100644..100644
@@ -47,12 +47,21 @@
47 47 let filesystem_error path error =
48 48 Internal (Printf.sprintf "%s: %s" path (Unix.error_message error))
49 49
50 Removed: let is_directory_result path =
50 Added: (* These report filesystem failures rather than hiding them, so that a directory
51 Added: made unreadable by permissions is distinguishable from one that is simply not
52 Added: a repository. A missing path is not a failure: it answers the question with
53 Added: [false].
54 Added:
55 Added: The plain names belong to these error-preserving functions. It is
56 Added: [repository_layout_or_none] below, which discards the distinction, that
57 Added: carries the qualifier — the surprising behaviour is the one worth naming. *)
58 Added:
59 Added: let is_directory path =
51 60 try Ok ((Unix.stat path).st_kind = Unix.S_DIR) with
52 61 | Unix.Unix_error ((Unix.ENOENT | Unix.ENOTDIR), _, _) -> Ok false
53 62 | Unix.Unix_error (error, _, _) -> Error (filesystem_error path error)
54 63
55 Removed: let file_exists_result path =
64 Added: let file_exists path =
56 65 try
57 66 ignore (Unix.stat path);
58 67 Ok true
@@ -60,33 +69,37 @@
60 69 | Unix.Unix_error ((Unix.ENOENT | Unix.ENOTDIR), _, _) -> Ok false
61 70 | Unix.Unix_error (error, _, _) -> Error (filesystem_error path error)
62 71
63 Removed: let is_git_directory_result path =
72 Added: (* A directory is a Git directory when it holds both HEAD and objects/. *)
73 Added: let is_git_directory path =
64 74 let ( let* ) = Result.bind in
65 Removed: let* directory = is_directory_result path in
75 Added: let* directory = is_directory path in
66 76 if not directory then Ok false
67 77 else
68 Removed: let* has_head = file_exists_result (Filename.concat path "HEAD") in
69 Removed: let* has_objects = is_directory_result (Filename.concat path "objects") in
78 Added: let* has_head = file_exists (Filename.concat path "HEAD") in
79 Added: let* has_objects = is_directory (Filename.concat path "objects") in
70 80 Ok (has_head && has_objects)
71 81
72 Removed: let repository_layout_result path =
82 Added: (* [Ok None] means "readable, but not a repository"; [Error _] means "could not
83 Added: tell". Keeping them apart is the whole point of this layer. *)
84 Added: let repository_layout path =
73 85 let ( let* ) = Result.bind in
74 86 let dotgit = Filename.concat path ".git" in
75 Removed: let* worktree = is_directory_result path in
87 Added: let* worktree = is_directory path in
76 88 if not worktree then Ok None
77 89 else
78 Removed: let* non_bare = is_git_directory_result dotgit in
90 Added: let* non_bare = is_git_directory dotgit in
79 91 if non_bare then Ok (Some { worktree = path; git_dir = dotgit })
80 92 else
81 Removed: let* bare = is_git_directory_result path in
93 Added: let* bare = is_git_directory path in
82 94 if bare then Ok (Some { worktree = path; git_dir = path }) else Ok None
83 95
84 Removed: let repository_layout path =
85 Removed: match repository_layout_result path with
86 Removed: | Ok layout -> layout
87 Removed: | Error _ -> None
96 Added: (** Collapse an unreadable path to [None], for the callers that cannot act on
97 Added: the difference anyway. Prefer {!repository_layout} where the distinction
98 Added: between "not a repository" and "could not tell" matters. *)
99 Added: let repository_layout_or_none path =
100 Added: match repository_layout path with Ok layout -> layout | Error _ -> None
88 101
89 Removed: let is_repository path = Option.is_some (repository_layout path)
102 Added: let is_repository path = Option.is_some (repository_layout_or_none path)
90 103
91 104 let repositories config =
92 105 try
@@ -98,7 +111,7 @@
98 111 collect repositories rest
99 112 | name :: rest ->
100 113 let path = Filename.concat config.Config.git_project_root name in
101 Removed: let* layout = repository_layout_result path in
114 Added: let* layout = repository_layout path in
102 115 collect
103 116 (if Option.is_some layout then name :: repositories
104 117 else repositories)
@@ -138,10 +151,10 @@
138 151 collect acc rest
139 152 | name :: rest -> (
140 153 let path = Filename.concat dir_path name in
141 Removed: let* is_dir = is_directory_result path in
154 Added: let* is_dir = is_directory path in
142 155 if not is_dir then collect acc rest
143 156 else
144 Removed: let* layout = repository_layout_result path in
157 Added: let* layout = repository_layout path in
145 158 match layout with
146 159 | Some layout ->
147 160 let description = description_of_layout layout in
@@ -166,7 +179,7 @@
166 179 let open_repository config name =
167 180 let* name = validate_repo_name name in
168 181 let path = Filename.concat config.Config.git_project_root name in
169 Removed: match repository_layout_result path with
182 Added: match repository_layout path with
170 183 | Error error -> Lwt_result.fail error
171 184 | Ok None -> Lwt_result.fail (Not_found ("not a Git repository " ^ name))
172 185 | Ok (Some ({ worktree; git_dir } as layout)) ->
@@ -730,10 +743,10 @@
730 743 collect acc rest
731 744 | name :: rest -> (
732 745 let path = Filename.concat dir_path name in
733 Removed: let* is_dir = is_directory_result path in
746 Added: let* is_dir = is_directory path in
734 747 if not is_dir then collect acc rest
735 748 else
736 Removed: let* layout = repository_layout_result path in
749 Added: let* layout = repository_layout path in
737 750 match layout with
738 751 | Some layout ->
739 752 let description = description_of_layout layout in
lib/resolvers.mli
index 9d5501ae..c4ae683c 100644..100644
@@ -23,7 +23,16 @@
23 23 type repository
24 24
25 25 val is_repository : string -> bool
26 Removed: val repository_layout : string -> repository_layout option
26 Added:
27 Added: val repository_layout : string -> (repository_layout option, error) result
28 Added: (** Locate a repository's worktree and git directory. [Ok None] means the path
29 Added: is readable but is not a repository; [Error _] means it could not be
30 Added: inspected. *)
31 Added:
32 Added: val repository_layout_or_none : string -> repository_layout option
33 Added: (** {!repository_layout} with unreadable paths collapsed to [None], for callers
34 Added: that cannot act on the difference. *)
35 Added:
27 36 val repositories : Config.t -> (string list, error) result
28 37
29 38 type repo_info = { repo_name : string; description : string }
test/test_discovery.ml
index 5900ad99..9bd5605b 100644..100644
@@ -34,7 +34,7 @@
34 34 Alcotest.(check bool)
35 35 "is_repository" true
36 36 (Ogit.Resolvers.is_repository bare);
37 Removed: match Ogit.Resolvers.repository_layout bare with
37 Added: match Ogit.Resolvers.repository_layout_or_none bare with
38 38 | Some { worktree; git_dir } ->
39 39 Alcotest.(check string) "worktree" bare worktree;
40 40 Alcotest.(check string) "git_dir" bare git_dir
@@ -48,7 +48,7 @@
48 48 Alcotest.(check bool)
49 49 "is_repository" true
50 50 (Ogit.Resolvers.is_repository clone);
51 Removed: match Ogit.Resolvers.repository_layout clone with
51 Added: match Ogit.Resolvers.repository_layout_or_none clone with
52 52 | Some { worktree; git_dir } ->
53 53 Alcotest.(check string) "worktree" clone worktree;
54 54 Alcotest.(check string)
@@ -78,7 +78,7 @@
78 78 Alcotest.(check bool)
79 79 "is_repository" true
80 80 (Ogit.Resolvers.is_repository repo);
81 Removed: match Ogit.Resolvers.repository_layout repo with
81 Added: match Ogit.Resolvers.repository_layout_or_none repo with
82 82 | Some { worktree; git_dir } ->
83 83 Alcotest.(check string) "worktree" repo worktree;
84 84 Alcotest.(check string)