Re: [RFC/PATCH] gitweb: link to toggle 'no merges' option

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 17, 2009, 21:14 UTC
Message-ID: <200912172214.27001.jnareb@gmail.com>
In-Reply-To: <cb7bb73a0912171241j56ecd2f1y3dc66cf3b86bd784@mail.gmail.com>

Giuseppe Bilotta wrote:

> 2009/12/17 Jakub Narebski <jnareb@gmail.com>:
>> On Thu, 17 Dec 2009 10:05 +0100, Giuseppe Bilotta wrote:

[...]

Show 15 quoted lines

>>> +     my $can_have_merges = grep(/^$action$/, @{$allowed_options{'--no-merges'}});
>>> +     my $has_merges = !grep(/^--no-merges$/, @extra_options);
>>> +
>>
>> Wouldn't it be better to use straight
>>
>>  +      my $no_merges = grep(/^--no-merges$/, @extra_options);
>>
>> Because $has_merges is true also for example for 'tree' view... which
>> absolutely doesn't make any sense whatsoever.
> 
> The reason why I have two vars is that one checks if we care about the
> option, and the other is to see if it's enabled or not. We don't want
> the 'show merges' toggle to appear in view which don't handle the
> --no-merge option.

Perhaps I didn't made myself clear.

What I wanted to ask is to switch $has_merges to $no_merges, not to remove $can_have_merges. Its a question of semantics of $has_merges, which do not mean that action has (handles) merges.

This is of course the question of style, but I think this would make code more maintainable.

Of course if you go %extra_options hash route this issue wouldn't matter.

-- 
Jakub Narebski
Poland