Giuseppe Bilotta wrote:
> 2009/12/17 Jakub Narebski <jnareb@xxxxxxxxx>:
>> On Thu, 17 Dec 2009 10:05 +0100, Giuseppe Bilotta wrote:
[...]
>>> + 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html