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

Driver: generate "external" pages #1183

Merged
merged 9 commits into from
Aug 23, 2024
Merged

Conversation

panglesd
Copy link
Collaborator

This adds generation of (basic) pages for:

  • A "list of all packages" page (at the root of the doc)
  • A landing page for each package (in the <pkgname>/ folder)
  • A landing page for the list of libraries (in the <pkgname>/lib/ folder)
  • A landing page for each library (in the <pkgname>/lib/<libname> folder)

This PR does not touch only the driver: I had to add the possibility of an empty --parent-id for the "list of all packages" page.

I also had to avoid "relative references" in the generated pages as they are did not resolve. I used `rooted absolute references instead, and I will investigate the bug.

This will hopefully be useful to test various ideas for the breadcrumbs.

@panglesd panglesd added the no changelog This pull request does not need a changelog entry label Jul 31, 2024
@panglesd
Copy link
Collaborator Author

(Hopefully, this can be made less clicky by having the landing page of a package be better (similar to odig's one / ocaml.org's one), but it's nice that library list and library landing page exists to avoid 404 pages eg on handcrafted path)

src/html/link.ml Outdated
let url_segs = for_linking ~is_flat url in
let filename =
match url_segs with
| [] -> Fpath.v "./"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What was the goal of this ? for_linking is defined just above and never returns an empty list.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strange, likely a leftover from some modifications that required that (maybe when the parent id was empty?).

I removed it.

@@ -0,0 +1,137 @@
open Packages
open Odoc_unit
let write_file file content = Bos.OS.File.write file content |> Result.get_ok
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fails because file might be in a directory that do not exists:

Suggested change
let write_file file content = Bos.OS.File.write file content |> Result.get_ok
let write_file file content =
Bos.OS.Dir.create (Fpath.parent file) |> Result.get_ok |> ignore;
Bos.OS.File.write file content |> Result.get_ok

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I'll use the already written "Util.write_file instead, which does the folder creation.

@jonludlam
Copy link
Member

I think it'd be useful to have some tests for the changed functionality in odoc itself.

src/odoc/asset.ml Outdated Show resolved Hide resolved
}

module PackageLanding = struct
let content pkg =
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've refactored the content functions to take advantage of Format in panglesd#32

panglesd and others added 9 commits August 23, 2024 13:15
Generate pages that are external of a package: landing pages for package,
library, library list and package list.

There is a problem currently with empty parent id. Currently using a "`a`"
container directory for the package list.
Useful for the list of packages landing page
Util.write_file is refactored to be more suitable.

Co-authored-by: Paul-Elliot <[email protected]>
Co-authored-by: Jules Aguillon <[email protected]>
@panglesd
Copy link
Collaborator Author

Rebased!

Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

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

Awesome! Let's merge!

@panglesd panglesd merged commit a45c770 into ocaml:master Aug 23, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog This pull request does not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants