Re: What's in git.git (stable frozen)

[Jeff King <peff@peff.net>]

On Sun, Jan 06, 2008 at 04:32:15AM -0800, Junio C Hamano wrote:

> Yeah, I like that much better, especially the renaming of
> $use_color to more descriptive (but is it really about "menu", I
> wonder?).

I thought that, too, but I didn't know what better word to use
(something like "display" doesn't make it clear that it doesn't involve
the diff).

> I might consider rewriting this part
> 
> > +my $menu_use_color = $repo->get_colorbool('color.interactive');
> > +my ($prompt_color, $header_color, $help_color) =
> > +	$menu_use_color ? (
> > +		$repo->get_color('color.interactive.prompt', 'bold blue'),
> > +		$repo->get_color('color.interactive.header', 'bold'),
> > +		$repo->get_color('color.interactive.help', 'red bold'),
> > +	) : ();
> 
> like this:
> 
> 	my ($prompt_color, $header_color, $help_color, $fraginfo_color);
>         if ($colored_prompt) {
>         	$prompt_color = ...;
>                 $header_color = ...;
>         }
> 	if ($colored_diff) {
>         	$fraginfo_color = ...;
> 	}

Actually, that's more or less how it's written before my patch (in fact,
you could eliminate that part of my patch and just move $normal_color
outside of the conditional). However I didn't like having the
declaration and the assignment so far apart (and duplicated). But I will
admit my version is a little nested. Please feel free to switch it when
you apply.

> or even like this:
> 
> 	my (%palette);
>         if ($colored_prompt) {
> 		my %default = (
>                 	prompt => 'bold blue',
>                         header => 'bold',
>                         ...
> 		);
>                 while (my ($k,$v) = each %default) {
>                     $palette{$k} = $repo->get_color("color.interactive.$k",$v);
> 		}
> 	}
> 
> But I realize that's going overboard.  Certainly the last one is
> doing unnecessary generalization for generalization's sake.

Yes, I considered making a %palette, as well, but it just seemed a
little gratuitous (and the nice thing about using variables is that they
catch typos better).

-Peff