git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 10/9 v5] difftool: fix regression in '--prompt' options
@ 2012-03-22 15:02 Tim Henigan
  2012-03-22 15:02 ` [PATCH 12/9 v5] t7800: add tests for difftool --dir-diff Tim Henigan
  2012-03-22 15:02 ` [PATCH 13/9 v5] difftool: add '--no-gui' as an option Tim Henigan
  0 siblings, 2 replies; 3+ messages in thread
From: Tim Henigan @ 2012-03-22 15:02 UTC (permalink / raw)
  To: gitster, git, davvid; +Cc: Tim Henigan

When difftool was changed to use Getopt::Long, it changed the way
that the '--prompt' and '--no-prompt' options were handled. The
expected behavior is that the two options may be given any number
of times. The last option given "wins".

For example, if a user sets "[alias] mdt = difftool --prompt", the
following must still run without error:

$ git mdt --no-prompt

The changes made during the switch to Getopt::Long broke this
behavior. This commit teaches difftool to handle them properly
again.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

This replaces 10/9 in the previous version of the series.

Changes in v5:
  - Based on an example provided by Thomas Rast on the Git dev list,
    the '--[no-]prompt' options are parsed by Getopt::Long again.


 git-difftool.perl |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 9f0f9a9..9892d1e 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -29,8 +29,8 @@ sub usage
 	my $exitcode = shift;
 	print << 'USAGE';
 usage: git difftool [-t|--tool=<tool>] [--tool-help]
-                    [-x|--extcmd=<cmd>]
-                    [-y|--no-prompt]   [-g|--gui]
+                    [-x|--extcmd=<cmd>] [-g|--gui]
+                    [--prompt] [-y|--no-prompt]
                     [-d|--dir-diff]
                     ['git diff' options]
 USAGE
@@ -131,15 +131,15 @@ sub setup_dir_diff
 
 # parse command-line options. all unrecognized options and arguments
 # are passed through to the 'git diff' command.
