expose Phase CLI option functions

Review Request #28 — Created Feb. 27, 2014 and discarded

lahosken
commons
pants-reviews
It would be lovely if our docs had all Pants' CLI options. (if you look quick, you might see a sneak preview at the generated-from-a-branch http://pantsbuild.github.io/goals_reference.html)

Towards making that happen, I want to expose some CLI-option functionality:

+ a way to get the optparse.OptionParser that a Phase would see.

+ a way to associate each OptionGroup within that OptionParser with a goal (Phase-part)

This here change gets the job done. Might be an awkward way to do it, though. If you point me at a way towards an elegant way, I wouldn't mind.


Loading file attachments...

  • 4
  • 0
  • 0
  • 0
  • 4
Description From Last Updated
Does this need to move? I *think* this could stay as-is. TR travis
I was confused about flow control here, and looked this up. Per http://docs.python.org/2/library/optparse.html#generating-help we see "If the help output is ... TR travis
Will the mystery of this change be revealed when seeing the builddict stuff? I don't see what this changes at ... TR travis
Fix indent. TR travis
LA
LA
  1. (Fun workflow fact: attaching a file to a review doesn't send out mail. I haven't looked at this review in a few days because I didn't think there was any feedback. D'oh.)
  2. 
      
LA
LA
  1. A screenshot attachment asks:
    > What do you think about starting with just the options provided by the task?
    
    Nervous. You can imagine an alternate universe in which Pants has a "publish" goal that depends on several other goals, e.g., "doc". Folks who want to ignore javadoc errors when publishing might be in the habit of running './pants goal publish --javadoc-ignore-errors'.
    
    That said, our choice of options-to-start-with doesn't affect the change in this here ReviewBoard. This here RB does some refactoring to expose an interface to ease generating that thing. Or to ease generating a variation on that thing that only shows options from those tasks. Or whatever.
  2. 
      
TR
  1. Overall this seems fine, and the global options are definitely easier to read. However, without seeing the builddict parts too I don't really understand the need for moving some of this around.
    
    Can you either clue me into what's coming, or do you want to back out parts not necessary as part of the global options refactor?
    1. Since I don't understand the phrase "back out the parts not necessary as part of the global options refactor", I'll post a gist:
      
      https://gist.github.com/lahosken/9359503#file-builddictionary-py-L273
      
      The relevant pseudo-code
      
        for phase, raw_goals in Phase.all():
          parser = phase.setup_parser_for_help() # get CLI option info via newly-exposed interface
                                                 # parser is an OptionParser, a collection of OptionGroups
                                                 # Roughly one OptionGroup for each task (and depended-on task)
          for goal in raw_goals:
            options_title = goal.title_for_option_group(phase) # get title via newly-exposed interface
            # use options_title to identify this goal's option_group out of parser's collection
      
      
  2. Does this need to move? I *think* this could stay as-is.
    1. If this means: instead of exposing this interface through Phase, let's expose it via a new commands.Goal method, I think it's a lovely idea. My puny attempts to import commands.Goal failed, so I gave up. But I can try harder.
      
      If this means something else, please clarify.
  3. I was confused about flow control here, and looked this up.
    
    Per http://docs.python.org/2/library/optparse.html#generating-help we see "If the help output is triggered by a help option, optparse exits after printing the help text." which is why pants exits after this call.
    
    What do you think about replacing with:
    
        parser.print_help()
    
    Functionally its the same - the message is printed and pants exits, but this is because pants has nothing else to do rather than a buried exit statement.
  4. Will the mystery of this change be revealed when seeing the builddict stuff? I don't see what this changes at present.
    1. This is part of the gyrations to expose the new title_for_option_group interface.
      
      There might be a more elegant way to expose that interface.
      
      Or... instead of using the title as an identifier, I could do the Pythonic-object thing of attaching some other field to the objects. That would have fewer weird functions floating around, at the cost of a weird field floating around.
  5. 
      
LA
LA
  1. Pardon me as I salvage some notes I'd scribbled onto Post-its:
    
    Travis and I talked about this.
    
    Prrrobably it makes sense for setup_for_help to not be a Phase method; instead it should be a function that takes a Phase arg.
    
    Mmmaybe that function lives in the same file as the global_options stuff which is then a little pile of functions that munge optparse-related thingies.
    1. ...and instead of showing all of the options that "dependent" phases give you, try just showing the section-links. E.g. if 'publish' depends on 'doc' , don't list all of doc-javadoc-this doc-javadoc-that. Just show 'doc:javadoc' as a link to that section.
  2. 
      
LA
LA
  1. Pushed an ephemeral generated-on-a-branch http://pantsbuild.github.io/goals_reference.html that shows command-line flags. Purty.
  2. 
      
LA
  1. Discarding this old-repo reviewboard in favor of new-repo reviewboard: https://rbcommons.com/s/twitter/r/181/
  2. 
      
LA
Review request changed

Status: Discarded

Loading...