Re: [PATCH 1/2] Added basic color support to git add --interactive

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2007, 05:36 UTC
Message-ID: <7v640ivagv.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071104045735.GA12359@segfault.peff.net>

Jeff King <peff@peff.net> writes:

Show 27 quoted lines

> On Fri, Nov 02, 2007 at 10:41:00PM -0500, Dan Zwell wrote:
>
>> +sub print_colored {
>> +	my $color = shift;
>> +	my $string = join("", @_);
>> +
>> +	if ($use_color) {
>> +		# Put a color code at the beginning of each line, a reset at the end
>> +		# color after newlines that are not at the end of the string
>> +		$string =~ s/(\n+)(.)/$1$color$2/g;
>> +		# reset before newlines
>> +		$string =~ s/(\n+)/$normal_color$1/g;
>> +		# codes at beginning and end (if necessary):
>> +		$string =~ s/^/$color/;
>> +		$string =~ s/$/$normal_color/ unless $string =~ /\n$/;
>> +	}
>> +	print $string;
>> +}
>
> This would probably be a bit more readable by marking the regex as
> multline using /m. Something like:
>
>   $string =~ s/^/$color/mg;
>   $string =~ s/.$/$&$normal_color/mg;
>
> which covers both the "start/end of line" and "start/end" of string
> cases.

I think you would end up spitting out:

        COLOR something RESET LF COLOR RESET LF

instead of:

	COLOR something RESET LF LF

when you get "something\n\n" if you did that. Not a big deal, though, as at this point we would be human I/O bound.

> Also, if there is to be pager support for showing diffs, perhaps
> print_colored needs to take a filehandle argument (or, even simpler,
> change "print_colored(...)" to "print color(...), so the caller can use
> print as usual).

Making it take a FH would be useful. With that, my proof-of-concept patch to add print_diff_hunk would become:

	sub print_diff_hunk {
        	my ($text) = @_;
		my $pager;

                if ($use_pager) {
	                open($pager, "| less");
		} else {
                	$pager = \*STDOUT;
		}
                for (@$text) {
			print_colored $pager $color ...
		}
                close($pager);
	}