[PATCH 1/5] add--interactive: refactor patch mode argument processing

[Jeff King <peff@peff.net>]

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@peff.net>
---
 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