From: nguyenki <nguyenki@ensibm.imag.fr>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: <git@vger.kernel.org>
Subject: Re: [PATCHv1] Export file in git-remote-mediawiki
Date: Sat, 09 Jun 2012 00:59:22 +0200 [thread overview]
Message-ID: <c23d4a113c735e093e3e9b06e4f16a70@ensibm.imag.fr> (raw)
In-Reply-To: <vpqwr3hrj6s.fsf@bauges.imag.fr>
On Fri, 08 Jun 2012 16:07:23 +0200, Matthieu Moy wrote:
>> Subject: Re: [PATCHv1] Export file in git-remote-mediawiki
>
> We usually write commit subject lines as "subsystem: description",
> hence
>
> git-remote-mediawiki: export "File:" attachments
>
Yes, it will be corrected in the next version.
> Kim Thuat NGUYEN <kim-thuat.nguyen@ensimag.imag.fr> writes:
>
>> From: nguyenkimthuat <nguyenkimthuat@gmail.com>
>>
>> This patch adds the functionnality to export the file attachements
>> from the local git's repository using the API of mediawiki. It also
>> provides the possibility for
>> an user to delete a page in the local repository Git which means the
>> page will be deleted in the wiki site after user do the 'push'.
>
> Please, avoid long lines (> 80 characters).
>
>> + open(my $g,"-|","git " . $_[0]);
>
> Space after , please.
>
>> + my %hashFiles = get_file_extensions_maybe($complete_file_name);
>
> What does this function do? My first understanding was that it
> queried
> the wiki for allowed file extensions, but why does it need the file
> name? It does nothing if $complete_file_name ends with .mw, but then
> why
> do you run it before entering the following if() statement?
>
>> if (substr($complete_file_name,-3) eq ".mw") {
>> my $title = substr($complete_file_name,0,-3);
>
This function will get a list of allowed file extensions. It need the
file name to verify if this name begins with .mw or not. If not, it does
nothing like
you said. But if $complete_file_name is not a wiki page (end with .mw),
the function will return the list of file extensions %hashFiles to
verify if this file is allowed in this condition:
} elsif (exists($hashFiles{$extension})) {
>> @@ -653,39 +666,74 @@ sub mw_push_file {
>> # special priviledges. A common
>> # convention is to replace the page
>> # with this content instead:
>> - $file_content = DELETED_CONTENT;
>> + mw_connect_maybe();
>> + my $re = $mediawiki->edit( {
>> + action => 'delete',
>> + title => $title,
>> + reason => $summary
>> + } )|| die $mediawiki-> {error}->{code} . ':' .
>> $mediawiki->{error}->{details};
>
> This is an unrelated topic, and should not appear in this patch.
>
> If you want to propagate page deletions, then you also need to deal
> with
> the case where the user is not allowed to do so (very common on
> MediaWiki). Also, if you change the code corresponding to the comment
> right above, you should update the comment too.
>
>> + elsif (exists($hashFiles{$extension}))
>> + {
>
Yes, i'll correct it in the next patch.
For the moment, i added these lines to deal with the case similar - the
case when an user tries to upload a file or pages but he doesn't have
sufficient rights or he failed to login.
+sub error_insufficient_right {
+ print STDERR "Can not delete or upload file and wiki pages\n";
+ print STDERR "You don't have right to do it\n";
+ print STDOUT "error $_[0]\"right insufficient\"\n";
+ return 0;
+}
@@ -726,17 +1011,32 @@ sub mw_push_file {
ignorewarnings=>1,
}, {
skip_encoding => 1 #
Helps with names with accentuated characters
- } ) || die $mediawiki->
{error}->{code} . ':' . $mediawiki->{error}->{details};
+ } );
+ if (!$res) {
+ if ($mediawiki->{error}->{code}
== 3) {
+ # Failed to upload,
user didn't login or he doesn't have sufficient rights
+ print STDERR 'Warning:
Error ' .
+
$mediawiki->{error}->{code} .
+ ' from mediwiki: '
. $mediawiki->{error}->{details} .
+ ".\n";
+ return
($newrevid,"insufficient-right");
+ } else {
+ # Other errors.
Shouldn't happen => just die()
+ die 'Fatal: Error ' .
+
$mediawiki->{error}->{code} .
+ ' from mediwiki: '
. $mediawiki->{error}->{details};
+ }
+ }
@@ -860,6 +1161,9 @@ sub mw_push_revision {
# accurate error message.
return error_non_fast_forward($remote);
}
+ if($status eq "insufficient-right") {
+ return
error_insufficient_right($remote);
+ }
if ($status ne "ok") {
die("Unknown error from
mw_push_file()");
}
>
>> } else {
>> print STDERR "$complete_file_name not a mediawiki file (Not
>> pushable on this version of git-remote-mediawiki).\n"
>> }
>
> Isn't the very point of this patch to remove this error message?
Now, the message is
+ print STDERR "$complete_file_name is not a permitted
file. Check the configuration of file uploads in your mediawiki \n"
+ }
>
>> @@ -825,3 +873,25 @@ sub mw_push_revision {
>> print STDOUT "ok $remote\n";
>> return 1;
>> }
>> +
>> +sub get_file_extensions_maybe {
>> + my $file_name = shift;
>> + my $est_mw_page = substr($file_name,-3) eq ".mw";
>
> English please. "est" is french ;-).
Corrected.
- my $est_mw_page = substr($file_name,-3) eq ".mw";
- if(!$est_mw_page) {
+ my $is_mw_page = substr($file_name,-3) eq ".mw";
+ if(!$is_mw_page) {
Thanks you so much for your advices.
Thuat.
next prev parent reply other threads:[~2012-06-08 22:59 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-08 13:27 [PATCHv1] Export file in git-remote-mediawiki Kim Thuat NGUYEN
2012-06-08 14:07 ` Matthieu Moy
2012-06-08 22:59 ` nguyenki [this message]
2012-06-10 13:01 ` Matthieu Moy
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c23d4a113c735e093e3e9b06e4f16a70@ensibm.imag.fr \
--to=nguyenki@ensibm.imag.fr \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=git@vger.kernel.org \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).