Re: [PATCHv1] Export file in git-remote-mediawiki

On Fri, 08 Jun 2012 16:07:23 +0200, Matthieu Moy wrote:

quoted

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 [off-list ref] writes:

quoted

From: nguyenkimthuat <redacted>

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).

quoted

+	open(my $g,"-|","git " . $_[0]);

Space after , please.

quoted

+	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?

quoted

 	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})) {

quoted

@@ -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.

quoted

+	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()");
                         }

quoted

 	} 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"
+       }

quoted

@@ -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.