Re: [PATCH v4] stash: infer "push" when command line starts with an option

[Phillip Wood <phillip.wood123@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 [<pathspec>]" 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 <deveshigurgaon@gmail.com>
> ---
> 
> 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] [<message>]`::
>   
> 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