From: Tim Henigan <tim.henigan@gmail.com>
To: David Aguilar <davvid@gmail.com>
Cc: gitster@pobox.com, git@vger.kernel.org
Subject: Re: [PATCH 1/9] difftool: parse options using Getopt::Long
Date: Sat, 17 Mar 2012 09:54:09 -0400 [thread overview]
Message-ID: <CAFouetgpz__1q7F_a3LQoeypCVBXOyTPWmL0e9=rw7vV65d7_g@mail.gmail.com> (raw)
In-Reply-To: <CAJDDKr7BTz-2THw1JaJEDcK1G4Uxwc88=gFsHCSONHAtqtypRw@mail.gmail.com>
On Fri, Mar 16, 2012 at 11:21 PM, David Aguilar <davvid@gmail.com> wrote:
> On Fri, Mar 16, 2012 at 6:57 PM, Tim Henigan <tim.henigan@gmail.com> wrote:
>
> I've also wanted to do the same in the past. The one thing holding me
> back was this note from the perldocs:
>
> "If pass_through is also enabled, options processing will terminate at
> the first unrecognized option, or non-option, whichever comes first."
>
> http://search.cpan.org/~jv/Getopt-Long-2.38/lib/Getopt/Long.pm
That sentence is listed under the documentation of the 'permute'
option. The documentation of 'pass_through' states that:
"Options that are unknown, ambiguous or supplied with an invalid
option value are passed through in @ARGV instead of being flagged as
errors. This makes it possible to write wrapper scripts that process
only part of the user supplied command line arguments, and pass the
remaining options to some other program.
If require_order is enabled, options processing will terminate at the
first unrecognized option, or non-option, whichever comes first.
However, if permute is enabled instead, results can become confusing."
So early termination would only be a problem if 'pass_through' was
enabled at the same time as 'require_order' or 'permute'. To verify,
I confirmed that 'git difftool --cached --diff-dir' works as expected.
> Is this indeed the case? I am a little ashamed that the difftool
> tests do not cover this area. That would be a valuable first step
> towards exploring this approach, IMO.
I will review the the test cases. If this goes forward, I still need
to add test cases to confirm the new '--dir-diff' option.
> BTW, I hate @ARGV parsing loops just as much as anyone! I was not
> ignorant of Getopt::Long, and no, I was not re-inventing the wheel for
> no reason. The reason it was done that way was so that we can forward
> everything we don't know about to git-diff.
I understand and was not implying anything different. I simply
thought this would be a positive change.
next prev parent reply other threads:[~2012-03-17 13:54 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-17 1:57 [PATCH 0/9] difftool: teach command to perform directory diffs Tim Henigan
2012-03-17 1:57 ` [PATCH 1/9] difftool: parse options using Getopt::Long Tim Henigan
2012-03-17 3:21 ` David Aguilar
2012-03-17 13:54 ` Tim Henigan [this message]
2012-03-18 3:29 ` David Aguilar
2012-03-17 1:57 ` [PATCH 2/9] difftool: exit(0) when usage is printed Tim Henigan
2012-03-17 1:57 ` [PATCH 3/9] difftool: remove explicit change of PATH Tim Henigan
2012-03-17 1:57 ` [PATCH 4/9] difftool: stop appending '.exe' to git Tim Henigan
2012-03-17 2:06 ` [PATCH 0/9] difftool: teach command to perform directory diffs Junio C Hamano
2012-03-17 2:26 ` Tim Henigan
2012-03-17 4:24 ` Junio C Hamano
2012-03-18 1:55 ` [PATCH 0/9 v2] " Tim Henigan
2012-03-18 1:55 ` [PATCH 1/9 v2] difftool: parse options using Getopt::Long Tim Henigan
2012-03-18 1:55 ` [PATCH 2/9 v2] difftool: exit(0) when usage is printed Tim Henigan
2012-03-18 1:55 ` [PATCH 3/9 v2] difftool: remove explicit change of PATH Tim Henigan
2012-03-18 1:55 ` [PATCH 4/9 v2] difftool: stop appending '.exe' to git Tim Henigan
2012-03-18 1:55 ` [PATCH 5/9 v2] difftool: eliminate setup_environment function Tim Henigan
2012-03-18 1:55 ` [PATCH 6/9 v2] difftool: replace system call with Git::command_noisy Tim Henigan
2012-03-18 1:55 ` [PATCH 7/9 v2] difftool: teach difftool to handle directory diffs Tim Henigan
2012-03-18 1:55 ` [PATCH 8/9 v2] difftool: teach dir-diff to copy modified files back to working tree Tim Henigan
2012-03-18 1:55 ` [PATCH 9/9 v2] difftool: print list of valid tools with '--tool-help' Tim Henigan
2012-03-19 21:00 ` [PATCH 0/9 v2] difftool: teach command to perform directory diffs Junio C Hamano
2012-03-20 2:52 ` David Aguilar
2012-03-20 5:52 ` Junio C Hamano
2012-03-20 13:07 ` Tim Henigan
2012-03-20 16:36 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFouetgpz__1q7F_a3LQoeypCVBXOyTPWmL0e9=rw7vV65d7_g@mail.gmail.com' \
--to=tim.henigan@gmail.com \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).