Re: [PATCH 10/9 v4] difftool: fix regression in '--prompt' options

Junio C Hamano [off-list ref] writes:

Tim Henigan [off-list ref] writes:

quoted

+# the '--prompt' and '--no-prompt' options require special treatment
+# because they may be specified more than once...the last one "wins".
+for (@ARGV) {
+	if (($_ eq "-y") or ($_ eq "--no-prompt")) {
+		$prompt = 0;
+	} elsif ($_ eq "--prompt") {
+		$prompt = 1;
+	} else {
+		push(@diffargs, $_);
+	}
+}

I really do not like the direction in which this series is going.  We do
not have a similar --no-gui option to defeat --gui option that may appear
earlier on the command line, but when we fix that bug (isn't it a bug?),
we would have to teach this loop about that option, wouldn't we?

In the end, won't you end up resurrecting the argument parsing loop that
you got rid of with the first patch in your series?  Isn't this working
around the problem introduced only because you are using Getopt::Long and
hitting its limitations?

Limitations?  You can basically steal code from git-send-email.  As an
example:

---- 8< ----
#!/usr/bin/perl

use strict;
use warnings;
use Getopt::Long;

my $foo = 0;
my $rc = GetOptions("foo!" => \$foo,
		    "n" => sub { $foo = 0; },
		    "y" => \$foo);

print "$foo\n";
---- 8< ----

$ ./getopt-test.perl -n
0
$ ./getopt-test.perl -y
1
$ ./getopt-test.perl --foo
1
$ ./getopt-test.perl --no-foo
0
$ ./getopt-test.perl --foo --no-foo
0
$ ./getopt-test.perl -y -n
0
$ ./getopt-test.perl --foo -n
0

--
Thomas Rast
trast@{inf,student}.ethz.ch

On Thu, Mar 22, 2012 at 4:19 AM, Thomas Rast [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

Tim Henigan [off-list ref] writes:

I really do not like the direction in which this series is going.  We do
not have a similar --no-gui option to defeat --gui option that may appear
earlier on the command line, but when we fix that bug (isn't it a bug?),
we would have to teach this loop about that option, wouldn't we?

Of course, Thomas is correct and the limitation was with my use of
Getopt::Long rather than the module itself.

v5 of the patch will use Getopt to parse the '--[no-]prompt' options
again.  I will also add a new patch to the series to implement
'--no-gui'.

Limitations?  You can basically steal code from git-send-email.  As an
example:

Thank you for the example...I should have found this when I read the
documentation earlier.