From: Phillip Wood Date: Mon, 13 Apr 2026 09:08:13 GMT Subject: Re: [PATCH v4] stash: infer "push" when command line starts with an option Message-ID: <00b92c0b-8602-45e4-bec3-c7c538bd288b@gmail.com> In-Reply-To: <20260412195204.4636-1-deveshigurgaon@gmail.com> On 12/04/2026 20:52, Deveshi Dwivedi wrote: > When "git stash" is run without the "push" subcommand, the command > tries to assume "push" but rejects any non-option arguments (i.e., > pathspecs without "--") to avoid treating a misspelled subcommand > name as a pathspec. The only exception is "-p", which sets > force_assume and allows pathspecs to follow. > > 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. The same applies to any command line that begins with an > option. > > A command line that begins with an option cannot be naming a "git > stash" subcommand, so unconditionally assume "push" in that case and > allow pathspec arguments to follow without requiring "--". This is > simpler and more robust than checking a specific list of options, > and remains correct even if push or other subcommands gain new > options in the future. > > Note that this does not check for negated options, so "git stash > --no-staged []" is still rejected. Handling negated > options would require teaching the inference logic about them > explicitly. That was true of the implementation in V3 which checked to see if any of the option variables were non-zero. Looking below, it now checks if the first argument begins with "-" which means that force_assume will be true when "--no-stage" is given. The implementation looks good, I've left a couple of comments below. > This was marked as #leftoverbits in [1]. > > [1] https://lore.kernel.org/git/xmqqtsu1jipp.fsf@gitster.g/ > > Signed-off-by: Deveshi Dwivedi > --- > > Changes since v3: > - Rewrote the approach per Junio and Phillip's suggestion: instead of > checking a specific list of push-only options, unconditionally > assume "push" whenever the command line begins with any option. > This is simpler and robust against future option additions, and > sidesteps the fact that -m and --include-untracked are not unique > to "push". > - Updated the test to reflect the new rule and switched cleanup to > test_when_finished per Junio's suggestion. > - Updated documentation accordingly. > > Documentation/git-stash.adoc | 7 ++++--- > builtin/stash.c | 6 ++++-- > t/t3903-stash.sh | 26 ++++++++++++++++++++++++-- > 3 files changed, 32 insertions(+), 7 deletions(-) > > diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc > index 235d57ddd8..135719611a 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 inferred and "assumed" might be easier to understand than "inferred" > +pathspec arguments are also accepted without `--`. > > `save [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-u | --include-untracked] [-a | --all] [-q | --quiet] []`:: > > diff --git a/builtin/stash.c b/builtin/stash.c > index 95c5005b0b..be96338d35 100644 > --- a/builtin/stash.c > +++ b/builtin/stash.c > @@ -1871,13 +1871,15 @@ static int push_stash(int argc, const char **argv, const char *prefix, > if (argc) { > int flags = PARSE_OPT_KEEP_DASHDASH; > > - if (push_assumed) > + if (push_assumed) { > flags |= PARSE_OPT_STOP_AT_NON_OPTION; > + if (argc > 1 && argv[1][0] == '-') > + force_assume = 1; We assume push was given if the first argument starts with '-'. That makes sense. If we get on invalid option we'll display the push usage as we did before. > + } > > argc = parse_options(argc, argv, prefix, options, > push_assumed ? git_stash_usage : > git_stash_push_usage, flags); > - force_assume |= patch_mode; > } > > if (argc) { > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 70879941c2..88f2b3c86b 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -410,8 +410,30 @@ 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 -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''someunknown'\''" err This is based on the existing test, but there is no need for "-e" and we normally match a single quote as "${SQ}" (which is defined by the test suite) or simply "." > +' > + > +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 && > + > + echo untracked >untracked-file && > + git stash --include-untracked untracked-file && > + test_path_is_missing untracked-file && > + git stash pop > ' This test looks good Thanks Phillip > test_expect_success 'stash --invalid-option' ' > > base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0