Pavel Volek <Pavel.Volek@xxxxxxxxxxxxxxx> writes:
> --- a/contrib/mw-to-git/git-remote-mediawiki
> +++ b/contrib/mw-to-git/git-remote-mediawiki
If the patch adds support for [[File:...]], then it should remove/adapt
the comment at the top of the file :
# Known limitations:
#
# - Only wiki pages are managed, no support for [[File:...]]
# attachments.
> @@ -212,59 +212,230 @@ sub get_mw_pages {
> my $user_defined;
> if (@tracked_pages) {
> $user_defined = 1;
> - # The user provided a list of pages titles, but we
> - # still need to query the API to get the page IDs.
> -
> - my @some_pages = @tracked_pages;
> - while (@some_pages) {
> - my $last = 50;
> - if ($#some_pages < $last) {
> - $last = $#some_pages;
> - }
> - my @slice = @some_pages[0..$last];
> - get_mw_first_pages(\@slice, \%pages);
> - @some_pages = @some_pages[51..$#some_pages];
> - }
> + get_mw_tracked_pages(\%pages);
> }
> if (@tracked_categories) {
> $user_defined = 1;
> - foreach my $category (@tracked_categories) {
> - if (index($category, ':') < 0) {
> - # Mediawiki requires the Category
> - # prefix, but let's not force the user
> - # to specify it.
> - $category = "Category:" . $category;
> - }
> - my $mw_pages = $mediawiki->list( {
> - action => 'query',
> - list => 'categorymembers',
> - cmtitle => $category,
> - cmlimit => 'max' } )
> - || die $mediawiki->{error}->{code} . ': ' . $mediawiki->{error}->{details};
> - foreach my $page (@{$mw_pages}) {
> - $pages{$page->{title}} = $page;
> - }
> - }
> + get_mw_tracked_categories(\%pages);
> }
> if (!$user_defined) {
> - # No user-provided list, get the list of pages from
> - # the API.
> - my $mw_pages = $mediawiki->list({
> - action => 'query',
> - list => 'allpages',
> - aplimit => 500,
> - });
> - if (!defined($mw_pages)) {
> - print STDERR "fatal: could not get the list of wiki pages.\n";
> - print STDERR "fatal: '$url' does not appear to be a mediawiki\n";
> - print STDERR "fatal: make sure '$url/api.php' is a valid page.\n";
> - exit 1;
> + get_mw_all_pages(\%pages);
> + }
> + return values(%pages);
> +}
The refactoring is welcome, but it would have been better to make it in
a separate patch. The patch as you made it is long and hard to review,
because it combines several new features, and refactoring.
> +sub get_mw_tracked_pages {
> + my $pages = shift;
> + # The user provided a list of pages titles, but we
> + # still need to query the API to get the page IDs.
> + my @some_pages = @tracked_pages;
> + while (@some_pages) {
> + my $last = 50;
> + if ($#some_pages < $last) {
> + $last = $#some_pages;
> + }
> + my @slice = @some_pages[0..$last];
> + get_mw_first_pages(\@slice, \%{$pages});
> + @some_pages = @some_pages[51..$#some_pages];
> + }
> +
> + # Get pages of related media files.
> + get_mw_linked_mediapages(\@tracked_pages, \%{$pages});
[...]
> +sub get_mw_linked_mediapages {
This is a nice feature, but I think it deserves to be configurable (if
the user explicitely specified one page, it actually seems strange to
import all the files it links to by default). Also, it should be
mentionned in the commit message.
Shouldn't the function be named get_mw_linked_mediafiles instead? In
general, the wording "media page" is used in many places in the code,
I prefer "media file" which is unambiguous.
> +sub get_mw_medafile_for_mediapage_revision {
medafile -> mediafile ?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html