Re: [PATCH] gitweb: new cgi parameter: option

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 12, 2007, 18:45 UTC
Message-ID: <7vy7hltq98.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200707121211.32813.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 5 quoted lines

> [*1*] At least in gitweb. If I understand correctly, you can use
> "git diff --cc tree1 tree2 tree2 ..." to get combined diff of specified
> tree-ish; I'm not sure if git-diff-tree support this. And I know that
> gitweb does not support this... at least for now. Would this be useful,
> I wonder?

I would say that would only be useful to satisfy curiosity. Luckily or unluckily I have not had real life use of that multiple tree comparison feature that is supported by "git diff" (multiple blob comparison is also available, which is mildly useful).

Show 5 quoted lines

>> +our $option = $cgi->param('option');
>> +if (defined $option) {
>> +	if (not grep(/^$option$/, keys %options)) {
>> +		die_error(undef, "Invalid option parameter");
>> +	}

"!exists $options{$option}" ?

Show 7 quoted lines

> I'd rather make it possible to pass multiple additional options, for
> example both '--remove-empty' (to speed up) and '--no-merges' for the
> history view. So I'd use
>
>   +our @options = $cgi->param('option');
>
> instead.

Good point.

> I'm also not sure if invalid option parameter for action should return
> error, or be simply ignored.

I'm mildly against "simply ignoring".

> By the way, gitweb uses shortened names for paramaters. Perhaps 'opt'
> or 'op' instead of 'options' here and in href subroutine (below)?

Or even 'o' ;-).