On Fri, 02 Nov 2007 22:06:08 -0700
Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Dan Zwell <dzwell@xxxxxxxxx> writes:
>
> How big is Term::ANSIColor, and how universally available is it?
> Implementing the ANSI "ESC [ %d m" arithmetic color.c in Perl
> ourselves does not feel too much effort, compared to the
> potential hassle of dealing with extra dependencies and
> potential drift between scripts and C implementation.
20K on my machine, and part of the core library since Perl 5.6.0.
This was released in 2000. With your addition (the eval check to make
sure the module is loaded), nobody should be harmed if they don't have a
modern perl, either. My vote would be to reimplement the coloring if we
actually notice these problems.
<snip>
>
> By the way, coloring the diff text itself may be just the matter
> of doing something like this (except that you now need to snarf
> OLD, NEW, METAINFO and FRAGINFO colors for diff configuration as
> well.
>
> In addition to a small matter of testing, a more practical issue
> would be to add PAGER support there, I think.
You mean in general, so that users can view a hunk in the PAGER, then
be prompted for what to do with it? (Because it doesn't solve the color
problems, because calling "diff --color" here creates other problems.)
>
> ---
>
> git-add--interactive.perl | 32 ++++++++++++++++++++++++--------
> 1 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 2bce5a1..1063a34 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -388,6 +388,27 @@ sub parse_diff {
> return @hunk;
> }
>
> +sub print_diff_hunk {
> + my ($text) = @_;
> + for (@$text) {
> + if (!$use_color) {
> + print;
> + next;
> + }
> + if (/^\+/) {
> + print_colored $new_color, $_;
> + } elsif (/^\-/) {
> + print_colored $old_color, $_;
> + } elsif (/^\@/) {
> + print_colored $fraginfo_color, $_;
> + } elsif (/^ /) {
> + print_colored $normal_color, $_;
> + } else {
> + print_colored $metainfo_color, $_;
> + }
> + }
> +}
> +
> sub hunk_splittable {
> my ($text) = @_;
>
> @@ -610,9 +631,7 @@ sub patch_update_cmd {
> my ($ix, $num);
> my $path = $it->{VALUE};
> my ($head, @hunk) = parse_diff($path);
> - for (@{$head->{TEXT}}) {
> - print;
> - }
> + print_diff_hunk($head->{TEXT});
> $num = scalar @hunk;
> $ix = 0;
>
> @@ -654,9 +673,7 @@ sub patch_update_cmd {
> if (hunk_splittable($hunk[$ix]{TEXT})) {
> $other .= '/s';
> }
> - for (@{$hunk[$ix]{TEXT}}) {
> - print;
> - }
> + print_diff_hunk($hunk[$ix]{TEXT});
> print_colored $prompt_color, "Stage this hunk
> [y/n/a/d$other/?]? "; my $line = <STDIN>;
> if ($line) {
> @@ -794,8 +811,7 @@ sub diff_cmd {
> HEADER => $status_head, },
> @mods);
> return if (!@them);
> - system(qw(git diff-index -p --cached HEAD --),
> - map { $_->{VALUE} } @them);
> + system(qw(git diff -p --cached HEAD --), map { $_->{VALUE} }
> @them); }
>
> sub quit_cmd {
>
In a previous incantation of this thread, coloring the diff output was
discussed. Your patch works, I tested it, but it does not highlight
whitespace at the end of lines or space/tab errors. If this is the only
case that more than one color may appear per line, it should not be
hard to match it as a special case (assuming this check isn't disabled
in .gitconfig), and print the rest of the line as we otherwise would.
Dan
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
