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

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

quoted

+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