Re: [PATCH v3 21/28] git-remote-mediawiki: Put long code into a subroutine

2013-06-11 Thread Célestin Matte 
Oops, forgot to take this into account before sending v4 of my series of
patch. I just noticed that, sorry...

Le 11/06/2013 17:42, Junio C Hamano a écrit :
> I am guessing that the new sub, parse_command, uses a local @cmd and
> this is an attempt to avoid using the same name, but this renaming
> of the variable is not explained.

This is indeed what I intended to do.

> I also wonder if you need this global @cmd/@cmds.  Instead of
> passing cmdref, wouldn't it be simpler to have the helper split the
> line, i.e. something like...
> 
>   sub parse_command {
>   my ($line) = @_;
> my @cmd = split(/ /, $line);
>   unless (defined @cmd) {
>   return 0;
>   }
> ... check capabilities, list, etc
>   return 1;
>   }
> 
> while () {
>   chomp;
> if (!parse_command($_)) {
>   unknown command, aborting
> last;
>   }
>   }
> 
> 


-- 
Célestin Matte
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 21/28] git-remote-mediawiki: Put long code into a subroutine

2013-06-11 Thread Junio C Hamano 
Célestin Matte  writes:

> Signed-off-by: Célestin Matte 
> Signed-off-by: Matthieu Moy 
> ---
>  contrib/mw-to-git/git-remote-mediawiki.perl |   50 
> +--
>  1 file changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
> b/contrib/mw-to-git/git-remote-mediawiki.perl
> index c404e7b..a66cef4 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -126,28 +126,13 @@ $wiki_name =~ s{[^/]*://}{};
>  $wiki_name =~ s/^.*@//;
>  
>  # Commands parser
> -my @cmd;
> +my @cmds;

I am guessing that the new sub, parse_command, uses a local @cmd and
this is an attempt to avoid using the same name, but this renaming
of the variable is not explained.

I also wonder if you need this global @cmd/@cmds.  Instead of
passing cmdref, wouldn't it be simpler to have the helper split the
line, i.e. something like...

sub parse_command {
my ($line) = @_;
my @cmd = split(/ /, $line);
unless (defined @cmd) {
return 0;
}
... check capabilities, list, etc
return 1;
}

while () {
chomp;
if (!parse_command($_)) {
unknown command, aborting
last;
}
}

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 21/28] git-remote-mediawiki: Put long code into a subroutine

2013-06-09 Thread Célestin Matte 
Signed-off-by: Célestin Matte 
Signed-off-by: Matthieu Moy 
---
 contrib/mw-to-git/git-remote-mediawiki.perl |   50 +--
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
b/contrib/mw-to-git/git-remote-mediawiki.perl
index c404e7b..a66cef4 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -126,28 +126,13 @@ $wiki_name =~ s{[^/]*://}{};
 $wiki_name =~ s/^.*@//;
 
 # Commands parser
-my @cmd;
+my @cmds;
 while () {
chomp;
-   @cmd = split(/ /);
-   if (defined($cmd[0])) {
+   @cmds = split(/ /);
+   if (defined($cmds[0])) {
# Line not blank
-   if ($cmd[0] eq "capabilities") {
-   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") if 
(defined($cmd[2]));
-   mw_list($cmd[1]);
-   } elsif ($cmd[0] eq "import") {
-   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") if ($cmd[1] eq 
"" || $cmd[2] eq "" || defined($cmd[3]));
-   mw_option($cmd[1],$cmd[2]);
-   } elsif ($cmd[0] eq "push") {
-   mw_push($cmd[1]);
-   } else {
-   print STDERR "Unknown command. Aborting...\n";
+   if (!parse_command(\@cmds)) {
last;
}
} else {
@@ -167,6 +152,33 @@ sub exit_error_usage {
 die "ERROR: git-remote-mediawiki module was not called with a correct 
number of parameters\n";
 }
 
+sub parse_command {
+   my $cmdref = shift;
+   my @cmd = @{$cmdref};
+   if ($cmd[0] eq "capabilities") {
+   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") if (defined($cmd[2]));
+   mw_list($cmd[1]);
+   } elsif ($cmd[0] eq "import") {
+   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")
+   if ($cmd[1] eq "" || $cmd[2] eq "" || defined($cmd[3]));
+   mw_option($cmd[1],$cmd[2]);
+   } elsif ($cmd[0] eq "push") {
+   mw_push($cmd[1]);
+   } else {
+   print STDERR "Unknown command. Aborting...\n";
+   return 0;
+   }
+   return 1;
+}
+
 # MediaWiki API instance, created lazily.
 my $mediawiki;
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html