Add Reason support by ryyppy · Pull Request #156 · ocaml/odoc

ryyppy · 2018-07-09T17:53:06Z

Tracking Branch for #129

Things that are being worked on:

Add --lang flag for html command ~~(currently not building b/c of type error)~~
Copy test cases to reason syntax & implement as much Reason syntax generation as possible
Apply refmt on code snippets inside block comments
~~Use location info to determine ml or re formatting for Code_blocks~~
~~Provide mechanics to configure refmt's heuristic constructor list (prevent explicit_arity attributes) -> use ODOC_REFMT_HEURISTICS env variable~~

Test Cases:

The semantics of this flag are subject of change though.

This commit introduces following changes: - Move language Variant from Html_page to Html_tree - Introduce a `~lang` parameter for the Documentation module - Manipulate block level code snippets by applying refmt - Attach a classname `ml` / `re` for each translated code snippet - If a codesnippet could not be translated, keep the OCaml representation

This adds language detection by checking the file extension of the compilation unit. Depending on the target language (~to_lang), it will do the proper Reason -> OCaml or OCaml -> Reason transformation in `Code_block`s. In case of to_lang = from_lang, no code transformation will be applied.

ryyppy · 2018-07-20T10:15:08Z

The most intrusive changes are in the src/html/documentation.ml module.
I introduced a ~from_lang / ~to_lang parameter for block_elements which give context on which parser / printer logic to use. Rest of the work is dedicated to the signature markup rendering part.

ryyppy · 2018-08-07T19:10:24Z

src/html/markup.mli

-module Type : sig
-  val path :
-    [< Html_types.span_content_fun ] Html.elt list ->
+module ML : sig


I introduced namespaces for Markup... common Markup is still in the toplevel of the module, e.g. keyword

ryyppy · 2018-08-07T19:11:02Z

src/html/markup.mli


-val arrow : [> Html_types.span ] Html.elt
-(** "->" with a non breaking hyphen, styled as a keyword. *)
+module RE : sig


It's probably worth introducing a common module interface to make the signature more DRY

That seems like a good idea, but we can do it later, during some future refactoring/cleanup.

ryyppy · 2018-08-07T19:12:34Z

src/html/to_html_tree.ml

    match t with
-    | Var s -> [Markup.Type.var ("'" ^ s)]
-    | Any  -> [Markup.Type.var "_"]
+    | Var s -> [Markup.ML.Type.var ("'" ^ s)]


I guess this kind of pattern can be solved with first class modules as well? dropping the namespace and just reify a specific module implementation for each language? This solution was quicker and the least intrusive

Is this comment in reference to similarity between this code, and the corresponding function in to_re_html_tree.ml? If so, yes, if many of the functions are similar, it might be worthwhile to factor the common parts out, and use either first-class modules or a functor. As with most other things, we can save that for a later refactoring, though :)

It could, however, make the diff much smaller, and, so, review much easier. I did a diff between to_html_tree.ml and to_re_html_tree.re to help the review, and the diff was pretty small, so a lot of code seems to be repeated.

I don't know how you would factor this out... I was thinking of using include statements, but most bindings of this module are recursively anded... not sure if you can override specific functions then?

IIRC, in the diff between to_html_tree.ml and to_re_html_tree.ml, the skeleton of the functions is the same, and only some details change (what delimiter to use in records, variants, whether there is a semicolon, etc). So, we would factor out these details into ML and Re modules, and turn To_html_tree into a functor that contains the remaining common code.

ryyppy · 2018-08-07T19:13:58Z

src/html/to_re_html_tree.mli

+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *)
+
+val compilation_unit : ?theme_uri:Html_tree.uri -> Model.Lang.Compilation_unit.t -> Html_tree.t


This should probably just reuse the module definition of to_html_tree.mli... I was not too sure how to do this here, also I thought I would let this be for now and redesign this in a more unified way

You could do include module type of To_html_tree, I believe, but it is indeed better to wait to unify.

