Re: [PATCH/RFC] Export file attachements in git-remote-mediawiki

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

NGUYEN Kim Thuat <kim-thuat.nguyen@ensimag.imag.fr> writes:

> +# Get the list of file extensions supported by the current version of mediawiki 
> +my @list_file_extensions = get_file_extensions();

You should do it only on demand (like $mediawiki is created lazily).

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

Space after ",".

> @@ -642,8 +651,14 @@ sub mw_push_file {
>  	my $old_sha1 = $diff_info_split[2];
>  	my $page_created = ($old_sha1 eq NULL_SHA1);
>  	my $page_deleted = ($new_sha1 eq NULL_SHA1);
> +	my $file_deleted = ($new_sha1 eq NULL_SHA1);

This line looks suspiciously similar to the previous one. Do you need
another variable for the same value?

> +	my @extensions = split(/\./,$complete_file_name);

Space after "," (many more instances after).

> -	} else {
> -		print STDERR "$complete_file_name not a mediawiki file (Not pushable on this version of git-remote-mediawiki).\n"
> +	} elsif (exists($hashFiles{$extension})) {
> +		# Deleting and uploading a file require the priviledge of the user
> +		if ($file_deleted) {
> +			mw_connect_maybe();
> +			my $res = $mediawiki->edit( {
> +			action => 'delete',
> +			title => $path,
> +			reason => $summary } )

Indent the body of {} please.

> +		} else {
> +			my $content = run_git_raw("cat-file blob $new_sha1");
> +			mw_connect_maybe();
> +				$mediawiki->{config}->{upload_url} = "$url/index.php/Special:Upload";

Broken indentation.

Does this work on wiki configured in foreign languages, like french that
has Spécial:Téléverser instead?

> +			} ) || die $mediawiki-> {error}->{code} . ':' . $mediawiki->{error}->{details};
> +			$newrevid = get_reviId_filepage();

This queries the wiki to get the last revision id. The existing code (to
deal with page) could get this from the response of the API to the edit
request, like this:

		$newrevid = $result->{edit}->{newrevid};

Your version is much more inefficient (many requests each time you
upload a file), and has a race condition (what happens if someone else
creates a revision at the same time?). Isn't there a better way?

> +			print STDERR "Pushed file: $new_sha1 - $complete_file_name\n";
> +			 }

Broken indentation.

> +	else {
> +		print STDERR "$complete_file_name is not supported on this version of Mediawiki.\n"

It's not a matter of version, it's a matter of configuration.

> +sub get_reviId_filepage() {

Strange name (two consecutive i?). If this function fetches the last
wiki revision, why not call it get_last_mw_revid or something like this?

> +	mw_connect_maybe();
> +
> +	my $max_rev_num_file = 0;
> +
> +	my @list_file_pages = get_mw_media_pages();
> +
> +	foreach my $file_page (@list_file_pages) {
> +	my $id = $file_page->{pageid};

Broken indentation.

> +	my $query = {
> +		action => 'query',
> +		prop => 'revisions',
> +		rvprop => 'ids',
> +		pageids => $id,
> +		};
> +
> +		my $result = $mediawiki->api($query);

Broken indentation.

> +sub get_file_extensions {
> +	mw_connect_maybe();
> +
> +	my $query = {
> +	action => 'query',
> +	meta => 'siteinfo',
> +	siprop => 'fileextensions'
> +	};

Broken indentation.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/