{"thread":{"id":"30772","subject":"[PATCHv2 1/2] git-remote-mediawiki: import \"File:\" attachments","startedAt":"2012-06-11T19:29:04Z","lastAt":"2012-06-12T09:56:18Z","messageCount":7,"participants":["Pavel Volek","Matthieu Moy","Simon Perrat","konglu@minatec.inpg.fr"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"193365","messageId":"1339442945-8561-1-git-send-email-Pavel.Volek@ensimag.imag.fr","threadId":"30772","inReplyTo":null,"subject":"[PATCHv2 1/2] git-remote-mediawiki: import \"File:\" attachments","fromName":"Pavel Volek","fromEmail":"pavel.volek@ensimag.imag.fr","sentAt":"2012-06-11T19:29:04Z","receivedAt":"2012-06-11T19:29:04Z","isPatch":false,"sender":{"key":"pavel.volek@ensimag.imag.fr","avatar":null},"body":"From: Volek Pavel <me@pavelvolek.cz>\n\nThe current version of the git-remote-mediawiki supports only import and export\nof the pages, doesn't support import and export of file attachments which are\nalso exposed by MediaWiki API. This patch adds the functionality to import file\nattachments and description pages for these files.\n\nSigned-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>\nSigned-off-by: NGUYEN Kim Thuat <Kim-Thuat.Nguyen@ensimag.imag.fr>\nSigned-off-by: ROUCHER IGLESIAS Javier <roucherj@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n contrib/mw-to-git/git-remote-mediawiki | 225 ++++++++++++++++++++++++++++++++-\n 1 file changed, 220 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex c18bfa1..14008ad 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -13,9 +13,6 @@\n #\n # Known limitations:\n #\n-# - Only wiki pages are managed, no support for [[File:...]]\n-#   attachments.\n-#\n # - Poor performance in the best case: it takes forever to check\n #   whether we're up-to-date (on fetch or push) or to fetch a few\n #   revisions from a large wiki, because we use exclusively a\n@@ -71,6 +68,9 @@ chomp(@tracked_pages);\n my @tracked_categories = split(/[ \\n]/, run_git(\"config --get-all remote.\". $remotename .\".categories\"));\n chomp(@tracked_categories);\n \n+# Import media files too.\n+my $import_media = run_git(\"config --get --bool remote.\". $remotename .\".mediaimport\");\n+\n my $wiki_login = run_git(\"config --get remote.\". $remotename .\".mwLogin\");\n # TODO: ideally, this should be able to read from keyboard, but we're\n # inside a remote helper, so our stdin is connect to git, not to a\n@@ -225,6 +225,11 @@ sub get_mw_pages {\n \t\t\tget_mw_first_pages(\\@slice, \\%pages);\n \t\t\t@some_pages = @some_pages[51..$#some_pages];\n \t\t}\n+\n+\t\t# Get pages of related media files.\n+\t\tif ($import_media) {\n+\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n+\t\t}\n \t}\n \tif (@tracked_categories) {\n \t\t$user_defined = 1;\n@@ -244,6 +249,12 @@ sub get_mw_pages {\n \t\t\tforeach my $page (@{$mw_pages}) {\n \t\t\t\t$pages{$page->{title}} = $page;\n \t\t\t}\n+\n+\t\t\t# Get pages of related media files.\n+\t\t\tif ($import_media) {\n+\t\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n+\t\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%pages);\n+\t\t\t}\n \t\t}\n \t}\n \tif (!$user_defined) {\n@@ -263,10 +274,140 @@ sub get_mw_pages {\n \t\tforeach my $page (@{$mw_pages}) {\n \t\t\t$pages{$page->{title}} = $page;\n \t\t}\n+\n+\t\tif ($import_media) {\n+\t\t\t# Attach list of all pages for meadia files from the API,\n+\t\t\t# they are in a different namespace, only one namespace\n+\t\t\t# can be queried at the same moment\n+\t\t\tmy $mw_pages = $mediawiki->list({\n+\t\t\t\taction => 'query',\n+\t\t\t\tlist => 'allpages',\n+\t\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n+\t\t\t\taplimit => 500\n+\t\t\t});\n+\t\t\tif (!defined($mw_pages)) {\n+\t\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n+\t\t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n+\t\t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t\t\t\texit 1;\n+\t\t\t}\n+\t\t\tforeach my $page (@{$mw_pages}) {\n+\t\t\t\t$pages{$page->{title}} = $page;\n+\t\t\t}\n+\t\t}\n \t}\n \treturn values(%pages);\n }\n \n+sub get_mw_pages_for_linked_mediafiles {\n+\tmy $titles = shift;\n+\tmy @titles = @{$titles};\n+\tmy $pages = shift;\n+\n+\t# pattern 'page1|page2|...' required by the API\n+\tmy $mw_titles = join('|', @titles);\n+\n+\t# Media files could be included or linked from\n+\t# a page, get all related\n+\tmy $query = {\n+\t\taction => 'query',\n+\t\tprop => 'links|images',\n+\t\ttitles => $mw_titles,\n+\t\tplnamespace => get_mw_namespace_id(\"File\"),\n+\t\tpllimit => 500\n+\t};\n+\tmy $result = $mediawiki->api($query);\n+\n+\twhile (my ($id, $page) = each(%{$result->{query}->{pages}})) {\n+\t\tmy @titles;\n+\t\tif (defined($page->{links})) {\n+\t\t\tmy @link_titles = map $_->{title}, @{$page->{links}};\n+\t\t\tpush(@titles, @link_titles);\n+\t\t}\n+\t\tif (defined($page->{images})) {\n+\t\t\tmy @image_titles = map $_->{title}, @{$page->{images}};\n+\t\t\tpush(@titles, @image_titles);\n+\t\t}\n+\t\tif (@titles) {\n+\t\t\tget_mw_first_pages(\\@titles, \\%{$pages});\n+\t\t}\n+\t}\n+}\n+\n+# Returns MediaWiki id for a canonical namespace name.\n+# Ex.: \"File\", \"Project\".\n+# Looks for the namespace id in the local configuration\n+# variables, if it is not found asks MW API.\n+sub get_mw_namespace_id {\n+\tmw_connect_maybe();\n+\n+\tmy $name = shift;\n+\n+\t# Look at configuration file, if the record\n+\t# for that namespace is already stored.\n+\t# Namespaces are stored in form: \"Name_of_namespace:Id_namespace\",\n+\t# Ex.: \"File:6\".\n+\tmy @tracked_namespaces = split(/[ \\n]/, run_git(\"config --get-all remote.\". $remotename .\".namespaces\"));\n+\tchomp(@tracked_namespaces);\n+\tif (@tracked_namespaces) {\n+\t\tforeach my $ns (@tracked_namespaces) {\n+\t\t\tmy @ns_split = split(/:/, $ns);\n+\t\t\tif ($ns_split[0] eq $name) {\n+\t\t\t\treturn $ns_split[1];\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t# NS not found => get namespace id from MW and store it in\n+\t# configuration file.\n+\tmy $query = {\n+\t\taction => 'query',\n+\t\tmeta => 'siteinfo',\n+\t\tsiprop => 'namespaces'\n+\t};\n+\tmy $result = $mediawiki->api($query);\n+\n+\twhile (my ($id, $ns) = each(%{$result->{query}->{namespaces}})) {\n+\t\tif (defined($ns->{canonical}) && ($ns->{canonical} eq $name)) {\n+\t\t\trun_git(\"config --add remote.\". $remotename .\".namespaces \". $name .\":\". $ns->{id});\n+\t\t\treturn $ns->{id};\n+\t\t}\n+\t}\n+\tdie \"Namespace $name was not found on MediaWiki.\";\n+}\n+\n+sub download_mw_mediafile {\n+\tmy $filename = shift;\n+\n+\t$mediawiki->{config}->{files_url} = $url;\n+\n+\tmy $file = $mediawiki->download( { title => $filename } );\n+\tif (!defined($file)) {\n+\t\tprint STDERR \"\\tFile \\'$filename\\' could not be downloaded.\\n\";\n+\t\texit 1;\n+\t} elsif ($file eq \"\") {\n+\t\tprint STDERR \"\\tFile \\'$filename\\' does not exist on the wiki.\\n\";\n+\t\texit 1;\n+\t} else {\n+\t\treturn $file;\n+\t}\n+}\n+\n+sub download_mw_mediafile_from_archive {\n+\tmy $url = shift;\n+\tmy $file;\n+\n+\tmy $ua = LWP::UserAgent->new;\n+\tmy $response = $ua->get($url);\n+\tif ($response->code) {\n+\t\t$file = $response->decoded_content;\n+\t} else {\n+\t\tprint STDERR \"Error downloading a file from archive.\\n\";\n+\t}\n+\n+\treturn $file;\n+}\n+\n sub run_git {\n \topen(my $git, \"-|:encoding(UTF-8)\", \"git \" . $_[0]);\n \tmy $res = do { local $/; <$git> };\n@@ -466,6 +607,13 @@ sub import_file_revision {\n \tmy %commit = %{$commit};\n \tmy $full_import = shift;\n \tmy $n = shift;\n+\tmy $mediafile_import = shift;\n+\tmy $mediafile;\n+\tmy %mediafile;\n+\tif ($mediafile_import) {\n+\t\t$mediafile = shift;\n+\t\t%mediafile = %{$mediafile};\n+\t}\n \n \tmy $title = $commit{title};\n \tmy $comment = $commit{comment};\n@@ -485,6 +633,10 @@ sub import_file_revision {\n \tif ($content ne DELETED_CONTENT) {\n \t\tprint STDOUT \"M 644 inline $title.mw\\n\";\n \t\tliteral_data($content);\n+\t\tif ($mediafile_import) {\n+\t\t\tprint STDOUT \"M 644 inline $mediafile{title}\\n\";\n+\t\t\tliteral_data($mediafile{content});\n+\t\t}\n \t\tprint STDOUT \"\\n\\n\";\n \t} else {\n \t\tprint STDOUT \"D $title.mw\\n\";\n@@ -525,6 +677,52 @@ sub get_more_refs {\n \t}\n }\n \n+sub get_mw_mediafile_for_page_revision {\n+\t# Name of the file on Wiki, with the prefix.\n+\tmy $mw_filename = shift;\n+\tmy $timestamp = shift;\n+\tmy %mediafile;\n+\n+\t# Search if on MediaWiki exists a media file with given\n+\t# timestamp. In that case download the file.\n+\tmy $query = {\n+\t\taction => 'query',\n+\t\tprop => 'imageinfo',\n+\t\ttitles => $mw_filename,\n+\t\tiistart => $timestamp,\n+\t\tiiend => $timestamp,\n+\t\tiiprop => 'timestamp|archivename|url',\n+\t\tiilimit => 1\n+\t};\n+\tmy $result = $mediawiki->api($query);\n+\n+\tmy ($fileid, $file) = each ( %{$result->{query}->{pages}} );\n+\t# If not defined it means there is no revision of the file for\n+\t# given timestamp.\n+\tif (defined($file->{imageinfo})) {\n+\t\t# Get real name of media file.\n+\t\tmy $filename;\n+\t\tif (index($mw_filename, 'File:') == 0) {\n+\t\t\t$filename = substr $mw_filename, 5;\n+\t\t} else {\n+\t\t\t$filename = substr $mw_filename, 6;\n+\t\t}\n+\t\t$mediafile{title} = $filename;\n+\n+\t\tmy $fileinfo = pop(@{$file->{imageinfo}});\n+\t\t$mediafile{timestamp} = $fileinfo->{timestamp};\n+\t\t# If this is an old version of the file, the file has to be\n+\t\t# obtained from the archive. Otherwise it can be downloaded\n+\t\t# by MediaWiki API download() function.\n+\t\tif (defined($fileinfo->{archivename})) {\n+\t\t\t$mediafile{content} = download_mw_mediafile_from_archive($fileinfo->{url});\n+\t\t} else {\n+\t\t\t$mediafile{content} = download_mw_mediafile($mw_filename);\n+\t\t}\n+\t}\n+\treturn %mediafile;\n+}\n+\n sub mw_import {\n \t# multiple import commands can follow each other.\n \tmy @refs = (shift, get_more_refs(\"import\"));\n@@ -580,6 +778,7 @@ sub mw_import_ref {\n \n \t\t$n++;\n \n+\t\tmy $page_title = $result->{query}->{pages}->{$pagerevid->{pageid}}->{title};\n \t\tmy %commit;\n \t\t$commit{author} = $rev->{user} || 'Anonymous';\n \t\t$commit{comment} = $rev->{comment} || '*Empty MediaWiki Message*';\n@@ -596,9 +795,25 @@ sub mw_import_ref {\n \t\t}\n \t\t$commit{date} = DateTime::Format::ISO8601->parse_datetime($last_timestamp);\n \n-\t\tprint STDERR \"$n/\", scalar(@revisions), \": Revision #$pagerevid->{revid} of $commit{title}\\n\";\n+\t\t# Differentiates classic pages and media files.\n+\t\tmy @prefix = split (\":\", $page_title);\n \n-\t\timport_file_revision(\\%commit, ($fetch_from == 1), $n);\n+\t\tmy %mediafile;\n+\t\tif ($prefix[0] eq \"File\" || $prefix[0] eq \"Image\") {\n+\t\t\t# The name of the file is the same as the media page.\n+\t\t\tmy $filename = $page_title;\n+\t\t\t%mediafile = get_mw_mediafile_for_page_revision($filename, $rev->{timestamp});\n+\t\t}\n+\t\t# If this is a revision of the media page for new version\n+\t\t# of a file do one common commit for both file and media page.\n+\t\t# Else do commit only for that page.\n+\t\tprint STDERR \"$n/\", scalar(@revisions), \": Revision #$pagerevid->{revid} of $commit{title}\\n\";\n+\t\tif (%mediafile) {\n+\t\t\tprint STDERR \"\\tDownloading file $mediafile{title}, version $mediafile{timestamp}\\n\";\n+\t\t\timport_file_revision(\\%commit, ($fetch_from == 1), $n, 1, \\%mediafile);\n+\t\t} else {\n+\t\t\timport_file_revision(\\%commit, ($fetch_from == 1), $n, 0);\n+\t\t}\n \t}\n \n \tif ($fetch_from == 1 && $n == 0) {\n-- \n1.7.10.2.552.gaa3bb87\n"},{"id":"193366","messageId":"1339442945-8561-2-git-send-email-Pavel.Volek@ensimag.imag.fr","threadId":"30772","inReplyTo":"1339442945-8561-1-git-send-email-Pavel.Volek@ensimag.imag.fr","subject":"[PATCHv2 2/2] git-remote-mediawiki: refactoring get_mw_pages function","fromName":"Pavel Volek","fromEmail":"pavel.volek@ensimag.imag.fr","sentAt":"2012-06-11T19:29:05Z","receivedAt":"2012-06-11T19:29:05Z","isPatch":false,"sender":{"key":"pavel.volek@ensimag.imag.fr","avatar":null},"body":"From: Volek Pavel <me@pavelvolek.cz>\n\nSplits the code in the get_mw_pages function into three separate functions.\nOne for getting list of all pages and all file attachments, second for pages\nin category specified in configuration file and files related to these pages\nand the last function to get from MW a list of specified pages with related\nfile attachments.\n\nSigned-off-by: Pavel Volek <Pavel.Volek@ensimag.imag.fr>\nSigned-off-by: NGUYEN Kim Thuat <Kim-Thuat.Nguyen@ensimag.imag.fr>\nSigned-off-by: ROUCHER IGLESIAS Javier <roucherj@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n contrib/mw-to-git/git-remote-mediawiki | 144 ++++++++++++++++++---------------\n 1 file changed, 79 insertions(+), 65 deletions(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki\nindex 14008ad..c0c0df7 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki\n+++ b/contrib/mw-to-git/git-remote-mediawiki\n@@ -212,91 +212,105 @@ sub get_mw_pages {\n \tmy $user_defined;\n \tif (@tracked_pages) {\n \t\t$user_defined = 1;\n-\t\t# The user provided a list of pages titles, but we\n-\t\t# still need to query the API to get the page IDs.\n-\n-\t\tmy @some_pages = @tracked_pages;\n-\t\twhile (@some_pages) {\n-\t\t\tmy $last = 50;\n-\t\t\tif ($#some_pages < $last) {\n-\t\t\t\t$last = $#some_pages;\n-\t\t\t}\n-\t\t\tmy @slice = @some_pages[0..$last];\n-\t\t\tget_mw_first_pages(\\@slice, \\%pages);\n-\t\t\t@some_pages = @some_pages[51..$#some_pages];\n-\t\t}\n-\n-\t\t# Get pages of related media files.\n-\t\tif ($import_media) {\n-\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n-\t\t}\n+\t\tget_mw_tracked_pages(\\%pages);\n \t}\n \tif (@tracked_categories) {\n \t\t$user_defined = 1;\n-\t\tforeach my $category (@tracked_categories) {\n-\t\t\tif (index($category, ':') < 0) {\n-\t\t\t\t# Mediawiki requires the Category\n-\t\t\t\t# prefix, but let's not force the user\n-\t\t\t\t# to specify it.\n-\t\t\t\t$category = \"Category:\" . $category;\n-\t\t\t}\n-\t\t\tmy $mw_pages = $mediawiki->list( {\n-\t\t\t\taction => 'query',\n-\t\t\t\tlist => 'categorymembers',\n-\t\t\t\tcmtitle => $category,\n-\t\t\t\tcmlimit => 'max' } )\n-\t\t\t    || die $mediawiki->{error}->{code} . ': ' . $mediawiki->{error}->{details};\n-\t\t\tforeach my $page (@{$mw_pages}) {\n-\t\t\t\t$pages{$page->{title}} = $page;\n-\t\t\t}\n-\n-\t\t\t# Get pages of related media files.\n-\t\t\tif ($import_media) {\n-\t\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n-\t\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%pages);\n-\t\t\t}\n-\t\t}\n+\t\tget_mw_tracked_categories(\\%pages);\n \t}\n \tif (!$user_defined) {\n-\t\t# No user-provided list, get the list of pages from\n-\t\t# the API.\n+\t\tget_mw_all_pages(\\%pages);\n+\t}\n+\treturn values(%pages);\n+}\n+\n+sub get_mw_all_pages {\n+\tmy $pages = shift;\n+\t# No user-provided list, get the list of pages from the API.\n+\tmy $mw_pages = $mediawiki->list({\n+\t\taction => 'query',\n+\t\tlist => 'allpages',\n+\t\taplimit => 500\n+\t});\n+\tif (!defined($mw_pages)) {\n+\t\tprint STDERR \"fatal: could not get the list of wiki pages.\\n\";\n+\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n+\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n+\t\texit 1;\n+\t}\n+\tforeach my $page (@{$mw_pages}) {\n+\t\t$pages->{$page->{title}} = $page;\n+\t}\n+\n+\tif ($import_media) {\n+\t\t# Attach list of all pages for meadia files from the API,\n+\t\t# they are in a different namespace, only one namespace\n+\t\t# can be queried at the same moment\n \t\tmy $mw_pages = $mediawiki->list({\n \t\t\taction => 'query',\n \t\t\tlist => 'allpages',\n-\t\t\taplimit => 500,\n+\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n+\t\t\taplimit => 500\n \t\t});\n \t\tif (!defined($mw_pages)) {\n-\t\t\tprint STDERR \"fatal: could not get the list of wiki pages.\\n\";\n+\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n \t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n \t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n \t\t\texit 1;\n \t\t}\n \t\tforeach my $page (@{$mw_pages}) {\n-\t\t\t$pages{$page->{title}} = $page;\n+\t\t\t$pages->{$page->{title}} = $page;\n+\t\t}\n+\t}\n+}\n+\n+sub get_mw_tracked_pages {\n+\tmy $pages = shift;\n+\t# The user provided a list of pages titles, but we\n+\t# still need to query the API to get the page IDs.\n+\tmy @some_pages = @tracked_pages;\n+\twhile (@some_pages) {\n+\t\tmy $last = 50;\n+\t\tif ($#some_pages < $last) {\n+\t\t\t$last = $#some_pages;\n \t\t}\n+\t\tmy @slice = @some_pages[0..$last];\n+\t\tget_mw_first_pages(\\@slice, \\%{$pages});\n+\t\t@some_pages = @some_pages[51..$#some_pages];\n+\t}\n+\n+\t# Get pages of related media files.\n+\tif ($import_media) {\n+\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%{$pages});\n+\t}\n+}\n \n+sub get_mw_tracked_categories {\n+\tmy $pages = shift;\n+\tforeach my $category (@tracked_categories) {\n+\t\tif (index($category, ':') < 0) {\n+\t\t\t# Mediawiki requires the Category\n+\t\t\t# prefix, but let's not force the user\n+\t\t\t# to specify it.\n+\t\t\t$category = \"Category:\" . $category;\n+\t\t}\n+\t\tmy $mw_pages = $mediawiki->list( {\n+\t\t\taction => 'query',\n+\t\t\tlist => 'categorymembers',\n+\t\t\tcmtitle => $category,\n+\t\t\tcmlimit => 'max' } )\n+\t\t\t|| die $mediawiki->{error}->{code} . ': '\n+\t\t\t\t. $mediawiki->{error}->{details};\n+\t\tforeach my $page (@{$mw_pages}) {\n+\t\t\t$pages->{$page->{title}} = $page;\n+\t\t}\n+\n+\t\t# Get pages of related media files.\n \t\tif ($import_media) {\n-\t\t\t# Attach list of all pages for meadia files from the API,\n-\t\t\t# they are in a different namespace, only one namespace\n-\t\t\t# can be queried at the same moment\n-\t\t\tmy $mw_pages = $mediawiki->list({\n-\t\t\t\taction => 'query',\n-\t\t\t\tlist => 'allpages',\n-\t\t\t\tapnamespace => get_mw_namespace_id(\"File\"),\n-\t\t\t\taplimit => 500\n-\t\t\t});\n-\t\t\tif (!defined($mw_pages)) {\n-\t\t\t\tprint STDERR \"fatal: could not get the list of pages for media files.\\n\";\n-\t\t\t\tprint STDERR \"fatal: '$url' does not appear to be a mediawiki\\n\";\n-\t\t\t\tprint STDERR \"fatal: make sure '$url/api.php' is a valid page.\\n\";\n-\t\t\t\texit 1;\n-\t\t\t}\n-\t\t\tforeach my $page (@{$mw_pages}) {\n-\t\t\t\t$pages{$page->{title}} = $page;\n-\t\t\t}\n+\t\t\tmy @titles = map $_->{title}, @{$mw_pages};\n+\t\t\tget_mw_pages_for_linked_mediafiles(\\@titles, \\%{$pages});\n \t\t}\n \t}\n-\treturn values(%pages);\n }\n \n sub get_mw_pages_for_linked_mediafiles {\n-- \n1.7.10.2.552.gaa3bb87\n"},{"id":"193377","messageId":"vpq7gvd1t4j.fsf@bauges.imag.fr","threadId":"30772","inReplyTo":"1339442945-8561-1-git-send-email-Pavel.Volek@ensimag.imag.fr","subject":"Re: [PATCHv2 1/2] git-remote-mediawiki: import \"File:\" attachments","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-11T20:38:36Z","receivedAt":"2012-06-11T20:38:36Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Pavel Volek <Pavel.Volek@ensimag.imag.fr> writes:\n\n> +\t\t# Get pages of related media files.\n> +\t\tif ($import_media) {\n> +\t\t\tget_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%pages);\n> +\t\t}\n\nThe comment is useless given the function name.\n\n> +# Returns MediaWiki id for a canonical namespace name.\n> +# Ex.: \"File\", \"Project\".\n> +# Looks for the namespace id in the local configuration\n> +# variables, if it is not found asks MW API.\n\nFunctions are usually specified in imperative form, hence \"Return\", not\n\"Returns\" for example.\n\n> +\tmy $file = $mediawiki->download( { title => $filename } );\n\nI'd call that $file_content, to avoid confusion with the file name.\n\nOther than that, the patch looks good (but I didn't review very\ncarefully).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"193423","messageId":"CA+hdvHhn1G=T=KjNvgXa-2M6oh2znbHmfZFZYPdqKhtybh_MoQ@mail.gmail.com","threadId":"30772","inReplyTo":"1339442945-8561-2-git-send-email-Pavel.Volek@ensimag.imag.fr","subject":"Re: [PATCHv2 2/2] git-remote-mediawiki: refactoring get_mw_pages function","fromName":"Simon Perrat","fromEmail":"simon.perrat@gmail.com","sentAt":"2012-06-12T09:06:07Z","receivedAt":"2012-06-12T09:06:07Z","isPatch":false,"sender":{"key":"simon.perrat@gmail.com","avatar":null},"body":"2012/6/11 Pavel Volek <Pavel.Volek@ensimag.imag.fr>:\n\n> +sub get_mw_all_pages {\n> +       my $pages = shift;\n> +       # No user-provided list, get the list of pages from the API.\n> +       my $mw_pages = $mediawiki->list({\n> +               action => 'query',\n> +               list => 'allpages',\n> +               aplimit => 500\n> +       });\n\nIndentation should be 8 columns wide.\n\n> +       if ($import_media) {\n> +               # Attach list of all pages for meadia files from the API,\n\nme*dia\n\n> +\n> +       # Get pages of related media files.\n\nThis comment seems to be paraphrasing the line below, could be removed maybe.\n\n> +       if ($import_media) {\n> +               get_mw_pages_for_linked_mediafiles(\\@tracked_pages, \\%{$pages});\n> +       }\n> +}\n\n\nNot much to say on this patch, as this is basically splitting existing code.\n"},{"id":"193425","messageId":"20120612112432.Horde.uOUWUHwdC4BP1wrQWv4jzXA@webmail.minatec.grenoble-inp.fr","threadId":"30772","inReplyTo":"CA+hdvHhn1G=T=KjNvgXa-2M6oh2znbHmfZFZYPdqKhtybh_MoQ@mail.gmail.com","subject":"Re: [PATCHv2 2/2] git-remote-mediawiki: refactoring get_mw_pages function","fromName":"","fromEmail":"konglu@minatec.inpg.fr","sentAt":"2012-06-12T09:24:32Z","receivedAt":"2012-06-12T09:24:32Z","isPatch":false,"sender":{"key":"konglu@minatec.inpg.fr","avatar":null},"body":"\nSimon Perrat <simon.perrat@gmail.com> a écrit :\n\n> 2012/6/11 Pavel Volek <Pavel.Volek@ensimag.imag.fr>:\n>\n>> +sub get_mw_all_pages {\n>> +       my $pages = shift;\n>> +       # No user-provided list, get the list of pages from the API.\n>> +       my $mw_pages = $mediawiki->list({\n>> +               action => 'query',\n>> +               list => 'allpages',\n>> +               aplimit => 500\n>> +       });\n>\n> Indentation should be 8 columns wide.\n\nIndentation is correct here.\n"},{"id":"193426","messageId":"vpqmx48x4o5.fsf@bauges.imag.fr","threadId":"30772","inReplyTo":"CA+hdvHhn1G=T=KjNvgXa-2M6oh2znbHmfZFZYPdqKhtybh_MoQ@mail.gmail.com","subject":"Re: [PATCHv2 2/2] git-remote-mediawiki: refactoring get_mw_pages function","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-06-12T09:25:46Z","receivedAt":"2012-06-12T09:25:46Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Simon Perrat <simon.perrat@gmail.com> writes:\n\n> 2012/6/11 Pavel Volek <Pavel.Volek@ensimag.imag.fr>:\n>\n>> +sub get_mw_all_pages {\n>> +       my $pages = shift;\n>> +       # No user-provided list, get the list of pages from the API.\n>> +       my $mw_pages = $mediawiki->list({\n>> +               action => 'query',\n>> +               list => 'allpages',\n>> +               aplimit => 500\n>> +       });\n>\n> Indentation should be 8 columns wide.\n\nMore precisely, indentation in Git is done with tabs, and should display\nwell with 1 tab == 8 characters.\n\nIt is actually the case in Pavel's patch. When seen as patches, the\nfirst column is used for + and -, so the first tab is rendered as only 7\ncharacters, but that's normal.\n\nOr did I miss something?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"193429","messageId":"CA+hdvHhvnku_h95LT7se7JWOZh4cG2UB4AfwAs2jV1j2hefDQw@mail.gmail.com","threadId":"30772","inReplyTo":"vpqmx48x4o5.fsf@bauges.imag.fr","subject":"Re: [PATCHv2 2/2] git-remote-mediawiki: refactoring get_mw_pages function","fromName":"Simon Perrat","fromEmail":"simon.perrat@gmail.com","sentAt":"2012-06-12T09:56:18Z","receivedAt":"2012-06-12T09:56:18Z","isPatch":false,"sender":{"key":"simon.perrat@gmail.com","avatar":null},"body":"2012/6/12 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:\n> Simon Perrat <simon.perrat@gmail.com> writes:\n>\n>> 2012/6/11 Pavel Volek <Pavel.Volek@ensimag.imag.fr>:\n>>\n>>> +sub get_mw_all_pages {\n>>> +       my $pages = shift;\n>>> +       # No user-provided list, get the list of pages from the API.\n>>> +       my $mw_pages = $mediawiki->list({\n>>> +               action => 'query',\n>>> +               list => 'allpages',\n>>> +               aplimit => 500\n>>> +       });\n>>\n>> Indentation should be 8 columns wide.\n>\n> More precisely, indentation in Git is done with tabs, and should display\n> well with 1 tab == 8 characters.\n>\n> It is actually the case in Pavel's patch. When seen as patches, the\n> first column is used for + and -, so the first tab is rendered as only 7\n> characters, but that's normal.\n>\n> Or did I miss something?\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n\nMy bad, I was mistaken by my mailer, which displayed these two lines\nwith a four-columns tab while the others were eight-columns.\n"}]}