Skip to content

Conversation

dbuenzli
Copy link
Contributor

@dbuenzli dbuenzli commented Feb 18, 2021

This PR is built on top of #589. The relevant commits are from the 15 of february.

It does the following (see the indiviual commits for more details).

  1. Regularizes the markup for declarations (no special casing when there is no docstring).
  2. Classifies the docstring div as .spec-doc (using .doc here was non trivial w.r.t. to the current usage of this class by the stylesheet maybe we can clarify/simplify later).
  3. Classifies all the direct children of .odoc-content.

Regarding 3. I'm not happy about the .odoc-unattached asides since they make markup structure differ between modules and mld pages. E.g. you can have h1 (aside p) in modules and you will have h1 p in mlds, which makes more cases to treat than needed. But I'll try to unnest them in another PR.

@dbuenzli dbuenzli mentioned this pull request Feb 24, 2021
@Drup
Copy link
Contributor

Drup commented Feb 25, 2021

Looks good to me. I think we could make the code a little bit regular by adding a few "smart div constructors" and only use those everywhere. This would ensure changes are consistent, and that we cover everything.

This no longer special cases the markup for declarations
that have no docstring. Before it would be:

    div (div.spec div)   # With docstring
    div.spec             # Without docstring

With this patch this becomes:

   div (div.spec div)    # With docstring
   div (div.spec)        # Without docstring

The regularity simplifies stylesheets development.
`div (div.spec div)` becomes `div (div.spec div.spec-doc)`.
Unattached trees are classified by `.odoc-unattached`
Include divs is classified by `.odoc-include`
Declarations are classified by `.odoc-spec`
@dbuenzli dbuenzli force-pushed the classify-page-content branch from 882686c to 1ca3e98 Compare February 25, 2021 17:16
@jonludlam
Copy link
Member

Format has failed - could you dune build @fmt && dune promote please?

@dbuenzli
Copy link
Contributor Author

Would you rather consider merging #600 that I just rebased @Drup vouched both of them and it has these commits with a final format commit.

@dbuenzli dbuenzli closed this Feb 25, 2021
@jonludlam
Copy link
Member

that's fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants