The existing parser was very inflexible and tree-like;
seeing "--patch=$mode" would put us into a conditional for
parsing the options for that particular $mode. That made it
very difficult to add additional options that would work for
all patch modes.
This cleanup turns the argument processing into a proper
loop ready to handle multiple arguments. To replace the
mode-specific parsing, we restructure the mode definitions
to better reflect the various cases (i.e., no revision given
versus "HEAD" versus an arbitrary revision).
As a bonus, our parsing is now a little more robust. For
example, we catch the bogus "add--interactive --patch HEAD
--", which is meaningless, but was silently accepted before.
In practice this probably doesn't matter, since we are
always called by other parts of git, but it might catch a
bug in another part of git.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
git-add--interactive.perl | 212 ++++++++++++++++++++++++---------------------
1 files changed, 112 insertions(+), 100 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 8f0839d..c5cd300 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -93,74 +93,86 @@ sub apply_patch_for_stash;
my %patch_modes = (
'stage' => {
- DIFF => 'diff-files -p',
- APPLY => sub { apply_patch 'apply --cached', @_; },
- APPLY_CHECK => 'apply --cached',
- VERB => 'Stage',
- TARGET => '',
- PARTICIPLE => 'staging',
- FILTER => 'file-only',
- IS_REVERSE => 0,
+ default => 'index',
+ index => {
+ DIFF => 'diff-files -p',
+ APPLY => sub { apply_patch 'apply --cached', @_; },
+ APPLY_CHECK => 'apply --cached',
+ VERB => 'Stage',
+ TARGET => '',
+ PARTICIPLE => 'staging',
+ FILTER => 'file-only',
+ IS_REVERSE => 0,
+ },
},
'stash' => {
- DIFF => 'diff-index -p HEAD',
- APPLY => sub { apply_patch 'apply --cached', @_; },
- APPLY_CHECK => 'apply --cached',
- VERB => 'Stash',
- TARGET => '',
- PARTICIPLE => 'stashing',
- FILTER => undef,
- IS_REVERSE => 0,
+ default => 'head',
+ head => {
+ DIFF => 'diff-index -p',
+ APPLY => sub { apply_patch 'apply --cached', @_; },
+ APPLY_CHECK => 'apply --cached',
+ VERB => 'Stash',
+ TARGET => '',
+ PARTICIPLE => 'stashing',
+ FILTER => undef,
+ IS_REVERSE => 0,
+ },
},
- 'reset_head' => {
- DIFF => 'diff-index -p --cached',
- APPLY => sub { apply_patch 'apply -R --cached', @_; },
- APPLY_CHECK => 'apply -R --cached',
- VERB => 'Unstage',
- TARGET => '',
- PARTICIPLE => 'unstaging',
- FILTER => 'index-only',
- IS_REVERSE => 1,
+ 'reset' => {
+ default => 'head',
+ head => {
+ DIFF => 'diff-index -p --cached',
+ APPLY => sub { apply_patch 'apply -R --cached', @_; },
+ APPLY_CHECK => 'apply -R --cached',
+ VERB => 'Unstage',
+ TARGET => '',
+ PARTICIPLE => 'unstaging',
+ FILTER => 'index-only',
+ IS_REVERSE => 1,
+ },
+ nothead => {
+ DIFF => 'diff-index -R -p --cached',
+ APPLY => sub { apply_patch 'apply --cached', @_; },
+ APPLY_CHECK => 'apply --cached',
+ VERB => 'Apply',
+ TARGET => ' to index',
+ PARTICIPLE => 'applying',
+ FILTER => 'index-only',
+ IS_REVERSE => 0,
+ },
},
- 'reset_nothead' => {
- DIFF => 'diff-index -R -p --cached',
- APPLY => sub { apply_patch 'apply --cached', @_; },
- APPLY_CHECK => 'apply --cached',
- VERB => 'Apply',
- TARGET => ' to index',
- PARTICIPLE => 'applying',
- FILTER => 'index-only',
- IS_REVERSE => 0,
- },
- 'checkout_index' => {
- DIFF => 'diff-files -p',
- APPLY => sub { apply_patch 'apply -R', @_; },
- APPLY_CHECK => 'apply -R',
- VERB => 'Discard',
- TARGET => ' from worktree',
- PARTICIPLE => 'discarding',
- FILTER => 'file-only',
- IS_REVERSE => 1,
- },
- 'checkout_head' => {
- DIFF => 'diff-index -p',
- APPLY => sub { apply_patch_for_checkout_commit '-R', @_ },
- APPLY_CHECK => 'apply -R',
- VERB => 'Discard',
- TARGET => ' from index and worktree',
- PARTICIPLE => 'discarding',
- FILTER => undef,
- IS_REVERSE => 1,
- },
- 'checkout_nothead' => {
- DIFF => 'diff-index -R -p',
- APPLY => sub { apply_patch_for_checkout_commit '', @_ },
- APPLY_CHECK => 'apply',
- VERB => 'Apply',
- TARGET => ' to index and worktree',
- PARTICIPLE => 'applying',
- FILTER => undef,
- IS_REVERSE => 0,
+ 'checkout' => {
+ default => 'index',
+ index => {
+ DIFF => 'diff-files -p',
+ APPLY => sub { apply_patch 'apply -R', @_; },
+ APPLY_CHECK => 'apply -R',
+ VERB => 'Discard',
+ TARGET => ' from worktree',
+ PARTICIPLE => 'discarding',
+ FILTER => 'file-only',
+ IS_REVERSE => 1,
+ },
+ head => {
+ DIFF => 'diff-index -p',
+ APPLY => sub { apply_patch_for_checkout_commit '-R', @_ },
+ APPLY_CHECK => 'apply -R',
+ VERB => 'Discard',
+ TARGET => ' from index and worktree',
+ PARTICIPLE => 'discarding',
+ FILTER => undef,
+ IS_REVERSE => 1,
+ },
+ nothead => {
+ DIFF => 'diff-index -R -p',
+ APPLY => sub { apply_patch_for_checkout_commit '', @_ },
+ APPLY_CHECK => 'apply',
+ VERB => 'Apply',
+ TARGET => ' to index and worktree',
+ PARTICIPLE => 'applying',
+ FILTER => undef,
+ IS_REVERSE => 0,
+ },
},
);
@@ -1546,45 +1558,45 @@ EOF
sub process_args {
return unless @ARGV;
- my $arg = shift @ARGV;
- if ($arg =~ /--patch(?:=(.*))?/) {
- if (defined $1) {
- if ($1 eq 'reset') {
- $patch_mode = 'reset_head';
+
+ while (@ARGV) {
+ if ($ARGV[0] =~ /--patch(?:=(.*))?/) {
+ $patch_mode = defined $1 ? $1 : 'stage';
+ }
+ else {
+ last;
+ }
+ shift @ARGV;
+ }
+
+ if (@ARGV && $ARGV[0] ne '--') {
+ $patch_mode_revision = shift @ARGV;
+ }
+ @ARGV or die "missing --";
+ shift @ARGV;
+
+ if (defined $patch_mode) {
+ my $mode = $patch_modes{$patch_mode}
+ or die "unknown --patch mode: $patch_mode";
+
+ my $submode;
+ if (!defined $patch_mode_revision) {
+ $submode = $mode->{default};
+ if ($submode eq 'head') {
$patch_mode_revision = 'HEAD';
- $arg = shift @ARGV or die "missing --";
- if ($arg ne '--') {
- $patch_mode_revision = $arg;
- $patch_mode = ($arg eq 'HEAD' ?
- 'reset_head' : 'reset_nothead');
- $arg = shift @ARGV or die "missing --";
- }
- } elsif ($1 eq 'checkout') {
- $arg = shift @ARGV or die "missing --";
- if ($arg eq '--') {
- $patch_mode = 'checkout_index';
- } else {
- $patch_mode_revision = $arg;
- $patch_mode = ($arg eq 'HEAD' ?
- 'checkout_head' : 'checkout_nothead');
- $arg = shift @ARGV or die "missing --";
- }
- } elsif ($1 eq 'stage' or $1 eq 'stash') {
- $patch_mode = $1;
- $arg = shift @ARGV or die "missing --";
- } else {
- die "unknown --patch mode: $1";
}
- } else {
- $patch_mode = 'stage';
- $arg = shift @ARGV or die "missing --";
}
- die "invalid argument $arg, expecting --"
- unless $arg eq "--";
- %patch_mode_flavour = %{$patch_modes{$patch_mode}};
- }
- elsif ($arg ne "--") {
- die "invalid argument $arg, expecting --";
+ elsif ($patch_mode_revision eq 'HEAD') {
+ $submode = 'head';
+ }
+ else {
+ $submode = 'nothead';
+ }
+
+ if (!exists $mode->{$submode}) {
+ die "mode '$patch_mode' cannot handle '$submode'";
+ }
+ %patch_mode_flavour = %{$mode->{$submode}};
}
}
--
1.7.5.4.31.ge4d5e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html