From: "Simon.Cathebras" <Simon.Cathebras@ensimag.imag.fr>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com, peff@peff.net,
Guillaume Sasdy <guillaume.sasdy@ensimag.imag.fr>,
Simon Perrat <simon.perrat@ensimag.imag.fr>,
Charles Roussel <charles.roussel@ensimag.imag.fr>,
Julien Khayat <julien.khayat@ensimag.imag.fr>,
Matthieu Moy <matthieu.moy@imag.fr>
Subject: Re: [PATCH 2/6] Test environment of git-remote-mediawiki
Date: Wed, 13 Jun 2012 19:00:15 +0200 [thread overview]
Message-ID: <4FD8C71F.1070508@ensimag.imag.fr> (raw)
In-Reply-To: <CACBZZX5v_cZTddWB3vPsepL2LPzP8zd+pOC5w+=SV8aYJpL3HA@mail.gmail.com>
On 13/06/2012 12:14, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Jun 12, 2012 at 11:12 PM, Simon Cathebras
> <simon.cathebras@ensimag.imag.fr> wrote:
>
> Some comments on your perl style:
>
>> +use Switch;
>> +use encoding 'utf8';
>> +use DateTime::Format::ISO8601;
>> +use open ':encoding(utf8)';
>> +use constant SLASH_REPLACEMENT => "%2F";
>> +
>> +#Parsing of the config file
>> +
>> +my $configfile = "$ENV{'CURR_DIR'}/test.config";
>> +my %config;
>> +open (CONFIG,"< $configfile") || die "can't open $configfile: $!";
>> +while (<CONFIG>)
> You probably want to use lexical filehandles instead:
>
> open my $config, "<", $configfile or die "...";
> while (<$config>)
>
> And also note the three-arg open I'm using, might want to change the
> rest to use that.
We corrected it. The correction will appears in our next patch.
>> +{
>> + chomp;
>> + s/#.*//;
>> + s/^\s+//;
>> + s/\s+$//;
>> + next unless length;
>> + my ($key, $value) = split (/\s*=\s*/,$_, 2);
>> + $config{$key} = $value;
>> + last if ($key eq 'LIGHTTPD' and $value eq 'false');
>> + last if ($key eq 'PORT');
>> +}
> And add:
>
> close $config or die "can't close $configfile: $!";
>
> Also you can do:
>
> while (my $line =<$config>) {
> chomp $line;
>
> Which IMO makes any code that's more than 2-3 lines long less
> confusing as there's no doubt what $_ refers to.
Same.
>> +my $wiki_address = "http://$config{'SERVER_ADDR'}".":"."$config{'PORT'}";
>> +my $wiki_url = "$wiki_address/$config{'WIKI_DIR_NAME'}/api.php";
>> +my $wiki_admin = "$config{'WIKI_ADMIN'}";
>> +my $wiki_admin_pass = "$config{'WIKI_PASSW'}";
>> +my $mw = MediaWiki::API->new;
>> +$mw->{config}->{api_url} = $wiki_url;
>> +
>> +sub mediawiki_clean_filename {
>> + my $filename = shift;
>> + $filename =~ s/@{[SLASH_REPLACEMENT]}/\//g;
>> + # [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
>> + # Do a variant of URL-encoding, i.e. looks like URL-encoding,
>> + # but with _ added to prevent MediaWiki from thinking this is
>> + # an actual special character.
>> + $filename =~ s/[\[\]\{\}\|]/sprintf("_%%_%x", ord($&))/ge;
>> + # If we use the uri escape before
>> + # we should unescape here, before anything
>> +
>> + return $filename;
>> +}
>> +
>> +sub mediawiki_smudge_filename {
>> + my $filename = shift;
>> + $filename =~ s/\//@{[SLASH_REPLACEMENT]}/g;
>> + $filename =~ s/ /_/g;
>> + # Decode forbidden characters encoded in mediawiki_clean_filename
>> + $filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf("%c", hex($1))/ge;
>> + return $filename;
>> +}
> I don't how in the big picture you're aiming to encode article names
> as filenames but if you can at all avoid doing so (i.e. the user
> doesn't need to be exposed to those files) it's much simpler just to
> take the article name, sha1sum it and use the first 5-10 bytes of that
> as the filename.
>
> With all the filesystems and OS's out there and their odd rules it can
> be tricky to write code that takes an arbitrary string and attempts to
> cast it to a valid filename or path.
Actually, we copy paste this code from git-remote-mediawiki. As a test
environment, this strategy appears to be a bad one (in addition of the
copy paste itself...).
Anyway, we decided to remove this code and modify our tests.
As a matter this code's purpose in git-remote-mediawiki is to change the
forbidden character in page name for wiki's, into understandable ones.
So, in my opinion, using sha1sum would be useless.
>> +# wiki_login<name> <password>
>> +#
>> +# Logs the user with<name> and<password> in the global variable
>> +# of the mediawiki $mw
>> +sub wiki_login {
>> + $mw->login( { lgname => "$_[0]",lgpassword => "$_[1]" } )
>> + || die "getpage: login failed";
>> +}
>> +
>> +# wiki_getpage<wiki_page> <dest_path>
>> +#
>> +# fetch a page<wiki_page> from the wiki referenced in the global variable
>> +# $mw and copies its content in directory dest_path
>> +sub wiki_getpage {
>> + my $pagename = $_[0];
>> + my $destdir = $_[1];
>> +
>> + my $page = $mw->get_page( { title => $pagename } );
>> + if (!defined($page)) {
>> + die "getpage: wiki does not exist";
>> + }
>> +
>> + my $content = $page->{'*'};
>> + if (!defined($content)) {
>> + die "getpage: page does not exist";
>> + }
>> +
>> + # Replace spaces by underscore in the page name
>> + $pagename=$page->{'title'};
>> + $pagename = mediawiki_smudge_filename $pagename;
>> + open(my $file, ">$destdir/$pagename.mw");
>> + print $file "$content";
>> + close ($file);
>> +
>> +}
>> +
>> +# wiki_delete_page<page_name>
>> +#
>> +# delete the page with name<page_name> from the wiki referenced
>> +# in the global variable $mw
>> +sub wiki_delete_page {
>> + my $pagename = $_[0];
>> +
>> + my $exist=$mw->get_page({title => $pagename});
>> +
>> + if (defined($exist->{'*'})){
>> + $mw->edit({ action => 'delete',
>> + title => $pagename})
>> + || die $mw->{error}->{code} . ": " . $mw->{error}->{details};
>> + } else {
>> + die "no page with such name found: $pagename\n";
>> + }
>> +}
>> +
>> +# wiki_editpage<wiki_page> <wiki_content> <wiki_append> [-c=<category>] [-s=<summary>]
>> +#
>> +# Edit a page named<wiki_page> with content<wiki_content> on the wiki
>> +# referenced with the global variable $mw
>> +# If<wiki_append> == true : append<wiki_content> at the end of the actual
>> +# content of the page<wiki_page>
>> +# If<wik_page> doesn't exist, that page is created with the<wiki_content>
>> +sub wiki_editpage {
>> + my $wiki_page = mediawiki_clean_filename $_[0];
>> + my $wiki_content = $_[1];
>> + my $wiki_append = $_[2];
>> + my $summary = "";
>> + my ($summ, $cat) = ();
>> + GetOptions('s=s' => \$summ, 'c=s' => \$cat);
>> +
>> + my $append = 0;
>> + if (defined($wiki_append)&& $wiki_append eq 'true') {
>> + $append=1;
>> + }
>> +
>> + my $previous_text ="";
>> +
>> + if ($append) {
>> + my $ref = $mw->get_page( { title => $wiki_page } );
>> + $previous_text = $ref->{'*'};
>> + }
>> +
>> + my $text = $wiki_content;
>> + if (defined($previous_text)) {
>> + $text="$previous_text$text";
>> + }
>> +
>> + # Eventually, add this page to a category.
>> + if (defined($cat)) {
>> + my $category_name="[[Category:$cat]]";
>> + $text="$text\n $category_name";
>> + }
>> + if(defined($summ)){
>> + $summary=$summ;
>> + }
>> +
>> + $mw->edit( { action => 'edit', title => $wiki_page, summary => $summary, text => "$text"} );
>> +}
>> +
>> +# wiki_getallpagename [<category>]
>> +#
>> +# Fetch all pages of the wiki referenced by the global variable $mw
>> +# and print the names of each one in the file all.txt with a new line
>> +# ("\n") between these.
>> +# If the argument<category> is defined, then this function get only the pages
>> +# belonging to<category>.
>> +sub wiki_getallpagename {
>> + # fetch the pages of the wiki
>> + if (defined($_[0])) {
>> + my $mw_pages = $mw->list ( { action => 'query',
>> + list => 'categorymembers',
>> + cmtitle => "Category:$_[0]",
>> + cmnamespace => 0,
>> + cmlimit=> 500 },
>> + )
>> + || die $mw->{error}->{code}.": ".$mw->{error}->{details};
>> + open(my $file, ">all.txt");
>> + foreach my $page (@{$mw_pages}) {
>> + print $file "$page->{title}\n";
>> + }
>> + close ($file);
>> +
>> + } else {
>> + my $mw_pages = $mw->list({
>> + action => 'query',
>> + list => 'allpages',
>> + aplimit => 500,
>> + })
>> + || die $mw->{error}->{code}.": ".$mw->{error}->{details};
>> + open(my $file, ">all.txt");
>> + foreach my $page (@{$mw_pages}) {
>> + print $file "$page->{title}\n";
>> + }
>> + close ($file);
>> + }
>> +}
>> +
>> +# Main part of this script: parse the command line arguments
>> +# and select which function to execute
>> +my $fct_to_call = shift;
>> +
>> +&wiki_login($wiki_admin,$wiki_admin_pass);
>> +
>> +switch ($fct_to_call) {
>> + case "get_page" {&wiki_getpage(@ARGV)}
>> + case "delete_page" {&wiki_delete_page(@ARGV)}
>> + case "edit_page" {&wiki_editpage(@ARGV)}
>> + case "getallpagename" {&wiki_getallpagename(@ARGV)}
>> + else { die("test-gitmw.pl ERROR: wrong argument")}
>> +}
> When you're calling bareword functions you can just call them as as
> wiki_login(..) not&wiki_login(..).
>
> Also please don't use Switch.pm at all, it's a source filter, is
> deprecated from the Perl core, and it's much better to write this as:
>
> my %functions_to_call = qw(
> get_page wiki_getpage
> delete_page wiki_delete_page
> edit_page wiki_editpage
> getallpagename wiki_getallpagename
> );
> die "$0 ERROR: wrong argument" unless exists $functions_to_call{$fct_to_call};
> &{$functions_to_call{$fct_to_call}}(@ARGV);
Corrected
Thanks ! :)
--
CATHEBRAS Simon
2A-ENSIMAG
Filière Ingéniérie des Systèmes d'Information
Membre Bug-Buster
next prev parent reply other threads:[~2012-06-13 17:00 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-11 20:27 [PATCH]Test environment for git-remote-mediawiki Simon.Cathebras
2012-06-11 20:28 ` [PATCHv3 1/6] Scripts to install, delete and clear a MediaWiki Simon Cathebras
2012-06-11 20:28 ` [PATCHv3 2/6] Test environment of git-remote-mediawiki Simon Cathebras
2012-06-11 20:28 ` [PATCHv3 3/6] Test file for git-remote-mediawiki clone Simon Cathebras
2012-06-11 21:07 ` konglu
2012-06-12 13:58 ` Simon.Cathebras
2012-06-11 20:28 ` [PATCHv3 4/6] Tests for git-remote-mediawiki pull Simon Cathebras
2012-06-11 21:09 ` konglu
2012-06-12 13:58 ` Simon Perrat
2012-06-12 21:12 ` [PATCH 1/6] Scripts to install, delete and clear a MediaWiki Simon Cathebras
2012-06-12 21:12 ` [PATCH 2/6] Test environment of git-remote-mediawiki Simon Cathebras
2012-06-13 7:56 ` Matthieu Moy
2012-06-13 8:10 ` Simon.Cathebras
2012-06-13 10:14 ` Ævar Arnfjörð Bjarmason
2012-06-13 17:00 ` Simon.Cathebras [this message]
2012-06-13 17:03 ` [PATCHv5 1/5] Scripts to install, delete and clear a MediaWiki Simon Cathebras
2012-06-13 17:03 ` [PATCHv5 2/5] Test environment of git-remote-mediawiki Simon Cathebras
2012-06-13 17:03 ` [PATCHv5 3/5] Test file for git-remote-mediawiki clone Simon Cathebras
2012-06-13 17:03 ` [PATCHv5 4/5] Tests for git-remote-mediawiki pull and push Simon Cathebras
2012-06-13 17:03 ` [PATCHv5 5/5] Tests of UTF8 character with git-remote-mediawiki Simon Cathebras
2012-06-14 8:57 ` [PATCHv5 1/5] Scripts to install, delete and clear a MediaWiki Matthieu Moy
2012-06-14 8:57 ` [PATCH 1/3] chmod -x test-gitmw-lib.sh Matthieu Moy
2012-06-14 8:57 ` [PATCH 2/3] Coding style Matthieu Moy
2012-06-14 8:57 ` [PATCH 3/3] Explicit error when curl_exec() fails Matthieu Moy
2012-06-14 9:23 ` Simon.Cathebras
2012-06-14 12:45 ` Matthieu Moy
2012-06-14 9:20 ` [PATCHv5 1/5] Scripts to install, delete and clear a MediaWiki Simon.Cathebras
2012-06-14 16:17 ` Matthieu Moy
2012-06-14 8:58 ` Matthieu Moy
2012-06-12 21:12 ` [PATCH 3/6] Test file for git-remote-mediawiki clone Simon Cathebras
2012-06-12 21:34 ` konglu
2012-06-13 7:20 ` Simon.Cathebras
2012-06-12 21:12 ` [PATCH 4/6] Tests for git-remote-mediawiki pull and push Simon Cathebras
2012-06-12 21:12 ` [PATCH 5/6] Tests of UTF8 character with git-remote-mediawiki Simon Cathebras
2012-06-12 21:18 ` Simon.Cathebras
2012-06-12 21:52 ` konglu
2012-06-13 7:30 ` Simon.Cathebras
2012-06-12 21:45 ` konglu
2012-06-11 20:28 ` [PATCHv3 5/6] Test file for git-remote-mediawiki push Simon Cathebras
2012-06-11 20:28 ` =?y?q?=5BPATCHv3=206/6=5D=20Tests=20of=20UTF8=20character=20with=20git-remote-mediawiki?= Simon Cathebras
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FD8C71F.1070508@ensimag.imag.fr \
--to=simon.cathebras@ensimag.imag.fr \
--cc=avarab@gmail.com \
--cc=charles.roussel@ensimag.imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=guillaume.sasdy@ensimag.imag.fr \
--cc=julien.khayat@ensimag.imag.fr \
--cc=matthieu.moy@imag.fr \
--cc=peff@peff.net \
--cc=simon.perrat@ensimag.imag.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).