* [PATCH] stash: infer "push" when push-specific options are given
@ 2026-04-04 14:36 Deveshi Dwivedi
2026-04-04 15:19 ` Mirko Faina
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-04 14:36 UTC (permalink / raw)
To: git; +Cc: ben.knoble, mroik, quentin.bernet, gitster, Deveshi Dwivedi
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
unambiguously a "push" option, and the user's intent is clear. The
same applies to other push-specific options like "--staged",
"--keep-index", "--include-untracked", and "--pathspec-from-file".
Expand the set of options that force the "push" assumption to
include all push-specific options, so that pathspec arguments are
accepted without requiring "--" or the explicit "push" subcommand
when the command line already contains a push-only option.
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>
---
builtin/stash.c | 4 +++-
t/t3903-stash.sh | 21 +++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index 95c5005b0b..197814241c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1877,7 +1877,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, options,
push_assumed ? git_stash_usage :
git_stash_push_usage, flags);
- force_assume |= patch_mode;
+ force_assume |= patch_mode || stash_msg ||
+ keep_index != -1 || only_staged ||
+ include_untracked || pathspec_from_file;
}
if (argc) {
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 70879941c2..9812b64989 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -414,6 +414,27 @@ test_expect_success 'dont assume push with non-option args' '
test_grep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err
'
+test_expect_success 'assume push when options imply push' '
+ git reset --hard &&
+ 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
+'
+
test_expect_success 'stash --invalid-option' '
echo bar5 >file &&
echo bar6 >file2 &&
base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
--
2.52.0.230.gd8af7cadaa
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] stash: infer "push" when push-specific options are given
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
@ 2026-04-04 15:19 ` Mirko Faina
2026-04-04 16:03 ` [PATCH v2] " Deveshi Dwivedi
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Mirko Faina @ 2026-04-04 15:19 UTC (permalink / raw)
To: Deveshi Dwivedi; +Cc: git, ben.knoble, quentin.bernet, gitster
On Sat, Apr 04, 2026 at 02:36:40PM +0000, Deveshi Dwivedi wrote:
> +test_expect_success 'assume push when options imply push' '
> + git reset --hard &&
> + 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
> +'
> +
> test_expect_success 'stash --invalid-option' '
> echo bar5 >file &&
> echo bar6 >file2 &&
The last 'untracked-file' remains in the working tree after the pop. I
know it doesn't affect later tests but can you clean it up anyway? I
recently fixed a test that failed due to poor cleanup and I would like
to see people prevent that too.
Thank you
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] stash: infer "push" when push-specific options are given
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
2026-04-04 15:19 ` Mirko Faina
@ 2026-04-04 16:03 ` Deveshi Dwivedi
2026-04-04 23:40 ` Mirko Faina
2026-04-05 11:09 ` [PATCH v3] " Deveshi Dwivedi
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-04 16:03 UTC (permalink / raw)
To: git; +Cc: ben.knoble, mroik, quentin.bernet, gitster, Deveshi Dwivedi
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
unambiguously a "push" option, and the user's intent is clear. The
same applies to other push-specific options like "--staged",
"--keep-index", "--include-untracked", and "--pathspec-from-file".
Expand the set of options that force the "push" assumption to
include all push-specific options, so that pathspec arguments are
accepted without requiring "--" or the explicit "push" subcommand
when the command line already contains a push-only option.
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 v1:
- Clean up untracked-file after stash pop in test
builtin/stash.c | 4 +++-
t/t3903-stash.sh | 22 ++++++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index 95c5005b0b..197814241c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1877,7 +1877,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, options,
push_assumed ? git_stash_usage :
git_stash_push_usage, flags);
- force_assume |= patch_mode;
+ force_assume |= patch_mode || stash_msg ||
+ keep_index != -1 || only_staged ||
+ include_untracked || pathspec_from_file;
}
if (argc) {
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 70879941c2..07bee22c27 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -414,6 +414,28 @@ test_expect_success 'dont assume push with non-option args' '
test_grep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err
'
+test_expect_success 'assume push when options imply push' '
+ git reset --hard &&
+ 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 &&
+ rm -f untracked-file
+'
+
test_expect_success 'stash --invalid-option' '
echo bar5 >file &&
echo bar6 >file2 &&
base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
--
2.52.0.230.gd8af7cadaa
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] stash: infer "push" when push-specific options are given
2026-04-04 16:03 ` [PATCH v2] " Deveshi Dwivedi
@ 2026-04-04 23:40 ` Mirko Faina
2026-04-05 7:02 ` Deveshi Dwivedi
0 siblings, 1 reply; 17+ messages in thread
From: Mirko Faina @ 2026-04-04 23:40 UTC (permalink / raw)
To: Deveshi Dwivedi; +Cc: git, ben.knoble, quentin.bernet, gitster, Mirko Faina
On Sat, Apr 04, 2026 at 04:03:57PM +0000, Deveshi Dwivedi wrote:
> +test_expect_success 'assume push when options imply push' '
> + git reset --hard &&
> + 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 &&
> + rm -f untracked-file
> +'
> +
This leaves 'file' in the staging area. Using "git reset --hard" like
you did at the start is probably the easiest way to clean up.
Apart from the testing, since the following is not true anymore...
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.
...you should probably change the documentation for "git push" as well.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] stash: infer "push" when push-specific options are given
2026-04-04 23:40 ` Mirko Faina
@ 2026-04-05 7:02 ` Deveshi Dwivedi
0 siblings, 0 replies; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-05 7:02 UTC (permalink / raw)
To: Mirko Faina; +Cc: git, ben.knoble, quentin.bernet, gitster
> > +test_expect_success 'assume push when options imply push' '
> > + git reset --hard &&
> > + 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 &&
> > + rm -f untracked-file
> > +'
> > +
>
> This leaves 'file' in the staging area. Using "git reset --hard" like
> you did at the start is probably the easiest way to clean up.
>
Understood, will use "git reset --hard" here as well.
> Apart from the testing, since the following is not true anymore...
>
> 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.
>
> ...you should probably change the documentation for "git push" as well.
Yes, I will update the documentation as well and send a v3 shortly.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
2026-04-04 15:19 ` Mirko Faina
2026-04-04 16:03 ` [PATCH v2] " Deveshi Dwivedi
@ 2026-04-05 11:09 ` Deveshi Dwivedi
2026-04-06 18:15 ` Mirko Faina
2026-04-09 20:22 ` Junio C Hamano
2026-04-12 19:52 ` [PATCH v4] stash: infer "push" when command line starts with an option Deveshi Dwivedi
2026-04-19 16:54 ` [PATCH v5] stash: assume " Deveshi Dwivedi
4 siblings, 2 replies; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-05 11:09 UTC (permalink / raw)
To: git; +Cc: ben.knoble, mroik, quentin.bernet, gitster, Deveshi Dwivedi
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
unambiguously a "push" option, and the user's intent is clear. The
same applies to other push-specific options like "--staged",
"--keep-index", "--include-untracked", and "--pathspec-from-file".
Expand the set of options that force the "push" assumption to
include all push-specific options, so that pathspec arguments are
accepted without requiring "--" or the explicit "push" subcommand
when the command line already contains a push-only option.
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 v2:
- Clean up staged file with git reset --hard at end of test
- Update documentation to reflect new push inference behavior
Documentation/git-stash.adoc | 7 ++++---
builtin/stash.c | 4 +++-
t/t3903-stash.sh | 23 +++++++++++++++++++++++
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 235d57ddd8..e4e77c2f07 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
+any push-specific option is given, the "push" subcommand is inferred
+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..197814241c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1877,7 +1877,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, options,
push_assumed ? git_stash_usage :
git_stash_push_usage, flags);
- force_assume |= patch_mode;
+ force_assume |= patch_mode || stash_msg ||
+ keep_index != -1 || only_staged ||
+ include_untracked || pathspec_from_file;
}
if (argc) {
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 70879941c2..f021dc9068 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -414,6 +414,29 @@ test_expect_success 'dont assume push with non-option args' '
test_grep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err
'
+test_expect_success 'assume push when options imply push' '
+ git reset --hard &&
+ 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 &&
+ rm -f untracked-file &&
+ git reset --hard
+'
+
test_expect_success 'stash --invalid-option' '
echo bar5 >file &&
echo bar6 >file2 &&
base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
--
2.52.0.230.gd8af7cadaa
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-05 11:09 ` [PATCH v3] " Deveshi Dwivedi
@ 2026-04-06 18:15 ` Mirko Faina
2026-04-07 9:36 ` Phillip Wood
2026-04-09 20:22 ` Junio C Hamano
1 sibling, 1 reply; 17+ messages in thread
From: Mirko Faina @ 2026-04-06 18:15 UTC (permalink / raw)
To: Deveshi Dwivedi; +Cc: git, ben.knoble, quentin.bernet, gitster, Mirko Faina
On Sun, Apr 05, 2026 at 11:09:53AM +0000, 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
> unambiguously a "push" option, and the user's intent is clear. The
> same applies to other push-specific options like "--staged",
> "--keep-index", "--include-untracked", and "--pathspec-from-file".
>
> Expand the set of options that force the "push" assumption to
> include all push-specific options, so that pathspec arguments are
> accepted without requiring "--" or the explicit "push" subcommand
> when the command line already contains a push-only option.
>
> 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>
Just realized, "--include-untracked" is not specific only to 'push' as
'show' accepts it too as an argument. "--keep-index" as well, but since
'save' is deprecated I don't think anyone would mind and should be fine
to leave it as is (though this is my opinion, should wait for others to
see what they think).
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-06 18:15 ` Mirko Faina
@ 2026-04-07 9:36 ` Phillip Wood
2026-04-09 19:22 ` Deveshi Dwivedi
2026-04-09 20:31 ` Junio C Hamano
0 siblings, 2 replies; 17+ messages in thread
From: Phillip Wood @ 2026-04-07 9:36 UTC (permalink / raw)
To: Mirko Faina, Deveshi Dwivedi; +Cc: git, ben.knoble, quentin.bernet, gitster
On 06/04/2026 19:15, Mirko Faina wrote:
> On Sun, Apr 05, 2026 at 11:09:53AM +0000, 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
>> unambiguously a "push" option, and the user's intent is clear. The
>> same applies to other push-specific options like "--staged",
>> "--keep-index", "--include-untracked", and "--pathspec-from-file".
>>
>> Expand the set of options that force the "push" assumption to
>> include all push-specific options, so that pathspec arguments are
>> accepted without requiring "--" or the explicit "push" subcommand
>> when the command line already contains a push-only option.
>>
>> 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>
>
> Just realized, "--include-untracked" is not specific only to 'push' as
> 'show' accepts it too as an argument.
"create" accepts "-m" as well so that's not unique either. I agree with
Junio's suggestion in the link above that we should assume "push" when
there is no subcommand given and error out if we see an unsupported
option. That does not require the arguments to be unique to "push". A
complete implementation would also support negated options like "git
stash --no-stage [<pathspec>]". What is implemented in this patch maybe
sufficient in practice but it would be good to mention the limitations
in the commit message.
Thanks
Phillip
> "--keep-index" as well, but since
> 'save' is deprecated I don't think anyone would mind and should be fine
> to leave it as is (though this is my opinion, should wait for others to
> see what they think).
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-07 9:36 ` Phillip Wood
@ 2026-04-09 19:22 ` Deveshi Dwivedi
2026-04-09 19:37 ` Mirko Faina
2026-04-09 20:31 ` Junio C Hamano
1 sibling, 1 reply; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-09 19:22 UTC (permalink / raw)
To: phillip.wood; +Cc: Mirko Faina, git, ben.knoble, quentin.bernet, gitster
> "create" accepts "-m" as well so that's not unique either. I agree with
> Junio's suggestion in the link above that we should assume "push" when
> there is no subcommand given and error out if we see an unsupported
> option. That does not require the arguments to be unique to "push". A
> complete implementation would also support negated options like "git
> stash --no-stage [<pathspec>]". What is implemented in this patch maybe
> sufficient in practice but it would be good to mention the limitations
> in the commit message.
>
> Thanks
>
> Phillip
Sure, I will send a v4 with an updated commit message. Thank you.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-09 19:22 ` Deveshi Dwivedi
@ 2026-04-09 19:37 ` Mirko Faina
0 siblings, 0 replies; 17+ messages in thread
From: Mirko Faina @ 2026-04-09 19:37 UTC (permalink / raw)
To: Deveshi Dwivedi
Cc: phillip.wood, git, ben.knoble, quentin.bernet, gitster,
Mirko Faina
On Fri, Apr 10, 2026 at 12:52:06AM +0530, Deveshi Dwivedi wrote:
> > "create" accepts "-m" as well so that's not unique either. I agree with
> > Junio's suggestion in the link above that we should assume "push" when
> > there is no subcommand given and error out if we see an unsupported
> > option. That does not require the arguments to be unique to "push". A
> > complete implementation would also support negated options like "git
> > stash --no-stage [<pathspec>]". What is implemented in this patch maybe
> > sufficient in practice but it would be good to mention the limitations
> > in the commit message.
> >
> > Thanks
> >
> > Phillip
> Sure, I will send a v4 with an updated commit message. Thank you.
I think the documentation should change as well as it can be ambiguous.
From reading "push-specific options" I'm assuming it would work only
with options that are unique to push, which is not the case, while
others might assume the correct behaviour which works with any option
that push accepts.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-05 11:09 ` [PATCH v3] " Deveshi Dwivedi
2026-04-06 18:15 ` Mirko Faina
@ 2026-04-09 20:22 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-04-09 20:22 UTC (permalink / raw)
To: Deveshi Dwivedi; +Cc: git, ben.knoble, mroik, quentin.bernet
Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:
> +test_expect_success 'assume push when options imply push' '
> + git reset --hard &&
> + 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 &&
A comment on these three lines.
> + git stash pop &&
> + rm -f untracked-file &&
> + git reset --hard
> +'
> +
I suspect that they are meant to be "clean-up after we are done with
the test, to avoid interfering with the next test", but if so,
"clean-up at the very end" is not a very effective strategy to do
so. Imagine that one of the previous steps fails, breaking all
later commands in the &&- cascade. Sitting at the very end, your
clean-up sequence will not run. Unless the tester is running this
test script with the "-i" option, the test will move on to the next
piece. Installing clean-up handler with test_when_finished may be
a cleaner approach.
test_expect_success 'do this test' '
test_when_finished "git stash clear; git reset --hard" &&
git reset --hard &&
... do all the dirty things in the working tree ...
test_when_finished "rm -f untracked-file" &&
echo untracked >untracked-file &&
git stash --include-untracked untracked-file &&
test_path_is_missing untracked-file
'
You can use more than one test_when_finished in a single test.
It is often done to add an upfront blunt hammer at the beginning to
do a clean-up without worrying too much about where exactly in the
command sequence a breakage may happen (e.g., we may fail before we
run our first "git add", or "git stash", and "git reset --hard" or
"git stash clear" may be an unnecessary no-op, but we do not worry
too much about the clean-up step doing potentially unnecessary
things.
Or you would set up a clean-up handler immediately before you create
a thing that you want to make sure you clean up. If the command
sequence fails before you echo the string into untracked-file to
create it, there is no point preparing to remove it when you are
done.
Both approaches are commmonly used.
> test_expect_success 'stash --invalid-option' '
> echo bar5 >file &&
> echo bar6 >file2 &&
>
> base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] stash: infer "push" when push-specific options are given
2026-04-07 9:36 ` Phillip Wood
2026-04-09 19:22 ` Deveshi Dwivedi
@ 2026-04-09 20:31 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-04-09 20:31 UTC (permalink / raw)
To: Phillip Wood
Cc: Mirko Faina, Deveshi Dwivedi, git, ben.knoble, quentin.bernet
Phillip Wood <phillip.wood123@gmail.com> writes:
> "create" accepts "-m" as well so that's not unique either. I agree with
> Junio's suggestion in the link above that we should assume "push" when
> there is no subcommand given and error out if we see an unsupported
> option.
Yeah, if -X were unique for "pop" and -Y were unique for "push", it
is tempting to DWIM "git stash -X" to "git stash pop -X" while
DWIMming "git stash -Y" to "git stash push -Y", but the thing is
that the urgency of each "stash" subcommand is different. As the
"the boss is here and tells me to work on this completely unrelated
thing, clear the desk as quickly as possible to switch context"
command, "push" deserves to have more quick access than other
commands.
It also makes it resilient if we said "a command line that begins
with an option cannot be naming any 'git stash' subcommand, so we
will unconditionally insert 'push' before that first option",
because 'push' may later acquire "-X" or 'save' may acquire "-Y" and
making these options no longer unique to a single subcommand. A
version of Git may have treated "git stash -Y" as "git stash push -Y"
but if the next version that has "git stash save -Y" stopped accepting
"git stash -Y" as "git stash push -Y" because -Y is not unique, the
end users will be unhappy.
And of course it is far easier to document and teach.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4] stash: infer "push" when command line starts with an option
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
` (2 preceding siblings ...)
2026-04-05 11:09 ` [PATCH v3] " Deveshi Dwivedi
@ 2026-04-12 19:52 ` Deveshi Dwivedi
2026-04-13 9:08 ` Phillip Wood
2026-04-13 15:09 ` Junio C Hamano
2026-04-19 16:54 ` [PATCH v5] stash: assume " Deveshi Dwivedi
4 siblings, 2 replies; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-12 19:52 UTC (permalink / raw)
To: git; +Cc: ben.knoble, mroik, quentin.bernet, gitster, Deveshi Dwivedi
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.
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
+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;
+ }
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
+'
+
+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
'
test_expect_success 'stash --invalid-option' '
base-commit: 2855562ca6a9c6b0e7bc780b050c1e83c9fcfbd0
--
2.52.0.230.gd8af7cadaa
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4] stash: infer "push" when command line starts with an option
2026-04-12 19:52 ` [PATCH v4] stash: infer "push" when command line starts with an option Deveshi Dwivedi
@ 2026-04-13 9:08 ` Phillip Wood
2026-04-13 15:09 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Phillip Wood @ 2026-04-13 9:08 UTC (permalink / raw)
To: Deveshi Dwivedi, git; +Cc: ben.knoble, mroik, quentin.bernet, gitster
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4] stash: infer "push" when command line starts with an option
2026-04-12 19:52 ` [PATCH v4] stash: infer "push" when command line starts with an option Deveshi Dwivedi
2026-04-13 9:08 ` Phillip Wood
@ 2026-04-13 15:09 ` Junio C Hamano
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2026-04-13 15:09 UTC (permalink / raw)
To: Deveshi Dwivedi; +Cc: git, ben.knoble, mroik, quentin.bernet
Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:
> 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.
I think "run without the 'push' subcommand" above should be "run
without any subcommand (on the command line)". "git stash pop
-- paths" is run without the "push" subcommand, and obviously we do
not want it to assume "push".
> 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.
Good.
> 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 is unexpected, and unfortunate. I would have expected, since
we are now sending any thing that is not unrecognised to "push", it
would largely be the matter of removing special casing code about
push_assumed from push_stash() and adding some to its caller, which
is cmd_stash(). It would first look at its table of subcommands and
if it finds a hit, calls the handler. If not, and if there is no
argument or if the first argument begins with a dash "if (argv[1] &&
argv[1][0] == '-')", then unshift "push" into the argv[] array, and
call push_stash(), which would complain if the command line (with
"push" prepended) does not make sense to it. If the above two
conditions were not met, cmd_stash() would not call push_stash() but
complain that it did not get a valid command. Or something like that.
That way, there is nothing that makes "--no-something" any more
special than "--something", no?
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v5] stash: assume "push" when command line starts with an option
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
` (3 preceding siblings ...)
2026-04-12 19:52 ` [PATCH v4] stash: infer "push" when command line starts with an option Deveshi Dwivedi
@ 2026-04-19 16:54 ` Deveshi Dwivedi
2026-04-21 15:28 ` Phillip Wood
4 siblings, 1 reply; 17+ messages in thread
From: Deveshi Dwivedi @ 2026-04-19 16:54 UTC (permalink / raw)
To: git; +Cc: ben.knoble, mroik, quentin.bernet, gitster, Deveshi Dwivedi
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v5] stash: assume "push" when command line starts with an option
2026-04-19 16:54 ` [PATCH v5] stash: assume " Deveshi Dwivedi
@ 2026-04-21 15:28 ` Phillip Wood
0 siblings, 0 replies; 17+ messages in thread
From: Phillip Wood @ 2026-04-21 15:28 UTC (permalink / raw)
To: Deveshi Dwivedi, git; +Cc: ben.knoble, mroik, quentin.bernet, gitster
On 19/04/2026 17:54, Deveshi Dwivedi wrote:
This has changed more than I was expecting it to, but I think the
approach of checking if argv[0] starts with '-' in cmd_stash() makes
sense. I've left a few comments below.
> 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);
As all we do with '--' is remove it now that we don't need to check if
it was passed, there is not much point in asking parse_options() to keep
it for us. You can just pass '0' here and drop the if statement below.
>
> - 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]);
If there's no option then we die straight away which makes sense. Part
of me wonders if we should also show the usage for "git stash" in case
the user misspelled a subcommand name. The other part of me finds the
way git is so keen to print reams of usage at the slightest provocation
quite annoying, but at least the "git stash" usage is fairly small.
Looking at the history we used to complain about an unknown subcommand
but that was changed without any explanation in 8c3713cede (stash:
eliminate crude option parsing, 2020-02-17)
> /*
> - * `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.
> */
I'm not really sure if this change to the usage is an improvement or
not. If we document that an option without a subcommand means "push"
then isn't it confusing to show the usage for "git stash" rather than
"git stash push"?
> - 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++;
Taking a shallow copy avoids the memory leak mentioned in the comment
you edited above. That might be worth doing but it should be done as a
separate preparatory step because it is orthogonal to the other changes
here. We should use COPY_ARRAY() rather than memcpy() and we should also
allocate 'argc + 2' and copy 'argc + 1' elements to keep the array NULL
terminated as it was prior to 2e875b6cb4 (builtin/stash: fix various
trivial memory leaks, 2024-08-01).
Thanks
Phillip
> + 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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-04-21 15:28 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-04 14:36 [PATCH] stash: infer "push" when push-specific options are given Deveshi Dwivedi
2026-04-04 15:19 ` Mirko Faina
2026-04-04 16:03 ` [PATCH v2] " Deveshi Dwivedi
2026-04-04 23:40 ` Mirko Faina
2026-04-05 7:02 ` Deveshi Dwivedi
2026-04-05 11:09 ` [PATCH v3] " Deveshi Dwivedi
2026-04-06 18:15 ` Mirko Faina
2026-04-07 9:36 ` Phillip Wood
2026-04-09 19:22 ` Deveshi Dwivedi
2026-04-09 19:37 ` Mirko Faina
2026-04-09 20:31 ` Junio C Hamano
2026-04-09 20:22 ` Junio C Hamano
2026-04-12 19:52 ` [PATCH v4] stash: infer "push" when command line starts with an option Deveshi Dwivedi
2026-04-13 9:08 ` Phillip Wood
2026-04-13 15:09 ` Junio C Hamano
2026-04-19 16:54 ` [PATCH v5] stash: assume " Deveshi Dwivedi
2026-04-21 15:28 ` Phillip Wood
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox