[PATCH] git svn dcommit : new option --interactive

[PATCH] git svn dcommit : new option --interactive

[Frédéric Heitzmann]

I use git svn daily, at work, and it is so useful that I started using it to
track some files which are not supposed to find their way up to the SVN
server.
For instance, it may be useful to 'git add & commit' some reference data
files, refactor some code, check against the data files, git commit, ..., then discard
commits with data files.
This way, it is very easy to monitor the functional changes in your code.

Unfortunately, it may happen that I forget to remove useless commits, and 'git
svn dcommit' everything ... forever.

These 2 patches add a --interactive option to 'git svn dcommit', which provides
an interactive mode, similar to git-send-email.

 Documentation/git-svn.txt              |    8 ++++
 git-svn.perl                           |   71 +++++++++++++++++++++++++++++++-
 t/t9160-git-svn-dcommit-interactive.sh |   64 ++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletions(-)

[PATCH 1/2] git svn dcommit: new option --interactive.

[Frédéric Heitzmann]

Allow the user to check the patch set before it is commited to SNV. It is then
possible to accept/discard one patch, accept all, or quit.

This interactive mode is similar with 'git send email' behaviour. However,
'git svn dcommit' returns as soon as one patch is discarded.

Part of the code was taken from git-send-email.perl

Thanks-to: Eric Wong <normalperson@yhbt.net> for the initial idea.
Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---

 I would have preferred not duplicating the code snippets taken from
 git-send-email ('ask' function, Term related code, ...) but I preferred not
 to spoil Git.pm with it.
 Any comment on a better way to factor perl code would be appreciated.

 Documentation/git-svn.txt |    8 +++++
 git-svn.perl              |   71 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index ed5eca1..08188a5 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -214,6 +214,14 @@ discouraged.
 	version 1.5 can make use of it. 'git svn' currently does not use it
 	and does not set it automatically.
 
+--interactive;;
+	Ask the user to confirm that a patch set should actually be sent to SVN.
+	For each patch, one may answer "yes" (accept this patch), "no" (discard this
+	patch), "all" (accept all patches), or "quit".
+	+
+	'git svn dcommit' returns immediately if answer if "no" or "quit", without
+	commiting anything to SVN.
+
 'branch'::
 	Create a branch in the SVN repository.
 
diff --git a/git-svn.perl b/git-svn.perl
index 89f83fd..fd5eaa2 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -87,7 +87,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase, $_fetch_parent,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format, $_commit_url, $_tag, $_merge_info);
+	$_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);
 $Git::SVN::_follow_parent = 1;
 $_q ||= 0;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
@@ -158,6 +158,7 @@ my %cmd = (
 			  'revision|r=i' => \$_revision,
 			  'no-rebase' => \$_no_rebase,
 			  'mergeinfo=s' => \$_merge_info,
+			  'interactive|i' => \$_interactive,
 			%cmt_opts, %fc_opts } ],
 	branch => [ \&cmd_branch,
 	            'Create a branch in the SVN repository',
@@ -251,6 +252,27 @@ my %cmd = (
 		{} ],
 );
 
