Re: [PATCH 05/18] Turn double-negated expressions into simple expressions

["Célestin Matte" <celestin.matte@ensimag.fr>]

Le 07/06/2013 06:12, Eric Sunshine a écrit :
> On Thu, Jun 6, 2013 at 3:34 PM, Célestin Matte
> <celestin.matte@ensimag.fr> wrote:
>> Signed-off-by: Célestin Matte <celestin.matte@ensimag.fr>
>> Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>> ---
>>  contrib/mw-to-git/git-remote-mediawiki.perl |    8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
>> index 68fd129..a6c7de2 100755
>> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
>> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
>> @@ -136,16 +136,16 @@ while (<STDIN>) {
>>         if (defined($cmd[0])) {
>>                 # Line not blank
>>                 if ($cmd[0] eq "capabilities") {
>> -                       die("Too many arguments for capabilities\n") unless (!defined($cmd[1]));
>> +                       die("Too many arguments for capabilities\n") if (defined($cmd[1]));
>>                         mw_capabilities();
>>                 } elsif ($cmd[0] eq "list") {
>> -                       die("Too many arguments for list\n") unless (!defined($cmd[2]));
>> +                       die("Too many arguments for list\n") if (defined($cmd[2]));
>>                         mw_list($cmd[1]);
>>                 } elsif ($cmd[0] eq "import") {
>> -                       die("Invalid arguments for import\n") unless ($cmd[1] ne "" && !defined($cmd[2]));
>> +                       die("Invalid arguments for import\n") if ($cmd[1] eq "" || defined($cmd[2]));
>>                         mw_import($cmd[1]);
>>                 } elsif ($cmd[0] eq "option") {
>> -                       die("Too many arguments for option\n") unless ($cmd[1] ne "" && $cmd[2] ne "" && !defined($cmd[3]));
>> +                       die("Too many arguments for option\n") if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
> 
> Not new in this patch, but isn't this diagnostic misleading? It will
> (falsely) claim "too many arguments" if $cmd[1] or $cmd[2] is an empty
> string. Perhaps it should be reworded like the 'import' diagnostic and
> say "Invalid arguments for option".

We could even be more precise and separate the cases, i.e., die("Too
many arguments") when too many arguments are defined and die("Invalid
arguments") when there are empty strings.
Not sure if I should integrate it in this patch, though.


-- 
Célestin Matte