Skip to content
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

Better docs #733

Merged
merged 29 commits into from
Oct 1, 2021
Merged

Better docs #733

merged 29 commits into from
Oct 1, 2021

Conversation

jonludlam
Copy link
Member

@jonludlam jonludlam force-pushed the better-docs branch 2 times, most recently from ef0e1b5 to 59e7f70 Compare September 23, 2021 08:21
@asavahista
Copy link
Contributor

asavahista commented Sep 23, 2021

note: edits from now on will contain version (currently: (e2)) to help identify changes
also note: all .mld file names will always be named with the file extension for consistency/ease of searching, even when talking about the compiled .html

i wonder if the navigation links at the top of the main section would be helpful on the sidebar as well? or at least a link for home/top and/or up i guess

also a sitemap - or links to more (ideally almost all) pages at the intro page - it's currently less than one page anyway so i don't think it would hurt (i feel like this would be useful as some links are only (?) discoverable through links named e.g. "separate page")

anyway, i've found a few potential (syntax) issues in my first pass:

general issues

(probably issues with odoc and therefore not exactly in scope for this PR):

  • (e2) (this one is in scope) i think it would be slightly better namespacing-wise to move all documentation pages to a subdirectory - this would also prevent most naming conflicts but the main upside of this is the better separation
  • (e2) (nitpick) (low priority) the occasional long line (example). there's probably a tool to find these; you can probably hack one together in a shell easily enough
  • some codeblocks (example) are being detected as ReasonML for some reason - which is making the comments not parse properly. if the docs no longer contain Reason i think it can be removed - the other fix would be adding the css class lang-ocaml to every code block that's highlighted. if it isn't already, you could also add lang-plaintext to all codeblocks that aren't supposed to be highlighted
  • to put it bluntly, particularly terrible contrast (example) - this is a css problem. the blue has contrast ratio 1.74:1; the red has contrast ratio 1.79:1, both of which fail all recommended web contrast guidelines
  • (e2) page titles should probably use the name in the {0 title} title - stripped of formatting of course
    • the same should probably apply to titles in the navigation line at the top of the contents section

specific files

  • contributing.mld
  • driver.mld
  • ocamldoc_differences.mld
    • (nitpick) mld instead of .mld: 1 2
  • features.mld
  • parent_child_sec.mld
  • odoc_for_authors.mld
    • (e2) (semantic) regarding paragraph headings it might be nice to say that if an author wants to use level 4 headings as subsubsubsections, they should consider splitting into multiple files instead
    • (e2) (nitpick) "TOC" should probably be the full name "table of contents" just for consistency, and just in case the reader isn't familiar with the acronym
    • (e2) (semantic) i feel like using_dune.mld is short enough (and relevant enough) to move here. if it takes too much space it can be moved to the end - i think the important part is that it is easy to find (e.g. in the table of contents). it could be named "getting started with/using dune" or something
      • this can be ignored if dune_wrapping.mld is merged into using_dune.mld
    • (e2) might want to link to ocamldoc_differences.mld in the section linking to ocamldoc's syntax. ocamldoc_differences.mld is already linked from odoc.mld, but i think it should be here as well
  • using_dune.mld
    • (e2) (semantic) it should probably have a link to dune_wrapping.mld, if not include it completely
  • dune_wrapping.mld
    • (e2) (semantic) if it's not merged into using_dune.mld, maybe it could be namespaced (?) under it instead? e.g. using_dune/dune_wrapping.mld - the main difference being that using_dune.mld would probably appear in the top navigation line as a parent of this page

@asavahista
Copy link
Contributor

asavahista commented Sep 23, 2021

i haven't checked everything yet - but i'm thinking of editing the post above (with a changelog) rather than making a new post when (well... if) i do, if that's fine of course

for anyone else interested in reading over this - it will probably be useful looking at the directory listing of the built site - since a lot of the .mlds in doc/ are dependencies only

@jonludlam
Copy link
Member Author

@asavahista This is excellent, thank you very much!

@asavahista
Copy link
Contributor

side note: i believe this would close #573 (or otherwise partially invalidate it)?
if it doesn't completely close it i still believe a new issue with a checklist could/should be made (or even better, a project)

@asavahista
Copy link
Contributor

it would probably help if gh-pages was rebuilt, there have been a considerable number of commits since last build

@jonludlam
Copy link
Member Author

I've done a small update following @asavahista's suggestions - more are to come though. Updated http://jonludlam.github.io/odoc with the current state.

General issues

  • clarification required: in "move all documentation pages to a subdirectory" comment, does this mean source mld files or output html files?

  • codeblocks detected as ReasonML: To a large extent this is simply a problem with odoc syntax, there's no way (currently) to specify the language. This is being addressed (Parse code blocks metadata ocaml-doc/odoc-parser#2) but won't be fixed for odoc 2.0. In general I'm happy for the odoc website to be 'warts and all' for now so I don't want to post-process the output.

  • The suggestions about the dune pages bring up an interesting point: we could of course have more of a hierarchy with /dune/index.html dune/wrapping.html and so on, but doing so would complicate the driver marginally, and as the driver is intended to be a very simple script that explains how to use odoc I'm wary of adding too much to it. I think for now we'll keep the simple structure and just merge the two dune-specific pages together.

