git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
@ 2008-10-11  0:21 Brandon Casey
  2008-10-11  0:24 ` Shawn O. Pearce
  0 siblings, 1 reply; 11+ messages in thread
From: Brandon Casey @ 2008-10-11  0:21 UTC (permalink / raw)
  To: Git Mailing List

Since dbf5e1e9, the '--no-validate' option is a Getopt::Long boolean
option. The '--no-' prefix (as in --no-validate) for boolean options
is not supported in Getopt::Long version 2.32 which was released with
Perl 5.8.0. This version only supports '--no' as in '--novalidate'.
More recent versions of Getopt::Long, such as version 2.34, support
either prefix. So use the older form in the tests.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t9001-send-email.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d098a01..561ae7d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -109,7 +109,7 @@ test_expect_success 'allow long lines with --no-validate' '
 		--from="Example <nobody@example.com>" \
 		--to=nobody@example.com \
 		--smtp-server="$(pwd)/fake.sendmail" \
-		--no-validate \
+		--novalidate \
 		$patches longline.patch \
 		2>errors
 '
-- 
1.6.0.2.468.gd5b83

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2008-10-11  0:21 [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-' Brandon Casey
@ 2008-10-11  0:24 ` Shawn O. Pearce
  2008-10-11  0:44   ` Brandon Casey
  0 siblings, 1 reply; 11+ messages in thread
From: Shawn O. Pearce @ 2008-10-11  0:24 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List

Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Since dbf5e1e9, the '--no-validate' option is a Getopt::Long boolean
> option. The '--no-' prefix (as in --no-validate) for boolean options
> is not supported in Getopt::Long version 2.32 which was released with
> Perl 5.8.0. This version only supports '--no' as in '--novalidate'.
> More recent versions of Getopt::Long, such as version 2.34, support
> either prefix. So use the older form in the tests.

Ouch.

Should we update our docs?

Actually, if 2.32 doesn't support the --no-validate syntax than
this is a regression in Git.  Even if it is what many would call a
bug in Getopt::Long in Perl, I think Git 1.6.1 should still honor
--no-validate like it did in Git 1.6.0.
 
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index d098a01..561ae7d 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -109,7 +109,7 @@ test_expect_success 'allow long lines with --no-validate' '
>  		--from="Example <nobody@example.com>" \
>  		--to=nobody@example.com \
>  		--smtp-server="$(pwd)/fake.sendmail" \
> -		--no-validate \
> +		--novalidate \
>  		$patches longline.patch \
>  		2>errors
>  '

-- 
Shawn.

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2008-10-11  0:24 ` Shawn O. Pearce
@ 2008-10-11  0:44   ` Brandon Casey
  0 siblings, 0 replies; 11+ messages in thread
From: Brandon Casey @ 2008-10-11  0:44 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List

Shawn O. Pearce wrote:
> Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> Since dbf5e1e9, the '--no-validate' option is a Getopt::Long boolean
>> option. The '--no-' prefix (as in --no-validate) for boolean options
>> is not supported in Getopt::Long version 2.32 which was released with
>> Perl 5.8.0. This version only supports '--no' as in '--novalidate'.
>> More recent versions of Getopt::Long, such as version 2.34, support
>> either prefix. So use the older form in the tests.
> 
> Ouch.
> 
> Should we update our docs?
> 
> Actually, if 2.32 doesn't support the --no-validate syntax than
> this is a regression in Git.  Even if it is what many would call a
> bug in Getopt::Long in Perl, I think Git 1.6.1 should still honor
> --no-validate like it did in Git 1.6.0.

If it makes any difference, none of the other boolean options would
work in the documented '--no-' form either, such as --no-thread,
--no-signed-off-by-cc, etc.  '--no-validate' is probably the only one
that used to work.


Update the docs?

---[no-]validate::
+--[no-|no]validate::

hmm. not sure.

-brandon

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

* [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
@ 2009-04-24 23:18 Brandon Casey
  0 siblings, 0 replies; 11+ messages in thread
From: Brandon Casey @ 2009-04-24 23:18 UTC (permalink / raw)
  To: git

The '--no-thread' option is a Getopt::Long boolean option. The '--no-'
prefix (as in --no-thread) for boolean options is not supported in
Getopt::Long version 2.32 which was released with Perl 5.8.0. This version
only supports '--no' as in '--nothread'.  More recent versions of
Getopt::Long, such as version 2.34, support either prefix. So use the older
form in the tests.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t9001-send-email.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d9420e0..ce26ea4 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -616,7 +616,7 @@ test_expect_success 'in-reply-to but no threading' '
 		--from="Example <nobody@example.com>" \
 		--to=nobody@example.com \
 		--in-reply-to="<in-reply-id@example.com>" \
-		--no-thread \
+		--nothread \
 		$patches |
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
-- 
1.6.2.4.24.gde59d2

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

* [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-01-29 13:30 Testsuite regression with perl 5.8.0 [Re: [ANNOUNCE] Git v2.3.0-rc2] Tom G. Christensen
@ 2015-01-30  6:24 ` Tom G. Christensen
  2015-01-30 23:05   ` brian m. carlson
  0 siblings, 1 reply; 11+ messages in thread
From: Tom G. Christensen @ 2015-01-30  6:24 UTC (permalink / raw)
  To: git

The '--no-xmailer' option is a Getopt::Long boolean option. The
'--no-' prefix (as in --no-xmailer) for boolean options is not
supported in Getopt::Long version 2.32 which was released with Perl 5.8.0.
This version only supports '--no' as in '--noxmailer'.  More recent
versions of Getopt::Long, such as version 2.34, support either prefix. So
use the older form in the tests.

See also:

d2559f734bba7fe5257720356a92f3b7a5b0d37c
907a0b1e04ea31cb368e9422df93d8ebb0187914
84eeb687de7a6c7c42af3fb51b176e0f412a979e
3fee1fe87144360a1913eab86af9ad136c810076

Signed-off-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
---
 t/t9001-send-email.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index af6a3e8..30df6ae 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1580,20 +1580,20 @@ do_xmailer_test () {
 
 test_expect_success $PREREQ '--[no-]xmailer without any configuration' '
 	do_xmailer_test 1 "--xmailer" &&
-	do_xmailer_test 0 "--no-xmailer"
+	do_xmailer_test 0 "--noxmailer"
 '
 
 test_expect_success $PREREQ '--[no-]xmailer with sendemail.xmailer=true' '
 	test_config sendemail.xmailer true &&
 	do_xmailer_test 1 "" &&
-	do_xmailer_test 0 "--no-xmailer" &&
+	do_xmailer_test 0 "--noxmailer" &&
 	do_xmailer_test 1 "--xmailer"
 '
 
 test_expect_success $PREREQ '--[no-]xmailer with sendemail.xmailer=false' '
 	test_config sendemail.xmailer false &&
 	do_xmailer_test 0 "" &&
-	do_xmailer_test 0 "--no-xmailer" &&
+	do_xmailer_test 0 "--noxmailer" &&
 	do_xmailer_test 1 "--xmailer"
 '
 
-- 
2.2.1

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-01-30  6:24 ` [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-' Tom G. Christensen
@ 2015-01-30 23:05   ` brian m. carlson
  2015-01-31  2:40     ` Kyle J. McKay
  0 siblings, 1 reply; 11+ messages in thread
From: brian m. carlson @ 2015-01-30 23:05 UTC (permalink / raw)
  To: Tom G. Christensen; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

On Fri, Jan 30, 2015 at 07:24:45AM +0100, Tom G. Christensen wrote:
>The '--no-xmailer' option is a Getopt::Long boolean option. The
>'--no-' prefix (as in --no-xmailer) for boolean options is not
>supported in Getopt::Long version 2.32 which was released with Perl 5.8.0.
>This version only supports '--no' as in '--noxmailer'.  More recent
>versions of Getopt::Long, such as version 2.34, support either prefix. So
>use the older form in the tests.
>
>See also:
>
>d2559f734bba7fe5257720356a92f3b7a5b0d37c
>907a0b1e04ea31cb368e9422df93d8ebb0187914
>84eeb687de7a6c7c42af3fb51b176e0f412a979e
>3fee1fe87144360a1913eab86af9ad136c810076
>
>Signed-off-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
>---
> t/t9001-send-email.sh | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
>index af6a3e8..30df6ae 100755
>--- a/t/t9001-send-email.sh
>+++ b/t/t9001-send-email.sh
>@@ -1580,20 +1580,20 @@ do_xmailer_test () {
>
> test_expect_success $PREREQ '--[no-]xmailer without any configuration' '
> 	do_xmailer_test 1 "--xmailer" &&
>-	do_xmailer_test 0 "--no-xmailer"
>+	do_xmailer_test 0 "--noxmailer"

I don't think this is an adequate fix.  The documented option is 
--no-xmailer.  If your version of Getopt::Long is not capable of that, 
then the program doesn't work as documented, and the test is correctly 
failing.  --noxmailer is not documented at all, so it's not something we 
should be testing.

We should probably require a certain version of Getopt::Long or 
explicitly handle this in the parsing code itself.  I think the former 
is a better choice, since no security-supported OS still ships with such 
a positively ancient version.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-01-30 23:05   ` brian m. carlson
@ 2015-01-31  2:40     ` Kyle J. McKay
  2015-02-02  1:33       ` Junio C Hamano
  2015-02-12 23:12       ` Junio C Hamano
  0 siblings, 2 replies; 11+ messages in thread
From: Kyle J. McKay @ 2015-01-31  2:40 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Git mailing list, Tom G. Christensen

On Jan 30, 2015, at 15:05, brian m. carlson wrote:
> On Fri, Jan 30, 2015 at 07:24:45AM +0100, Tom G. Christensen wrote:
>> The '--no-xmailer' option is a Getopt::Long boolean option. The
>> '--no-' prefix (as in --no-xmailer) for boolean options is not
>> supported in Getopt::Long version 2.32 which was released with Perl  
>> 5.8.0.
>> This version only supports '--no' as in '--noxmailer'.  More recent
>> versions of Getopt::Long, such as version 2.34, support either  
>> prefix. So
>> use the older form in the tests.
>>
>> See also:
>>
>> d2559f734bba7fe5257720356a92f3b7a5b0d37c
>> 907a0b1e04ea31cb368e9422df93d8ebb0187914
>> 84eeb687de7a6c7c42af3fb51b176e0f412a979e
>> 3fee1fe87144360a1913eab86af9ad136c810076
>>
>> Signed-off-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
>> ---
>> t/t9001-send-email.sh | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
>> index af6a3e8..30df6ae 100755
>> --- a/t/t9001-send-email.sh
>> +++ b/t/t9001-send-email.sh
>> @@ -1580,20 +1580,20 @@ do_xmailer_test () {
>>
>> test_expect_success $PREREQ '--[no-]xmailer without any  
>> configuration' '
>> 	do_xmailer_test 1 "--xmailer" &&
>> -	do_xmailer_test 0 "--no-xmailer"
>> +	do_xmailer_test 0 "--noxmailer"
>
> I don't think this is an adequate fix.  The documented option is -- 
> no-xmailer.  If your version of Getopt::Long is not capable of that,  
> then the program doesn't work as documented, and the test is  
> correctly failing.  --noxmailer is not documented at all, so it's  
> not something we should be testing.

It is not alone.  From the git-send-email help these are all boolean  
options:

> git send-email
>
>   Composing:
>     --[no-]xmailer
>     --[no-]annotate
>
>   Automating:
>     --[no-]cc-cover
>     --[no-]to-cover
>     --[no-]signed-off-by-cc
>     --[no-]suppress-from
>     --[no-]chain-reply-to
>     --[no-]thread
>
>   Administering:
>     --[no-]validate
>     --[no-]format-patch


Anything done to fix --no-xmailer should be applied for all the other  
--no-... options as well.

> We should probably require a certain version of Getopt::Long or  
> explicitly handle this in the parsing code itself.  I think the  
> former is a better choice, since no security-supported OS still  
> ships with such a positively ancient version.

I don't really like that second option because all the .perl files have:

> use 5.008;

So either that needs to change or the code should properly deal with  
the version of Getopt::Long that comes with 5.8.0.

Since it's really not very difficult or invasive to add support for  
the no- variants, here's a patch to do so:

-- 8< --
Subject: [PATCH] git-send-email.perl: support no- prefix with older GetOptions

Only Perl version 5.8.0 or later is required, but that comes with
an older Getopt::Long (2.32) that does not support the 'no-'
prefix.  Support for that was added in Getopt::Long version 2.33.

Since the help only mentions the 'no-' prefix and not the 'no'
prefix, add explicit support for the 'no-' prefix when running
with older GetOptions versions.

Reported-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---
 git-send-email.perl | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 3092ab35..a18a7959 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -299,6 +299,7 @@ my $rc = GetOptions("h" => \$help,
 		    "bcc=s" => \@bcclist,
 		    "no-bcc" => \$no_bcc,
 		    "chain-reply-to!" => \$chain_reply_to,
+		    "no-chain-reply-to" => sub {$chain_reply_to = 0},
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-option=s" => \@smtp_server_options,
 		    "smtp-server-port=s" => \$smtp_server_port,
@@ -311,25 +312,34 @@ my $rc = GetOptions("h" => \$help,
 		    "smtp-domain:s" => \$smtp_domain,
 		    "identity=s" => \$identity,
 		    "annotate!" => \$annotate,
+		    "no-annotate" => sub {$annotate = 0},
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
+		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
+		    "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
 		    "cc-cover|cc-cover!" => \$cover_cc,
+		    "no-cc-cover" => sub {$cover_cc = 0},
 		    "to-cover|to-cover!" => \$cover_to,
+		    "no-to-cover" => sub {$cover_to = 0},
 		    "confirm=s" => \$confirm,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
 		    "thread!" => \$thread,
+		    "no-thread" => sub {$thread = 0},
 		    "validate!" => \$validate,
+		    "no-validate" => sub {$validate = 0},
 		    "transfer-encoding=s" => \$target_xfer_encoding,
 		    "format-patch!" => \$format_patch,
+		    "no-format-patch" => sub {$format_patch = 0},
 		    "8bit-encoding=s" => \$auto_8bit_encoding,
 		    "compose-encoding=s" => \$compose_encoding,
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
+		    "no-xmailer" => sub {$use_xmailer = 0},
 	 );
 
 usage() if $help;
--

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-01-31  2:40     ` Kyle J. McKay
@ 2015-02-02  1:33       ` Junio C Hamano
  2015-02-02 16:11         ` Kyle J. McKay
  2015-02-12 23:12       ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2015-02-02  1:33 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: brian m. carlson, Git mailing list, Tom G. Christensen

"Kyle J. McKay" <mackyle@gmail.com> writes:

>> use 5.008;
>
> So either that needs to change or the code should properly deal with  
> the version of Getopt::Long that comes with 5.8.0.
>
> Since it's really not very difficult or invasive to add support for  
> the no- variants, here's a patch to do so:

Doesn't that approach add "what does --no-no-chain-rely-to even
mean?" confusion to the resulting system?  If that is not the case,
then I am all for it, but otherwise, let's not.

People can easily spell --noxmailer if they want to stay at an older
Getopt::Long, and over time these ancient ones will be upgraded
away.



> -- 8< --
> Subject: [PATCH] git-send-email.perl: support no- prefix with older GetOptions
>
> Only Perl version 5.8.0 or later is required, but that comes with
> an older Getopt::Long (2.32) that does not support the 'no-'
> prefix.  Support for that was added in Getopt::Long version 2.33.
>
> Since the help only mentions the 'no-' prefix and not the 'no'
> prefix, add explicit support for the 'no-' prefix when running
> with older GetOptions versions.
>
> Reported-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
> ---
>  git-send-email.perl | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 3092ab35..a18a7959 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -299,6 +299,7 @@ my $rc = GetOptions("h" => \$help,
>  		    "bcc=s" => \@bcclist,
>  		    "no-bcc" => \$no_bcc,
>  		    "chain-reply-to!" => \$chain_reply_to,
> +		    "no-chain-reply-to" => sub {$chain_reply_to = 0},
>  		    "smtp-server=s" => \$smtp_server,
>  		    "smtp-server-option=s" => \@smtp_server_options,
>  		    "smtp-server-port=s" => \$smtp_server_port,
> @@ -311,25 +312,34 @@ my $rc = GetOptions("h" => \$help,
>  		    "smtp-domain:s" => \$smtp_domain,
>  		    "identity=s" => \$identity,
>  		    "annotate!" => \$annotate,
> +		    "no-annotate" => sub {$annotate = 0},
>  		    "compose" => \$compose,
>  		    "quiet" => \$quiet,
>  		    "cc-cmd=s" => \$cc_cmd,
>  		    "suppress-from!" => \$suppress_from,
> +		    "no-suppress-from" => sub {$suppress_from = 0},
>  		    "suppress-cc=s" => \@suppress_cc,
>  		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
> +		    "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
>  		    "cc-cover|cc-cover!" => \$cover_cc,
> +		    "no-cc-cover" => sub {$cover_cc = 0},
>  		    "to-cover|to-cover!" => \$cover_to,
> +		    "no-to-cover" => sub {$cover_to = 0},
>  		    "confirm=s" => \$confirm,
>  		    "dry-run" => \$dry_run,
>  		    "envelope-sender=s" => \$envelope_sender,
>  		    "thread!" => \$thread,
> +		    "no-thread" => sub {$thread = 0},
>  		    "validate!" => \$validate,
> +		    "no-validate" => sub {$validate = 0},
>  		    "transfer-encoding=s" => \$target_xfer_encoding,
>  		    "format-patch!" => \$format_patch,
> +		    "no-format-patch" => sub {$format_patch = 0},
>  		    "8bit-encoding=s" => \$auto_8bit_encoding,
>  		    "compose-encoding=s" => \$compose_encoding,
>  		    "force" => \$force,
>  		    "xmailer!" => \$use_xmailer,
> +		    "no-xmailer" => sub {$use_xmailer = 0},
>  	 );
>  
>  usage() if $help;
> --

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-02-02  1:33       ` Junio C Hamano
@ 2015-02-02 16:11         ` Kyle J. McKay
  2015-02-02 20:12           ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Kyle J. McKay @ 2015-02-02 16:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, Git mailing list, Tom G. Christensen

On Feb 1, 2015, at 17:33, Junio C Hamano wrote:

> "Kyle J. McKay" <mackyle@gmail.com> writes:
>
>>> use 5.008;
>>
>> So either that needs to change or the code should properly deal with
>> the version of Getopt::Long that comes with 5.8.0.
>>
>> Since it's really not very difficult or invasive to add support for
>> the no- variants, here's a patch to do so:
>
> Doesn't that approach add "what does --no-no-chain-rely-to even
> mean?" confusion to the resulting system?  If that is not the case,
> then I am all for it, but otherwise, let's not.

No.  You have to append the '!' to get the automagic no prefix  
alternative(s), so while 'chain-reply-to!' means support chain-reply- 
to, nochain-reply-to and (if you have a new enough Getopt::Long) no- 
chain-reply-to, just using 'no-chain-reply-to' without the trailing  
'!' means that nono-chain-reply-to and no-no-chain-reply-to remain  
invalid options that will generate an error.

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-02-02 16:11         ` Kyle J. McKay
@ 2015-02-02 20:12           ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2015-02-02 20:12 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: brian m. carlson, Git mailing list, Tom G. Christensen

"Kyle J. McKay" <mackyle@gmail.com> writes:

> On Feb 1, 2015, at 17:33, Junio C Hamano wrote:
>
>> "Kyle J. McKay" <mackyle@gmail.com> writes:
>>
>>>> use 5.008;
>>>
>>> So either that needs to change or the code should properly deal with
>>> the version of Getopt::Long that comes with 5.8.0.
>>>
>>> Since it's really not very difficult or invasive to add support for
>>> the no- variants, here's a patch to do so:
>>
>> Doesn't that approach add "what does --no-no-chain-rely-to even
>> mean?" confusion to the resulting system?  If that is not the case,
>> then I am all for it, but otherwise, let's not.
>
> No.  You have to append the '!' to get the automagic no prefix
> alternative(s), so while 'chain-reply-to!' means support chain-reply- 
> to, nochain-reply-to and (if you have a new enough Getopt::Long) no- 
> chain-reply-to, just using 'no-chain-reply-to' without the trailing
> !' means that nono-chain-reply-to and no-no-chain-reply-to remain
> invalid options that will generate an error.

Ahh, I missed that ! suffix (or lack thereof).

Thanks.

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

* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
  2015-01-31  2:40     ` Kyle J. McKay
  2015-02-02  1:33       ` Junio C Hamano
@ 2015-02-12 23:12       ` Junio C Hamano
  1 sibling, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2015-02-12 23:12 UTC (permalink / raw)
  To: Git mailing list; +Cc: brian m. carlson, Kyle J. McKay, Tom G. Christensen

"Kyle J. McKay" <mackyle@gmail.com> writes:

> Since it's really not very difficult or invasive to add support for  
> the no- variants, here's a patch to do so:

I am inclined to replace the tc/t9001-noxmailer topic that has been
sitting on my Undecided pile with this patch and move it forward.

    * tc/t9001-noxmailer (2015-01-30) 1 commit
     - t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'

Please stop me if I am missing something obviously wrong with this
plan.

Thanks.



> -- 8< --
> Subject: [PATCH] git-send-email.perl: support no- prefix with older GetOptions
>
> Only Perl version 5.8.0 or later is required, but that comes with
> an older Getopt::Long (2.32) that does not support the 'no-'
> prefix.  Support for that was added in Getopt::Long version 2.33.
>
> Since the help only mentions the 'no-' prefix and not the 'no'
> prefix, add explicit support for the 'no-' prefix when running
> with older GetOptions versions.
>
> Reported-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
> ---
>  git-send-email.perl | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 3092ab35..a18a7959 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -299,6 +299,7 @@ my $rc = GetOptions("h" => \$help,
>  		    "bcc=s" => \@bcclist,
>  		    "no-bcc" => \$no_bcc,
>  		    "chain-reply-to!" => \$chain_reply_to,
> +		    "no-chain-reply-to" => sub {$chain_reply_to = 0},
>  		    "smtp-server=s" => \$smtp_server,
>  		    "smtp-server-option=s" => \@smtp_server_options,
>  		    "smtp-server-port=s" => \$smtp_server_port,
> @@ -311,25 +312,34 @@ my $rc = GetOptions("h" => \$help,
>  		    "smtp-domain:s" => \$smtp_domain,
>  		    "identity=s" => \$identity,
>  		    "annotate!" => \$annotate,
> +		    "no-annotate" => sub {$annotate = 0},
>  		    "compose" => \$compose,
>  		    "quiet" => \$quiet,
>  		    "cc-cmd=s" => \$cc_cmd,
>  		    "suppress-from!" => \$suppress_from,
> +		    "no-suppress-from" => sub {$suppress_from = 0},
>  		    "suppress-cc=s" => \@suppress_cc,
>  		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
> +		    "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
>  		    "cc-cover|cc-cover!" => \$cover_cc,
> +		    "no-cc-cover" => sub {$cover_cc = 0},
>  		    "to-cover|to-cover!" => \$cover_to,
> +		    "no-to-cover" => sub {$cover_to = 0},
>  		    "confirm=s" => \$confirm,
>  		    "dry-run" => \$dry_run,
>  		    "envelope-sender=s" => \$envelope_sender,
>  		    "thread!" => \$thread,
> +		    "no-thread" => sub {$thread = 0},
>  		    "validate!" => \$validate,
> +		    "no-validate" => sub {$validate = 0},
>  		    "transfer-encoding=s" => \$target_xfer_encoding,
>  		    "format-patch!" => \$format_patch,
> +		    "no-format-patch" => sub {$format_patch = 0},
>  		    "8bit-encoding=s" => \$auto_8bit_encoding,
>  		    "compose-encoding=s" => \$compose_encoding,
>  		    "force" => \$force,
>  		    "xmailer!" => \$use_xmailer,
> +		    "no-xmailer" => sub {$use_xmailer = 0},
>  	 );
>  
>  usage() if $help;
> --

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

end of thread, other threads:[~2015-02-12 23:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-11  0:21 [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-' Brandon Casey
2008-10-11  0:24 ` Shawn O. Pearce
2008-10-11  0:44   ` Brandon Casey
  -- strict thread matches above, loose matches on Subject: below --
2009-04-24 23:18 Brandon Casey
2015-01-29 13:30 Testsuite regression with perl 5.8.0 [Re: [ANNOUNCE] Git v2.3.0-rc2] Tom G. Christensen
2015-01-30  6:24 ` [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-' Tom G. Christensen
2015-01-30 23:05   ` brian m. carlson
2015-01-31  2:40     ` Kyle J. McKay
2015-02-02  1:33       ` Junio C Hamano
2015-02-02 16:11         ` Kyle J. McKay
2015-02-02 20:12           ` Junio C Hamano
2015-02-12 23:12       ` Junio C Hamano

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