-
Notifications
You must be signed in to change notification settings - Fork 94
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
Dedicated sidebar file #1250
Dedicated sidebar file #1250
Conversation
e6ff8d0
to
0031767
Compare
src/odoc/sidebar.ml
Outdated
open_out_bin (Fs.File.to_string output) | ||
in | ||
let output = Format.formatter_of_out_channel output_channel in | ||
Format.fprintf output "%s" text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best practice to close the channel. We have Fun.protect
in utils to help close properly.
src/odoc/sidebar.ml
Outdated
("modules", `Array (List.map (Tree.to_json toc_to_json) units)); | ||
] | ||
|
||
let sidebar_to_json ({ pages; libraries } : Odoc_document.Sidebar.t) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this function be in the HTML generator ? That's where the other JSON function are located.
The HTML generator is used in these functions.
src/odoc/sidebar.ml
Outdated
let sidebar_to_json ({ pages; libraries } : Odoc_document.Sidebar.t) = | ||
let pages = List.map pages_to_json pages in | ||
let libraries = List.map libs_to_json libraries in | ||
`Object [ ("pages", `Array pages); ("libraries", `Array libraries) ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON is slightly confusing with two things called pages. Shouldn't this be called manuals
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, anyway this changes to a more generic sidebar json in #1251.
And not the index file, to avoid doing multiple times the same thing
0031767
to
180676a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, let's merge !
Several changes: - Entries are now defined in the `odoc_index` library, - Entries can have new kinds (pages, source, ...) - Indexes have the form of "skeletons of entries", that can be folded. - Indexes can be created by odoc with the `odoc compile-index` command, and then consumed by sherlodoc. These changes come from: - ocaml/odoc#1228 - ocaml/odoc#1232 - ocaml/odoc#1233 - ocaml/odoc#1244 - ocaml/odoc#1250 - ocaml/odoc#1251
This PR adds a
odoc sidebar-generate
command which takes an index (.odoc-index
) as input and produces a sidebar file (.odoc-sidebar
or.json
if--json
is passed).This allows to reduce the time spent HTML generating, as sidebar files are much smaller than index files (they don't contain values and other entries that are not shown in the sidebar anyway).
The json format is likely to change with #1247.