ryyppy · 2018-08-07T19:14:35Z

src/html/utils.ml

    | None -> hd @ tl
    | Some sep -> hd @ sep :: tl
+
+(**


I left this code in here, since there were some discussions on how to introduce refmt without coupling it to the odoc core

ryyppy · 2018-08-07T19:15:18Z

src/model/location_.ml

 let location : 'a with_location -> span = fun {location; _} ->
  location

+let file_ext : span -> string = fun span ->


Required to be able to determine the from_lang attribute... if .mli -> OCaml, if .rei -> Reason

ryyppy · 2018-08-07T19:15:43Z

src/html/html_tree.ml

 module Html = Tyxml.Html
 module Paths = Model.Paths

+type lang = OCaml | Reason


I introduced the language type in here, not sure if this is the right module

ryyppy · 2018-08-07T19:17:36Z

src/odoc/html_page.ml

-let from_odoc ~env ?theme_uri ~output:root_dir input =
+let to_html_tree_page ?theme_uri ~lang v =
+  match lang with
+  | Html.Html_tree.Reason -> Html.To_html_tree.RE.page ?theme_uri v


I don't like this... I felt like we should use something like first-class-modules, but I was not entirely sure how to do this the right way... feedback welcome!

There may be som nice solution with first-class modules, but I suspect it's not worth looking for, for something so simple as this match expression :)

ryyppy · 2018-08-07T19:18:15Z

src/odoc/html_page.ml

+  | Html.Html_tree.Reason -> Html.To_html_tree.RE.compilation_unit ?theme_uri v
+  | Html.Html_tree.OCaml -> Html.To_html_tree.ML.compilation_unit ?theme_uri v
+
+let from_odoc ~env ?(lang=Html.Html_tree.OCaml) ?theme_uri ~output:root_dir input =


The CLI has an optional lang parameter, which defaults to OCaml

aantron

Looks good, thanks! I'm happy to merge this PR after a brief discussion, and we can finish off things here or in additional PRs.

I think module declarations, module type declarations, and type declarations are missing the final ; (see e.g. module.html in the HTML expect tests).
The most dubious thing in the PR is, I think, the duplication of most of the code in to_html_tree.ml into to_re_html_tree.ml. But we can figure out how to factor the common code out in a future PR or in general cleanup.
How should the new --lang argument be integrated with build systems?

aantron · 2018-08-08T02:12:18Z

src/html/markup.mli


-val arrow : [> Html_types.span ] Html.elt
-(** "->" with a non breaking hyphen, styled as a keyword. *)
+module RE : sig


That seems like a good idea, but we can do it later, during some future refactoring/cleanup.

aantron · 2018-08-08T02:15:52Z

src/html/to_html_tree.ml

    match t with
-    | Var s -> [Markup.Type.var ("'" ^ s)]
-    | Any  -> [Markup.Type.var "_"]
+    | Var s -> [Markup.ML.Type.var ("'" ^ s)]


Is this comment in reference to similarity between this code, and the corresponding function in to_re_html_tree.ml? If so, yes, if many of the functions are similar, it might be worthwhile to factor the common parts out, and use either first-class modules or a functor. As with most other things, we can save that for a later refactoring, though :)

It could, however, make the diff much smaller, and, so, review much easier. I did a diff between to_html_tree.ml and to_re_html_tree.re to help the review, and the diff was pretty small, so a lot of code seems to be repeated.

aantron · 2018-08-08T02:44:28Z

src/html/to_re_html_tree.mli

+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *)
+
+val compilation_unit : ?theme_uri:Html_tree.uri -> Model.Lang.Compilation_unit.t -> Html_tree.t


You could do include module type of To_html_tree, I believe, but it is indeed better to wait to unify.

aantron · 2018-08-08T02:50:53Z

src/odoc/html_page.ml

-let from_odoc ~env ?theme_uri ~output:root_dir input =
+let to_html_tree_page ?theme_uri ~lang v =
+  match lang with
+  | Html.Html_tree.Reason -> Html.To_html_tree.RE.page ?theme_uri v