+use Term::ReadLine;
+package FakeTerm;
+sub new {
+	my ($class, $reason) = @_;
+	return bless \$reason, shift;
+}
+sub readline {
+	my $self = shift;
+	die "Cannot use readline on FakeTerm: $$self";
+}
+package main;
+
+my $term = eval {
+	$ENV{"GIT_SVN_NOTTY"}
+		? new Term::ReadLine 'git-svn', \*STDIN, \*STDOUT
+		: new Term::ReadLine 'git-svn';
+};
+if ($@) {
+	$term = new FakeTerm "$@: going non-interactive";
+}
+
 my $cmd;
 for (my $i = 0; $i < @ARGV; $i++) {
 	if (defined $cmd{$ARGV[$i]}) {
@@ -361,6 +383,31 @@ sub version {
 	exit 0;
 }
 
+sub ask {
+	my ($prompt, %arg) = @_;
+	my $valid_re = $arg{valid_re};
+	my $default = $arg{default};
+	my $resp;
+	my $i = 0;
+	return defined $default ? $default : undef
+		unless defined $term->IN and defined fileno($term->IN) and
+		       defined $term->OUT and defined fileno($term->OUT);
+	while ($i++ < 10) {
+		$resp = $term->readline($prompt);
+		if (!defined $resp) { # EOF
+			print "\n";
+			return defined $default ? $default : undef;
+		}
+		if ($resp eq '' and defined $default) {
+			return $default;
+		}
+		if (!defined $valid_re or $resp =~ /$valid_re/) {
+			return $resp;
+		}
+	}
+	return undef;
+}
+
 sub do_git_init_db {
 	unless (-d $ENV{GIT_DIR}) {
 		my @init_db = ('init');
@@ -546,6 +593,28 @@ sub cmd_dcommit {
 		     "If these changes depend on each other, re-running ",
 		     "without --no-rebase may be required."
 	}
+
+	if (defined $_interactive){
+		my $ask_default = "y";
+		foreach my $d (@$linear_refs){
+			print "debug : d = $d\n";
+			my ($fh, $ctx) = command_output_pipe(qw(show --summary), "$d");
+			while (<$fh>){
+				print $_;
+			}
+			command_close_pipe($fh, $ctx);
+			$_ = ask("Commit this patch to SVN? ([y]es (default)|[n]o|[q]uit|[a]ll): ",
+			         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
+			         default => $ask_default);
+			die "Commit this patch reply required" unless defined $_;
+			if (/^[nq]/i) {
+				exit(0);
+			} elsif (/^a/i) {
+				last;
+			}
+		}
+	}
+
 	my $expect_url = $url;
 	Git::SVN::remove_username($expect_url);
 	while (1) {
-- 
1.7.6.447.gb9176

[PATCH 2/2] git svn dcommit: add a test serie for 'git svn dcommit --interactive'

[Frédéric Heitzmann]

test several combinations of potential answers to 'git svn dcommit
--interactive'. For each of them, test whether patches were commited to SVN or
not

Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>
---
 t/t9160-git-svn-dcommit-interactive.sh |   64 ++++++++++++++++++++++++++++++++
 1 files changed, 64 insertions(+), 0 deletions(-)
 create mode 100644 t/t9160-git-svn-dcommit-interactive.sh

diff --git a/t/t9160-git-svn-dcommit-interactive.sh b/t/t9160-git-svn-dcommit-interactive.sh
new file mode 100644
index 0000000..e38d9fa
--- /dev/null
+++ b/t/t9160-git-svn-dcommit-interactive.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+#
+# Copyright (c) 2011 Frédéric Heitzmann
+
+test_description='git svn dcommit --interactive series'
+. ./lib-git-svn.sh
+
+test_expect_success 'initialize repo' '
+	svn_cmd mkdir -m"mkdir test-interactive" "$svnrepo/test-interactive" &&
+	git svn clone "$svnrepo/test-interactive" test-interactive &&
+	cd test-interactive &&
+	touch foo && git add foo && git commit -m"foo: first commit" &&
+	git svn dcommit
+	'
+
+test_expect_success 'answers: y [\n] yes' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "y
+
+y" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD) = $(git rev-parse remotes/git-svn)
+	)
+	'
+
+test_expect_success 'answers: yes yes no' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "yes
+yes
+no" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD^^^) = $(git rev-parse remotes/git-svn) &&
+		git reset --hard remotes/git-svn
+	)
+	'
+
+test_expect_success 'answers: yes quit' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "yes
+quit" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD^^^) = $(git rev-parse remotes/git-svn) &&
+		git reset --hard remotes/git-svn
+	)
+	'
+
+test_expect_success 'answers: all' '
+	(
+		echo "change #1" >> foo && git commit -a -m"change #1" &&
+		echo "change #2" >> foo && git commit -a -m"change #2" &&
+		echo "change #3" >> foo && git commit -a -m"change #3" &&
+		( echo "all" | GIT_SVN_NOTTY=1 git svn dcommit --interactive ) &&
+		test $(git rev-parse HEAD) = $(git rev-parse remotes/git-svn) &&
+		git reset --hard remotes/git-svn
+	)
+	'
+
+test_done
-- 
1.7.6.447.gb9176

Re: [PATCH 1/2] git svn dcommit: new option --interactive.

[Eric Wong]

Frédéric Heitzmann <frederic.heitzmann@gmail.com> wrote:
> Allow the user to check the patch set before it is commited to SNV. It is then
> possible to accept/discard one patch, accept all, or quit.
> 
> This interactive mode is similar with 'git send email' behaviour. However,
> 'git svn dcommit' returns as soon as one patch is discarded.
> 
> Part of the code was taken from git-send-email.perl

> Thanks-to: Eric Wong <normalperson@yhbt.net> for the initial idea.
> Signed-off-by: Frédéric Heitzmann <frederic.heitzmann@gmail.com>

I agree with this feature, a few comments inline.

>  I would have preferred not duplicating the code snippets taken from
>  git-send-email ('ask' function, Term related code, ...) but I preferred not
>  to spoil Git.pm with it.
>  Any comment on a better way to factor perl code would be appreciated.

We should put this into Git.pm at some point.
(Somebody should refactor git-svn.perl into separate files too... :x)

>  Documentation/git-svn.txt |    8 +++++
>  git-svn.perl              |   71 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 78 insertions(+), 1 deletions(-)

Tests and feature should be the same patch

> +	return defined $default ? $default : undef
> +		unless defined $term->IN and defined fileno($term->IN) and
> +		       defined $term->OUT and defined fileno($term->OUT);

Things to make life easier for (mainly) C programmers:

* Use C-style "&&" and "||" for conditionals.  "and" and "or" are lower
  precedence and better used for control flow (see perlop(1) manpage).

* Also, use parentheses for defined(foo) to disambiguate multiple
  conditions/statements.

-- 
Eric Wong

Re: [PATCH 1/2] git svn dcommit: new option --interactive.

[Frédéric Heitzmann]

Le 06/09/2011 22:26, Eric Wong a écrit :
> Frédéric Heitzmann<frederic.heitzmann@gmail.com>  wrote:
>> Allow the user to check the patch set before it is commited to SNV. It is then
>> possible to accept/discard one patch, accept all, or quit.
>>
>> This interactive mode is similar with 'git send email' behaviour. However,
>> 'git svn dcommit' returns as soon as one patch is discarded.
>>
>> Part of the code was taken from git-send-email.perl
>> Thanks-to: Eric Wong<normalperson@yhbt.net>  for the initial idea.
>> Signed-off-by: Frédéric Heitzmann<frederic.heitzmann@gmail.com>
> I agree with this feature, a few comments inline.
>
>>   I would have preferred not duplicating the code snippets taken from
>>   git-send-email ('ask' function, Term related code, ...) but I preferred not
>>   to spoil Git.pm with it.
>>   Any comment on a better way to factor perl code would be appreciated.
> We should put this into Git.pm at some point.
> (Somebody should refactor git-svn.perl into separate files too... :x)
>
>>   Documentation/git-svn.txt |    8 +++++
>>   git-svn.perl              |   71 ++++++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 78 insertions(+), 1 deletions(-)
> Tests and feature should be the same patch
>> +	return defined $default ? $default : undef
>> +		unless defined $term->IN and defined fileno($term->IN) and
>> +		       defined $term->OUT and defined fileno($term->OUT);
> Things to make life easier for (mainly) C programmers:
>
> * Use C-style "&&" and "||" for conditionals.  "and" and "or" are lower
>    precedence and better used for control flow (see perlop(1) manpage).
>
> * Also, use parentheses for defined(foo) to disambiguate multiple
>    conditions/statements.
>
My fault : I copied-pasted the 'ask' function from git-send-email.
Even if I rewrite it a litlle, it should not prevent anyone to mutalize 
some code into Git.pm.
And, indeed, it will improve readability.

I wait a few days to see if anyone else has some comments and I send a 
V2 patch serie.

Thanks for reviewing.

--
Fred