On Fri, Oct 12, 2007 at 11:13:14PM -0500, Dan Zwell wrote:

quoted text
> Recently there was some talk of color for git-add--interactive, but the 
> person who said he already had a patch didn't produce it.

Neat, thanks for working on this.

quoted text
> There is one problem--a block is commented out, because adding the "--color" 
> option to git-diff-files somehow breaks git-add--interactive, and I would 

I believe it's because add--interactive parses the output of
git-diff-files, so it expects unadorned diffs. I think you may be stuck
re-coloring the diffs yourself, which is a little ugly.

quoted text
> tabs. Feel free to replace the colors that I chose with something that 
> better conforms to the "git style", if there is such a thing.

Two suggestions:
  - every color should be configurable (e.g., see diff color options)
  - where possible, use existing color config (e.g., for diffs)

You will never come up with a color scheme that satisfies everyone
(e.g., white text on black background versus black text on white
background), so configurability is a good idea (not to mention that
nobody will ever agree on what looks "good").

quoted text
> +if ($color_config=~"true" || -t STDOUT && $color_config=~"auto") {

Shouldn't these just be 'eq' instead of a regex?

quoted text
> +			print_ansi_color "bold";
>  			print "$opts->{HEADER}\n";
> +			print_ansi_color "clear";

ISTR some terminals had issues with leaving ANSI attributes set across a
newline. That was the reason for the color_fprintf_ln business in
color.[ch]. You might replace this with something like:

  print_color_ln 'bold', $opts->{HEADER};

where "print_color_ln" turns off the attribute before the newline.

quoted text
> +	# FIXME: the following breaks git, and I'm not sure why. When
> +	# the following is uncommented, git no longer asks whether we
> +	# want to add given hunks.
> +	#my @diff;
> +	#if ($use_color) {
> +	#    #@diff = run_cmd_pipe(qw(git diff-files --color -p --), $path);
> +	#}
> +	#else {
> +	#    #@diff = run_cmd_pipe(qw(git diff-files -p --), $path);
> +	#}
>  	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);

See how we are pulling the diff into lines? Look a few lines below and
you will see that we start parsing without regard to the color.

Unfortunately, that parsed form ends up being output to the user, so we
will have to do colorization at that point (fortunately, diff
colorization with regexes isn't _that_ hard).

quoted text
> +	print_ansi_color "blue";
>  	print <<\EOF ;
>  y - stage this hunk
>  n - do not stage this hunk
> @@ -555,6 +582,7 @@ k - leave this hunk undecided, see previous undecided 
> hunk
>  K - leave this hunk undecided, see previous hunk
>  s - split the current hunk into smaller hunks
>  EOF
> +	print_ansi_color "clear";

Hrm, splitting this with print_color_ln as I mentioned above would be a
little painful. Maybe something like this (totally untested):

# Turn on ansi attributes at the beginning of the string and at
# the beginning of each line, but then turn them off before each
# newline. This should give the effect of covering the whole string
# with the attribute, but not have attributes cross newline boundaries.
sub color_print {
  my $attr = shift;
  local $_ = shift;
  if ($use_color) {
    s/^/Term::ANSIColor::color($attr)/mge;
    s/\n/Term::ANSIColor::color('reset') . $&/ge;
  }
  print $_;
}

-Peff
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html