There may be som nice solution with first-class modules, but I suspect it's not worth looking for, for something so simple as this match expression :)

aantron · 2018-08-08T02:57:46Z

test/html/expect/ml/markup.html

@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<html xmlns="http://www.w3.org/1999/xhtml"><head><title>Markup (test_package.Markup)</title><link rel="stylesheet" href="../../odoc.css"/><meta charset="utf-8"/><meta name="viewport" content="width=device-width,initial-scale=1.0"/><script src="../../highlight.pack.js"></script><script>hljs.initHighlightingOnLoad();</script></head><body><div class="content"><header><nav><a href="../index.html">Up</a> – <a href="../index.html">test_package</a> &#x00BB; Markup</nav><h1>Module <code>Markup</code></h1><p>Here, we test the rendering of comment markup.</p><nav class="toc"><ul><li><a href="#sections">Sections</a><ul><li><a href="#subsection-headings">Subsection headings</a><ul><li><a href="#sub-subsection-headings">Sub-subsection headings</a></li><li><a href="#anchors">Anchors</a></li></ul></li></ul></li><li><a href="#styling">Styling</a></li><li><a href="#links-and-references">Links and references</a></li><li><a href="#preformatted-text">Preformatted text</a></li><li><a href="#lists">Lists</a></li><li><a href="#unicode">Unicode</a></li><li><a href="#raw-html">Raw HTML</a></li><li><a href="#tags">Tags</a></li></ul></nav></header><section><header><h2 id="sections"><a href="#sections" class="anchor"></a>Sections</h2><p>Let's get these done first, because sections will be used to break up the rest of this test.</p><p>Besides the section heading above, there are also</p></header><section><header><h3 id="subsection-headings"><a href="#subsection-headings" class="anchor"></a>Subsection headings</h3><p>and</p></header><section><header><h4 id="sub-subsection-headings"><a href="#sub-subsection-headings" class="anchor"></a>Sub-subsection headings</h4><p>but odoc has banned deeper headings. There are also title headings, but they are only allowed in mld files.</p></header></section><section><header><h4 id="anchors"><a href="#anchors" class="anchor"></a>Anchors</h4><p>Sections can have attached <a href="index.html#anchors"><span>Anchors</span></a>, and it is possible to <a href="index.html#anchors"><span>link</span></a> to them. Links to section headers should not be set in source code style.</p></header></section></section></section><section><header><h2 id="styling"><a href="#styling" class="anchor"></a>Styling</h2><p>This paragraph has some styled elements: <b>bold</b> and <i>italic</i>, <b><i>bold italic</i></b>, <em>emphasis</em>, <em><em>emphasis</em> within emphasis</em>, <b><i>bold italic</i></b>, super<sup>script</sup>, sub<sub>script</sub>. The line spacing should be enough for superscripts and subscripts not to look odd.</p><p><code>code</code> is a different kind of markup that doesn't allow nested markup.</p><p>It's possible for two markup elements to appear <b>next</b> <i>to</i> each other and have a space, and appear <b>next</b><i>to</i> each other with no space. It doesn't matter <b>how</b> <i>much</i> space it was in the source: in this sentence, it was two space characters. And in this one, there is <b>a</b> <i>newline</i>.</p><p>Code can appear <b>inside <code>other</code> markup</b>. Its display shouldn't be affected.</p></header></section><section><header><h2 id="links-and-references"><a href="#links-and-references" class="anchor"></a>Links and references</h2><p>This is a <a href="#">link</a>. It sends you to the top of this page. Links can have markup inside them: <a href="#"><b>bold</b></a>, <a href="#"><i>italics</i></a>, <a href="#"><em>emphasis</em></a>, <a href="#">super<sup>script</sup></a>, <a href="#">sub<sub>script</sub></a>, and <a href="#"><code>code</code></a>. Links can also be nested <em><a href="#">inside</a></em> markup. Links cannot be nested inside each other. This link has no replacement text: <a href="#">#</a>. The text is filled in by odoc. This is a shorthand link: <a href="#">#</a>. The text is also filled in by odoc in this case.</p><p>This is a reference to <a href="index.html#val-foo"><code>foo</code></a>. References can have replacement text: <a href="index.html#val-foo"><span>the value foo</span></a>. Except for the special lookup support, references are pretty much just like links. The replacement text can have nested styles: <a href="index.html#val-foo"><span><b>bold</b></span></a>, <a href="index.html#val-foo"><span><i>italic</i></span></a>, <a href="index.html#val-foo"><span><em>emphasis</em></span></a>, <a href="index.html#val-foo"><span>super<sup>script</sup></span></a>, <a href="index.html#val-foo"><span>sub<sub>script</sub></span></a>, and <a href="index.html#val-foo"><span><code>code</code></span></a>. It's also possible to surround a reference in a style: <b><a href="index.html#val-foo"><code>foo</code></a></b>. References can't be nested inside references, and links and references can't be nested inside each other.</p></header></section><section><header><h2 id="preformatted-text"><a href="#preformatted-text" class="anchor"></a>Preformatted text</h2><p>This is a code block:</p><pre><code>let foo = ()
+<html xmlns="http://www.w3.org/1999/xhtml"><head><title>Markup (test_package.Markup)</title><link rel="stylesheet" href="../../odoc.css"/><meta charset="utf-8"/><meta name="viewport" content="width=device-width,initial-scale=1.0"/><script src="../../highlight.pack.js"></script><script>hljs.initHighlightingOnLoad();</script></head><body><div class="content"><header><nav><a href="../index.html">Up</a> – <a href="../index.html">test_package</a> &#x00BB; Markup</nav><h1>Module <code>Markup</code></h1><p>Here, we test the rendering of comment markup.</p><nav class="toc"><ul><li><a href="#sections">Sections</a><ul><li><a href="#subsection-headings">Subsection headings</a><ul><li><a href="#sub-subsection-headings">Sub-subsection headings</a></li><li><a href="#anchors">Anchors</a></li></ul></li></ul></li><li><a href="#styling">Styling</a></li><li><a href="#links-and-references">Links and references</a></li><li><a href="#preformatted-text">Preformatted text</a></li><li><a href="#lists">Lists</a></li><li><a href="#unicode">Unicode</a></li><li><a href="#raw-html">Raw HTML</a></li><li><a href="#tags">Tags</a></li></ul></nav></header><section><header><h2 id="sections"><a href="#sections" class="anchor"></a>Sections</h2><p>Let's get these done first, because sections will be used to break up the rest of this test.</p><p>Besides the section heading above, there are also</p></header><section><header><h3 id="subsection-headings"><a href="#subsection-headings" class="anchor"></a>Subsection headings</h3><p>and</p></header><section><header><h4 id="sub-subsection-headings"><a href="#sub-subsection-headings" class="anchor"></a>Sub-subsection headings</h4><p>but odoc has banned deeper headings. There are also title headings, but they are only allowed in mld files.</p></header></section><section><header><h4 id="anchors"><a href="#anchors" class="anchor"></a>Anchors</h4><p>Sections can have attached <a href="index.html#anchors"><span>Anchors</span></a>, and it is possible to <a href="index.html#anchors"><span>link</span></a> to them. Links to section headers should not be set in source code style.</p></header></section></section></section><section><header><h2 id="styling"><a href="#styling" class="anchor"></a>Styling</h2><p>This paragraph has some styled elements: <b>bold</b> and <i>italic</i>, <b><i>bold italic</i></b>, <em>emphasis</em>, <em><em>emphasis</em> within emphasis</em>, <b><i>bold italic</i></b>, super<sup>script</sup>, sub<sub>script</sub>. The line spacing should be enough for superscripts and subscripts not to look odd.</p><p><code>code</code> is a different kind of markup that doesn't allow nested markup.</p><p>It's possible for two markup elements to appear <b>next</b> <i>to</i> each other and have a space, and appear <b>next</b><i>to</i> each other with no space. It doesn't matter <b>how</b> <i>much</i> space it was in the source: in this sentence, it was two space characters. And in this one, there is <b>a</b> <i>newline</i>.</p><p>Code can appear <b>inside <code>other</code> markup</b>. Its display shouldn't be affected.</p></header></section><section><header><h2 id="links-and-references"><a href="#links-and-references" class="anchor"></a>Links and references</h2><p>This is a <a href="#">link</a>. It sends you to the top of this page. Links can have markup inside them: <a href="#"><b>bold</b></a>, <a href="#"><i>italics</i></a>, <a href="#"><em>emphasis</em></a>, <a href="#">super<sup>script</sup></a>, <a href="#">sub<sub>script</sub></a>, and <a href="#"><code>code</code></a>. Links can also be nested <em><a href="#">inside</a></em> markup. Links cannot be nested inside each other. This link has no replacement text: <a href="#">#</a>. The text is filled in by odoc. This is a shorthand link: <a href="#">#</a>. The text is also filled in by odoc in this case.</p><p>This is a reference to <a href="index.html#val-foo"><code>foo</code></a>. References can have replacement text: <a href="index.html#val-foo"><span>the value foo</span></a>. Except for the special lookup support, references are pretty much just like links. The replacement text can have nested styles: <a href="index.html#val-foo"><span><b>bold</b></span></a>, <a href="index.html#val-foo"><span><i>italic</i></span></a>, <a href="index.html#val-foo"><span><em>emphasis</em></span></a>, <a href="index.html#val-foo"><span>super<sup>script</sup></span></a>, <a href="index.html#val-foo"><span>sub<sub>script</sub></span></a>, and <a href="index.html#val-foo"><span><code>code</code></span></a>. It's also possible to surround a reference in a style: <b><a href="index.html#val-foo"><code>foo</code></a></b>. References can't be nested inside references, and links and references can't be nested inside each other.</p></header></section><section><header><h2 id="preformatted-text"><a href="#preformatted-text" class="anchor"></a>Preformatted text</h2><p>This is a code block:</p><pre><code class="ml">let foo = ()


