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

From: Mark Lodato <lodatom@gmail.com>
Date: Feb 14, 2010, 14:58 UTC
Message-ID: <ca433831002140658r30aa539fy5480cae8298d6d6c@mail.gmail.com>
In-Reply-To: <20100214064408.GB20630@coredump.intra.peff.net>

On Sun, Feb 14, 2010 at 1:44 AM, Jeff King <peff@peff.net> wrote:

Show 11 quoted lines

> 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