Re: [PATCHv2 2/3] grep: Colorize filename, line number, and separator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Mar 7, 2010 at 2:02 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Mark Lodato <lodatom@xxxxxxxxx> writes:
>
>> +     if (color) {
>>               if (!value)
>>                       return config_error_nonbool(var);
>> -             color_parse(value, var, opt->color_match);
>> -             return 0;
>> +             color_parse(value, var, color);
>> +             if (!strcmp(color, GIT_COLOR_RESET))
>> +                     color[0] = '\0';
>
> I don't know this "optimization" is warranted.  I can understand that you
> are trying to help the user to save a handful of useless bytes per line of
> output in the normal case, but doesn't "color.cmd.foo = normal" set the
> value to an empty string already if that is what the user wants?

Ah, I didn't know about the "normal" color.  I tried using a blank
value, which defaulted to "reset", so I thought there was no way to
say "no ANSI sequence".  The above was my workaround to get no
sequence.  Since the "normal" color exists, this workaround should be
removed.

The squash-able patch below removes the workaround, and also fixes
another bug: color was not reset at the end of the line for hunk
separators ("--").

As an aside, perhaps a blank color value should default to no ANSI
sequence, rather than the reset sequence?


diff --git i/builtin-grep.c w/builtin-grep.c
index e423eac..c519fcf 100644
--- i/builtin-grep.c
+++ w/builtin-grep.c
@@ -318,8 +318,6 @@
 		if (!value)
 			return config_error_nonbool(var);
 		color_parse(value, var, color);
-		if (!strcmp(color, GIT_COLOR_RESET))
-			color[0] = '\0';
 	}
 	return 0;
 }
diff --git i/grep.c w/grep.c
index 1e0b1e6..b641305 100644
--- i/grep.c
+++ w/grep.c
@@ -533,12 +533,15 @@

 	if (opt->pre_context || opt->post_context) {
 		if (opt->last_shown == 0) {
-			if (opt->show_hunk_mark)
-				output_color(opt, "--\n", 3, opt->color_sep);
-			else
+			if (opt->show_hunk_mark) {
+				output_color(opt, "--", 2, opt->color_sep);
+				opt->output(opt, "\n", 1);
+			} else
 				opt->show_hunk_mark = 1;
-		} else if (lno > opt->last_shown + 1)
-			output_color(opt, "--\n", 3, opt->color_sep);
+		} else if (lno > opt->last_shown + 1) {
+			output_color(opt, "--", 2, opt->color_sep);
+			opt->output(opt, "\n", 1);
+		}
 	}
 	opt->last_shown = lno;
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]