From: Jeff King <peff@peff.net>
Date: Sun, 04 Nov 2007 04:57:35 GMT
Subject: Re: [PATCH 1/2] Added basic color support to git add --interactive
Message-ID: <20071104045735.GA12359@segfault.peff.net>
In-Reply-To: <20071102224100.71665182@paradox.zwell.net>

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.

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).

-Peff