odoc: Don't add unecessary space in references and links

CHANGES.md

@@ -106,6 +106,7 @@ profile. This started with version 0.26.0.
 - \* Fix arrow type indentation with `break-separators=before` (#2598, @Julow)
 - Fix missing parentheses around a let in class expressions (#2599, @Julow)
 - Fix formatting of paragraphs in lists in documentation (#2607, @Julow)
+- Avoid unwanted space in references and links text in documentation (#2608, @Julow)
 
 ## 0.26.2 (2024-04-18)
 

lib/Fmt_odoc.ml

@@ -89,6 +89,10 @@ let str_normalized ?(escape = escape_all) c s =
     |> fun s -> list s space_break (fun s -> escape s |> str)
   else str (escape s)
 
+let rec drop_leading_spaces = function
+  | {Loc.value= `Space _; _} :: tl -> drop_leading_spaces tl
+  | elems -> elems
+
 let ign_loc ~f with_loc = f with_loc.Loc.value
 
 let fmt_verbatim_block ~loc s =
@@ -204,18 +208,10 @@ let list_block_elem c elems f =
       in
       f elem $ break )
 
-let space_elt c : inline_element with_location =
-  let sp = if c.conf.fmt_opts.wrap_docstrings.v then "" else " " in
-  Loc.(at (span []) (`Space sp))
-
 let non_wrap_space sp =
   if String.contains sp '\n' then force_newline else str sp
 
 let rec fmt_inline_elements c elements =
-  let wrap_elements opn cls ~always_wrap hd = function
-    | [] -> wrap_if always_wrap opn cls hd
-    | tl -> wrap opn cls (hd $ fmt_inline_elements c (space_elt c :: tl))
-  in
   let rec aux = function
     | [] -> noop
     | `Space sp :: `Word (("-" | "+") as w) :: t ->
@@ -252,20 +248,35 @@ let rec fmt_inline_elements c elements =
         in
         hovbox_if c.conf.fmt_opts.wrap_docstrings.v
           (1 + String.length s + 1)
-          (wrap_elements (str "{") (str "}") ~always_wrap:true
+          (fmt_markup_with_inline_elements c ~force_space:true
              (str_normalized c s) elems )
         $ aux t
     | `Reference (_kind, rf, txt) :: t ->
-        let rf = wrap (str "{!") (str "}") (fmt_reference rf) in
-        wrap_elements (str "{") (str "}") ~always_wrap:false rf txt $ aux t
+        let rf = str "{!" $ fmt_reference rf $ str "}" in
+        fmt_link_or_reference c rf txt $ aux t
     | `Link (url, txt) :: t ->
-        let url = wrap (str "{:") (str "}") (str_normalized c url) in
-        hovbox_if c.conf.fmt_opts.wrap_docstrings.v 2
-        @@ wrap_elements (str "{") (str "}") ~always_wrap:false url txt
-        $ aux t
+        let url = str "{:" $ str_normalized c url $ str "}" in
+        fmt_link_or_reference c url txt $ aux t
   in
   aux (List.map elements ~f:(ign_loc ~f:Fn.id))
 
+and fmt_link_or_reference c tag txt =
+  match txt with
+  | [] -> tag
+  | _ :: _ ->
+      hovbox_if c.conf.fmt_opts.wrap_docstrings.v 1
+        (fmt_markup_with_inline_elements c tag txt)
+
+(** Format a markup of the form [{tag elems}]. If [force_space] is [true], a
+    space will be added after the tag, even if it's not present in the source.
+*)
+and fmt_markup_with_inline_elements c ?(force_space = false) tag elems =
+  let leading_space, elems =
+    if force_space then (str " ", drop_leading_spaces elems)
+    else (noop, elems)
+  in
+  str "{" $ tag $ leading_space $ fmt_inline_elements c elems $ str "}"
+
 and fmt_nestable_block_element c elm =
   match elm.Loc.value with
   | `Paragraph elems -> hovbox 0 (fmt_inline_elements c elems)
@@ -348,12 +359,9 @@ let fmt_block_element c elm =
         | Some lbl -> str ":" $ str_normalized c lbl
         | None -> noop
       in
-      let elems =
-        if List.is_empty elems then elems else space_elt c :: elems
-      in
+      let tag = str lvl $ lbl in
       hovbox 0
-        (wrap (str "{") (str "}")
-           (str lvl $ lbl $ fmt_inline_elements c elems) )
+        (fmt_markup_with_inline_elements c ~force_space:true tag elems)
   | #nestable_block_element as value ->
       hovbox 0 (fmt_nestable_block_element c {elm with value})
 

test/passing/tests/doc.mld

@@ -157,3 +157,6 @@ and we shall get
 ]}
 
 For more about [odoc] commands, simply invoke [odoc --help] in your shell.
+
+Preserve the space between a link/reference and its text:
+{{:foo}bar} {{:foo} bar} {{!foo}bar} {{!foo} bar}

test/passing/tests/doc.mld.ref

@@ -1,7 +1,7 @@
 {0 Parent/Child Specification}
 This parent/child specification allows more flexible output support, e.g.,
 per library documentation. See
-{{:https://v3.ocaml.org/packages} v3.ocaml.org/packages}.
+{{:https://v3.ocaml.org/packages}v3.ocaml.org/packages}.
 
 The rules are;
 
@@ -160,3 +160,6 @@ and we shall get
 ]}
 
 For more about [odoc] commands, simply invoke [odoc --help] in your shell.
+
+Preserve the space between a link/reference and its text: {{:foo}bar}
+{{:foo} bar} {{!foo}bar} {{!foo} bar}

test/passing/tests/doc_comments-no-parse-docstrings.mli.ref

@@ -650,3 +650,7 @@ type x =
     {- baz}
     }
 *)
+
+(** Space before a reference or link text is preserved. A newline is turned
+    into a space. {{!ref}
+    with newline} and {{!ref} with space}. *)

test/passing/tests/doc_comments-no-wrap.mli.ref

@@ -477,8 +477,9 @@ end
     {i fooooooooooooo oooooooo oooooooooo}
     {b fooooooooooooo oooooooooooo oooooo ooooooo} *)
 
-(** {e foooooooo oooooooooo ooooooooo ooooooooo} {{!some ref} fooooooooooooo
-    oooooooo oooooooooo} {b fooooooooooooo oooooooooooo oooooo ooooooo} *)
+(** {e foooooooo oooooooooo ooooooooo ooooooooo}
+    {{!some ref} fooooooooooooo oooooooo oooooooooo}
+    {b fooooooooooooo oooooooooooo oooooo ooooooo} *)
 
 (** foooooooooooooooooooooooooooooooooooooooooooooooooo foooooooooooo
     {b eee + eee eee} *)
@@ -498,8 +499,7 @@ val k : int
        {i fooooooooooooo oooooooo oooooooooo
           {b fooooooooooooo oooooooooooo oooooo ooooooo}}} *)
 
-(** {e
-       {i fooooooooooooo oooooooo oooooooooo
+(** {e {i fooooooooooooo oooooooo oooooooooo
           {b fooooooooooooo oooooooooooo oooooo ooooooo}} foooooooo
        oooooooooo ooooooooo ooooooooo} *)
 
@@ -692,3 +692,6 @@ type x =
         + baz
      }
     } *)
+
+(** Space before a reference or link text is preserved. A newline is turned
+    into a space. {{!ref} with newline} and {{!ref} with space}. *)

test/passing/tests/doc_comments.mli

@@ -657,3 +657,7 @@ type x =
     {- baz}
     }
 *)
+
+(** Space before a reference or link text is preserved. A newline is turned
+    into a space. {{!ref}
+    with newline} and {{!ref} with space}. *)

test/passing/tests/doc_comments.mli.ref

@@ -477,8 +477,9 @@ end
     {i fooooooooooooo oooooooo oooooooooo}
     {b fooooooooooooo oooooooooooo oooooo ooooooo} *)
 
-(** {e foooooooo oooooooooo ooooooooo ooooooooo} {{!some ref} fooooooooooooo
-    oooooooo oooooooooo} {b fooooooooooooo oooooooooooo oooooo ooooooo} *)
+(** {e foooooooo oooooooooo ooooooooo ooooooooo}
+    {{!some ref} fooooooooooooo oooooooo oooooooooo}
+    {b fooooooooooooo oooooooooooo oooooo ooooooo} *)
 
 (** foooooooooooooooooooooooooooooooooooooooooooooooooo foooooooooooo
     {b eee + eee eee} *)
@@ -498,8 +499,7 @@ val k : int
        {i fooooooooooooo oooooooo oooooooooo
           {b fooooooooooooo oooooooooooo oooooo ooooooo}}} *)
 
-(** {e
-       {i fooooooooooooo oooooooo oooooooooo
+(** {e {i fooooooooooooo oooooooo oooooooooo
           {b fooooooooooooo oooooooooooo oooooo ooooooo}} foooooooo
        oooooooooo ooooooooo ooooooooo} *)
 
@@ -686,3 +686,6 @@ type x =
         + baz
      }
     } *)
+
+(** Space before a reference or link text is preserved. A newline is turned
+    into a space. {{!ref} with newline} and {{!ref} with space}. *)

vendor/odoc-parser/syntax.ml

@@ -281,13 +281,15 @@ and delimited_inline_element_list :
   let first_token = peek input in
   match first_token.value with
   | `Space _ ->
-      junk input;
+      (* Preserve leading spaces in markups
+         junk input; *)
       consume_elements ~at_start_of_line:false []
       (* [~at_start_of_line] is [false] here because the preceding token was some
          some markup like '{b', and we didn't move to the next line, so the next
          token will not be the first non-whitespace token on its line. *)
   | `Single_newline _ ->
-      junk input;
+      (* Preserve leading spaces in markups
+         junk input; *)
       consume_elements ~at_start_of_line:true []
   | `Blank_line _ as blank ->
       (* In case the markup is immediately followed by a blank line, the error