Skip to content

feat: use output from new doc-ci with odoc 3 features in the package documentation area #3124

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

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

panglesd
Copy link
Contributor

@panglesd panglesd commented May 21, 2025

Replaces #3123, but PR is on main.

This PR uses the output from the new doc-ci @jonludlam has been working on. The docs are built with odoc 3, benefiting from several features, some of them that were previously "emulated" on this repo (such as a global sidebar).

This currently implements:

  • A global sidebar as computed by odoc 3
  • Better breadcrumbs
  • Source code pages
  • Redirections for url changes

This PR is not yet complete, however all pushed commits can already be reviewed, so I think it was already worth opening.
I tried to make the commits as atomic as possible to ease the review, which can then be made "commit by commit". All commits compile, although they may fail at runtime (eg due to wrong format for deserializing).

I'll post updates and warn when I consider the PR complete!

@panglesd panglesd force-pushed the odoc-3.0-reworked branch 3 times, most recently from 9a5f418 to 62d32b9 Compare May 21, 2025 13:04
@sabine sabine marked this pull request as draft May 26, 2025 10:20
@sabine
Copy link
Collaborator

sabine commented May 26, 2025

I converted this to a draft, but feel free to ping me any time when it makes sense to test this. I will put this on https://staging.ocaml.org, and hook it up with the staging docs-ci.

@sabine sabine changed the title Documentation: use output from new doc-ci with odoc 3 features Package Documentation Area: use output from new doc-ci with odoc 3 features May 26, 2025
@sabine sabine changed the title Package Documentation Area: use output from new doc-ci with odoc 3 features feat: use output from new doc-ci with odoc 3 features in the package documentation area May 26, 2025
@yawaramin
Copy link
Contributor

Does this make #1963 obsolete?

@jonludlam
Copy link
Member

Does this make #1963 obsolete?

Yes.

@panglesd panglesd force-pushed the odoc-3.0-reworked branch from 52920b2 to 3f494dd Compare June 3, 2025 09:41
@panglesd
Copy link
Contributor Author

panglesd commented Jun 3, 2025

@sabine @jonludlam This is ready for testing on staging!

(I don't know what makes the "deployability" CI fail but I don't think it's related)

sabine added a commit that referenced this pull request Jun 6, 2025
@sabine
Copy link
Collaborator

sabine commented Jun 6, 2025

Deploying on staging.ocaml.org (https://deploy.ci.ocaml.org/job/2025-06-06/074257-ocluster-build-384752)

@jonludlam
Copy link
Member

The source rendering needs some CSS love:

Screenshot 2025-06-09 at 14 43 27

@panglesd
Copy link
Contributor Author

Did you take the screenshot from staging.ocaml.org? Here is how the same page looks for me:

Light mode:
image

Dark mode:
image

I'll try to reproduce with different browser and settings, but if you have any hint, I'll take them.

@panglesd
Copy link
Contributor Author

Ok, testing on chromium yields the same CSS problem...

sabine added a commit that referenced this pull request Jun 10, 2025
@sabine
Copy link
Collaborator

sabine commented Jun 10, 2025

Deploying updated PR to staging.

@panglesd panglesd force-pushed the odoc-3.0-reworked branch from 6c9e7de to b30d4c1 Compare June 11, 2025 06:54
@panglesd panglesd marked this pull request as ready for review June 11, 2025 07:33
sabine added a commit that referenced this pull request Jun 11, 2025
sabine added a commit that referenced this pull request Jun 11, 2025
sabine added a commit that referenced this pull request Jun 20, 2025
Comment on lines 1292 to 1305
| Some "module" -> Module
| (Some ("page" | "leaf-page") | None)
when String.starts_with ~prefix:"Library " sidebar.node.content
->
Library
| Some ("page" | "leaf-page") -> Page
| Some "module-type" -> Module_type
| Some "parameter" -> Parameter
| Some "class" -> Class
| Some "class-type" -> Class_type
| Some "file" -> File
| Some "source" -> Source
| None -> Page
Copy link
Collaborator

Choose a reason for hiding this comment

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

makes sense to do this decoding in the Sidebar module in ocamlorg_package/, instead of here

panglesd and others added 5 commits July 1, 2025 14:10
panglesd and others added 22 commits July 1, 2025 14:10
From https://alpinejs.dev/directives/cloak :

> Sometimes, when you're using AlpineJS for a part of your template, there is a
> "blip" where you might see your uninitialized template after the page loads,
> but before Alpine loads.
>
> `x-cloak` addresses this scenario by hiding the element it's attached to until
> Alpine is fully loaded on the page.
>
> For `x-cloak` to work however, you must add the following CSS to the page.
>
> ```
> [x-cloak] { display: none !important; }
> ```
Since odoc 3, and the replacement of voodoo by odoc_driver as the odoc driver,
the layout for docs has changed. Since we do not want to break previously
existing links, we redirect the old layout to the new one.
Documentation assets are not present as json files. Therefore, if we do not find
a .json target, we try to load it as an asset.

This is not ideal: assets always require two requests to the documentation
server. 404 pages also trigger two requests...
Odoc 3's search has already a leading `/`.
`old_path` and `new_path` do not include the prefix, which makes the detection
of redirection harder.

Previously, the url `/p/odoc/3.0.0/doc/odoc.odoc/Odoc_odoc/Sidebar/index.html`
would trigger the rewriting:
`doc/Odoc_odoc/Sidebar/index.html`
->
`doc/odoc.odoc/Odoc_odoc/Sidebar/index.html`

which is a bug. Adding a / in the rewriting rules solves it
@panglesd panglesd force-pushed the odoc-3.0-reworked branch from 41a4b4c to 9ff58c1 Compare July 1, 2025 12:15
@shonfeder
Copy link
Contributor

shonfeder commented Jul 2, 2025

Hello! Congrats on this work, @panglesd!

@sabine, just FYI, I will be taking over responsibility for helping ensure this lands.

If any additional work is needed here, I'll respond, and if not I may ping with followups until it is in :)

Thanks!

@jonludlam
Copy link
Member

Can we please not merge this PR for a few days - I just need to fiddle with the storage on the new server and I'd rather not do that when it's being used! I'll update when the changes have been done.

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.

5 participants