Skip to content

Use webman/*.html and webman/api for OCaml.org manuals#12976

Merged
Octachron merged 13 commits into
ocaml:trunkfrom
shakthimaan:doc/build-manual-under-webman
Mar 27, 2024
Merged

Use webman/*.html and webman/api for OCaml.org manuals#12976
Octachron merged 13 commits into
ocaml:trunkfrom
shakthimaan:doc/build-manual-under-webman

Conversation

@shakthimaan
Copy link
Copy Markdown
Contributor

@shakthimaan shakthimaan commented Feb 14, 2024

Theis PR builds the HTML manual pages directly under webman/ and not webman/manual/ as preferred in ocaml/ocaml.org#534. This will further allow the following URLs to be served directly under OCaml.org for easy navigation and reference:

  https://ocaml.org/manual/<VERSION>/*.html
  https://ocaml.org/manual/<VERSION/api/*.html

Reference:

@Octachron
Copy link
Copy Markdown
Member

I am not sure how this change would impact ocaml.org ? It is not like the files built in webman/manual or webman are published directly.

Moreover, I have the impression that this PR would result in a different directory structure than the one described in the ocaml.org issue by mixing the manual html files and the api directory.


let process_dir = Filename.current_dir_name

let ocaml_version = "5.3.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The manual is shared between patch version, so 5.3 is probably better. And this version number is available as a configure time macro as "@OCAML_VERSION_SHORT@"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We want to make the manuals available for all OCaml releases, which includes minor versions as well. Hence, the x.y.z naming convention has been used for consistency.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Except that bugfix versions share the same features, manual and API that their main version. I am not convinced that it is useful to differentiate between them for the manual.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay. I will build using "@OCAML_VERSION_SHORT@" and shall update the PR. I now see the version handling at ocaml.org for version numbering at https://github.com/ocaml/ocaml.org/blob/86acab21cac4f887d2ce2b0d963813a78e3fa81d/src/global/url.ml#L42-L51. Thank you!

@shakthimaan
Copy link
Copy Markdown
Contributor Author

The HTML manual documentation generated for 5.3.0 has been deployed for evaluation at https://shakthimaan.github.io/5.3.0/. (Eventually, we will replace shakthimaan.github.io with ocaml.org.)

I am not sure how this change would impact ocaml.org ? It is not like the files built in webman/manual or webman are published directly.

Yes, the changes help in shorter URLs that can be easily remembered and shared. For example:

Current: https://v2.ocaml.org/releases/5.1/htmlman/index.html
Suggested: https://ocaml.org/5.3.0/index.html

Current: https://v2.ocaml.org/releases/5.1/api/Atomic.html
Suggested: https://ocaml.org/5.3.0/api/Atomic.html

this PR would result in a different directory structure than the one described in the ocaml.org issue by mixing the manual html files and the api directory.

Yes. This is in order to simplify the URLs as mentioned. The corresponding HTML files will be pushed to https://github.com/ocaml-web/html-compiler-manuals/ and served directly by ocaml.org.

Thanks for your valuable comments and feedback.

(P.S.: I am now part of the OCaml.org team and helping them with the documentation and tooling).

Comment thread manual/src/html_processing/Makefile Outdated

WEBDIR = ../webman
WEBDIRMAN = $(WEBDIR)/manual
VERSION=$(shell grep 'OCAML_VERSION_SHORT=' ../../../configure | awk -F'=' '{print $$2}')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The version number is available without shell invocation by including Makefile.common or Makefile.config.

Comment thread manual/src/html_processing/Makefile Outdated
@@ -1,3 +1,5 @@
include ../../../Makefile.config
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The compiler Makefile style for those definition is to define first a ROOTDIR directory:

ROOTDIR = ../../..
include $(ROOTDIR)/Makefile.common

Comment thread manual/src/html_processing/Makefile Outdated
WEBDIRMAN = $(WEBDIR)/manual
VERSION := $(OCAML_VERSION_MAJOR).$(OCAML_VERSION_MINOR)

WEBDIR = ../$(VERSION)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As discussed offline, I would propose to keep WEBDIR= ../webman and use WEBDIRMAN =$(WEBDIR)/$(VERSION) as the output directory. Like this we keep the structure one directory by backend while still having the manual lives under a ($VERSION) directory.


(* How to go from manual to api *)
let api_page_url = "../api"
let api_page_url = "../" ^ ocaml_version ^ "/api"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason why this path is not ./api?
If this is the case, a comment would be nice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The manual here refers to htmlman. Without the above defined path, I get the following error:

Fatal error: exception Sys_error("../htmlman/./5.3/api/compilerlibref/index.html: No such file or directory")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's update the comment to reflect that point: manualhtmlman manual.

Copy link
Copy Markdown
Member

@Octachron Octachron Mar 26, 2024

Choose a reason for hiding this comment

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

The explanation is erroneous in fact, which explains why the path htmlman/../5.3/api doesn't correspond to anything.

The root issue here is that the Process_manual.get_xfile function filters the api index using the ../* pattern when looking for files to add to the table of contents.

Using ../ocaml_version/api rather than just api allow to still match this pattern by accident while keeping the correct url.

In other words, the simpler fix is to keep api_page_url="api" and fix get_xfile with

-             not (starts_with ".." rf) &&
+             not (starts_with "api" rf) &&

|> append_child head;
create_element "script" ~attributes:["src","navigation.js"]
|> append_child head;
create_element "script" ~attributes:["src", "https://plausible.ci.dev/js/script.js"; "defer data-domain", "ocaml.org"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wouldn't mind a comment documenting what is the plausible script that we are linking here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW, I have a bad feeling to embed any random JavaScript from the Internet into the output of the manual -- it is then a burden for those who use an offline version of it. Instead, any ocaml.org specific stuff that is needed should be done in a post-processing step (or via a configure option or environment variable).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This script is already added in the postprocessing step for ocaml.org (notice the repertory name). I imagine we could add an offline flag to the post-processing script to skip this part of the postprocessing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, ok. Sorry for my lack of understanding about that. If I understand you correctly, the property that http://caml.inria.fr/distrib/ocaml-4.14/ocaml-4.14-refman-html.tar.gz (and future versions) allow offline browsing of the manual is fulfilled?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed, those are the archive version that are not post-processed for ocaml.org and directly generated from the manual source.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe in term of comment :

script for ocaml.org's instance of plausible analytics.

@Octachron
Copy link
Copy Markdown
Member

The PR looks good to merge, thanks! I would propose to add a change entry in the manual section (something like "better integration with ocaml.org"?) to keep track of the manual improvements.

@shakthimaan
Copy link
Copy Markdown
Contributor Author

Should I create a new PR with just the changes along with a change entry ? Thank you!

@Octachron
Copy link
Copy Markdown
Member

Octachron commented Mar 27, 2024

You can add the change entry in the changelog in a new commit in this PR. When merging, I will squash the PR into one commit since this is mostly a one feature PR with some back-and-forth and keeping the full history doesn't feel useful in those cases.

@Octachron Octachron merged commit 83535fc into ocaml:trunk Mar 27, 2024
samsa1 pushed a commit to samsa1/ocaml that referenced this pull request Mar 27, 2024
* Use webman/*.html and webman/api for OCaml.org manuals
* Specify OCaml version when building HTML manual
* Use webman/<version>/ directory structure
* Add  Plausible analytics script
@dra27 dra27 mentioned this pull request Apr 23, 2024
frou added a commit to frou/ocaml-docset that referenced this pull request May 17, 2024
I encountered (and reported) a bug with downloading the manual from the ocaml.org site.

Also, the `v2` subdomain seems to be deprecated.

Related reading:
* ocaml/ocaml#12976
* ocaml/ocaml.org#534
* ocaml/ocaml.org#465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants