-
-
-
-
-
-
This isn't needed if you make RB_COMMANDS a dictionary. You can just do: if RB_MAIN + args[0] in RB_COMMANDS:
-
-
Should avoid map where possible. These are hard to read. Do: _indent = max([len(key) for key in RB_COMMANDS]) - len(RB_MAIN)
-
This would be nicer as: _COMMANDS_LIST_STR = '\n'.join([ " %s-*s %s" % (_indent, cmd(len(RB_MAIN):], desc) for cmd, desc in RB_COMMANDS.iteritems() ])
-
I don't understand exactly what this is for. Can you add some docstrings on what it's for exactly? I feel like right now, this should probably just go in rb.py, until we have a good reason to give it its own file. There's no harm in other commands importing from rb.py if needed.
-
I'm not sure we want to split, do we? If for some reason there's "rb" elsewhere, this will break. I think instead we want to do: if cmd.startswith(RB_MAIN): name = cmd[len(RB_MAIN):]
-
Shouldn't split the fmt into its own string. Instead, just wrap if we need to. Also, instead of %s and passing name twice, use "%(name)s" and then have the format arguments be: % { 'name': name }
Rework 'rb' command, add rb-* commands description
Review Request #2549 — Created Aug. 15, 2011 and submitted
Options in 'rb' are parsed by 'optparse' module, added well-formatted help with sub-commands annotations.
Description | From | Last Updated |
---|---|---|
With the above, this would be "%(name)s" |
chipx86 | |
No blank line here. optparse is part of Python. |
chipx86 | |
"Available" |
chipx86 | |
No need for parens around the format arguments when there's just one value. |
chipx86 | |
This isn't needed if you make RB_COMMANDS a dictionary. You can just do: if RB_MAIN + args[0] in RB_COMMANDS: |
chipx86 | |
No need for parens. Should use { "name": args[0] } |
chipx86 | |
Should avoid map where possible. These are hard to read. Do: _indent = max([len(key) for key in RB_COMMANDS]) - len(RB_MAIN) |
chipx86 | |
This would be nicer as: _COMMANDS_LIST_STR = '\n'.join([ " %s-*s %s" % (_indent, cmd(len(RB_MAIN):], desc) for cmd, desc in RB_COMMANDS.iteritems() … |
chipx86 | |
I don't understand exactly what this is for. Can you add some docstrings on what it's for exactly? I feel … |
chipx86 | |
I'm not sure we want to split, do we? If for some reason there's "rb" elsewhere, this will break. I … |
chipx86 | |
Shouldn't split the fmt into its own string. Instead, just wrap if we need to. Also, instead of %s and … |
chipx86 | |
Description should be on the same line. It can then wrap. (Make sure the description aligns on each line). |
chipx86 | |
Still needs to be documented to explain why it's needed and how it differs from IndentedHelpFormatter. |
chipx86 | |
Do we mean to override _COMMANDS_LIST_STR, or append? Why is this down here instead of up above? It seems an … |
chipx86 |
-
-
Description should be on the same line. It can then wrap. (Make sure the description aligns on each line).
-
Still needs to be documented to explain why it's needed and how it differs from IndentedHelpFormatter.
-
Do we mean to override _COMMANDS_LIST_STR, or append? Why is this down here instead of up above? It seems an odd place for it.