Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdcompat dependency #304

Closed
hannesm opened this issue Mar 20, 2023 · 3 comments · Fixed by #305
Closed

stdcompat dependency #304

hannesm opened this issue Mar 20, 2023 · 3 comments · Fixed by #305

Comments

@hannesm
Copy link
Contributor

hannesm commented Mar 20, 2023

Dear Madam or Sir,

first of all thanks for maintaining this package. I've some trouble with the stdcompat dependency (which fails to install for me, reported ocamllibs/stdcompat#28).

So, I looked into OCaml 4.08 and omd without stdcompat, and the only two functions used from Stdcompat are List.find_map and String.for_all. Would you accept a patch (see below) that removes the stdcompat dependency (since omd requires OCaml 4.08).

WDYT?

diff --git a/dune-project b/dune-project
index e4dec6f..5270431 100644
--- a/dune-project
+++ b/dune-project
@@ -26,7 +26,6 @@ package installs both the OMD library and the command line tool `omd`.")
  (tags (org:ocamllabs org:mirage))
  (depends
   (ocaml (>= 4.08))
-   stdcompat
    uutf
    uucp
    uunf
diff --git a/omd.opam b/omd.opam
index b6dd14a..6011e09 100644
--- a/omd.opam
+++ b/omd.opam
@@ -23,7 +23,6 @@ bug-reports: "https://github.com/ocaml/omd/issues"
 depends: [
   "dune" {>= "2.7"}
   "ocaml" {>= "4.08"}
-  "stdcompat"
   "uutf"
   "uucp"
   "uunf"
diff --git a/src/dune b/src/dune
index 47c7825..45ccf64 100644
--- a/src/dune
+++ b/src/dune
@@ -1,7 +1,7 @@
 (library
  (name omd)
  (public_name omd)
- (libraries uutf uucp uunf stdcompat))
+ (libraries uutf uucp uunf))
 
 (rule
  (with-stdout-to
diff --git a/src/parser.ml b/src/parser.ml
index 5fbf669..d6ae4f9 100644
--- a/src/parser.ml
+++ b/src/parser.ml
@@ -1,5 +1,4 @@
 open Ast.Impl
-open Stdcompat
 
 type 'attr link_def =
   { label : string
@@ -1618,6 +1617,14 @@ let get_buf buf =
 let text buf acc =
   if Buffer.length buf = 0 then acc else Pre.R (Text ([], get_buf buf)) :: acc
 
+let for_all p s =
+  let n = String.length s in
+  let rec loop i =
+    if i = n then true
+    else if p (String.unsafe_get s i) then loop (succ i)
+    else false in
+  loop 0
+
 let inline_pre buf acc st =
   let pos = pos st in
   let rec gobble_open_backtick n =
@@ -1631,7 +1638,7 @@ let inline_pre buf acc st =
         let finish () =
           let content = Buffer.contents bufcode in
           let content =
-            if String.for_all (fun c -> c = ' ') content then content
+            if for_all (fun c -> c = ' ') content then content
             else if
               String.length content >= 2
               && content.[0] = ' '
diff --git a/src/strSlice.ml b/src/strSlice.ml
index 01a9e79..2a21682 100644
--- a/src/strSlice.ml
+++ b/src/strSlice.ml
@@ -1,5 +1,3 @@
-open Stdcompat
-
 type t =
   { base : string
   ; off : int
diff --git a/src/toc.ml b/src/toc.ml
index f962762..d2b5003 100644
--- a/src/toc.ml
+++ b/src/toc.ml
@@ -1,5 +1,4 @@
 open Ast.Util
-open Stdcompat
 
 let rec remove_links inline =
   match inline with
@@ -59,8 +58,16 @@ let rec find_start headers level number subsections =
 
 let unordered_list items = List ([], Bullet '*', Tight, items)
 
+let rec find_map f = function
+  | [] -> None
+  | x :: l ->
+     begin match f x with
+       | Some _ as result -> result
+       | None -> find_map f l
+     end
+
 let find_id attributes =
-  List.find_map
+  find_map
     (function k, v when String.equal "id" k -> Some v | _ -> None)
     attributes
 
@hannesm
Copy link
Contributor Author

hannesm commented Mar 20, 2023

(alternatively, the lower bound for OCaml could be increased to 4.13 -- I think this is as well fine, since most users of omd won't use old OCaml compilers)

@shonfeder
Copy link
Collaborator

Hi! Sounds like your patch is a good idea: I didn't realise stdcompat was so brittle (tho I see I also filed an issue there awhile back.)

Thanks for the patch!

@hannesm
Copy link
Contributor Author

hannesm commented Mar 24, 2023

Thanks for your reply, @shonfeder. What is the plan forward in terms of OCaml version lower bound? Is just putting it on 4.13 fine with you? I opened #305 that requires OCaml 4.13 (with 5.0 out, I don't think maintaining old 4.x Stdlibs is worth it).

samoht added a commit that referenced this issue Jul 23, 2023
Remove stdcompat dependency (fixes #304), require OCaml 4.13
avsm added a commit to avsm/opam-repository that referenced this issue Jul 24, 2023
CHANGES:

- Add an `empty` inline value (ocaml-community/omd#298 @cuihtlauac)

- Remove stdcompat dependency (ocaml-community/omd#304 ocaml-community/omd#305 @hannesm, review by @samoht)

- Minimum supported OCaml version is now 4.13 (ocaml-community/omd#304 @hannesm)
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

- Add an `empty` inline value (ocaml-community/omd#298 @cuihtlauac)

- Remove stdcompat dependency (ocaml-community/omd#304 ocaml-community/omd#305 @hannesm, review by @samoht)

- Minimum supported OCaml version is now 4.13 (ocaml-community/omd#304 @hannesm)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants