[PATCH v5] stash: assume "push" when command line starts with an option

[Deveshi Dwivedi <deveshigurgaon@gmail.com>]

When "git stash" is run without any subcommand (on the command line),
the command tries to assume "push" but rejects non-option arguments
(i.e., pathspecs without "--") to avoid treating a misspelled
subcommand name as a pathspec.

This means "git stash -m foo file" is rejected even though "-m" is
clearly an option and not a subcommand name, and the user's intent
is clear.

A command line that begins with an option cannot be naming a "git
stash" subcommand, so move the decision to cmd_stash(): when no
subcommand matches and the first argument starts with "-", assume
"push". When the first argument does not start with "-", reject it as
an unexpected token as before.

This simplifies push_stash() by removing push_assumed/force_assume
logic; the caller has already made the assumption decision.

This was marked as #leftoverbits in [1].

[1] https://lore.kernel.org/git/xmqqtsu1jipp.fsf@gitster.g/

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>

---

Changes since v4:
  - Move push-assumption logic from push_stash() to cmd_stash().
  - Remove stale push_assumed/force_assume handling.
  - Drop incorrect note about negated options being rejected.
  - Use "assumed" (not "inferred") in documentation.
  - Update tests/style (including ${SQ}) and add --no-keep-index coverage.
---
 Documentation/git-stash.adoc |  7 +++--
 builtin/stash.c              | 53 ++++++++++++++----------------------
 t/t3903-stash.sh             | 30 ++++++++++++++++++--
 3 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 235d57ddd8..946b51c7a5 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -61,9 +61,10 @@ COMMANDS
 +
 For quickly making a snapshot, you can omit "push".  In this mode,
 non-option arguments are not allowed to prevent a misspelled
-subcommand from making an unwanted stash entry.  The two exceptions to this
-are `stash -p` which acts as alias for `stash push -p` and pathspec elements,
-which are allowed after a double hyphen `--` for disambiguation.
+subcommand from making an unwanted stash entry.  Pathspec elements
+are allowed after a double hyphen `--` for disambiguation.  When
+the command line begins with an option, "push" is assumed and
+pathspec arguments are also accepted without `--`.
 
 `save [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-u | --include-untracked] [-a | --all] [-q | --quiet] [<message>]`::
 
diff --git a/builtin/stash.c b/builtin/stash.c
index 95c5005b0b..bf04cf58a6 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1831,9 +1831,8 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 }
 
 static int push_stash(int argc, const char **argv, const char *prefix,
-		      int push_assumed)
+		      const char * const usage[])
 {
-	int force_assume = 0;
 	int keep_index = -1;
 	int only_staged = 0;
 	int patch_mode = 0;
@@ -1868,26 +1867,14 @@ static int push_stash(int argc, const char **argv, const char *prefix,
 	};
 	int ret;
 
-	if (argc) {
-		int flags = PARSE_OPT_KEEP_DASHDASH;
-
-		if (push_assumed)
-			flags |= PARSE_OPT_STOP_AT_NON_OPTION;
-
+	if (argc)
 		argc = parse_options(argc, argv, prefix, options,
-				     push_assumed ? git_stash_usage :
-				     git_stash_push_usage, flags);
-		force_assume |= patch_mode;
-	}
+				     usage,
+				     PARSE_OPT_KEEP_DASHDASH);
 
-	if (argc) {
-		if (!strcmp(argv[0], "--")) {
-			argc--;
-			argv++;
-		} else if (push_assumed && !force_assume) {
-			die("subcommand wasn't specified; 'push' can't be assumed due to unexpected token '%s'",
-			    argv[0]);
-		}
+	if (argc && !strcmp(argv[0], "--")) {
+		argc--;
+		argv++;
 	}
 
 	parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,
@@ -1935,7 +1922,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
 static int push_stash_unassumed(int argc, const char **argv, const char *prefix,
 				struct repository *repo UNUSED)
 {
-	return push_stash(argc, argv, prefix, 0);
+	return push_stash(argc, argv, prefix, git_stash_push_usage);
 }
 
 static int save_stash(int argc, const char **argv, const char *prefix,
@@ -2387,7 +2374,6 @@ int cmd_stash(int argc,
 {
 	pid_t pid = getpid();
 	const char *index_file;
-	struct strvec args = STRVEC_INIT;
 	parse_opt_subcommand_fn *fn = NULL;
 	struct option options[] = {
 		OPT_SUBCOMMAND("apply", &fn, apply_stash),
@@ -2427,19 +2413,22 @@ int cmd_stash(int argc,
 	else if (!argc)
 		return !!push_stash_unassumed(0, NULL, prefix, repo);
 
-	/* Assume 'stash push' */
-	strvec_push(&args, "push");
-	strvec_pushv(&args, argv);
+	if (argv[0][0] != '-')
+		die("subcommand wasn't specified; 'push' can't be assumed due to unexpected token '%s'",
+		    argv[0]);
 
 	/*
-	 * `push_stash()` ends up modifying the array, which causes memory
-	 * leaks if we didn't copy the array here.
+	 * When the command line starts with an option, assume 'push'.
+	 * Unshift "push" into argv so that parse_options() skips it
+	 * as the subcommand name.  Use git_stash_usage so that invalid
+	 * options show the general stash usage rather than the
+	 * push-specific usage.
 	 */
-	DUP_ARRAY(args_copy, args.v, args.nr);
-
-	ret = !!push_stash(args.nr, args_copy, prefix, 1);
-
-	strvec_clear(&args);
+	ALLOC_ARRAY(args_copy, argc + 1);
+	args_copy[0] = "push";
+	memcpy(&args_copy[1], argv, argc * sizeof(const char *));
+	argc++;
+	ret = !!push_stash(argc, args_copy, prefix, git_stash_usage);
 	free(args_copy);
 	return ret;
 }
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 70879941c2..836cc29a6b 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -410,8 +410,34 @@ test_expect_success 'stash --staged with binary file' '
 '
 
 test_expect_success 'dont assume push with non-option args' '
-	test_must_fail git stash -q drop 2>err &&
-	test_grep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err
+	test_must_fail git stash someunknown 2>err &&
+	test_grep "subcommand wasn${SQ}t specified; ${SQ}push${SQ} can${SQ}t be assumed due to unexpected token ${SQ}someunknown${SQ}" err
+'
+
+test_expect_success 'assume push when command line starts with option' '
+	test_when_finished "git reset --hard" &&
+	test_when_finished "rm -f untracked-file" &&
+	echo changed >file &&
+	git add file &&
+	git stash -m "implied push" file &&
+	git stash pop &&
+
+	git add file &&
+	git stash --staged file &&
+	git stash pop &&
+
+	git add file &&
+	git stash --keep-index file &&
+	git stash pop &&
+
+	git add file &&
+	git stash --no-keep-index file &&
+	git stash pop &&
+
+	echo untracked >untracked-file &&
+	git stash --include-untracked untracked-file &&
+	test_path_is_missing untracked-file &&
+	git stash pop
 '
 
 test_expect_success 'stash --invalid-option' '

base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
-- 
2.52.0.230.gd8af7cadaa