Skip to content

HTML: Match rendering of polymorphic variant with normal variants #971

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 26, 2023
Merged

HTML: Match rendering of polymorphic variant with normal variants #971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ce1df23
Simplify css for docs on variants and poly-variants
panglesd Jul 4, 2023
c78ffab
Link to Ocamlary from index of pages
panglesd Jul 5, 2023
e4c5cea
Use documentedSrc instead of code in polyvariants
wikku Aug 3, 2023
1cae830
Consider extension constructor as constructors in css
panglesd Sep 6, 2023
a4dff22
css: use list-specific selector for list-specific properties
panglesd Sep 12, 2023
61666b2
compatibility with 4.02
panglesd Sep 12, 2023
1f24c7c
Add changelog entry for #971
panglesd Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Tags:

### Added
- Display 'private' keyword for private type extensions (@gpetiot, #1019)
- Fix rendering of polymorphic variants (@wikku, @panglesd, #971)

# 2.3.0

Expand Down
2 changes: 1 addition & 1 deletion doc/driver.mld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ let dep_libraries =
let odoc_libraries = [
"odoc_xref_test"; "odoc_xref2"; "odoc_odoc"; "odoc_html_support_files";
"odoc_model_desc"; "odoc_model"; "odoc_manpage"; "odoc_loader";
"odoc_latex"; "odoc_html"; "odoc_document"; "odoc_examples"; "odoc_parser" ];;
"odoc_latex"; "odoc_html"; "odoc_document"; "odoc_examples"; "odoc_parser"; "ocamlary" ];;

let all_libraries = dep_libraries @ odoc_libraries;;

Expand Down
3 changes: 2 additions & 1 deletion doc/library_mlds/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@
odoc_model_desc
odoc_odoc
odoc_xref2
odoc_xref_test))
odoc_xref_test
ocamlary))
3 changes: 3 additions & 0 deletions doc/library_mlds/ocamlary.mld
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{0 Ocamlary}

A demonstration of the rendering of most of the OCaml constructs {!module-Ocamlary}.
1 change: 1 addition & 0 deletions doc/odoc.mld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,5 @@ The main other pages of this site:
- {!page-dune} Dune
- {!page-parent_child_spec} Parent/Child specification
- {!page-interface} Interface guarantees
- {!page-ocamlary} A demonstration of the rendering of most of the OCaml constructs

