-
Notifications
You must be signed in to change notification settings - Fork 97
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
Cut the preamble at the first top-level heading #640
Conversation
It should be no heading. The idea is that the preamble never has any heading, it's what comes before the implicit level 0 heading ( If you do it the way it is in this PR then these headings won't appear in the
|
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.
Code looks ok to me.
It should be any heading. The idea is that the preamble never has any heading, it's what comes before the implicit level 0 heading (
module
or.mld
page title) and the first header. Basically the preamble is like an abstract, only a sequence of block level items.
I don't know, I think it's rather nice to be able to have some structure in your preamble. In any case, it's only in the first comment, and the specification is clear. And indeed, they do not appear in the TOC, which is fair.
src/document/generator.ml
Outdated
let doc_of_expansion ~decl_doc ~expansion_doc = | ||
Comment.standalone decl_doc @ Comment.standalone expansion_doc | ||
let make_page title kind url ?(header_title = make_name_from_path url) comments | ||
items = |
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.
Could you use labels for everything here ?
Also, comments are already lists, no need to have a list of lists.
There starts to be a lot of those helpers floating around, I think we'll need to put them in a dedicated module at some point.
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 labels would be good here, arguments have all different types and some are expected to be literals, for example:
make_page modname `Mod url [ t.doc; expansion_doc ] items
Also, comments are already lists, no need to have a list of lists.
I wanted to abstract the code that knows what comments are and that concatenate them in a specific way, instead of repeating that everywhere.
I've pushed some refactoring making my intent clearer.
Well what we are going to have is duplicates of #235 for lower levels... People do not apply these things religiously. Keep things simple and usable, in particular you don't want for people to have to scroll for pages to access the toc when the viewport becomes narrow. As mentioned in #235 take wikipedia pages as an example here. |
And again comment should not be the unit here. If I have two unattached doc strings paragraphs before the first heading these two paragraphs should go in the |
Just to make things clear. That's the model I'd like to give to
This model is simple, easily discovered and remembered. In particular it doesn't require conditional thinking and enforces good usability. I enjoin you to write down in words the documentation for the model that this PR proposes and compare it with the above and ponder on the fact that it will have to be (likely) discovered and remembered by users. Regarding page usability, if we allow people to section their preamble they may not realize that on narrow viewports this may push toc access much further down and hamper page usability. It also means that for example on Now @Drup "finds it nice" to be able to section the abstract, I don't think there's anything wrong with writing these sections in the content, especially on broad viewports, there will be no difference; so I'd like a concrete example that would not be well served by the model I give above which, again, is simple to discover, understand and remember, enforces better usability and leads to a more consistent markup structure for content. |
I think we were picturing a model where the preamble was more like the introduction section of an article, rather than the wikipedia-style lead section. The lead section model is appealing though, and would indeed require cutting the comment at the first heading of any sort. I disagree about having multiple comments though. I strongly prefer using only the first comment. It allows leaving the preamble without needing a section or OCaml value. It also keeps the mli and the docs in a similar state. Most mli files have no sections and using all comments before an item will lead to a lot of arbitrary stuff being pulled into preambles unintentionally. |
I think that in practice if you peruse a bit .mlis you'll mostly find:
But again in the "wikipedia-style", for those I don't think you really loose anything w.r.t. vision you had for preambles -- which I have to say I'm not too fond of anyways, I don't want to have to scroll through walls of text to access the api defs (even nowadays that we have a toc). Longer blurbs of text can be deferred to sections at the end of the module and/or
I could almost be convinced by that but it has two problems in my opinion.
If there's really a strong feeling that you want to be able to break an initial sequence of paragraphs I think we should rather introduce an explicit construct like
Most poorly documented .mlis (which are plenty) indeed do not have sections. But in any case I suspect that if an |
If the preamble contains a heading, this heading and the rest of the preamble is moved out of the 'header' to the 'content' of the page. That part of the preamble is rendered after the TOC and appears in it. Heading of level 2 or higher are kept in the preamble.
I think you've convinced me about no section headers in the pre-amble.
That isn't true when |
Ok I won't fight about this. It is true that sometimes the notion of comment "counts" in the ocamldoc language. |
The previous rule was to allow headings of level 2 or more in the preamble. This commit removes this rule and cut the preamble before any heading.
Following the discussion, I changed the rule to cut the preamble at the first heading, regardless of its level. |
Closes #235 and #631
If the preamble contains a level 1 heading, this heading and the rest of the preamble is moved out of the 'header' to the 'content' of the page.
That part of the preamble is rendered after the TOC and appears in it.
Heading of level 2 or higher are kept in the preamble.
cc @Drup, @dbuenzli