Adding support for multiline param help descriptions in Pants BUILD Dictionary

Review Request #3399 — Created Jan. 30, 2016 and submitted

caveness
pants
2848
pants-reviews
benjyw, dturner-tw, zundel

In response to a user reported error that target parameter help appeared incomplete, added support for parameter help strings that span multiple lines.

Prior to this change, get_arg_descriptions_from_docstring did not suppport multiline descriptions, as was noted in the comments. However, there are numerous instances in which existing parameter help spans multiple lines, and there's nothing enforcing the single-line description convention. In addition, especially for parameters that have longer names, it may be difficult to include meaningful descriptions that span only a single line. Thus, it seemed best to go ahead and add support for multiline descriptions.

Adding support for multiline descriptions also fixed the fact that no help was appearing in the Pants BUILD Dictionary for parameters where the help string included :param name: on one line and the description on the next line. This can be seen in the help for the provides parameter of contrib_plugin (screenshots attached).

As seen in the screenshots for android_binary below, adding support for mulitline descriptions altered the formatting of the tables containing those descriptions. In particular, the parameter names sometimes wrap, and the description text is aligned in the middle of the cell. If these changes go in, I'll open another github issue on that, given my very limited css skills.

https://travis-ci.org/pantsbuild/pants/builds/106130379

Loading file attachments...

  • 0
  • 0
  • 7
  • 0
  • 7
Description From Last Updated
DT
BE
  1. 
      
  2. Wouldn't it be simpler to check for any doc stanza (i.e., anything that matches :\w+) that isn't :param? Seems like overkill to test specifically for return, rtype and type, especially when there are other allowed stanzas.

  3. Spelling nit: s/subseqent/subsequent/

  4. You probably want to strip whitespace from the line, because there may be indentation ws in the docstrings. Please also exercise this case in the test.

  5. Spelling nit: s/subsquent/subsequent/

  6. 
      
LA
  1. Thank you for fixing this!
  2. Add a comment w/this regexp's purpose? The specific question that had me scratching my head: Is this supposed to catch :returns: and :raises: and that's why it's using wildcards?
    
    (Depending on what the reason is, it might make the code more self-documenting to use strings instead of wildcards... or might not, uh, depending...)
  3. 
      
CA
BE
  1. Thanka for the enhancements!

  2. 
      
ZU
  1. Ship It!
  2. 
      
ST
  1. Thanks!

  2. 
      
CA
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as bb939c4a774c290cc426b502ca98c62769df7242 and published to http://pantsbuild.github.io/

Loading...