22 changes: 13 additions & 9 deletions src/document/generator.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,9 @@ module Make (Syntax : SYNTAX) = struct
| Error e -> failwith (Url.Error.to_string e)
| Ok url ->
let anchor = Some url in
let attrs = [ "def"; Url.Anchor.string_of_kind url.kind ] in
let attrs =
[ "def"; "variant"; Url.Anchor.string_of_kind url.kind ]
in
let code =
O.documentedSrc (O.txt "| ") @ constructor id t.args t.res
in
Expand Down Expand Up @@ -688,12 +690,12 @@ module Make (Syntax : SYNTAX) = struct
let kind_approx, cstr, doc =
match item with
| Odoc_model.Lang.TypeExpr.Polymorphic_variant.Type te ->
("unknown", O.code (type_expr te), None)
("unknown", O.documentedSrc (type_expr te), None)
| Constructor { constant; name; arguments; doc; _ } -> (
let cstr = "`" ^ name in
( "constructor",
(match arguments with
| [] -> O.code (O.txt cstr)
| [] -> O.documentedSrc (O.txt cstr)
| _ ->
(* Multiple arguments in a polymorphic variant constructor correspond
to a conjunction of types, not a product: [`Lbl int&float].
Expand All @@ -715,7 +717,7 @@ module Make (Syntax : SYNTAX) = struct
let params =
if constant then O.txt "& " ++ params else params
in
O.code
O.documentedSrc
(O.txt cstr
++
if Syntax.Type.Variant.parenthesize_params then params
Expand All @@ -725,18 +727,20 @@ module Make (Syntax : SYNTAX) = struct
let markers = Syntax.Comment.markers in
try
let url = Url.Anchor.polymorphic_variant ~type_ident item in
let attrs = [ "def"; Url.Anchor.string_of_kind url.kind ] in
let attrs =
[ "def"; "variant"; Url.Anchor.string_of_kind url.kind ]
in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure exactly why you had to change the generator to fix this issue (odig's stylesheet have no problem with this without that) but, why not…

Copy link
Contributor

@dbuenzli dbuenzli Jun 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking more about this I'm not sure it's really a good idea to add more to the markup to distinguish polymorphic variants from regular ones. I don't really see a good use case for it (except for people making stylesheets more complex than they could be).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is related with odoc doing something more complex, when the size of the comment is small, the comment is in the same line and takes 50% of the space, otherwise it takes more space and is wrapped below (see the screenshot or some file where it happens).
I don't know if that's worth the trouble, but I think that's the difference with odig, which displays comments in a simpler way (from a quick look at the stylesheet, I might be wrong).

There might be a way to do it without, but since variant and fields had their classes, it felt natural to add one for polymorphic variants

let anchor = Some url in
let code = O.code (O.txt "| ") @ cstr in
let code = O.documentedSrc (O.txt "| ") @ cstr in
let doc = match doc with None -> [] | Some doc -> doc in
DocumentedSrc.Documented { attrs; anchor; code; doc; markers }
DocumentedSrc.Nested { attrs; anchor; code; doc; markers }
with Failure s ->
Printf.eprintf "ERROR: %s\n%!" s;
let code = O.code (O.txt "| ") @ cstr in
let code = O.documentedSrc (O.txt "| ") @ cstr in
let attrs = [ "def"; kind_approx ] in
let doc = [] in
let anchor = None in
DocumentedSrc.Documented { attrs; anchor; code; doc; markers }
DocumentedSrc.Nested { attrs; anchor; code; doc; markers }
in
let variants = List.map row t.elements in
let intro, ending =
Expand Down
9 changes: 4 additions & 5 deletions src/html_support_files/odoc.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,10 +549,10 @@ div.odoc-spec,.odoc-include {

.spec.type .variant, .spec.type .record {
margin-left: 2ch;
}

.spec.type li.variant, .spec.type li.record {
list-style: none;
display: flex;
flex-wrap: wrap;
row-gap: 4px;
}

.spec.type .record > code, .spec.type .variant > code {
Expand All @@ -569,9 +569,8 @@ div.odoc-spec,.odoc-include {
padding: 0.25em 0.5em;
margin-left: 10%;
border-radius: 3px;
flex-grow:1;
background: var(--main-background);
box-shadow: 2px 2px 4px lightgrey;
box-shadow: 1px 1px 2px lightgrey;
}

div.def {
Expand Down
18 changes: 9 additions & 9 deletions src/html_support_files/odoc_html_support_files.ml
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion src/ocamlary/ocamlary.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,11 @@ type variant =
(** This comment is also for [variant]. *)

(** This comment is for [poly_variant]. *)
type poly_variant = [ `TagA | `ConstrB of int ]
type poly_variant =
[ `TagA
(* This is a comment for [`TagA]. Disabled for compatibility
with 4.02. *)
| `ConstrB of int (* This is a comment for [`ConstrB] *) ]
(** Wow! It was a polymorphic variant! *)

(** This comment is for [full_gadt]. *)
Expand Down
2 changes: 1 addition & 1 deletion test/generators/html/Labels.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ <h2 id="L2"><a href="#L2" class="anchor"></a>Attached to nothing</h2>
</span>
</code>
<ol>
<li id="extension-X" class="def extension anchored">
<li id="extension-X" class="def variant extension anchored">
<a href="#extension-X" class="anchor"></a>
<code><span>| </span><span><span class="extension">X</span></span>
</code>
Expand Down
2 changes: 1 addition & 1 deletion test/generators/html/Ocamlary-ExtMod.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ <h1>Module <code><span>Ocamlary.ExtMod</span></code></h1>
</span>
</code>
<ol>
<li id="extension-Leisureforce" class="def extension anchored">
<li id="extension-Leisureforce" class="def variant extension anchored">
<a href="#extension-Leisureforce" class="anchor"></a>
<code><span>| </span>
<span><span class="extension">Leisureforce</span></span>
Expand Down
2 changes: 1 addition & 1 deletion test/generators/html/Ocamlary-module-type-TypeExt.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ <h1>Module type <code><span>Ocamlary.TypeExt</span></code></h1>
</span>
</code>
<ol>
<li id="extension-C" class="def extension anchored">
<li id="extension-C" class="def variant extension anchored">
<a href="#extension-C" class="anchor"></a>
<code><span>| </span><span><span class="extension">C</span></span>
</code>
Expand Down
2 changes: 1 addition & 1 deletion test/generators/html/Ocamlary-module-type-TypeExtPruned.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ <h1>Module type <code><span>Ocamlary.TypeExtPruned</span></code></h1>
</span>
</code>
<ol>
<li id="extension-C" class="def extension anchored">
<li id="extension-C" class="def variant extension anchored">
<a href="#extension-C" class="anchor"></a>
<code><span>| </span><span><span class="extension">C</span></span>
</code>
Expand Down
Loading