Add markdown generator #723

lubegasimon · 2021-09-09T06:19:08Z

Fixes #709

Todo
~~- [] Don’t render OCaml syntax, that is sig,end etc. This might require tweaking the document library slightly. (Edit: Disclaimer, no tweaking of the document library is needed!)~~

Add options to render markdown in different flavours. That is, pandoc and github’s markdown. (Stretch goal)
Add option to enable and disable links (suggested by @lpw25 in odoc-dev meeting 9/9/21)
Fix end keyword that is poorly aligned for submodules.
handling of labels
use unicode character rather than &nbsp (by @lpw25).
tie anchor tags to —generate-links options (@lpw25)
~~- [ ] Treat doc comments in modules and records a little different from those outside.~~
updating the docs to include the markdown generator

lubegasimon · 2021-09-16T05:39:31Z

@Drup, this is ready for review 🙂.

Drup

Here's a first set of comments. Could you post the result for various files so that people can look at the results.

Maybe we can try a version that inline submodules, like in the latex version. Not sure It'll give good results, but it might be more readable in a text editor than 1 file per module.

src/markdown/generator.ml

Drup · 2021-09-16T14:18:06Z

src/markdown/generator.ml

+          in
+          list ~sep:break (List.map f descrs) ++ continue rest
+      | Source content -> source_code content ++ continue rest
+      | Verbatim content -> str "%s" content ++ continue rest


That's not marked up as verbatim (it needs indentation or backticks).

I think Env is supposed to implement indentation ? (seems planned in the types but there's no implementation)

Indeed, indentation will be better

This must be resolved in the most recent commit using space ++ space — > (“ “)

This is not fixed. Indentation is not applied to every lines:

(** {v verbatim text v} *)

renders to:

verbatim text

Also, this should be rended the same way (existing indentation should be detected and normalized):

(** {v verbatim text v} *)

Drup · 2021-09-16T14:19:41Z

src/markdown/generator.ml

+            let bullet =
+              match list_typ with
+              | Unordered -> escaped "- "
+              | Ordered -> str "%d) " (n + 1)


Ordered lists in the standard are %d., might as well use that.

Also, you'll need to be pretty careful about breaks inside list items (it requires some indentation to preserve list-ness).

The indentation of list items is still broken:

(** {ul {- p1 p2} } *)

renders to:

- p1 p2

Drup · 2021-09-16T14:23:27Z

src/markdown/generator.ml

+  in
+  match label with
+  | Some _ -> command level (str " " ++ inline title)
+  | None -> command (level ^ " ") (inline title)


Not sure what's the difference between those two, but in any case, the label is missing. There is a pandoc extension for labels.

This extension is header_attributes.
We might need to add a "target" option (with values either pandoc or gh let's say) and to use the syntax only if it makes sense.

Anyway, for now, we need to at least use the <a id="..."> trick to make the label work.

Now that we have the flavour option, this is wanted.

There is no <a id...> on headings with --generate-links enabled.

Drup · 2021-09-16T14:24:49Z

src/markdown/generator.ml

+            let def = block i.Description.definition in
+            break ++ str "@" ++ key ++ str " : " ++ def ++ break ++ break
+          in
+          list ~sep:break (List.map f descrs) ++ continue rest


Why not use the definition_list extension from pandoc ? It'll be non-standard anyway.

Being that we now have a —md-flavour option, more of the renderer<Pandoc and GFM>-specific syntaxes will be added. Thanks!

src/markdown/generator.ml

Julow · 2021-09-16T18:35:31Z

src/markdown/generator.ml

+          in
+          list ~sep:break (List.map f descrs) ++ continue rest
+      | Source content -> source_code content ++ continue rest
+      | Verbatim content -> str "%s" content ++ continue rest


I think Env is supposed to implement indentation ? (seems planned in the types but there's no implementation)

src/markdown/generator.ml

Signed-off-by: lubegasimon <[email protected]>

lubegasimon · 2021-10-05T09:23:07Z

This PR is ready for another review 🙂.

Julow

Spacing between block elements is not right:

(** p1 *)

(** p2

   p3

   {ul
     {- a}
     {- b}
    } *)

renders to:

p1
p2
p3
- a

- b

We expect an empty line between two blocks.

Julow · 2021-10-06T08:53:44Z

src/markdown/generator.ml

+
+  val concat : t list -> t
+
+  val inline' : string list -> t


This function isn't necessary, it's used once and can be inlined to gain some readability.

Julow · 2021-10-06T08:54:36Z

src/odoc/markdown.ml

+  Odoc_markdown.Generator.args.md_flavour := args.md_flavour;
+  Odoc_markdown.Generator.args.generate_links := args.generate_links;
+  Odoc_markdown.Generator.render page


no need to use mutable fields.
A value of type Odoc_markdown.Generator.args can be passed directly to Odoc_markdown.Generator.render.
The type args above doesn't need to be a different type, it can be equal to Odoc_markdown.Generator.args.

Julow · 2021-10-06T08:57:35Z

test/generators/gen_rules/gen_rules.ml

@@ -100,6 +111,7 @@ let () =
        (html_target_rule, Fpath.v "html", Some "--flat");
        (latex_target_rule, Fpath.v "latex", None);
        (man_target_rule, Fpath.v "man", None);
+        (markdown_target_rule, Fpath.v "markdown", None);


We need to test md flavours as well as the generate-links option. Perhaps we can use other kinds of tests for that to avoid having a huge number of test outputs.

Julow · 2021-10-06T09:02:47Z

src/odoc/bin/main.ml

+    in
+    Arg.(
+      value & opt string ""
+      & info ~docv:"MARKDOWN_FLAVOUR" ~doc [ "md-flavour" ])


This option should be renamed flavour.

Julow · 2021-10-06T09:04:20Z

src/markdown/generator.ml

+  let level =
+    match level with
+    | 1 -> make_hashes 1
+    | 2 -> make_hashes 2
+    | 3 -> make_hashes 3
+    | 4 -> make_hashes 4
+    | 5 -> make_hashes 5
+    | 6 -> make_hashes 6
+    | _ -> ""
+    (* We can be sure that h6 will never be exceded! *)
+  in


Julow · 2021-10-06T09:06:04Z

src/markdown/generator.ml

+          cond
+            (source_code content nbsp ++ continue rest)
+            (backticks ++ source_code content nbsp ++ backticks ++ continue rest)


Why does this change formatting depending on generate-links ? I see no reason.

That’s because we would end up with links in back ticks which will confuse the reader, that is, [text](href) (text will be a link, but it won’t be displayed as a link rather as source code.

Do you have an example of links inside "source code" ?
Anyway, removing the backticks isn't the solution because we want them.

Julow · 2021-10-06T09:07:56Z

src/markdown/generator.ml

+      | Text "" -> continue rest
+      | Text s -> (
+          match s with
+          | "end" | "}" | "]" ->


Documentation containing these words is broken:

(** end } ] *)

The keywords must not be matched into any text, only when nested into a source_code with the right tag.

Hmm! only } and ] behave not right but yes, that’s a good catch. Thanks!

Julow · 2021-10-06T09:09:15Z

src/markdown/generator.ml

+       "type nums = [ | `One | `Two ]" would be rendered as "``|`````Monday`` "
+       if the spaces were missing.
+    *)
+    | Backticks -> Format.fprintf fmt " `` "


Should use a single backtick when possible (when there is no backticks in the enclosed code).

Julow · 2021-10-06T09:10:15Z

src/markdown/generator.ml

+      | Text b -> block b nbsp' ++ continue rest
+      | Heading h -> break ++ heading h nbsp' ++ break ++ break ++ continue rest
+      | Declaration { attr = _; anchor; content; doc } ->
+          let nbsp'' = nbsp ++ nbsp ++ nbsp ++ nbsp in


Move that code into documented_src because it needs both values.

Julow · 2021-10-06T09:11:00Z

src/markdown/generator.ml

+
+type args = { generate_links : bool ref; md_flavour : string ref }
+
+let args = { generate_links = ref true; md_flavour = ref "" }


Please don't use a global variable. The args value should be passed to every functions.

lubegasimon · 2021-12-01T05:18:34Z

Closed in favour of #791

lubegasimon marked this pull request as ready for review September 9, 2021 08:52

lubegasimon marked this pull request as draft September 9, 2021 08:56

lubegasimon force-pushed the add-markdown-generator branch 4 times, most recently from 9059f17 to b0cc543 Compare September 15, 2021 15:27

lubegasimon marked this pull request as ready for review September 15, 2021 15:28

Drup reviewed Sep 16, 2021

View reviewed changes

Julow reviewed Sep 16, 2021

View reviewed changes

lubegasimon added 4 commits October 2, 2021 01:25

add markdown generator

c7a5d68

Signed-off-by: lubegasimon <[email protected]>

add markdown commands,and options '--generate-links' and'--md-flavour'

8cf8bb8

Signed-off-by: lubegasimon <[email protected]>

test markdown generator

9ffa3eb

Signed-off-by: lubegasimon <[email protected]>

promote markdown generator tests

fc8e1c0

Signed-off-by: lubegasimon <[email protected]>

lubegasimon force-pushed the add-markdown-generator branch from 3fefc49 to fc8e1c0 Compare October 1, 2021 22:32

Julow requested changes Oct 6, 2021

View reviewed changes

lubegasimon closed this Dec 1, 2021

lubegasimon deleted the add-markdown-generator branch February 18, 2022 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add markdown generator #723

Add markdown generator #723

lubegasimon commented Sep 9, 2021 •

edited

Loading

lubegasimon commented Sep 16, 2021

Drup left a comment

Drup Sep 16, 2021

Julow Sep 16, 2021

lubegasimon Sep 17, 2021

lubegasimon Sep 21, 2021

Julow Oct 6, 2021

Drup Sep 16, 2021

Julow Oct 6, 2021

Drup Sep 16, 2021

Julow Sep 16, 2021

Julow Oct 6, 2021

Julow Oct 6, 2021

Drup Sep 16, 2021

lubegasimon Oct 1, 2021

Julow Sep 16, 2021

lubegasimon commented Oct 5, 2021

Julow left a comment

Julow Oct 6, 2021

Julow Oct 6, 2021

Julow Oct 6, 2021

Julow Oct 6, 2021

Julow Oct 6, 2021

Julow Oct 6, 2021

lubegasimon Oct 6, 2021

Julow Oct 7, 2021

Julow Oct 6, 2021

lubegasimon Oct 6, 2021 •

edited

Loading

Julow Oct 6, 2021

Julow Oct 6, 2021

Julow Oct 6, 2021

lubegasimon commented Dec 1, 2021


		type args = { generate_links : bool ref; md_flavour : string ref }

		let args = { generate_links = ref true; md_flavour = ref "" }

Add markdown generator #723

Add markdown generator #723

Conversation

lubegasimon commented Sep 9, 2021 • edited Loading

lubegasimon commented Sep 16, 2021

Drup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lubegasimon commented Oct 5, 2021

Julow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lubegasimon Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lubegasimon commented Dec 1, 2021

lubegasimon commented Sep 9, 2021 •

edited

Loading

lubegasimon Oct 6, 2021 •

edited

Loading