-my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $no_prompt, $prompt, $tool_help);
+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help);
 GetOptions('g|gui' => \$gui,
 	'd|dir-diff' => \$dirdiff,
 	'h' => \$help,
-	'prompt' => \$prompt,
+	'prompt!' => \$prompt,
+	'y' => sub { $prompt = 0; },
 	't|tool:s' => \$difftool_cmd,
 	'tool-help' => \$tool_help,
-	'x|extcmd:s' => \$extcmd,
-	'y|no-prompt' => \$no_prompt);
+	'x|extcmd:s' => \$extcmd);
 
 if (defined($help)) {
 	usage(0);
@@ -203,10 +203,11 @@ if (defined($dirdiff)) {
 	}
 } else {
 	if (defined($prompt)) {
-		$ENV{GIT_DIFFTOOL_PROMPT} = 'true';
-	}
-	elsif (defined($no_prompt)) {
-		$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+		if ($prompt) {
+			$ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+		} else {
+			$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+		}
 	}
 
 	$ENV{GIT_PAGER} = '';
-- 
1.7.10.rc1.40.g756bbcd

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

* [PATCH 12/9 v5] t7800: add tests for difftool --dir-diff
  2012-03-22 15:02 [PATCH 10/9 v5] difftool: fix regression in '--prompt' options Tim Henigan
@ 2012-03-22 15:02 ` Tim Henigan
  2012-03-22 15:02 ` [PATCH 13/9 v5] difftool: add '--no-gui' as an option Tim Henigan
  1 sibling, 0 replies; 3+ messages in thread
From: Tim Henigan @ 2012-03-22 15:02 UTC (permalink / raw)
  To: gitster, git, davvid; +Cc: Tim Henigan

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

This replaces 12/9 in the previous version of the series.

Changes in v5:

Based on an example provided by David Aguilar on the Git developer
list, the 'difftool --dir-diff from subdirectory' test is now wrapped
in parentheses "()".

>From David's email:
> If we wrap the subdir operations in parentheses, then the sub-shell
> saves us from having to do "cd ..".  It also helps prevent leakage
> from earlier failures, which is helpful when writing new tests.


 t/t7800-difftool.sh |   39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 41d8399..5f3ad3f 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -311,4 +311,43 @@ test_expect_success PERL 'difftool --tool-help' '
 	echo "$tool_help" | stdin_contains tool
 '
 
+test_expect_success PERL 'setup change in subdirectory' '
+	git checkout master &&
+	mkdir sub &&
+	echo master >sub/sub &&
+	git add sub/sub &&
+	git commit -m "added sub/sub" &&
+	echo test >>file &&
+	echo test >>sub/sub &&
+	git add . &&
+	git commit -m "modified both"
+'
+
+test_expect_success PERL 'difftool -d' '
+	diff=$(git difftool -d --extcmd ls branch) &&
+	echo "$diff" | stdin_contains sub &&
+	echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff' '
+	diff=$(git difftool --dir-diff --extcmd ls branch) &&
+	echo "$diff" | stdin_contains sub &&
+	echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
+	diff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&
+	echo "$diff" | stdin_contains sub &&
+	echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+	(
+		cd sub &&
+		diff=$(git difftool --dir-diff --extcmd ls branch) &&
+		echo "$diff" | stdin_contains sub &&
+		echo "$diff" | stdin_contains file
+	)
+'
+
 test_done
-- 
1.7.10.rc1.40.g756bbcd

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

* [PATCH 13/9 v5] difftool: add '--no-gui' as an option
  2012-03-22 15:02 [PATCH 10/9 v5] difftool: fix regression in '--prompt' options Tim Henigan
  2012-03-22 15:02 ` [PATCH 12/9 v5] t7800: add tests for difftool --dir-diff Tim Henigan
@ 2012-03-22 15:02 ` Tim Henigan
  1 sibling, 0 replies; 3+ messages in thread
From: Tim Henigan @ 2012-03-22 15:02 UTC (permalink / raw)
  To: gitster, git, davvid; +Cc: Tim Henigan

This commit teaches difftool to handle the '--no-gui' option. This option
negates the existing '--gui' option. The last setting given on the command
line wins.

This allows a user to configure "[alias] mdt = difftool --gui", but still
have the ability to override the setting without error:

$ git mdt --no-gui

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

This patch is new to the series. It fixes a potential bug pointed out
by Junio Hamano on the Git developers list [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/193601/focus=193633


 git-difftool.perl   |    7 ++++---
 t/t7800-difftool.sh |   13 +++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 9892d1e..d9be7d6 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -29,7 +29,8 @@ sub usage
 	my $exitcode = shift;
 	print << 'USAGE';
 usage: git difftool [-t|--tool=<tool>] [--tool-help]
-                    [-x|--extcmd=<cmd>] [-g|--gui]
+                    [-x|--extcmd=<cmd>]
+                    [-g|--gui] [--no-gui]
                     [--prompt] [-y|--no-prompt]
                     [-d|--dir-diff]
                     ['git diff' options]
@@ -132,7 +133,7 @@ sub setup_dir_diff
 # parse command-line options. all unrecognized options and arguments
 # are passed through to the 'git diff' command.
 my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt, $tool_help);
-GetOptions('g|gui' => \$gui,
+GetOptions('g|gui!' => \$gui,
 	'd|dir-diff' => \$dirdiff,
 	'h' => \$help,
 	'prompt!' => \$prompt,
@@ -169,7 +170,7 @@ if (defined($extcmd)) {
 		usage(1);
 	}
 }
-if (defined($gui)) {
+if ($gui) {
 	my $guitool = "";
 	$guitool = Git::config('diff.guitool');
 	if (length($guitool) > 0) {
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 5f3ad3f..bbe71e5 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -94,6 +94,19 @@ test_expect_success PERL 'difftool honors --gui' '
 	restore_test_defaults
 '
 
+test_expect_success PERL 'difftool --gui last setting wins' '
+	git config diff.guitool bogus-tool &&
+	git difftool --no-prompt --gui --no-gui &&
+
+	git config merge.tool bogus-tool &&
+	git config diff.tool bogus-tool &&
+	git config diff.guitool test-tool &&
+	diff=$(git difftool --no-prompt --no-gui --gui branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
 test_expect_success PERL 'difftool --gui works without configured diff.guitool' '
 	git config diff.tool test-tool &&
 
-- 
1.7.10.rc1.40.g756bbcd

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

end of thread, other threads:[~2012-03-22 15:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-22 15:02 [PATCH 10/9 v5] difftool: fix regression in '--prompt' options Tim Henigan
2012-03-22 15:02 ` [PATCH 12/9 v5] t7800: add tests for difftool --dir-diff Tim Henigan
2012-03-22 15:02 ` [PATCH 13/9 v5] difftool: add '--no-gui' as an option Tim Henigan

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 NNTP newsgroup(s).