Le 07/06/2013 06:12, Eric Sunshine a écrit :
> On Thu, Jun 6, 2013 at 3:34 PM, Célestin Matte
> <celestin.matte@xxxxxxxxxx> wrote:
>> Signed-off-by: Célestin Matte <celestin.matte@xxxxxxxxxx>
>> Signed-off-by: Matthieu Moy <matthieu.moy@xxxxxxxxxxxxxxx>
>> ---
>> 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
--
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