* [PATCH] Fix sloppy Getopt::Long.
@ 2009-05-05 18:16 Robin H. Johnson
2009-05-05 19:37 ` Junio C Hamano
[not found] ` <7vljpazuyg.fsf@alter.siamese.dyndns.org>
0 siblings, 2 replies; 6+ messages in thread
From: Robin H. Johnson @ 2009-05-05 18:16 UTC (permalink / raw)
To: git; +Cc: Robin H. Johnson
Getopt-Long v2.38 is much stricter about sloppy getopt usage. The
trailing pipe causes git-svn testcases to fail for all of the --stdin
argument calls.
Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
===
Should be applied to both the stable 1.6.2.x tree and the new 1.6.3
tree.
---
git-svn.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index c5965c9..ef1d30d 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -147,7 +147,7 @@ my %cmd = (
'dry-run|n' => \$_dry_run } ],
'set-tree' => [ \&cmd_set_tree,
"Set an SVN repository to a git tree-ish",
- { 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
+ { 'stdin' => \$_stdin, %cmt_opts, %fc_opts, } ],
'create-ignore' => [ \&cmd_create_ignore,
'Create a .gitignore per svn:ignore',
{ 'revision|r=i' => \$_revision
--
1.6.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix sloppy Getopt::Long.
2009-05-05 18:16 [PATCH] Fix sloppy Getopt::Long Robin H. Johnson
@ 2009-05-05 19:37 ` Junio C Hamano
2009-05-05 20:21 ` Robin H. Johnson
[not found] ` <7vljpazuyg.fsf@alter.siamese.dyndns.org>
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2009-05-05 19:37 UTC (permalink / raw)
To: Robin H. Johnson; +Cc: git
"Robin H. Johnson" <robbat2@gentoo.org> writes:
> Getopt-Long v2.38 is much stricter about sloppy getopt usage. The
> trailing pipe causes git-svn testcases to fail for all of the --stdin
> argument calls.
I am not objecting at all; just asking for clarification.
> - { 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
> + { 'stdin' => \$_stdin, %cmt_opts, %fc_opts, } ],
Is this "pipe" supposed to be followed by an alternative spelling of the
option, as in
'stdin|standard-input' => \$_stdin, ...
and is the sloppyness that it would be crazy to accept either --stdin or
just -- (without actual option name) for this option?
Could an older version of Getopt::Long() have accepted
$ command --foo --bar - other args
to set $_stdin to true with that "sloppy" syntax? If so people could have
relied on such a behaviour, which is a bit worrying.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix sloppy Getopt::Long.
2009-05-05 19:37 ` Junio C Hamano
@ 2009-05-05 20:21 ` Robin H. Johnson
0 siblings, 0 replies; 6+ messages in thread
From: Robin H. Johnson @ 2009-05-05 20:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Robin H. Johnson, git
On Tue, May 05, 2009 at 12:37:34PM -0700, Junio C Hamano wrote:
> "Robin H. Johnson" <robbat2@gentoo.org> writes:
>
> > Getopt-Long v2.38 is much stricter about sloppy getopt usage. The
> > trailing pipe causes git-svn testcases to fail for all of the --stdin
> > argument calls.
>
> I am not objecting at all; just asking for clarification.
>
> > - { 'stdin|' => \$_stdin, %cmt_opts, %fc_opts, } ],
> > + { 'stdin' => \$_stdin, %cmt_opts, %fc_opts, } ],
>
> Is this "pipe" supposed to be followed by an alternative spelling of the
> option, as in
> 'stdin|standard-input' => \$_stdin, ...
Yes. Short form or alternative long form.
> and is the sloppyness that it would be crazy to accept either --stdin or
> just -- (without actual option name) for this option?
Within the main loop, both '--' and '-' are treated as special cases
earlier on before the matching of options is done. '--' is the explicit
separator, while '-' is an argument (or a value to an option), not an
option in itself.
> Could an older version of Getopt::Long() have accepted
>
> $ command --foo --bar - other args
>
> to set $_stdin to true with that "sloppy" syntax? If so people could have
> relied on such a behaviour, which is a bit worrying.
As far as I can follow in the Getopt::Long code, with the old case of
'stdin|', the empty string case would never have matched anyway.
The v2.38 change in respect to this is not described in the upstream
CHANGES, but boils down to this single modification:
Getopt-Long-2.38/lib/Getopt/Long.pm:
@@ -777,7 +776,7 @@
# Option name
(?: \w+[-\w]* )
# Alias names, or "?"
- (?: \| (?: \? | \w[-\w]* )? )*
+ (?: \| (?: \? | \w[-\w]* ) )*
)?
--
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix sloppy Getopt::Long.
[not found] ` <20090506064949.GB29479@dcvr.yhbt.net>
@ 2009-05-06 16:13 ` Robin H. Johnson
2009-05-06 17:24 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Robin H. Johnson @ 2009-05-06 16:13 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, Robin H. Johnson, git
On Tue, May 05, 2009 at 11:49:49PM -0700, Eric Wong wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > "Robin H. Johnson" <robbat2@gentoo.org> writes:
> >
> > > Getopt-Long v2.38 is much stricter about sloppy getopt usage. The
> > > trailing pipe causes git-svn testcases to fail for all of the --stdin
> > > argument calls.
> > >
> > > Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
> >
> > Eric, I'll take this directly to my tree. Ok?
>
> The empty "" after the "|" was intended for the set-tree command to take
> a lone "-" as a parameter to also mean "--stdin".
>
> The following should work, too, but I don't have time to test right now:
>
> + { '' => \$_stdin, 'stdin' => \$_stdin,
> + %cmt_opts, %fc_opts, } ],
I confirm that it does correctly set the $_stdin variable (tested
briefly).
Testcase:
=====
use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
my $_stdin;
my $rc = GetOptions( 'stdin' => \$_stdin, '' => \$_stdin);
printf "rc:%s s:%s\n",$rc,$_stdin;
=====
--
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix sloppy Getopt::Long.
2009-05-06 16:13 ` Robin H. Johnson
@ 2009-05-06 17:24 ` Junio C Hamano
2009-05-08 18:28 ` Robin H. Johnson
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2009-05-06 17:24 UTC (permalink / raw)
To: Robin H. Johnson; +Cc: Eric Wong, Junio C Hamano, git
"Robin H. Johnson" <robbat2@gentoo.org> writes:
> On Tue, May 05, 2009 at 11:49:49PM -0700, Eric Wong wrote:
>> Junio C Hamano <gitster@pobox.com> wrote:
>> > "Robin H. Johnson" <robbat2@gentoo.org> writes:
>> >
>> > > Getopt-Long v2.38 is much stricter about sloppy getopt usage. The
>> > > trailing pipe causes git-svn testcases to fail for all of the --stdin
>> > > argument calls.
>> > >
>> > > Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
>> >
>> > Eric, I'll take this directly to my tree. Ok?
>>
>> The empty "" after the "|" was intended for the set-tree command to take
>> a lone "-" as a parameter to also mean "--stdin".
>>
>> The following should work, too, but I don't have time to test right now:
>>
>> + { '' => \$_stdin, 'stdin' => \$_stdin,
>> + %cmt_opts, %fc_opts, } ],
> I confirm that it does correctly set the $_stdin variable (tested
> briefly).
Wait a minute. Do you mean we would also need the above "explicit empty
argument sets $_stdin"? Wasn't it your earlier analysis/claim that the
caller already takes care of a lone "-"?
Or do you mean "yes it would also work but it is not necessary"?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix sloppy Getopt::Long.
2009-05-06 17:24 ` Junio C Hamano
@ 2009-05-08 18:28 ` Robin H. Johnson
0 siblings, 0 replies; 6+ messages in thread
From: Robin H. Johnson @ 2009-05-08 18:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Robin H. Johnson, Eric Wong, git
[-- Attachment #1: Type: text/plain, Size: 937 bytes --]
On Wed, May 06, 2009 at 10:24:50AM -0700, Junio C Hamano wrote:
> >> + { '' => \$_stdin, 'stdin' => \$_stdin,
> >> + %cmt_opts, %fc_opts, } ],
> > I confirm that it does correctly set the $_stdin variable (tested
> > briefly).
> Wait a minute. Do you mean we would also need the above "explicit empty
> argument sets $_stdin"? Wasn't it your earlier analysis/claim that the
> caller already takes care of a lone "-"?
I'd originally considered a single '-' as an argument not an option,
meaning stdin, not being processed within the getopt framework, but
instead being handled later.
> Or do you mean "yes it would also work but it is not necessary"?
Using Eric's change makes it explicit what is expected: Passing
'--stdin' is the same as passing '-'.
--
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-05-08 18:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-05 18:16 [PATCH] Fix sloppy Getopt::Long Robin H. Johnson
2009-05-05 19:37 ` Junio C Hamano
2009-05-05 20:21 ` Robin H. Johnson
[not found] ` <7vljpazuyg.fsf@alter.siamese.dyndns.org>
[not found] ` <20090506064949.GB29479@dcvr.yhbt.net>
2009-05-06 16:13 ` Robin H. Johnson
2009-05-06 17:24 ` Junio C Hamano
2009-05-08 18:28 ` Robin H. Johnson
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).