Re: [PATCH 5/5] Added diff hunk coloring to git-add--interactive

Jeff King [off-list ref] writes:

On Thu, Nov 22, 2007 at 04:56:24AM -0600, Dan Zwell wrote:

quoted

-		else { # set up colors
-			# Grab the 3 main colors in git color string format, with sane
-			# (visible) defaults:
-			$prompt_color = Git::color_to_ansi_code(
-				scalar $repo->config_default('color.interactive.prompt',
-					'bold blue'));

These were just added in the last patch. I know sometimes it is worth
showing the progression of work as the patches go, but in this case, I
think it is simpler for the reviewers if the first patch which adds a
chunk of code does it in the final way (even if you need to just say "I
did it this way because there will be reasons later on.").

If you are suggesting to reorganize the series like this:

 1/5 Fix to Git.pm for list context;

 2/5 Enhance Git.pm to allow config() methods to take default values;

 3/5 Enhance Git.pm with get_color() method;

 4/5 Teach git-add--interactive to read color settings from the
     config;

 5/5 Paint output from git-add--interactive in colors, including
     prompt, help and diff hunks.

I think that makes a very good sense.  The earlier part of the
series would be independent from colorization of "add -i" and
can go in before everything else to allow other potential users,
e.g. "git remote --color" ;-).  I do not see a strong reason to
have the separate "Basic color support with hardcoded color" at
the beginning, either.

I think it is a matter of taste to either:

 (1) Squash 4 and 5 in the above list into one; or

 (2) Split 5 into separate commits to color different parts.

Perhaps the former would be simpler and more appropriate for
this series.