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

[Junio C Hamano]

Célestin Matte celestin.ma...@ensimag.fr writes:

 Signed-off-by: Célestin Matte celestin.ma...@ensimag.fr
 Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
 ---
  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 (STDIN) {
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

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

[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 (STDIN) {
   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