New implementation of option reference/builddict generation.

Review Request #3303 — Created Jan. 6, 2016 and discarded

benjyw
pants
pants-reviews
lahosken, zundel

- New implementation is in the docgen backend, not the internal
backend, because repos might want to generate their own reference
and builddict, that knows about their custom plugins.

- However our internal sitegen is modified to use this new docgen too,
of course. To support this, two versions are generated: a standalone
page (by default), and version suitable for embedding in the
pantsbuild.github.io docs.

- The new pages have some minimal javascript, so they can expand/collapse
sections of the doc.

- We only generate HTML pages. The old builddict generator also generated
RST docs, but we didn't use them anywhere in practice.

- Future changes will tweak docstrings etc. so that the generated documentation makes sense.

CI passes: https://travis-ci.org/pantsbuild/pants/builds/100655484

Manually generated docs. See embedded versions here:

https://benjyw.github.io/pants/doc/docsite/build_dictionary.html
https://benjyw.github.io/pants/doc/docsite/reference.html

See standalone versions here:

https://benjyw.github.io/pants/doc/reference/build_dictionary.html
https://benjyw.github.io/pants/doc/reference/pants_reference.html

ZU
  1. First of all, thank you for taking this on. Besides cleaning up the reflect.py issues, I think the content pages are improved.

    I'm not all that fond of the default collapsed view in the BUILD dictionary (https://benjyw.github.io/pants/doc/docsite/build_dictionary.html) I'm not sure what this will do to the google search, but specifically what I don't like is that I can no longer use the browser search function (CMD-F) to search through the build dictionary.

    After looking over it a bit, I see there are links to expand/collapse all which I think make up for this. Can we do something to make those links more obvious? They look like plain text right now. Maybe we could use color, underlining, or just some extra characters around to make it obvious these are clickable entities:

    [ Expand All | Collapse All ]
    

    The Pants reference has a similar issue.

    Option Reference
    
    Expand All Collapse All
    > Global Options
    > Task Options
    > Subsystem Options
    

    It starts out collapsed at the top level and has just 3 headings. My personal preference is that that those 3 top headings always start expanded (or just not be collapsable) so that you can see the task names and subsystem names.

    1. Having everything in the build dictionary expanded by default makes the page unreadable, and the CMD-F problem can be solved by clicking "expand all" first.

      Having the top-level of the options reference expanded by default is a possibility, I actually started out that way, but I didn't like the way it looked in practice.

      I'm no web designer, but I can try and make those expand all/collapse all links more prominent.

    2. OK, was able to hack something that intercepts the cmd-F keystroke and expands all automatically, so that the find operation can see all content. Is that sufficient do you think?

    3. I just tried it, neat! works for me, I wonder if it will work for google search, but I'm ok with just publishing it and trying it out/

    4. We can finesse the search issue one way or another, I'm sure (sitemap? Something in the Search Console? I'll figure it out)

  2. build-support/bin/publish_docs.sh (Diff revision 1)
     
     

    Put your name after the TODO.

    bikeshed warning: If someone comes along and asks about this I'm not sure I'm on board. Are you sure that these relationships are generalizable and aren't specific to the way that Pants wants to build its website? (I don't feel that strongly about this)

    1. Well, the sitegen task is expecting to find the generated HTML files from reference and markdown in a known location in dist/. It will fail if they aren't there. We already have a mechanism for ensuring that tasks have inputs they need, namely products. It seems obvious to me that this should use them.

      The relationships don't need to be generalizable at all. We just need reference and markdown to populate products for the targets they act on, and for sitegen to require those products. Seems like the way Pants should work, no?

    2. Agreed

  3. Can you provide some comments about why these are commented out (or does it just need to be removed?)

    1. Oh, that was while I was testing to see if those RST docs were used anywhere. I'll undo that do avoid confusion, and a future commit will remove builddictionary.py entirely.

  4. nit: Date (if this is this new code)

  5. Can you expand the description a bit? Specifically that this generates 2 specific pages (BUILD dictionary and the options reference)

  6. I see you changed from 'options_reference' to 'reference' in several places. 'reference' seems to be a bit too vague to me. I still like 'options_reference' because all of this information is about how to tweak options to pants (even though it isn't necessarily command line options)

    1. True, but this also enumerates all the available goals, so it's more than just the options reference. But we can elide that formality if you prefer, and just call this options_reference.

  7. As per my comments in the summary, I was thinking we could remove 'class="collapsed"' from these list elements.

  8. I really like this high level summary of goals, tasks, subsystems, .... Is it all new?

    1. It is all new. Glad you like it.

  9. s/E.g./This is how you invoke the compile goal:/

  10. Mention that subsystem names are prefixed onto tasks? Setting the jvm subsystem options specific to the test.junit task is written as:

    jvm.test.junit
    
    1. I do mention that below, when I explain scopes. Seems a bit arbitrary to mention that here, before even explaining what scopes are.

  11. shouldn't it be scope bar.foo ? Anyway, its opposite of the way you declare scope in pants.ini

    1. Ooops, yes, that was a mistake. Good catch.

  12. (This is the text I was expecting to read in Subsystems as I pointed out above)

  13. just curoius why this is a mustache template. I don't see any substitutions, looks like just a static .js file.

    1. It doesn't substitute, but it is itself used as a substitute:

      <script type="text/javascript">
          {{> reference/reference.js}}
      </script>
      

      And the mustache implementation resolves that to reference/reference.js.mustache.

  14. src/python/pants/docs/docsite.css (Diff revision 1)
     
     

    TIL what a 'rem' is. I wonder if this was intentional or was suppsed to be 'em'?

    1. I actually didn't even notice that and have no idea what a rem is... I just know that good CSS style requires not mentioning units when specifying zero, for some reason.

    2. rem: Its like em, but doesn't inherit from the parent, just the root font size.

  15. src/python/pants/docs/docsite.json (Diff revision 1)
     
     

    Do we need to change this path? Its going to make google search and other links 404. (maybe manually add a redirect?)

    1. These paths are just where sitegen looks for content to pull into the site. As far as I can tell they don't affect the external URLs. Those are determined by the tree structure, which hasn't changed in this case.

    2. OK

  16. src/python/pants/docs/docsite.json (Diff revision 1)
     
     

    Same comment. changing this path will make google and other outside links 404.

    1. This does break a link because I renamed options_reference to reference, but if I change that back (even just in this file, regardless of the names of templates etc.) then the existing links should be fine.

    2. Ok, could you just change the name 'reference' back to 'options_reference' to preserve the URL?

    3. Yep, done.

  17. 
      
BE
LA
  1. I had a tougher-than-necessary time matching build symbol names to descriptions. Something in the .css that might help:
    
    .build-symbol-args td { vertical-align: top; top-border: 1px dotted white; } 
    Top alignment so name and description line up, a border so eye can follow a line from a "skinny" name to description that goes with it. ...but some other solution would probably be lovely too.
    
    
    Another tougher-than-necessary thing in lists of goal options: when an option name is long enough such that it (and perhaps its repeats) go to multiple lines, when I try to quickly scan down the list, my brain "sees" the bold blue name on the second line instead of the name on the first line--maybe because it's "indented" by the -- ? If those <li>s had a style, things that might help: give them a border-top. (Or maybe a first-line outdent?)
  2. s/I.e./E.g./ now that I'm looking at it.
  3. If you meant to zap all the newlines, here's one that remains. If you didn't, then never mind.
  4. If this had a class=something, that would make it easier to wrestle with the Larry-can't-scan-the-list whine. "single-option", maybe?
  5. 
      
BE
Review request changed

Status: Discarded

Change Summary:

Diffs got messed up. Replaced by https://rbcommons.com/s/twitter/r/3315/.

Loading...