-
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
Doc: parent-child convention for installed packages #1011
Conversation
This convention is already in use by Odig, Voodoo and Dune. It needs to be specified as it needs to change in order to support assets and complex hierarchies. Co-authored-by: Paul-Elliot <[email protected]>
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 think it does except for the index.mld
renaming bit.
doc/parent_child_spec.mld
Outdated
- [.mld] pages are installed in a package's [share] directory, under the | ||
[odoc-pages] sub-directory. | ||
- [index.mld] is the parent of every other pages. The driver can freely rename | ||
this page, for example, it can be named after the package. |
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 renaming bit doesn't work, it prevents from making links from other pages into the index
page (example).
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 index of package page is not generated using Odoc ? It can't make references to {!page-index}
, as that would be very ambiguous.
Though I believe we need references through the page hierarchy like {!page-packages.odoc.index}
but even then, the index
part isn't wanted.
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.
Also, we'll want to allow deeper hierarchies in the future and our first though is to generalize the index.mld
renaming:
If you have foo/bar/index.mld
, it makes a page named foo/bar.html
and have foo/bar/*.mld
as children.
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.
@dbuenzli you're right, linking to the index is currently broken. We can fix this by having links to page-index
resolve to the parent, but it all feels a little hacky. If anyone has any other bright ideas I'll be very happy to hear suggestions.
The index of package page is not generated using Odoc ? It can't make references to {!page-index}, as that would be very ambiguous.
It's only ambiguous knowing how the various drivers work - it's entirely unambiguous in the context of having a flat hierarchy of mld pages per package - ie, the view that the package author has. There can be only 1 index.mld
file, as they're all stored in the same directory (and dune at least enforces this - I presume other packaging tools do similarly.) When we have support for user-written hierarchical docs, we can be more explicit in what to name your mlds, how the hierarchy works, how to make links between pages correctly and whatnot.
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 index of package page is not generated using Odoc ?
I'm not sure what you are asking here. The index of the package page is generated by processing the index.mld
file provided by the package or one generated by the driver if none is provided.
Note that currently odig
is actually using the legacy --pkg
option (I'm planning to rewrite the driver any time soon). So I'm wondering whether currently odig is not maybe making something that is not described by this convention in this PR and we are missing a bit.
It can't make references to {!page-index}, as that would be very ambiguous.
I notice for the example I mention that while the link gets correctly rendered in the brzo
driver which I use for docsets on erratique.ch, it's neither rendered by odig
nor by ocaml.org.
Though I believe we need references through the page hierarchy like
{!page-packages.odoc.index}
but even then, theindex
part isn't wanted.
For cross packages links I don't really care if there's an additional .index
needed. However one thing that is important is not to have to hard code your package/parent name everywhere. Even for your example of deeper hierarchies you want to be able to refer to your parent's index page without having to name it.
Besides I guess that if we want these cross packages links to work. Then we need to go a bit beyond what is described here, on how things need to be compiled. (In the legacy pipeline the --pkg
option made all that clear).
If you have
foo/bar/index.mld
, it makes a page namedfoo/bar.html
and havefoo/bar/*.mld
as children.
I don't really care how the page end up being named but we need a reliable way to be able to refer to these in the sources. However in general the idea of solving these problems by renaming .mld
by the driver doesn't seem very sound. It's rather something you should specify at the cli level when you compile the source.
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 slighly annoying bit is that package cross references go through index
then e.g.
{!pkg.index.mypage}
)
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.
Ah but no in fact we simply do:
pkg <-- empty, non materialized page created by the driver
├── Mymodule
├── index.mld
├── myimage.gif
└── page.mld
and simply decrete that index.mld
is the landing page shown to users.
Note that this essentially comes back to the old --pkg
option.
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 think we should focus here on what currently works (or should work!)
I think it's useful to discuss more because we'll want deep hierarchy soon and it would be a shame to create conflicts now.
furthermore it is entirely impossible to link to that page because you've no idea what the driver might have named it
Maybe it's reasonable to say that we never want to reference a parent page by its name as it was said above ?
Though, the convention should tell how it should be renamed.
In my opinion these kind of shortcuts tend to bring in more problems than they solve because now you need to say something about what happens when you have both foo.mld and foo/index.mld.
Defining what happens in this case does not seem worse than having undefined references exist.
I think good looking URLs, then good looking references come before work in the driver.
It seems assets references proposed in #1002 have some kind of search procedure (not sure it's a good idea, but why not) what if we start treating pages references like assets ?
Yes I think we want that! It's currently impossible to solve a page name conflict without limiting what can be linked to (by limiting the search path, as it's done currently).
It's also a reason why we need dot references, a page outside of this hierarchy could do {!page-pkg.index.mypage}
.
We also want to do:
pkg <-- an mld page created by the driver but compiled like any other pages.
├── Mymodule
├── index.mld <-- Has a reference to 'mydir/index.mld'
├── myimage.gif
├── page.mld
└── mydir/
├── index.mld <-- Has a reference to 'index.mld' in the parent directory
└── myimage.gif
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.
Defining what happens in this case does not seem worse than having undefined references exist.
Well it goes without saying that if you use the reference it warns you for the fact it doesn't exist.
Yes I think we want that!
Actually I'm not sure. I think it would be clearer to have explicit parent notation, that is the moral equivalent of ..
which we all know perfectly is sadly missing from ocaml
's module system :-)
Maybe it's reasonable to say that we never want to reference a parent page by its name as it was said above ?
Though, the convention should tell how it should be renamed.
We definitively need that if we want to enable packages to escape their little island to link into other ones (in fact AFAIR that worked at some point in odoc
on the --pkg
option path).
pkg <-- an mld page created by the driver but compiled like any other pages. […] └── mydir/ ├── index.mld <-- Has a reference to 'index.mld' in the parent directory
But what do you put on this pkg
page ? how does it materialize ?
Basically I think the idea that the parent-child relationship occurs through concrete content (i.e. a .mld page) is not necessarily a good one and we should let it go.
I think we rather want to think in terms of nested bundles: understood as a directory with .mld
files, assets and possibly module files. The name of the directory defines the reference path/url prefix and the default page is index.mld
so a hierarchy like:
pkg/
├── Mymodule
├── index.mld <-- Has a reference to 'mydir/index.mld'
├── myimage.gif
├── page.mld
└── mydir/
├── index.mld <-- Has a reference to 'index.mld' in the parent directory
└── myimage.gif
Generates basically:
pkg/
├── Mymodule.html
├── index.html
├── myimage.gif
├── page.html
└── mydir/
├── index.html
└── myimage.gif
To which you can refer to, as far as the pages are concerned and according to where you are as:
- In another package.
{!page-pkg.index} # pkg/index.mld
{!page-pkg.page} # pkg/page.mld
{!page-pkg.mydir.index} # pkg/mydir/index.mld
- When you are in the
pkg
level
{!page.index} # pkg/index.mld
{!page.page} # pkg/page.mld
{!page.mydir.index}. # pkg/mydir/index.mld
- When ou are in the
mydir
level
{!page-^index} # pkg/index.mld
{!page-^page} # pkg/page.mld
{!page-index} # pkg/mydir/index.mld
That way we can actually already have structured documents now. The convention for a package pkg
is simply as follows:
- A "directory"
pkg
is created - The content of
odoc-pages
is rerooted to this directory - If there is no
index.mld
at that point, generate one for the package inpkg
- The modules are added to this directory.
Directory is methaphorical here this can happen through --parent
options.
This is a system that is extremely easy to use for end users as it naturally maps on the file system, no crazy renamings going on behind the scene or arbitrary attachements. The position in the file system mostly defines how your reference looks like. The only thing the user needs to be made aware of is the pkg
directory creation – and only if it needs to make references into other packages.
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.
- In another package.
{!page-pkg.index} # pkg/index.mld {!page-pkg.page} # pkg/page.mld {!page-pkg.mydir.index} # pkg/mydir/index.mld
Unless you have a pkg
hierarchy in your bundle in which case you need to write:
{!page-^pkg.index} # pkg/index.mld
{!page-^pkg.page} # pkg/page.mld
{!page-^pkg.mydir.index} # pkg/mydir/index.mld
Thanks to support from OCSF I can allocate a bit of time now to migrate the Personally I'm really not fond of the renaming business that it seems other drivers are doing. It puts more burden on document writers which must be made aware of it to make links on the I am also unconvinced by the current semantics of the I think we should simply let the
Materializes the references If we have that then, what do you think about the scheme I proposed in this comment ? This is compatible with the current state affairs, allows reference on a package's We would need to formalize the name resolution procedure a bit more but the idea is to make it mostly as the OCaml one in the module system with the addition of the moral equivalent of |
Sorry for the late answer. I gave some thinking on this, and I think you are right that the good way to do it is by having pages without content. I'd like to propose a slightly different way to implement that though. Also, I'm not going to speak much about the reference resolution/syntax, since I think it is a sufficiently disjoint topic! (not completely orthogonal, I agree) ContextI wrote some details about the current state, mostly for myself, but I think it is worth sharing. What we currently have is:
As a consequence of this, if a page I think we need cannot change this, to avoid generating many dead links. New featureHowever, as @dbuenzli suggests, we can simply add pages without content! And, in this case, I think we should just "unify" the two following scenario:
The first option would be "the right way to do it", the second option "kept for compatibility purpose". In the case of:
we could either say that it's forbidden, or say that it's allowed but cannot be referenced, or say that it's allowed, can be reference, and generate it's content from the list of children. This would allow to map exactly a file hierarchy to a page hierarchy, with references working naturally. Concretely, the The driver conventionAs an example, the commands to compile the following hierarchy:
would be:
So:
Comparison with @dbuenzli proposalI think it basically achieve the same result, but is more clearly retrocompatible and closer to what we have now (so hopefully easier to implement). In particular, with @dbuenzli's proposal:
Does that make sense? Convention until the above is accepted and implementedI think that we can already achieve the same result without implementing my proposal, by having the following convention:
So, taking the same example as above, the commands would be:
Thanks for reading until here! |
Thanks @panglesd !
That's useful thank you. I will try to go with that for now in my
Did you mean to define an
For my own understanding, could you be more specific about this ? Since it seems there was no support so far for hierarchical documents in any build system. It seems we just have the toplevel renaming business done by some of them, which leads to an
It seems to me that it captures the essence of what I wanted1: Here are few comments:
I think these comments could perhaps be solved by having a separate command for nodes:
Which would produce an
Footnotes
|
I meant that the convention for the user is to write an
Suppose
For instance, this implies that we cannot in But yes, I agree, we have should not restrict ourself too much!
Sure, I just came up with that for the example. Node seems a good pick, let's see in the PR later :)
Yes, its using
That's a possibility! I guess we should not introduce breaking changes, so maybe in another
Yes, why not!
+1! Thanks for all these comments, it will definitely help if/when I implement the feature.
Sorry if I misunderstood your proposal, and made invalid points! If you think it has some advantages, feel free to defend it. I'm curious about the opinion of @jonludlam and Julow, especially on the "Convention until the above is accepted and implemented" since it is what this PR was about initially! (Of course, I'm also curious about their opinion on the rest of the discussion!) |
Well I'm not very fond of your idea to encode little languages into the argument values. You can use as much options as you want. They are cheap to define, there's plenty of characters to choose from them and it avoids having to munge stuff in drivers and unmunge them in
It's much less obscure and actually shows directly in the invocation itself on which files the results depends.
See above. Solved by having both
I'm not sure I understand your points here. In any case new I think you should not take compatibility too much into account, it seems we have only three or four
I didn't think deeply about it but I can't really make a difference with what you are proposing. What's the difference ? You can still attach stuff to pages ? Maybe we can simply say no page can be a |
That's not really my idea, I'm just using what is already in odoc... But I agree that
Instead of using
Thanks, good to know!
What I understood from your proposal was that nodes did not have a corresponding |
Ah yes maybe. But I'm not against having to devise empty node We will however still need a discussion about scoping. Basically in your example above if in |
while still having reference to `page-index` work. Signed-off-by: Paul-Elliot <[email protected]>
It's not possible to reference the package main page and this cannot be fixed now by tweaking the convention.
doc/parent_child_spec.mld
Outdated
- If there is no installed [index.mld] page, the driver has to generate some | ||
content for the [pkg] page. | ||
|
||
When the rendering of source code is enabled, the source tree will be named |
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 don't think any driver that follows this spec renders source today. If we're just documenting how things work right now we should omit this section.
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.
doc/driver.mld
does that. This document doesn't describe what an existing driver does, rather what can be done by a driver today.
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 think it's better if we document what driver should do, so that there are no useless discrepancies between them.
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 think this paragraph should be kept then.
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.
doc/driver.mld is a bad example for this - it follows literally none of the other conventions in the paragraph, as the intent back when it was first written was to show how odoc can be used more generally than just for making docs from installed opam packages.
We haven't quite figured out a convention for the sources, and it's not at all clear to me that the hierarchy will share the same namespace as the other package pages. As a concrete example, on ocaml.org
cmdliner's pages currently appear under the hierarchy https://ocaml.org/p/cmdliner/latest/doc/ - e.g. tutorial.mld is https://ocaml.org/p/cmdliner/latest/doc/tutorial.html . I can easily imagine that there would be a desire for the source to appear under https://ocaml.org/p/cmdliner/latest/src/ in which case there's no namespace clash.
I'd quite like in this PR to just focus on what's actually implemented 'in the wild' and then we can have a separate discussion on these sorts of topics to bash out the wider question about what we should do to support the new features.
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 don't agree that what exists in the wild is in the scope of this PR but I removed the paragraph to not interferes with the future changes.
Thanks! |
@dbuenzli We are currently working on a better CLI and convention for building hierarchical docs. I remembered your message quoted above, so: If you did not work too much on porting odig to the new command set, you might want to wait a little bit to jump directly to the last command set! (the design will be posted for discussion as soon as it is ready). |
I kept on postponing that for now. I wanted to squeeze that soon (next week) but it may fall through again. When do you expect to post the design ? |
(Sorry for the delay, but I'll be able to answer more accurately this evening!) |
The document we have is clearly not finished enough and there would be no interest in sharing it now. It's hard to give an estimate on when it will be in a shareable state, but for sure that won't happen this week unfortunately. We do think it would be better to wait for the new design to be published and validated ; and maybe even implemented in |
Is there any reason why |
Let me give more details, since re-reading my post I agree it gives more an impression of being developed "closed doors" than I think is the case. In particular the use of "we" without being explicit who it is referring to. I'm a Tarides employee and work on odoc as part of this job. For this particular task (improving the CLI to enable hierarchical documentations, assets, render source code and a CLI friendlier to an incremental build system) I started working with @Julow on a proposal. Of course, this first stage of "in private" writing of the proposal will not replace a public discussion here. Many of this was discussed in odoc dev meetings. I don't know if those meetings are public or not? (I'm participating also to some ppxlib dev meeting, which are public, with public notes, which is working quite well. If odoc dev meetings are not public, I would be in favor of having them public, especially public notes.) I cannot compare to how odoc was before, since I was not there at the time. What has changed to give this impression? |
I see! We use a gh board to organize ourselves, usually with issues that more or less just link to a public one. This time it contained a bit more than that, and indeed it should have been a public issue. I added the content of the "issue" that was referenced in the PR. |
This convention is already in use by Odig, Voodoo and Dune. It needs to be specified as it needs to change in order to support assets and complex hierarchies.