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

Add support for Reason doc generation #1117

Open
rizo opened this issue Aug 9, 2018 · 16 comments

Comments

Projects
None yet
4 participants
@rizo
Copy link
Contributor

commented Aug 9, 2018

(Continued from ocaml/odoc#156 (comment))

We recently introduced support for Reason syntax generation in odoc.

Specifically, the following option is now implemented:

$ odoc html [--syntax=<ml|re>]   (default=ml)

It would be great if dune had the ability to pass this option to odoc and thus allow users to generate either or both syntaxes.

A few options were discussed:

  1. Introduce different targets for each syntax: @doc and @reason-doc (suggested by @rgrinberg).
  2. Decide which syntax to generate based on the file extension (suggested by @diml).

The second option seems very natural and doesn't require user-facing changes but it also doesn't allow the generation of both syntaxes at the same time.

Note: The --lang option mentioned in this issues was renamed to --syntax.

@rizo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2018

My suggestion is to implement the first option, albeit with some minor changes:

$ dune build @doc-re  # Pass `--syntax=re` to odoc
$ dune build @doc-ml  # Pass `--syntax=ml` to odoc
$ dune build @doc     # Call both above targets

I understand this would change the meaning of the current @doc target. My motivation comes from the fact that we will want to introduce the ability to switch the syntax in the generated HTML, thus I believe this should be the default behaviour.

(CC @aantron @ryyppy)

@diml

This comment has been minimized.

Copy link
Member

commented Aug 9, 2018

Does --lang=ml requires to have reason installed?

@rizo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2018

@diml No, and neither does --lang=re. This option only affects the HTML generation.

In general we will try to be careful with dependencies and will only introduce reason dependencies (like refmt) in a modular/optional way.

@diml

This comment has been minimized.

Copy link
Member

commented Aug 9, 2018

Ah yes, I meant --lang=re. Do you have your own reason pretty-printer? (just curious)

@rizo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2018

@diml The Reason HTML generation is only done for signature items where pretty-printing is applied directly to odoc's internal model. So, yes, it's an ad-hoc AST pretty-printer. It's not ideal, but the same applies to the OCaml syntax.

This also means that examples in code blocks are not currently translated (but will be in the future).

@ryyppy did actually implemented code translation/formatting but it's currently commented out. For some context see Combining ocamlformat & refmt – OCaml Discuss.

Coming back to your previous questions: I'm not sure if --lang=re will in the future require the reason library. I'd prefer to keep it optional via a custom odoc tag or a ppx.

I think I understand your concern: are you worried about dune build @doc requiring reason as a dependency even for OCaml-only projects?

@aantron

This comment has been minimized.

Copy link
Collaborator

commented Aug 9, 2018

The second option also makes it difficult to present OCaml-syntax projects for Reason people and vice versa. So, I prefer (some variant of) the first.

@ryyppy

This comment has been minimized.

Copy link
Collaborator

commented Aug 10, 2018

The second option seems very natural and doesn't require user-facing changes but it also doesn't allow the generation of both syntaxes at the same time.

I don't understand.. why isn't it possible to generate both version of the same time? I believe it's the contrary, with the first version you won't be able to mix ML / RE code. If I know the original file format, I can proactively build the contrary version as well?

also, @diml @aantron @rgrinberg, if we settle on a rough decision, I would like to take a stab at this and create a PR. We can figure out details on the way?

@rizo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2018

@ryyppy Technically you're right – both versions can be generated even with option 1. But in practice, in that case, the extension doesn't matter because odoc operates on cmti/cmi and not mli/rei files. With that approach dune would always generate both versions, which is less flexible.

Assuming that we won't introduce reason as a dependency, I believe this suggestion would work well.

FYI: We decided to rename the --lang flag to --syntax to be consistent with dune's naming.

@rizo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2018

As far as I understand, dune can already determine if reason installation is available. This would help with the unlikely event of odoc --syntax=re requiring something like refmt in the future. In that situation I imagine dune build @doc would only generate the OCaml version of the docs, and produce a warning informing that the Reason version couldn't be generated.

Does that sound sensible?

@aantron

This comment has been minimized.

Copy link
Collaborator

commented Aug 10, 2018

@rizo Location information in each .cmti/.cmt contains the source filename and extension, so I imagine Dune could easily extract that. Not arguing for this, though :)

@diml

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

My concern was indeed about introducing more dependencies than necessary for someone who would only want the OCaml syntax, and also about doing more work than necessary by default. For instance, I would imagine that OCaml developers only care about the OCaml syntax and that Reason developers only care about the Reason syntax, so it seems a bit odd to me to build both by default. The only case I can think of where one would want to build both is when publishing to a web-server. In some ways it feels like it should be a global user configuration parameter. In any case, I'm happy to leave all the design decisions to odoc maintainers.

BTW, with all the modern web technologies we have, I'm slightly surprised that such as choice has to be made at compile time instead of letting the reader decide :)

@diml

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

@ryyppy, independently of the final choice, the core of the work will be to adapt the odoc rules so that multiple trees of html documentation can be generated in parallel. You could indeed start by looking at this.

@ryyppy

This comment has been minimized.

Copy link
Collaborator

commented Aug 13, 2018

@diml Will start the work now! Can you assign this issue to me?

@diml

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

Done

@ryyppy ryyppy referenced this issue Oct 4, 2018

Closed

Add odoc syntax support #1397

1 of 3 tasks complete
@ryyppy

This comment has been minimized.

Copy link
Collaborator

commented Jan 19, 2019

Short update for this: We realized that adding a parameter to the odoc stanza doesn't make a lot of sense, so we decided to introduce an environment variable ODOC_SYNTAX in the meantime instead (thanks @rizo).

See: ocaml/odoc#280

Also, the latest idea was to make odoc render Reason & OCaml by default, the syntax variable should then be used to opt out of this behavior and only render one language.

So we should document on how to configure the odoc syntax in dune.

@diml

This comment has been minimized.

Copy link
Member

commented Jan 21, 2019

Ok. We should also make the rules in dune sensible to this variable, so that things are rebuilt when this variable changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.