Re: [PATCH] stash: allow "git stash -p <pathspec>" to assume push again
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 16, 2025, 19:10 UTC
- Message-ID
- <xmqqtt5ktlqm.fsf@gitster.g>
- In-Reply-To
- <6292feee7c4347efad31e9fb2a1763779b7df133.1747407473.git.phillip.wood@dunelm.org.uk>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 6 quoted lines
> From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Historically "git stash [<options>]" was assumed to mean "git stash save > [<options>]". Since 1ada5020b38 (stash: use stash_push for no verb form, > 2017-02-28) it is assumed to mean "git stash push [<options>]". As the > push subcommand supports pathspecs 9e140909f61 (stash: allow pathspecs
Can I safely do "pathspecs" -> "pathspecs," here? I found this sentence hard to read without a comma.
Show 12 quoted lines
> in the no verb form, 2017-02-28) allowed "git stash -p <pathspec>" to > mean "git stash push -p <pathspec>". This was broken in 8c3713cede7 > (stash: eliminate crude option parsing, 2020-02-17) which failed to > account for "push" being added to the start of argv in cmd_stash() > before it calls push_stash() and kept looking in argv[0] for "-p" after > moving the code to push_stash(). > > The support for assuming "push" when "-p" is given introduced in > 9e140909f61 is very narrow, neither "git stash -m <message> -p > <pathspec>" nor "git stash --patch <pathspec>" imply "push" and die > instead. Fix the regression introduced by 8c3713cede7 and relax the > behavior introduced in 9e140909f61 by passing
Hmph, is it too much work to have a patch that only fixes the regression and another that extends the feature on top as a separate patch? Not that I am opposed by the new feature, though.
Show 5 quoted lines
> PARSE_OPT_STOP_AT_NON_OPTION when push is being assumed and then setting > "force_assume" if "--patch" was present. This means "git stash > <pathspec> -p" still dies so do assume the user meant "push" if they > mistype a subcommand name but "git stash -m <message> -p <pathspec>" > will now succeed.
> Tests are added to prevent future regressions.
Nice.
Show 13 quoted lines
> +test_expect_success 'stash --patch <pathspec> stash and restores the file' ' > + cat file >expect-file && > + echo changed-file >file && > + echo changed-other-file >other-file && > + echo a | git stash -m "stash bar" --patch file && > + test_cmp expect-file file && > + echo changed-other-file >expect && > + test_cmp expect other-file && > + git stash pop && > + test_cmp expect other-file && > + echo changed-file >expect && > + test_cmp expect file > +'
OK.
> +test_expect_success 'stash <pathspec> -p is rejected' '
> + test_must_fail git stash file -p 2>err &&
> + test_grep "subcommand wasn${SQ}t specified; ${SQ}push${SQ} can${SQ}t be assumed due to unexpected token ${SQ}file${SQ}" err
> +'Good thing to test.