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

[...]
quoted text
>>> +     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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html