Do you know offhand what changed in this test?

I introduced a class attribute for each pre block (see https://github.com/ocaml/odoc/pull/156/files#diff-41083926a76829c938b1f88df53c48d7R274)

Thanks, I had seen that but not made the connection. I gave myself a TODO to get these expected strings indented nicely, so that the diffs are actually helpful.

aantron · 2018-08-08T03:02:39Z

src/html/jbuild

 ((name html)
  (preprocess (pps (bisect_ppx -conditional)))
-  (libraries (model tyxml))))
+  (libraries (model tyxml reason))))


Do we still have a dep on reason here? I think the code that uses reason is commented out, but not sure if I missed some.

Dune 1.1.0 doesn't support the --dev flag, which is part of the odoc build for now. See https://travis-ci.org/ocaml/odoc/jobs/413265420#L793

aantron · 2018-08-08T17:28:20Z

Looks like the remaining problem is a failure on 4.03, because Filename.extension was only added in 4.04. Since odoc depends on fpath, we can probably deal with this by replacing Filename.extension with some combination of these functions from fpath.

aantron · 2018-08-08T17:32:19Z

You could also use Filename.check_suffix instead of extracting the extension.

aantron · 2018-08-08T21:41:52Z

@diml, @rgrinberg, We'd like to be able to choose, through Dune, whether odoc outputs OCaml syntax, or Reason syntax. This is controlled by passing --lang=re or --lang=ml to odoc html (--lang=ml is the default). What do you recommend for achieving this?

