git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing
@ 2013-06-13 10:07 benoit.person
  2013-06-13 10:07 ` [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm benoit.person
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: benoit.person @ 2013-06-13 10:07 UTC (permalink / raw)
  To: git; +Cc: Celestin Matte, Jeff King, Matthieu Moy, Benoit Person

From: Benoit Person <benoit.person@ensimag.fr>

The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
preview content without pushing would be a nice thing to have.

This serie is a second attempt to achieve it:
  - It adds a new GitMediawiki.pm package to share code between the new tool and 
    `git-remote-mediawiki.perl`. (PATCH 1 & 2)
  - It creates a new "meta"-command `git mw` with subcommand handling (PATCH 3)
  - It adds a new subcommand named `preview` to `git mw` (PATCH 4)

changes from the V0:
  - add new package GitMediawiki
    - move some of git-remote-mediawiki functions into the package
    - update git-remote-mediawiki to use those "moved" functions
    - add a hacky-way to install it in the Makefile
    - use it in the new git mw tool
  - add a way to give to the preview tool blobs as argument
  - add a fallback when the upstream's branch remote is not a mediawiki remote
  - update the `autoload` option to use `git web--browse` and not `xdg-open`
  - update the way we find the upstream's branch remote name

For now, this PATCH/RFC is based on the 'next' branch merged with the 
bp/mediawiki-credential patch. For the final version, I will try 
to rebase it on celestin's work with perlcritic.

[1] https://github.com/moy/Git-Mediawiki/issues/7

Benoit Person (4):
  git-mw: Introduction of GitMediawiki.pm
  git-mw: Moving some functions from git-remote-mediawiki.perl to
    GitMediawiki.pm
  git-mw: Adding git-mw.perl script
  git-mw: Adding preview tool in git-mw.perl

 contrib/mw-to-git/GitMediawiki.pm           |  94 +++++++++++
 contrib/mw-to-git/Makefile                  |  22 ++-
 contrib/mw-to-git/git-mw.perl               | 247 ++++++++++++++++++++++++++++
 contrib/mw-to-git/git-remote-mediawiki.perl |  80 ++-------
 4 files changed, 373 insertions(+), 70 deletions(-)
 create mode 100644 contrib/mw-to-git/GitMediawiki.pm
 create mode 100644 contrib/mw-to-git/git-mw.perl

-- 
1.8.3.GIT

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm
  2013-06-13 10:07 [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing benoit.person
@ 2013-06-13 10:07 ` benoit.person
  2013-06-13 11:44   ` Matthieu Moy
  2013-06-13 10:07 ` [PATCH/RFC 2/4] git-mw: Moving some functions from git-remote-mediawiki.perl to GitMediawiki.pm benoit.person
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: benoit.person @ 2013-06-13 10:07 UTC (permalink / raw)
  To: git; +Cc: Celestin Matte, Jeff King, Matthieu Moy, Benoit Person

From: Benoit Person <benoit.person@ensimag.fr>

The GitMediawiki.pm goal is to share code betwwen several scripts in
Git-Mediawiki (for now, git-mw.perl introduced in this patch serie and
git-remote-mediawiki.perl)

Signed-off-by: Benoit Person <benoit.person@ensimag.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>

---
 contrib/mw-to-git/GitMediawiki.pm | 24 ++++++++++++++++++++++++
 contrib/mw-to-git/Makefile        | 19 +++++++++++++++++--
 2 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 contrib/mw-to-git/GitMediawiki.pm

diff --git a/contrib/mw-to-git/GitMediawiki.pm b/contrib/mw-to-git/GitMediawiki.pm
new file mode 100644
index 0000000..8a0ffc7
--- /dev/null
+++ b/contrib/mw-to-git/GitMediawiki.pm
@@ -0,0 +1,24 @@
+package GitMediawiki;
+
+use 5.008;
+use strict;
+use Git;
+
+BEGIN {
+
+our ($VERSION, @ISA, @EXPORT, @EXPORT_OK);
+
+# Totally unstable API.
+$VERSION = '0.01';
+
+require Exporter;
+
+@ISA = qw(Exporter);
+
+@EXPORT = ();
+
+# Methods which can be called as standalone functions as well:
+@EXPORT_OK = ();
+}
+
+1; # Famous last words
\ No newline at end of file
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index f149719..fe30e7f 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -4,14 +4,29 @@
 #
 ## Build git-remote-mediawiki
 
+GIT_MEDIAWIKI_PM=GitMediawiki.pm
 SCRIPT_PERL=git-remote-mediawiki.perl
 GIT_ROOT_DIR=../..
 HERE=contrib/mw-to-git/
 
 SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \
+                -s --no-print-directory instlibdir)
 
 all: build
 
-build install clean:
+install_pm:
+	cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)
+
+build:
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+                build-perl-script
+
+install: install_pm
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+                install-perl-script
+
+clean:
 	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
-                $@-perl-script
+                clean-perl-script
+	rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
\ No newline at end of file
-- 
1.8.3.GIT

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH/RFC 2/4] git-mw: Moving some functions from git-remote-mediawiki.perl to GitMediawiki.pm
  2013-06-13 10:07 [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing benoit.person
  2013-06-13 10:07 ` [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm benoit.person
@ 2013-06-13 10:07 ` benoit.person
  2013-06-13 10:07 ` [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script benoit.person
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: benoit.person @ 2013-06-13 10:07 UTC (permalink / raw)
  To: git; +Cc: Celestin Matte, Jeff King, Matthieu Moy, Benoit Person

From: Benoit Person <benoit.person@ensimag.fr>

Moving mediawiki_clean_filename, mediawiki_smudge_filename and mw_connect_maybe
Since we have a clean namespace in a perl module, we also rename them into more
concise ones (clean_filename, smudge_filename and connect_maybe). It also
delete the side effects of mw_connect_maybe requiring it to be called with
parameters and returning the new instance of the Mediawiki::Api if it needs to
be created

Signed-off-by: Benoit Person <benoit.person@ensimag.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>

---
 contrib/mw-to-git/GitMediawiki.pm           | 72 +++++++++++++++++++++++++-
 contrib/mw-to-git/git-remote-mediawiki.perl | 80 +++++------------------------
 2 files changed, 84 insertions(+), 68 deletions(-)

diff --git a/contrib/mw-to-git/GitMediawiki.pm b/contrib/mw-to-git/GitMediawiki.pm
index 8a0ffc7..acf4e43 100644
--- a/contrib/mw-to-git/GitMediawiki.pm
+++ b/contrib/mw-to-git/GitMediawiki.pm
@@ -18,7 +18,77 @@ require Exporter;
 @EXPORT = ();
 
 # Methods which can be called as standalone functions as well:
-@EXPORT_OK = ();
+@EXPORT_OK = qw(clean_filename smudge_filename connect_maybe);
+}
+
+# Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced
+use constant SLASH_REPLACEMENT => '%2F';
+
+sub clean_filename {
+	my $filename = shift;
+	$filename =~ s{@{[SLASH_REPLACEMENT]}}{/}g;
+	# [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
+	# Do a variant of URL-encoding, i.e. looks like URL-encoding,
+	# but with _ added to prevent MediaWiki from thinking this is
+	# an actual special character.
+	$filename =~ s/[\[\]\{\}\|]/sprintf("_%%_%x", ord($&))/ge;
+	# If we use the uri escape before
+	# we should unescape here, before anything
+
+	return $filename;
+}
+
+sub smudge_filename {
+	my $filename = shift;
+	$filename =~ s{/}{@{[SLASH_REPLACEMENT]}}g;
+	$filename =~ s/ /_/g;
+	# Decode forbidden characters encoded in clean_filename
+	$filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf('%c', hex($1))/ge;
+	return $filename;
+}
+
+sub connect_maybe {
+	my $wiki = shift;
+	if ($wiki) {
+		return $wiki;
+	}
+
+	my $remote_name = shift;
+	my $remote_url = shift;
+	my ($wiki_login, $wiki_password, $wiki_domain);
+
+	git_cmd_try {
+		$wiki_login = Git::config("remote.${remote_name}.mwLogin");
+		$wiki_password = Git::config("remote.${remote_name}.mwPassword");
+		$wiki_domain = Git::config("remote.${remote_name}.mwDomain");}
+		"%s failed w/ code %d";
+
+	$wiki = MediaWiki::API->new;
+	$wiki->{config}->{api_url} = "${remote_url}/api.php";
+	if ($wiki_login) {
+		my %credential = (
+			'url' => $remote_url,
+			'username' => $wiki_login,
+			'password' => $wiki_password
+		);
+		Git::credential(\%credential);
+		my $request = {lgname => $credential{username},
+			       lgpassword => $credential{password},
+			       lgdomain => $wiki_domain};
+		if ($wiki->login($request)) {
+			Git::credential(\%credential, 'approve');
+			print {*STDERR} qq(Logged in mediawiki user "$credential{username}".\n);
+		} else {
+			print {*STDERR} qq(Failed to log in mediawiki user "$credential{username}" on ${remote_url}\n);
+			print {*STDERR} '  (error ' .
+				$wiki->{error}->{code} . ': ' .
+				$wiki->{error}->{details} . ")\n";
+			Git::credential(\%credential, 'reject');
+			exit 1;
+		}
+	}
+
+	return $wiki;
 }
 
 1; # Famous last words
\ No newline at end of file
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
index be17e89..d679035 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -13,6 +13,8 @@
 
 use strict;
 use MediaWiki::API;
+use Git;
+use GitMediawiki qw(clean_filename smudge_filename connect_maybe);
 use DateTime::Format::ISO8601;
 
 # By default, use UTF-8 to communicate with Git and the user
@@ -24,9 +26,6 @@ use IPC::Open2;
 
 use warnings;
 
-# Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced
-use constant SLASH_REPLACEMENT => "%2F";
-
 # It's not always possible to delete pages (may require some
 # privileges). Deleted pages are replaced with this content.
 use constant DELETED_CONTENT => "[[Category:Deleted]]\n";
@@ -159,36 +158,6 @@ while (<STDIN>) {
 # MediaWiki API instance, created lazily.
 my $mediawiki;
 
-sub mw_connect_maybe {
-	if ($mediawiki) {
-		return;
-	}
-	$mediawiki = MediaWiki::API->new;
-	$mediawiki->{config}->{api_url} = "$url/api.php";
-	if ($wiki_login) {
-		my %credential = (
-			'url' => $url,
-			'username' => $wiki_login,
-			'password' => $wiki_passwd
-		);
-		Git::credential(\%credential);
-		my $request = {lgname => $credential{username},
-			       lgpassword => $credential{password},
-			       lgdomain => $wiki_domain};
-		if ($mediawiki->login($request)) {
-			Git::credential(\%credential, 'approve');
-			print STDERR "Logged in mediawiki user \"$credential{username}\".\n";
-		} else {
-			print STDERR "Failed to log in mediawiki user \"$credential{username}\" on $url\n";
-			print STDERR "  (error " .
-				$mediawiki->{error}->{code} . ': ' .
-				$mediawiki->{error}->{details} . ")\n";
-			Git::credential(\%credential, 'reject');
-			exit 1;
-		}
-	}
-}
-
 sub fatal_mw_error {
 	my $action = shift;
 	print STDERR "fatal: could not $action.\n";
@@ -293,7 +262,7 @@ sub get_mw_first_pages {
 
 # Get the list of pages to be fetched according to configuration.
 sub get_mw_pages {
-	mw_connect_maybe();
+	$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 
 	print STDERR "Listing pages on remote wiki...\n";
 
@@ -480,7 +449,7 @@ my %basetimestamps;
 # avoid a loop onto all tracked pages. This is useful for the fetch-by-rev
 # option.
 sub get_last_global_remote_rev {
-	mw_connect_maybe();
+	$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 
 	my $query = {
 		action => 'query',
@@ -496,7 +465,7 @@ sub get_last_global_remote_rev {
 # Get the last remote revision concerning the tracked pages and the tracked
 # categories.
 sub get_last_remote_revision {
-	mw_connect_maybe();
+	$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 
 	my %pages_hash = get_mw_pages();
 	my @pages = values(%pages_hash);
@@ -552,29 +521,6 @@ sub mediawiki_smudge {
 	return $string."\n";
 }
 
-sub mediawiki_clean_filename {
-	my $filename = shift;
-	$filename =~ s/@{[SLASH_REPLACEMENT]}/\//g;
-	# [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
-	# Do a variant of URL-encoding, i.e. looks like URL-encoding,
-	# but with _ added to prevent MediaWiki from thinking this is
-	# an actual special character.
-	$filename =~ s/[\[\]\{\}\|]/sprintf("_%%_%x", ord($&))/ge;
-	# If we use the uri escape before
-	# we should unescape here, before anything
-
-	return $filename;
-}
-
-sub mediawiki_smudge_filename {
-	my $filename = shift;
-	$filename =~ s/\//@{[SLASH_REPLACEMENT]}/g;
-	$filename =~ s/ /_/g;
-	# Decode forbidden characters encoded in mediawiki_clean_filename
-	$filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf("%c", hex($1))/ge;
-	return $filename;
-}
-
 sub literal_data {
 	my ($content) = @_;
 	print STDOUT "data ", bytes::length($content), "\n", $content;
@@ -775,7 +721,7 @@ sub mw_import_ref {
 		return;
 	}
 
-	mw_connect_maybe();
+	$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 
 	print STDERR "Searching revisions...\n";
 	my $last_local = get_last_local_revision();
@@ -888,7 +834,7 @@ sub mw_import_revids {
 		my %commit;
 		$commit{author} = $rev->{user} || 'Anonymous';
 		$commit{comment} = $rev->{comment} || EMPTY_MESSAGE;
-		$commit{title} = mediawiki_smudge_filename($page_title);
+		$commit{title} = smudge_filename($page_title);
 		$commit{mw_revision} = $rev->{revid};
 		$commit{content} = mediawiki_smudge($rev->{'*'});
 
@@ -949,7 +895,7 @@ sub mw_upload_file {
 	}
 	# Deleting and uploading a file requires a priviledged user
 	if ($file_deleted) {
-		mw_connect_maybe();
+		$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 		my $query = {
 			action => 'delete',
 			title => $path,
@@ -965,7 +911,7 @@ sub mw_upload_file {
 		# Don't let perl try to interpret file content as UTF-8 => use "raw"
 		my $content = run_git("cat-file blob $new_sha1", "raw");
 		if ($content ne "") {
-			mw_connect_maybe();
+			$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 			$mediawiki->{config}->{upload_url} =
 				"$url/index.php/Special:Upload";
 			$mediawiki->edit({
@@ -1013,7 +959,7 @@ 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);
-	$complete_file_name = mediawiki_clean_filename($complete_file_name);
+	$complete_file_name = clean_filename($complete_file_name);
 
 	my ($title, $extension) = $complete_file_name =~ /^(.*)\.([^\.]*)$/;
 	if (!defined($extension)) {
@@ -1036,7 +982,7 @@ sub mw_push_file {
 			$file_content = run_git("cat-file blob $new_sha1");
 		}
 
-		mw_connect_maybe();
+		$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 
 		my $result = $mediawiki->edit( {
 			action => 'edit',
@@ -1220,7 +1166,7 @@ sub mw_push_revision {
 }
 
 sub get_allowed_file_extensions {
-	mw_connect_maybe();
+	$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 
 	my $query = {
 		action => 'query',
@@ -1244,7 +1190,7 @@ my %cached_mw_namespace_id;
 # Return MediaWiki id for a canonical namespace name.
 # Ex.: "File", "Project".
 sub get_mw_namespace_id {
-	mw_connect_maybe();
+	$mediawiki = connect_maybe($mediawiki, $remotename, $url);
 	my $name = shift;
 
 	if (!exists $namespace_id{$name}) {
-- 
1.8.3.GIT

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-13 10:07 [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing benoit.person
  2013-06-13 10:07 ` [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm benoit.person
  2013-06-13 10:07 ` [PATCH/RFC 2/4] git-mw: Moving some functions from git-remote-mediawiki.perl to GitMediawiki.pm benoit.person
@ 2013-06-13 10:07 ` benoit.person
  2013-06-13 13:01   ` Matthieu Moy
  2013-06-24 14:26   ` Matthieu Moy
  2013-06-13 10:07 ` [PATCH/RFC 4/4] git-mw: Adding preview tool in git-mw.perl benoit.person
  2013-06-13 11:23 ` [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing Matthieu Moy
  4 siblings, 2 replies; 17+ messages in thread
From: benoit.person @ 2013-06-13 10:07 UTC (permalink / raw)
  To: git; +Cc: Celestin Matte, Jeff King, Matthieu Moy, Benoit Person

From: Benoit Person <benoit.person@ensimag.fr>

This script will be used for all tools and command related to a mediawiki
remote. In this commit we introduce the tool, the way it parses argument
and subcommands and an example of subcommand: "help". It also updates
the Makefile so that the new tool is installed properly.

Signed-off-by: Benoit Person <benoit.person@ensimag.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>

---
 contrib/mw-to-git/Makefile    |  7 ++++---
 contrib/mw-to-git/git-mw.perl | 46 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 contrib/mw-to-git/git-mw.perl

diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index fe30e7f..c0633b1 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -6,6 +6,7 @@
 
 GIT_MEDIAWIKI_PM=GitMediawiki.pm
 SCRIPT_PERL=git-remote-mediawiki.perl
+SCRIPT_PERL+=git-mw.perl
 GIT_ROOT_DIR=../..
 HERE=contrib/mw-to-git/
 
@@ -19,14 +20,14 @@ install_pm:
 	cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)
 
 build:
-	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
                 build-perl-script
 
 install: install_pm
-	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
                 install-perl-script
 
 clean:
-	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
                 clean-perl-script
 	rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM)
\ No newline at end of file
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
new file mode 100644
index 0000000..a2f0aa1
--- /dev/null
+++ b/contrib/mw-to-git/git-mw.perl
@@ -0,0 +1,46 @@
+#!/usr/bin/perl
+
+# Copyright (C) 2013
+#     Benoit Person <benoit.person@ensimag.imag.fr>
+#     Celestin Matte <celestin.matte@ensimag.imag.fr>
+# License: GPL v2 or later
+
+# Set of tools for git repo with a mediawiki remote.
+# Documentation & bugtracker: https://github.com/moy/Git-Mediawiki/
+
+use strict;
+use warnings;
+
+use Getopt::Long;
+
+my %commands = (
+	'help' =>
+		[\&help, {}, \&help]
+);
+
+# Search for sub-command
+my $cmd = $commands{'help'};
+for (my $i = 0; $i < @ARGV; $i++) {
+	if (defined $commands{$ARGV[$i]}) {
+		$cmd = $commands{$ARGV[$i]};
+		splice @ARGV, $i, 1;
+		last;
+	}
+};
+GetOptions( %{$cmd->[1]},
+	'help|h' => \&{$cmd->[2]});
+
+# Launch command
+&{$cmd->[0]};
+
+############################## Help Functions ##################################
+
+sub help {
+	print <<'END';
+usage: git mw <command> <args>
+
+git mw commands are:
+    Help        Display help information about git mw
+END
+	exit;
+}
\ No newline at end of file
-- 
1.8.3.GIT

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH/RFC 4/4] git-mw: Adding preview tool in git-mw.perl
  2013-06-13 10:07 [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing benoit.person
                   ` (2 preceding siblings ...)
  2013-06-13 10:07 ` [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script benoit.person
@ 2013-06-13 10:07 ` benoit.person
  2013-06-13 12:13   ` Matthieu Moy
  2013-06-13 11:23 ` [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing Matthieu Moy
  4 siblings, 1 reply; 17+ messages in thread
From: benoit.person @ 2013-06-13 10:07 UTC (permalink / raw)
  To: git; +Cc: Celestin Matte, Jeff King, Matthieu Moy, Benoit Person

From: Benoit Person <benoit.person@ensimag.fr>

This final commit adds the preview subcommand to git mw. It works as such:
1- Find the remote name of the current branch's upstream and check if it's a
mediawiki one.
1b- If it's not found or if it's not a mediawiki one. It will list all the
mediawiki remotes configured and ask the user to replay the command with the
--remote option set.
2- Parse the content of the local file (or blob) (given as argument) using
the distant mediawiki's API
3- Retrieve the current page on the distant mediawiki
4- Replaces all content in that page with the newly parsed one
5- Convert relative links into absolute
6- Save the result on disk

The command accepts those options:
  --autoload | -a tries to launch the newly generated file in the user's
                  default browser (using git web--browse)
  --remote | -r provides a way to select the distant mediawiki in which
                the user wants to preview his file (or blob)
  --output | -o enables the user to choose the output filename. Default
                output filename is based on the input filename in which
                the extension '.mw' is replaced with '.html'
  --blob | -b tells the script that the last argument is a blob and not
              a filename

Signed-off-by: Benoit Person <benoit.person@ensimag.fr>
Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>

---
 contrib/mw-to-git/git-mw.perl | 203 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 202 insertions(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
index a2f0aa1..79c6cd0 100644
--- a/contrib/mw-to-git/git-mw.perl
+++ b/contrib/mw-to-git/git-mw.perl
@@ -12,10 +12,37 @@ use strict;
 use warnings;
 
 use Getopt::Long;
+use URI::URL qw(url);
+use IO::File;
+use LWP::Simple;
+use HTML::TreeBuilder;
+
+use Git;
+use MediaWiki::API;
+use GitMediawiki qw(smudge_filename connect_maybe);
+
+#preview parameters
+my $file_name;
+my $remote_name = '';
+my $preview_file_name = '';
+my $autoload = 0;
+my $blob = 0;
+sub file {
+	$file_name = shift;
+	return $file_name;
+}
 
 my %commands = (
 	'help' =>
-		[\&help, {}, \&help]
+		[\&help, {}, \&help],
+	'preview' =>
+		[\&preview, {
+			'<>' => \&file,
+			'output|o=s' => \$preview_file_name,
+			'remote|r=s' => \$remote_name,
+			'autoload|a' => \$autoload,
+			'blob|b' => \$blob
+		}, \&preview_help]
 );
 
 # Search for sub-command
@@ -33,6 +60,179 @@ GetOptions( %{$cmd->[1]},
 # Launch command
 &{$cmd->[0]};
 
+############################# Preview Functions ################################
+
+sub preview_help {
+	print <<'END';
+usage: git mw preview [--remote|-r <remote name>] [--autoload|-a]
+                      [--output|-o <output filename>] <filename>
+
+    -r, --remote    Specify which mediawiki should be used
+    -o, --output    Name of the output file
+    -a, --autoload  Autoload the page in your default web browser
+END
+	exit;
+}
+
+sub preview {
+	my $wiki;
+	my ($remote_url, $wiki_page_name);
+	my ($content, $content_tree, $template, $html_tree, $mw_content_text);
+	my $file_content;
+
+	# file_name argumeent is mandatory
+	if (! defined $file_name) {
+		die "File not set, see `git mw help` \n";
+	}
+
+	if ($blob) { # blob mode
+		$blob = $file_name;
+		if ($blob =~ /(.+):(.+)/) {
+			$file_name = $2;
+		}
+	} else { # file mode
+		if (! -e $file_name) {
+			die "File $file_name does not exists \n";
+		}
+	}
+
+	# Default preview_file_name is file_name with .html ext
+	if ($preview_file_name eq '') {
+		$preview_file_name = $file_name;
+		$preview_file_name =~ s/\.[^.]+$/.html/;
+	}
+
+	# Transform file_name into a mediawiki page name
+	$wiki_page_name = smudge_filename($file_name);
+	$wiki_page_name =~ s/\.[^.]+$//;
+
+	if ($remote_name eq '') {
+		# Search current branch upstream branch remote
+		$remote_name = git_cmd_try {
+			my $current_branch = 
+				Git::command_oneline('symbolic-ref', '--short', 'HEAD');
+			Git::config("branch.$current_branch.remote") }
+			"%s failed w/ code %d";
+
+		if ($remote_name) {
+			$remote_url = mediawiki_remote_url_maybe($remote_name);
+		}
+
+		# Search all possibles mediawiki remotes
+		if (! $remote_url) {
+			my @remotes = git_cmd_try {
+				Git::command('remote'); }
+				"%s failed w/ code%d";
+
+			my @valid_remotes = ();
+			foreach my $remote (@remotes) {
+				$remote_url = mediawiki_remote_url_maybe($remote);
+				if ($remote_url) {
+					push(@valid_remotes, $remote);
+				}
+			}
+
+			if ($#valid_remotes == 0) {
+				print "Can not find any mediawiki remote in this repo. \n";
+				exit 1;
+			} else {
+				print "There are multiple mediawiki remotes, which of:\n";
+				foreach my $remote (@remotes) {
+					print "\t$remote\n";
+				}
+				print "do you want ? Use the -r option to specify the remote\n";
+			}
+
+			exit 0;
+		}
+	} else {
+		# Check remote name
+		my @remotes = git_cmd_try {
+			Git::command('remote') }
+			"%s failed w/ code %d";
+		grep { $_ eq $remote_name } @remotes
+			or die "$remote_name is not a remote";
+	
+		# Find remote url
+		$remote_url = mediawiki_remote_url_maybe($remote_name);
+		if (! $remote_url) {
+			die "the remote you selected is not a mediawiki remote";
+		}
+	}
+
+	# Create and connect to the wiki if necessary
+	$wiki = connect_maybe($wiki, $remote_name, $remote_url);
+
+	# Read file content
+	if ($blob) {
+		$file_content = git_cmd_try {
+			Git::command('cat-file', 'blob', $blob); }
+			"%s failed w/ code %d";
+	} else {
+		open my $read_fh, "<", $file_name
+			or die "could not open $file_name: $!";
+		$file_content = do { local $/ = undef; <$read_fh> };
+		close $read_fh;
+	}
+
+	# Load template page
+	$template = get("$remote_url/index.php?title=$wiki_page_name")
+		or die "You need to create $wiki_page_name before previewing it";
+	$html_tree = HTML::TreeBuilder->new;
+	$html_tree->parse($template);
+
+	# Load new content
+	$content = $wiki->api({
+		action => 'parse',
+		text => $file_content,
+		title => $wiki_page_name
+	}) or die 'No response from distant mediawiki';
+	$content = $content->{'parse'}->{'text'}->{'*'};
+	$content_tree = HTML::TreeBuilder->new;
+	$content_tree->parse($content);
+
+	# Replace old content with new one
+	$mw_content_text = $html_tree->look_down('id', 'mw-content-text');
+	$mw_content_text->delete_content();
+	$mw_content_text->push_content($content_tree);
+
+	# Transform relative links into absolute ones
+	for (@{ $html_tree->extract_links() }) {
+		my ($link, $element, $attr) = @$_;
+		my $url = url($link)->canonical;
+		$element->attr($attr, URI->new_abs($url, $remote_url));
+	}
+
+	# Save the preview file
+	my $save_fh = IO::File->new($preview_file_name, 'w')
+		or die "Could not open: $!";
+	$save_fh->print($html_tree->as_HTML);
+
+	# Auto-loading in browser
+	if ($autoload) {
+		system('git', 'web--browse', $preview_file_name);
+	} else {
+		print "preview file saved as: $preview_file_name\n";
+	}
+
+	exit;
+}
+
+sub mediawiki_remote_url_maybe {
+	my $remote = shift;
+
+	# Find remote url
+	my $remote_url = git_cmd_try {
+		Git::config("remote.$remote.url") }
+		"%s failed w/ code %d";
+	if ($remote_url =~ s/mediawiki::(.*)/$1/) {
+		return url($remote_url)->canonical;
+	} else {
+		return undef;
+	}
+
+}
+
 ############################## Help Functions ##################################
 
 sub help {
@@ -41,6 +241,7 @@ usage: git mw <command> <args>
 
 git mw commands are:
     Help        Display help information about git mw
+    Preview 	Parse and render local file into HTML
 END
 	exit;
 }
\ No newline at end of file
-- 
1.8.3.GIT

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing
  2013-06-13 10:07 [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing benoit.person
                   ` (3 preceding siblings ...)
  2013-06-13 10:07 ` [PATCH/RFC 4/4] git-mw: Adding preview tool in git-mw.perl benoit.person
@ 2013-06-13 11:23 ` Matthieu Moy
  4 siblings, 0 replies; 17+ messages in thread
From: Matthieu Moy @ 2013-06-13 11:23 UTC (permalink / raw)
  To: benoit.person; +Cc: git, Celestin Matte, Jeff King

benoit.person@ensimag.fr writes:

> For now, this PATCH/RFC is based on the 'next' branch merged with the 
> bp/mediawiki-credential patch. For the final version, I will try 
> to rebase it on celestin's work with perlcritic.

Actually, it seems based on an old "next" branch. This hunk

--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -13,6 +13,8 @@
 
 use strict;
 use MediaWiki::API;
+use Git;

Fails to apply because "use Git;" is already there in today's next.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm
  2013-06-13 10:07 ` [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm benoit.person
@ 2013-06-13 11:44   ` Matthieu Moy
  0 siblings, 0 replies; 17+ messages in thread
From: Matthieu Moy @ 2013-06-13 11:44 UTC (permalink / raw)
  To: benoit.person; +Cc: git, Celestin Matte, Jeff King

benoit.person@ensimag.fr writes:

> +install_pm:
> +	cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)

Better use "install", which is roughly like "cp".

Also, this fails if the target dir does not exist, ie. if one did not
run "make install" at the toplevel Git. It's OK, but perhaps you should
add a comment in the Makefile like '# Run "make install" from Git's
toplevel before using this'.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 4/4] git-mw: Adding preview tool in git-mw.perl
  2013-06-13 10:07 ` [PATCH/RFC 4/4] git-mw: Adding preview tool in git-mw.perl benoit.person
@ 2013-06-13 12:13   ` Matthieu Moy
  0 siblings, 0 replies; 17+ messages in thread
From: Matthieu Moy @ 2013-06-13 12:13 UTC (permalink / raw)
  To: benoit.person; +Cc: git, Celestin Matte, Jeff King

benoit.person@ensimag.fr writes:

> From: Benoit Person <benoit.person@ensimag.fr>
>
> This final commit adds the preview subcommand to git mw. It works as such:
> 1- Find the remote name of the current branch's upstream and check if it's a
> mediawiki one.
> 1b- If it's not found or if it's not a mediawiki one. It will list all the
> mediawiki remotes configured and ask the user to replay the command with the
> --remote option set.
> 2- Parse the content of the local file (or blob) (given as argument) using
> the distant mediawiki's API
> 3- Retrieve the current page on the distant mediawiki
> 4- Replaces all content in that page with the newly parsed one
> 5- Convert relative links into absolute
> 6- Save the result on disk
>
> The command accepts those options:
>   --autoload | -a tries to launch the newly generated file in the user's
>                   default browser (using git web--browse)
>   --remote | -r provides a way to select the distant mediawiki in which
>                 the user wants to preview his file (or blob)
>   --output | -o enables the user to choose the output filename. Default
>                 output filename is based on the input filename in which
>                 the extension '.mw' is replaced with '.html'
>   --blob | -b tells the script that the last argument is a blob and not
>               a filename

A commit messages that answers the "what?" and "how?" questions (as
opposed to "why?") is always suspicious: doesn't the message belong
elsewhere?

Here, you have a nice user documentation for command-line options, and
the actual user doc is much poorer:

> +sub preview_help {
> +	print <<'END';
> +usage: git mw preview [--remote|-r <remote name>] [--autoload|-a]
> +                      [--output|-o <output filename>] <filename>
> +
> +    -r, --remote    Specify which mediawiki should be used
> +    -o, --output    Name of the output file
> +    -a, --autoload  Autoload the page in your default web browser
> +END

(shorter description, missing --blob)

> +	} else { # file mode
> +		if (! -e $file_name) {
> +			die "File $file_name does not exists \n";

We're just setting a convention to use ${var} in string interpolation
(Celestin's perlcritic patch series), so better do it right now ;-).

Did you try "make perlcritic" on your code?

> +	# Default preview_file_name is file_name with .html ext
> +	if ($preview_file_name eq '') {

EMPTY ?

> +	if ($remote_name eq '') {

EMPTY ?

> +	# Load template page
> +	$template = get("$remote_url/index.php?title=$wiki_page_name")
> +		or die "You need to create $wiki_page_name before previewing it";

I got hit again by the HTTPS certificate validation failure. It would
make sense to have a more detailed error message, including the URL,
because having the same error:

You need to create Accueil before previewing it at /home/moy/local/usr-wheezy/libexec/git-core/git-mw line 182.

for any kind of HTTP failure is a painful. Doesn't "get" return an HTTP
code? If so, your message would make sense for 404 errors, but not for
the others.

> +	$mw_content_text = $html_tree->look_down('id', 'mw-content-text');

Unfortunately, this doesn't seem standard. It doesn't work on
https://ensiwiki.ensimag.fr/index.php/Accueil at least (which is my main
use-case :-( ).

At least, you should check $mw_content_text and have a nice error
message here. As much as possible, you should allow a way to solve it
(make the lookup configurable in .git/config, or allow the user to
specify an arbitrary HTML template to plug onto, or display the raw,
incomplete, HTML).

I replaced 'mw-content-text' with 'bodyContent' and it worked.

Then I got

Wide character in print at /usr/lib/perl/5.14/IO/Handle.pm line 159.

but the file was generated. There are encoding problems: the title says
"Le Wiki des &Atilde;&copy;tudiants et enseignants" (it should be a É).

I guess you fed the API with an improper encoding (double UTF-8
encoding, or UTF-8 announced as latin-1 or so), and the API returned you
some hard-coded, badly encoded, rendered HTML.

> @@ -41,6 +241,7 @@ usage: git mw <command> <args>
>  
>  git mw commands are:
>      Help        Display help information about git mw
> +    Preview 	Parse and render local file into HTML
>  END

Lower-case help and preview.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-13 10:07 ` [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script benoit.person
@ 2013-06-13 13:01   ` Matthieu Moy
  2013-06-15 13:22     ` Benoît Person
  2013-06-24 14:26   ` Matthieu Moy
  1 sibling, 1 reply; 17+ messages in thread
From: Matthieu Moy @ 2013-06-13 13:01 UTC (permalink / raw)
  To: benoit.person; +Cc: git, Celestin Matte, Jeff King

benoit.person@ensimag.fr writes:

> From: Benoit Person <benoit.person@ensimag.fr>
>
> This script will be used for all tools and command related to a mediawiki
> remote. In this commit we introduce the tool, the way it parses argument
> and subcommands and an example of subcommand: "help". It also updates
> the Makefile so that the new tool is installed properly.

How does the "make" Vs "make install" work? How does a developer run the
tool without installing?

I first tried:

$ ../../bin-wrappers/git mw
git: 'mw' is not a git command. See 'git --help'.

Then, this first seem OK:

$ ./git-mw 
usage: git mw <command> <args>

git mw commands are:
    Help        Display help information about git mw
    Preview     Parse and render local file into HTML

BUT, this will take the installed GitMediawiki.pm if it is available,
and we don't want this (if one hacks GitMediawiki.pm locally, one wants
the new hacked to be taken into account without "make install"ing it).

To understand better how it works, try adding this in git-mw.perl:

  print "$_\n" for @INC;

I get this:

/home/moy/local/usr-squeeze/share/perl/5.14.2
/home/moy/local/usr-squeeze/src/MediaWiki-API-0.39/blib/lib
/etc/perl
/usr/local/lib/perl/5.14.2
/usr/local/share/perl/5.14.2
/usr/lib/perl5
/usr/share/perl5
/usr/lib/perl/5.14
/usr/share/perl/5.14
/usr/local/lib/site_perl
.

The '.' is there, but it comes after the hardcoded
/home/moy/local/usr-squeeze/share/perl/5.14.2 (which has to comes first,
to let the install version be robust to whatever comes after).

I think you need an equivalent of Git's toplevel bin-wrappers/git, or
perhaps use the same bin-wrapper/git but let "make install" in
contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib

BTW, I just noticed we had a Git::SVN, so perhaps GitMediawiki should be
Git::MediaWiki.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-13 13:01   ` Matthieu Moy
@ 2013-06-15 13:22     ` Benoît Person
  2013-06-16 19:59       ` Matthieu Moy
  0 siblings, 1 reply; 17+ messages in thread
From: Benoît Person @ 2013-06-15 13:22 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Celestin Matte, Jeff King

The V3 is ready, but I am still not sure about what is the best way to
do it for this issue though.

On 13 June 2013 15:01, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> benoit.person@ensimag.fr writes:
> How does the "make" Vs "make install" work? How does a developer run the
> tool without installing?
Well it does not work without installing but I think you know that now :)

> I first tried:
>
> $ ../../bin-wrappers/git mw
> git: 'mw' is not a git command. See 'git --help'.
>
> Then, this first seem OK:
>
> $ ./git-mw
> usage: git mw <command> <args>
>
> git mw commands are:
>     Help        Display help information about git mw
>     Preview     Parse and render local file into HTML
>
> BUT, this will take the installed GitMediawiki.pm if it is available,
> and we don't want this (if one hacks GitMediawiki.pm locally, one wants
> the new hacked to be taken into account without "make install"ing it).
>
> To understand better how it works, try adding this in git-mw.perl:
>
>   print "$_\n" for @INC;
>
> I get this:
>
> /home/moy/local/usr-squeeze/share/perl/5.14.2
> /home/moy/local/usr-squeeze/src/MediaWiki-API-0.39/blib/lib
> /etc/perl
> /usr/local/lib/perl/5.14.2
> /usr/local/share/perl/5.14.2
> /usr/lib/perl5
> /usr/share/perl5
> /usr/lib/perl/5.14
> /usr/share/perl/5.14
> /usr/local/lib/site_perl
> .
>
> The '.' is there, but it comes after the hardcoded
> /home/moy/local/usr-squeeze/share/perl/5.14.2 (which has to comes first,
> to let the install version be robust to whatever comes after).
Thanks for the explanations

> I think you need an equivalent of Git's toplevel bin-wrappers/git, or
> perhaps use the same bin-wrapper/git but let "make install" in
> contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib
Typo s/make install/make/ ?

For now, I have implemented that one : each time you do `make`, if
there is changes in GitMediawiki.pm, it gets copied to $GITPERLLIB
(perl/blib/lib). But I am not sure it's the best approach here. If we
want something entirely self-contained for GitMediawiki, creating a
new git wrapper seems like the best way. But then, we could say that
since GitMediawiki Makefile uses Git toplevel Makefile, it's not
entirely self-contained :/ maybe it's the "copying file" that makes it
weird ?

> BTW, I just noticed we had a Git::SVN, so perhaps GitMediawiki should be
> Git::MediaWiki.
For that one, I am not really sure Git::Mediawiki makes more sense
than GitMediawiki. The point of the GitMediawiki.pm package is to
contain all the stuff for the bidirectionnal-thingy. So they are not
really Git-related, nor Mediawiki-related. Making it part of a "Git"
directory / namespace does not really feels right, even if it's how
it's done for SVN :/ .

Thank you for the all the reviews, (it works for Ensiwiki now \o/)

Benoit Person

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-15 13:22     ` Benoît Person
@ 2013-06-16 19:59       ` Matthieu Moy
  0 siblings, 0 replies; 17+ messages in thread
From: Matthieu Moy @ 2013-06-16 19:59 UTC (permalink / raw)
  To: Benoît Person; +Cc: git, Celestin Matte, Jeff King

Benoît Person <benoit.person@ensimag.fr> writes:

>> I think you need an equivalent of Git's toplevel bin-wrappers/git, or
>> perhaps use the same bin-wrapper/git but let "make install" in
>> contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib
> Typo s/make install/make/ ?

Yes.

> For that one, I am not really sure Git::Mediawiki makes more sense
> than GitMediawiki. The point of the GitMediawiki.pm package is to
> contain all the stuff for the bidirectionnal-thingy. So they are not
> really Git-related, nor Mediawiki-related.

I'd say they are related to both, not to neither.

> Making it part of a "Git" directory / namespace does not really feels
> right, even if it's how it's done for SVN :/ .

Well, it's the part of Git's perl library which deals with SVN
interaction, so it makes sense to have it be a subdirectory of Git. I'd
say it makes as much sense for Mediawiki interaction.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-13 10:07 ` [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script benoit.person
  2013-06-13 13:01   ` Matthieu Moy
@ 2013-06-24 14:26   ` Matthieu Moy
  2013-06-24 16:38     ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Matthieu Moy @ 2013-06-24 14:26 UTC (permalink / raw)
  To: benoit.person; +Cc: git, Celestin Matte, Jeff King

benoit.person@ensimag.fr writes:

> diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
> new file mode 100644
> index 0000000..a2f0aa1
> --- /dev/null
> +++ b/contrib/mw-to-git/git-mw.perl

*.perl scripts are usually executable in Git's tree (although it's
usually better to run the non-*.perl version).

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-24 14:26   ` Matthieu Moy
@ 2013-06-24 16:38     ` Junio C Hamano
  2013-06-24 16:56       ` Matthieu Moy
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2013-06-24 16:38 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: benoit.person, git, Celestin Matte, Jeff King

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> benoit.person@ensimag.fr writes:
>
>> diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
>> new file mode 100644
>> index 0000000..a2f0aa1
>> --- /dev/null
>> +++ b/contrib/mw-to-git/git-mw.perl
>
> *.perl scripts are usually executable in Git's tree (although it's
> usually better to run the non-*.perl version).

Good eyes.  But if we encourage people to run non-*.perl version,
perhaps we should drop the executable bit from the source, no?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-24 16:38     ` Junio C Hamano
@ 2013-06-24 16:56       ` Matthieu Moy
  2013-06-24 17:00         ` Jeff King
                           ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Matthieu Moy @ 2013-06-24 16:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: benoit.person, git, Celestin Matte, Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> benoit.person@ensimag.fr writes:
>>
>>> diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
>>> new file mode 100644
>>> index 0000000..a2f0aa1
>>> --- /dev/null
>>> +++ b/contrib/mw-to-git/git-mw.perl
>>
>> *.perl scripts are usually executable in Git's tree (although it's
>> usually better to run the non-*.perl version).
>
> Good eyes.  But if we encourage people to run non-*.perl version,
> perhaps we should drop the executable bit from the source, no?

But by default, I'd say consistency is most important so if other *.perl
are executable, we should do the same (otherwise my "ls" shows different
colors and it's ugly ;-) ).

But it may make sense to change the convention, i.e. run a "chmod -x
*.perl" in Git's tree (in any case, people can still run "perl
foo.perl").

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-24 16:56       ` Matthieu Moy
@ 2013-06-24 17:00         ` Jeff King
  2013-06-24 17:05         ` Benoit Person
  2013-06-24 17:23         ` Junio C Hamano
  2 siblings, 0 replies; 17+ messages in thread
From: Jeff King @ 2013-06-24 17:00 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Junio C Hamano, benoit.person, git, Celestin Matte

On Mon, Jun 24, 2013 at 06:56:17PM +0200, Matthieu Moy wrote:

> > Good eyes.  But if we encourage people to run non-*.perl version,
> > perhaps we should drop the executable bit from the source, no?
> 
> But by default, I'd say consistency is most important so if other *.perl
> are executable, we should do the same (otherwise my "ls" shows different
> colors and it's ugly ;-) ).
> 
> But it may make sense to change the convention, i.e. run a "chmod -x
> *.perl" in Git's tree (in any case, people can still run "perl
> foo.perl").

You'd probably want to also change the shell scripts, too, which are
marked executable in the repo (but the source-able shell "libraries" are
not).

I don't remember the details, but I think there may be some magic with
the --valgrind test option that depends on the executable bit to
distinguish those two.

-Peff

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-24 16:56       ` Matthieu Moy
  2013-06-24 17:00         ` Jeff King
@ 2013-06-24 17:05         ` Benoit Person
  2013-06-24 17:23         ` Junio C Hamano
  2 siblings, 0 replies; 17+ messages in thread
From: Benoit Person @ 2013-06-24 17:05 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Junio C Hamano, benoit.person, git, Celestin Matte, Jeff King

Le 2013-06-24 18:56, Matthieu Moy a écrit :
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> benoit.person@ensimag.fr writes:
>>>
>>>> diff --git a/contrib/mw-to-git/git-mw.perl 
>>>> b/contrib/mw-to-git/git-mw.perl
>>>> new file mode 100644
>>>> index 0000000..a2f0aa1
>>>> --- /dev/null
>>>> +++ b/contrib/mw-to-git/git-mw.perl
>>>
>>> *.perl scripts are usually executable in Git's tree (although it's
>>> usually better to run the non-*.perl version).
>>
>> Good eyes.  But if we encourage people to run non-*.perl version,
>> perhaps we should drop the executable bit from the source, no?
>
> But by default, I'd say consistency is most important so if other 
> *.perl
> are executable, we should do the same (otherwise my "ls" shows 
> different
> colors and it's ugly ;-) ).
So it's really a *nice* catch then :) .

> But it may make sense to change the convention, i.e. run a "chmod -x
> *.perl" in Git's tree (in any case, people can still run "perl
> foo.perl").
For what I've seen so far of git.git, the best way would be to make it
executable in this patch serie and send another patch applying that
'chmod -x'-thingy ?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script
  2013-06-24 16:56       ` Matthieu Moy
  2013-06-24 17:00         ` Jeff King
  2013-06-24 17:05         ` Benoit Person
@ 2013-06-24 17:23         ` Junio C Hamano
  2 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2013-06-24 17:23 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: benoit.person, git, Celestin Matte, Jeff King

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> benoit.person@ensimag.fr writes:
>>>
>>>> diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
>>>> new file mode 100644
>>>> index 0000000..a2f0aa1
>>>> --- /dev/null
>>>> +++ b/contrib/mw-to-git/git-mw.perl
>>>
>>> *.perl scripts are usually executable in Git's tree (although it's
>>> usually better to run the non-*.perl version).
>>
>> Good eyes.  But if we encourage people to run non-*.perl version,
>> perhaps we should drop the executable bit from the source, no?
>
> But by default, I'd say consistency is most important so if other *.perl
> are executable, we should do the same.

Ah, I should have been more clear.  I meant "perhaps we should drop
executable bits from existing *.perl source files."

> But it may make sense to change the convention, i.e. run a "chmod -x
> *.perl" in Git's tree (in any case, people can still run "perl
> foo.perl").

Yes, that is exactly what I meant.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-06-24 17:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-13 10:07 [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing benoit.person
2013-06-13 10:07 ` [PATCH/RFC 1/4] git-mw: Introduction of GitMediawiki.pm benoit.person
2013-06-13 11:44   ` Matthieu Moy
2013-06-13 10:07 ` [PATCH/RFC 2/4] git-mw: Moving some functions from git-remote-mediawiki.perl to GitMediawiki.pm benoit.person
2013-06-13 10:07 ` [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script benoit.person
2013-06-13 13:01   ` Matthieu Moy
2013-06-15 13:22     ` Benoît Person
2013-06-16 19:59       ` Matthieu Moy
2013-06-24 14:26   ` Matthieu Moy
2013-06-24 16:38     ` Junio C Hamano
2013-06-24 16:56       ` Matthieu Moy
2013-06-24 17:00         ` Jeff King
2013-06-24 17:05         ` Benoit Person
2013-06-24 17:23         ` Junio C Hamano
2013-06-13 10:07 ` [PATCH/RFC 4/4] git-mw: Adding preview tool in git-mw.perl benoit.person
2013-06-13 12:13   ` Matthieu Moy
2013-06-13 11:23 ` [PATCH/RFC V2 0/4] git-remote-mediawiki: new tool to preview local changes without pushing Matthieu Moy

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