Jeff King <peff@peff.net> writes:

quoted text
> 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+)(.)/$color/g;
>> +		# reset before newlines
>> +		$string =~ s/(\n+)/$normal_color/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.

quoted text
> 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);
	}
-
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