From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 07 Jun 2013 04:12:54 GMT
Subject: Re: [PATCH 05/18] Turn double-negated expressions into simple expressions
Message-ID: <CAPig+cSDxhT=WVMQz_88z0xEmPVENDSyp5mE7XWCf_99yQTkTA@mail.gmail.com>
In-Reply-To: <1370547263-13558-6-git-send-email-celestin.matte@ensimag.fr>

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".

>                         mw_option($cmd[1],$cmd[2]);
>                 } elsif ($cmd[0] eq "push") {
>                         mw_push($cmd[1]);
> --