Specific pages:

  • contributing.mld
    • fixed all identified issues
  • driver.mld
    • docs.ocaml.org links replaced with v3.ocaml.org (and in parent_child_spec.mld)
    • ... clarified
    • HTML comments - these are mdx-specific, don't need them in the mld file.
  • ocamldoc_differences.mld
    • mld replaced with .mld
  • features.mld
    • fixed capitalisation
    • fixed Paths_types rendering
    • separate page link renamed
    • wide comment shortened
  • parent_child_spec.mld
    • mld -> .mld, [odoc] to [.odoc] and [odocl] -> [.odocl] (where appropriate) - also changed indefinite articles! (an [mld] file -> a [.mld] file :-) )
    • replaced {[ ... ]} with {v ... v} for non-OCaml code blocks
    • section names capitalised
    • \( replaced with (
  • odoc_for_authors.mld
    • Suggestion to use multiple files rather than 4-level headings inserted
    • TOC acronym expanded
    • added link to ocamldoc_differences.mld
    • simple examples added to drivers part at the top
  • using_dune.mld and dune_wrapping.mld
    • merged

@asavahista
Copy link
Contributor

clarification: the source mld files - generally nobody will need to browse the build output
re: merging the dune files together - i think that's fine

side note: ci is producing some weird failures, not sure if that's a concern

@asavahista
Copy link
Contributor

ok so... i don't think i'll be able to comment on the actual content (e.g. whether it's sufficient/comprehensive enough) before i actually need to use it

(apologies in advance if this post is a bit messy. as with the previous post i tend to jump around when making the list)

(of course, feel free to ignore any of the nitpicks in the following list, they are mostly nitpicks after all!)

some offtopic stuff one thing i've been thinking about re: contributing - i feel like a code tour (note: currently vs code only) style guide to the codebase would be pretty sweet (and very, very helpful for figuring out where to start debugging an issue/implementing a feature request). it would be quite difficult to get right, but potentially worth the effort.
  • general
    • low priority but just mentioning again that adding links to most/all pages from the index page may be nice
    • Dune, Esy, Opam, Odig, Voodoo, jq, tidy - not an issue. just noting that formatting of cli tool names is a bit inconsistent (vs odoc, ocamldoc, diff etc.)
  • contributing.mld
    • other paths should probably be code spans too: 1 2 3
      • 4? (library name formatting should probably be consistent with the default formatting, which seems to be code spans)
    • dune doesn't capitalize "cram" (in "cram tests") so the docs probably shouldn't either. of course this isn't a big deal - to be fair dune/jane street itself seems pretty inconsistent with the capitalization of these. i guess it's mostly just personal preference on my part, since names of CLI programs are normally in lowercase
      • similar with Expect-tests
    • another possible capitalization issue? (linking to header without alt text like last time
    • developer mode -> "developer tools"
    • (semantic) might want to mention that the shortcut to open this is ctrl + shift + i on most (all?) browsers
    • more accidental markdown formatting? (_don't_)
    • diff vs diff vs diff. either way is fine tbh
    • odoc -> odoc
    • section link Testing -> testing
    • (semantic) the wording is a bit odd in some places:
      • "the string that the code emitted" -> "the output of the code", or more simply just "the output"?
      • "human developer" - similarly 🤔, i think "developer" means basically the same thing. especially given that this is contributing.mld
      • "tested by our CI tests" - just "CI" instead of "CI tests" should be fine. but also it's the line right before the section on CI so maybe it's unnecessary. even if it's not completely removed i think it can be changed to, say, "we run coverage analysis in our CI pipeline (see below for more details)"
  • driver.mld
    • more file extensions missing formatting? 1 2 3 4 - might be worth doing a grep -r mld (etc.) for those?
    • <!-- $MDX skip --> still there
    • Odoc, odoc not odoc (also voodoo). also mdx -> MDX? and html -> HTML in that region
    • path/to/x.html -> path/to/x.html
    • quite a number of code blocks have scrollbars for me. not really an issue, considering a lot of them are also very tall, just a heads up. can't (or shouldn't) really do anything about the shell ones, of course
  • odoc_for_authors.mld - (** - probably an unintended space. seems like the space after it being missing is a HTML quirk
    • Preamble -> preamble
    • (semantic) more odd wording (?):
      • "structurally placed in the HTML" - specifically, the word "structurally". "[placed/located] immediately after the preamble in the HTML" is an option, removing just the "structurally" is fine too

Also format the code to reduce the number of overflows
@jonludlam
Copy link
Member Author

I believe I have made most of the updates suggested - many thanks to everyone who helped with this! I'll merge this now so we have something we can release, and we can continue updating and improving it in further PRs.

@jonludlam jonludlam merged commit 37196b0 into ocaml:master Oct 1, 2021
jonludlam added a commit to jonludlam/opam-repository that referenced this pull request Oct 5, 2021
CHANGES:

Breaking changes
- Remove odoc-parser into a separate repository (@jonludlam, ocaml/odoc#700)

Additions
- OCaml 4.13 support (@Octachron, ocaml/odoc#687, ocaml/odoc#689)
- Better errors/warnings (@Julow, ocaml/odoc#692, ocaml/odoc#717, ocaml/odoc#720, ocaml/odoc#732)
- ModuleType 'Alias' support (@jonludlam, ocaml/odoc#703)
- Improved test suite (@lubega-simon, ocaml/odoc#697)
- Improved documentation (@lubega-simon, @jonludlam, ocaml/odoc#702, ocaml/odoc#733)
- Strengthen module types (@jonludlam, ocaml/odoc#731)

Bugs fixed
- `uwt` now can be documented (@jonludlam, ocaml/odoc#708)
- Fix resolution involving deeply nested substitutions (@jonludlam, ocaml/odoc#727)
- Fix off-by-one error in error reporting (@asavahista, ocaml/odoc#736)
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