-
Notifications
You must be signed in to change notification settings - Fork 89
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
Specify children order in frontmatter #1193
Conversation
297519c
to
531e8d4
Compare
src/driver/odoc_driver.ml
Outdated
@@ -582,6 +582,20 @@ let run libs verbose packages_dir odoc_dir odocl_dir html_dir stats nb_workers | |||
(fun () -> render_stats env nb_workers) | |||
in | |||
|
|||
List.iter |
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.
Is all of this just debug ? Otherwise, it seems too verbose and opaque.
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.
Yes, this is only showing filtered logs for debugging. I'll remove it.
src/model/comment.ml
Outdated
let fm, content = | ||
let fm, rev_content = | ||
List.fold_left | ||
(fun (fm_acc, content_acc) doc -> | ||
match doc.Location_.value with | ||
| `Code_block (Some "meta", content, None) -> | ||
(parse_frontmatter content.Location_.value :: fm_acc, content_acc) | ||
(content.Location_.value :: fm_acc, content_acc) |
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 previous code was more logical (parse each blocks and concatenate the values rather than concatenate the blocks and parse something no one wrote) and allowed more graceful handling of errors with locations.
Anyway, that's a temporary syntax.
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.
Indeed, we are going to have the frontmatter consistently at the top, so there won't be any distinction.
So, I suggest leaving it as is.
|
||
type library = { name : string; units : Paths.Identifier.RootModule.t list } | ||
|
||
type page_hierarchy = { hierarchy_name : string; pages : toc } |
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.
I didn't expect the page hierarchy to have a name and be an extra level on top of a toc
. What if this hierarchy contains an index.mld
and a children order ? Is here an extra indentation with the name repeated ?
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.
This "hierarchy_name" corresponds to the name of the library's package: the one passed in the -P
option of odoc compile-index
.
It is currently turned into a "{hierarchy_name}'s Pages"
title:
# Package1's Pages
- The best way to do foo
- Guide
- example
- tutorial
# Package2's Pages
- Blazingly fast bar
- Guide
- example
- tutorial
# Library1's modules
- Foo
- Bar
We could possibly remove this hierarchy_name
, but I would argue that it is better done in another PR (this PR is not introducing the behavior).
(Maybe:
# Pages
- The best way to do foo
- Guide
- example
- tutorial
- Blazingly fast bar
- Guide
- example
- tutorial
# Libraries
- library1
- Foo
- Bar
)
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.
Makes total sense! I think the hierarchy name is something we'd want in the breadcrumbs and so for clarity, we should also have it in the sidebar.
All warnings have been implemented, and the code has been cleaned up. Only remains a review for the implementation details, on my side this is ready to be merged. |
Page don't and won't use dot references as in `page1.page2`.
Use ``` children: page1 page2 page3 ``` in the frontmatter
Children order specifies order in an index page for the current directory. It has to specify what is a page and what is a directory, as we can have: ``` doc/ index foo foo/ index bar ``` In the specification of the order in `doc/index`, we must make the difference between the foo page and the foo directory. This was not practical with references. So instead, the order specification has its own type.
Allow sequences of spaces in a row: `children: page1 page2`.
Unexpose unnecessary functions open modules use mli for model/frontmatter
Co-authored-by: Jules Aguillon <[email protected]>
Directly take children order into account. Co-authored-by: Jules Aguillon <[email protected]>
Co-authored-by: Jules Aguillon <[email protected]>
Co-authored-by: Jules Aguillon <[email protected]>
But alias it to Id. It was bringing too many values in scope (eg shadowing compare).
03389b1
to
a5db29c
Compare
Now that we have a convention for hierarchical pages, we need a way for the author to specify the order.
This PR does that by using the frontmatter, using this syntax:
The trailing
/
for directories is needed to distinguish them from files.This PR is mostly ready, but:
children
list is not found (should it happen duringlinking
of the page or during thecompile-index
command?)children
list in an index page:PageToc
.