In the future, we might want to generate both outputs, and give users the option to switch between them. I guess Reason output wouldn't require support from Dune at that point.

rgrinberg · 2018-08-09T08:07:40Z

@aantron you should open a dune ticket to discuss this. There's a couple of options here. The most natural one is to create separate targets for reason/ocaml documentation and then use 2 aliases to control them. For example, @doc and @reason-doc. Would you like to take a stab at implementing this in dune?

ghost · 2018-08-09T08:45:37Z

What about the following simple rule: if the original source file is a .mli, output in OCaml syntax, if it is a .rei, output in reason syntax?

jordwalke · 2019-02-03T06:20:59Z

@diml That would work, except there are reasons to have .mli files which generate Reason docs (or vice versa). Many people building Reason apps want to try out opam libraries and the more we can do to make that natural, the more exposure the opam libraries would receive.

ryyppy added 8 commits July 9, 2018 19:51

Add infra for Reason support

180e5b0

Merge branch 'master' into reason-syntax-support

67fc899

Add --lang flag support

a11602d

The semantics of this flag are subject of change though.

Adapt html test to output ml / re in subdirs

9d3b1ba

Restore old expected mld.html result

8767385

Fix Reason Val case

e4435c7

Remove unnecessary print_endline

1d9d0e1

rizo mentioned this pull request Jul 31, 2018

Extract all examples and run them (e.g., in CI) #130

Closed

ryyppy added 5 commits August 3, 2018 17:27

Add Reason formatting for GADT and objects (not done)

c341324

Merge branch 'master' into reason-syntax-support

f73e41d

Remove refmt functionality

4354122

Add Reason method syntax

7ffa16e

Update Reason tests

82f57bd

ryyppy commented Aug 7, 2018

View reviewed changes

aantron self-requested a review August 7, 2018 21:25

Reduce diff during review

f992139

aantron reviewed Aug 8, 2018

View reviewed changes

aantron and others added 2 commits August 8, 2018 08:24

Travis: pin Dune to 1.0.0

2579703

Dune 1.1.0 doesn't support the --dev flag, which is part of the odoc build for now. See https://travis-ci.org/ocaml/odoc/jobs/413265420#L793

Add semicolon for module, type, class etc

576227f

ryyppy added 2 commits August 8, 2018 17:49

Remove reason from jbuild file

d75d03f

Update Reason tests

66f918e

ryyppy added 3 commits August 8, 2018 20:09

Fall back to Filename.check_suffix

e95ddcc

Reason: Add semicolon format for exn

4aa9270

Update Reason tests

8989218

aantron merged commit c71e87b into ocaml:master Aug 8, 2018

aantron pushed a commit that referenced this pull request Aug 8, 2018

Add Reason syntax support (#156)

7f27e13

aantron mentioned this pull request Aug 8, 2018

Reason/OCaml syntax switching #129

Closed

rizo mentioned this pull request Aug 9, 2018

Add support for Reason doc generation ocaml/dune#1117

Closed

rizo mentioned this pull request Aug 22, 2018

Add a --fragment option to odoc html #89

Closed

aantron mentioned this pull request Sep 10, 2018

Support BuckleScript (Backport to 4.02.3) #179

Merged

aantron mentioned this pull request Sep 25, 2018

Refactor HTML Generation #201

Merged

3 tasks

aantron mentioned this pull request Oct 16, 2018

odoc 1.3.0: OCaml documentation generator ocaml/opam-repository#12811

Merged

ryyppy changed the title ~~Add Reason support (WIP)~~ Add Reason support Jan 19, 2019

davesnx mentioned this pull request Aug 1, 2025

Support for rendering both re and ml syntaxes, within the same generation #1342

Open

Conversation

ryyppy commented Jul 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryyppy commented Jul 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aantron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aantron commented Aug 8, 2018

Uh oh!

aantron commented Aug 8, 2018

Uh oh!

aantron commented Aug 8, 2018

Uh oh!

rgrinberg commented Aug 9, 2018

Uh oh!

ghost commented Aug 9, 2018

Uh oh!

jordwalke commented Feb 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryyppy commented Jul 9, 2018 •

edited

Loading