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

Le 07/06/2013 06:12, Eric Sunshine a écrit :

On Thu, Jun 6, 2013 at 3:34 PM, Célestin Matte
[off-list ref] wrote:

quoted

Signed-off-by: Célestin Matte <redacted>
Signed-off-by: Matthieu Moy <redacted>
---
 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