Re: [PATCH v3 21/28] git-remote-mediawiki: Put long code into a subroutine
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
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
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