Re: [PATCH] Add an optional argument for --color options

[Mark Lodato <lodatom@gmail.com>]

On Sun, Feb 14, 2010 at 1:44 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Feb 13, 2010 at 05:01:15PM -0500, Mark Lodato wrote:
>
>> If the argument is not valid for a diff-family program, a completely
>> unhelpful usage message is shown.  It seems that all the other diff
>> options silently ignore invalid inputs, so this is consistent.  Perhaps
>> this aspect should be tweaked.
>
> Hmm...the only one I see that silently ignores is "--submodule=bogus".
> But it seems that "git log -Bfoobar" fails but does not print a useful
> message.  Probably both should be fixed, and your option should follow
> the same convention as those.

Just wondering, why does diff use a separate option parsing mechanism
than the rest of the code?  Would it be worthwhile to switch to
parse_opt?  This may make the code cleaner, and it would definitely
make the command-line interface more consistent with the rest of the
suite.  From a user's point of view, the biggest win would be "-h"
printing all of the options, like all the non-diff commands do.

> My only complaint in that respect is that there are no tests.  However,
> I'm not sure we can get a very satisfying test, since the test scripts
> may or may not have stdout going to a tty.

Perhaps I can throw the tests in Jonathan's "tests for automatic use
of pager", t7006-pager?  Or, create a new test that mimics his?

Thanks for the feedback,
Mark