* The more I think about it, the more I like that you're basically constructing options to match a function signature. But I can imagine that this might be too restrictive for some uses. It might be worth having an alternate interface that provides an options object like optparse does.
It is matching options to a function signiture, and for complex methods, this might be too restrictive (for example, you'd get methods with 10 - 20 params). That said, many cases don't have this many things that can be set.
Although your point about providing an alternate interface is a good one...
* Any reason why you aren't using new-style classes?
Ignorance. I'll look into it...
* I think the code would be easier to navigate in a single module.
It might be easier to navigate (eg: when you want to go to a method implmentation), but from a maintenance point of view I feel modular code is easier... Mostly this is instint from other languages (mostly c++).
* Your code alternates between underscore_names and camelCaseNames. Might be better to stick with one or the other. (PEP 8 suggests the former.)
Yeah, I coded it before I looked at PEP8. Initially it was all camel case, and I attempted to retrofit it. It seems like classes should still be CamelCase, yes?
* File specific comments:
Argument.py:
Drop the get_name method; name attribute is already accessible. (The property builtin makes getters and setters generally unnecessary.)
CommandArgument.py:
Drop the get_param_name method; param_name attribute is already accessible
Flag.py:
__init__ should have defaults where applicable (e.g. the comments say you can provide None for short_flag, but it doesn't default to this). Should also probably produce an error if neither short_flag nor long_flag is set. (Or do this in Command._add_options)
In get_value, use "self.argument is None" not "self.argument == None".
get_flag_name should default to long_flag, not short_flag
Command.py:
add_flag should call _validate_param_name *before* _add_options (in case an exception is raised and caught).
In _get_params, for arg_index in range(len(method_args)): arg = method_args[arg_index] could be replaced with for arg_index, arg in enumerate(method_args):
Will do...
MulipleLines.py:
Can you use the (standard library) textwrap module instead? Seems like they do about the same thing, but I haven't looked in too much detail.
Doh! Yep, they do the same thing. This is another case of my not checking the standard library well enough.
Hope these comments are at least marginally useful. ;)
Yes, very useful. Thanks very much for spending the time to go over it...
-- http://mail.python.org/mailman/listinfo/python-list
