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

Reason/OCaml syntax switching #129

Closed
aantron opened this issue Apr 17, 2018 · 16 comments
Closed

Reason/OCaml syntax switching #129

aantron opened this issue Apr 17, 2018 · 16 comments
Assignees

Comments

@aantron
Copy link
Contributor

aantron commented Apr 17, 2018

This is about outputting docs in Reason syntax.

Nothing needs to be done at the parsing end, because odoc reads docs from parsed ASTs (after the OCaml or Reason parser is done running), so the source syntax doesn't affect odoc.

https://discuss.ocaml.org/t/1841/13, https://discuss.ocaml.org/t/1841/47. The Ocsigen website has an example of switching (http://ocsigen.org/lwt/3.2.1/api/Lwt). cc @Drup

@aantron aantron added the output label Apr 17, 2018
@ryyppy
Copy link
Member

ryyppy commented Jul 6, 2018

Hey 👋
I am currently investing some time into this and try to submit a PR soon!

Hope this is okay?

@rizo
Copy link
Collaborator

rizo commented Jul 6, 2018

The way I see this feature implemented and run is:

  • Add the --syntax=ml|re option to odoc html.
  • Implement an alternative HTML tree encoder for Reason.
  • Generate both HTML trees.
  • Have a button that changes the directory between ml and re trees.

@aantron What do you think about this approach?

@Drup
Copy link
Contributor

Drup commented Jul 6, 2018

Yeah, or you could just compile refmt to js and do the conversion of the fly.

@rizo
Copy link
Collaborator

rizo commented Jul 6, 2018

@Drup I think it would be harder to achieve that for the signature items' markup. For examples, yes. But the markup is not purely textual, it has links and styling.

@ryyppy ryyppy mentioned this issue Jul 9, 2018
17 tasks
@aantron
Copy link
Contributor Author

aantron commented Jul 9, 2018

Hope this is okay?

It is, and quite welcome :)

@rizo, I think that works. My original thought was to generate both OCaml and Reason output side-by-side in each HTML file, and use CSS to hide/show the right one. But this is probably bad for accessibility, navigation, etc., so separate output trees are likely to be better.

@ryyppy
Copy link
Member

ryyppy commented Jul 10, 2018

My original thought was to generate both OCaml and Reason output side-by-side in each HTML file

I liked this idea as well... in my initial draft, I will just generate the whole documentation in ML / RE syntax side by side. Later on we can think of combining the output

@aantron aantron changed the title Reason syntax and syntax switching Reason/OCaml syntax switching Aug 8, 2018
@aantron
Copy link
Contributor Author

aantron commented Aug 8, 2018

@ryyppy added Reason syntax output in #156. Right now, odoc generates one of either ML or Reason syntax each time it is run. What's left on this topic is to decide whether we want to always generate both.

@leostera leostera mentioned this issue Oct 8, 2018
13 tasks
@ryyppy ryyppy self-assigned this Oct 8, 2018
@ryyppy
Copy link
Member

ryyppy commented Oct 8, 2018

@aantron @Ostera @rizo So my idea to complete all of this was to basically generate both versions, but always hide one unselected version via CSS. By providing the --syntax flag, you will be able to decide on a default language to display whenever you load the docs for the first time.

As soon as this is done, we can add a JS-driven toggle for both syntaxes in the doc header.

@lpw25
Copy link
Contributor

lpw25 commented Oct 8, 2018

Not hard to do but can we make sure to have an option that only generates one or the other rather than both. Personally, I think that setting should be the default, but that's less important than having the option.

@rizo
Copy link
Collaborator

rizo commented Oct 8, 2018

Totally agreed with @lpw25. Maybe the --syntax option could be changed to accept an ordered non-empty set of ml|re with ml as the default. The first syntax would be the one shown by default.

Additionally it would stop producing two file-system sub-trees for each syntax prefixed with .../re/ and .../ml – reverting back to what current stable odoc does. This would mean that exclusively one of the syntaxes would be visible for the users (re xor ml), but depending on the --syntax option, both would be potentially generated. The syntax toggle button could later be added to allow the switch.

I think this should be relatively simple to achieve with the current HTML generation code. @ryyppy is this something you want to work on next? I think we should do this before releasing odoc.

@ryyppy
Copy link
Member

ryyppy commented Oct 8, 2018

@rizo Yeah, after doing some integration work in Dune, I think this is the right way to do it.
Another thing I wanted to have is the ability to use an ENV variable to set the --syntax parameter, mostly for the dune integration.

I think I can get this done this week.

@rizo
Copy link
Collaborator

rizo commented Oct 8, 2018

Great, let me know if you need help with this!

I wanted to have is the ability to use an ENV variable to set the --syntax parameter

I believe you should be able to use Cmdliner.Arg.env_var for that.

@jordwalke
Copy link

Is there anything left to do on this one? I'm eager to get started trying out odoc. I like the discussion here about always generating both, but allowing doc authors to have total control over it if they want.

@ryyppy
Copy link
Member

ryyppy commented Feb 4, 2019

@jordwalke i am currently working on the syntax toggle feature... as soon as we get this in, we can switch the behavior of odoc to render both syntaxes by default and use the --syntax switch to opt into one specific syntax instead.

Will let you know as soon as this is done, then we should try to rebuild some essential doc pages of e.g. BuckleScript with the new odoc changes.

@yunti
Copy link

yunti commented May 21, 2019

@ryyppy How is it going with this PR? Need any help?

@github-actions
Copy link

github-actions bot commented May 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants