From: Mark Lodato Date: Sun, 14 Feb 2010 14:58:58 GMT Subject: Re: [PATCH] Add an optional argument for --color options Message-ID: In-Reply-To: <20100214064408.GB20630@coredump.intra.peff.net> On Sun, Feb 14, 2010 at 1:44 AM, Jeff King 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