Skip to content
This repository has been archived by the owner on Mar 29, 2024. It is now read-only.

fixed 'Usage' to be on one line instead of two #132

Merged
merged 1 commit into from Oct 30, 2014

Conversation

twobraids
Copy link
Contributor

a comma was left out of the code that produces the Help usage line. That caused it to print on two lines instead of just one. This PR adds the missing comma.

also in the same line, when a command line argument has been given a value AND the user has listed --help, we ought show the item that command line argument as an optional value. In the context of the line provided, it is not optional, it is required.

This is the case when a program is a launcher and the non-switch argument is a class that represents the app to run. Git itself is a fine example:

$ git commit --help

the usage line in help should list the word "commit':

    git commit  ...

earlier configman didn't get this right and would instead show:

git [ command ]

@@ -45,6 +45,8 @@
import functools
import warnings

from inspect import isclass, ismodule
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would favor just import inspect. It seems to be the pattern we have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha ha, just noticed that inspect is already imported on line 42

@peterbe
Copy link
Contributor

peterbe commented Oct 29, 2014

So what's the best way to manually test this?
I haven't started looking at the code yet (despite noticing an import style thing).

@twobraids
Copy link
Contributor Author

@peterbe log into staging processor 1 and issue these commands:

export PYTHONPATH=/home/lars/socorro:/home/lars/configman:/data/socorro/socorro-virtualenv/lib/python2.6/site-packages
export PATH=$PATH:/home/lars/socorro/scripts
socorro --help | grep lars
socorro fetch --help | grep lars

and compare the lines that make it through grep

@peterbe
Copy link
Contributor

peterbe commented Oct 29, 2014

Does staging processor 1 have this patch included in it? Is there no way I can verify it locally?

@twobraids
Copy link
Contributor Author

no, staging processor does not have this patch, except for my own local copy in my own directory. Notice that the PYTHONPATH is set to pick up configman (and socorro) from my own local copy.

If you'd rather not use stage, get this version of configman loaded in an independent location on your socorro virtual machine. Set up your PYTHONPATH to look at that configman and your regular installation of socorro. Invoke the same commands that I did above.

Alternatively, try this is bash:

cat > t1.py << MARKER
from configman import Namespace, RequiredConfig, ConfigurationManager, class_converter

class Alpha(RequiredConfig):
    required_config = Namespace()
    required_config.add_option(
        'foo',
        doc='the foo',
        default=38,
        is_argument=True,
    )

required_config = Namespace()
required_config.add_option(
    'command',
    doc='the command (class) to load',
    default=None,
    from_string_converter=class_converter,
    is_argument=True,
)
cm = ConfigurationManager(
    required_config,
)
MARKER
export PYTHONPATH=$HOME/work/configman
python t1.py --help | grep t1.py
python t1.py Alpha --help | grep t1.py

the output should be something like:

t1.py [OPTIONS]...  command
t1.py [OPTIONS]...  Alpha [ foo ] 

@peterbe
Copy link
Contributor

peterbe commented Oct 29, 2014

And it works!

  --foo
    the foo
    (default: 38)

#--------------------------------------------------------------------------
def test_output_summary_with_argument_2(self):
"""test_output_summary: the output from help where one item is a
non-switch argument with no default value"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This docstring is the same as the test_output_summary_with_argument_1 one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that out because I couldn't see what this test does differently.

@peterbe
Copy link
Contributor

peterbe commented Oct 29, 2014

r+ with nits.

twobraids added a commit that referenced this pull request Oct 30, 2014
fixed 'Usage' to be on one line instead of two
@twobraids twobraids merged commit 9c4fd2a into mozilla:master Oct 30, 2014
@twobraids twobraids deleted the better_usage branch February 25, 2022 19:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants