From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> "git stash -p <pathspec>" should imply "git stash push -p <pathspec>" but that was broken by a code cleanup in c3713cede7 (stash: eliminate crude option parsing, 2020-02-17). This regression is fixed in the first patch. Although "-p" implies the "push" subcommand "--patch" has never implied "push". That is fixed in the second patch. Thanks to Junio for his comments on V1. Changes since V1: - Split out the regression fix into its own patch Base-Commit: 1a8a4971cc6c179c4dd711f4a7f5d7178f4b3ab7 Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fstash-assume-push-with-dash-p%2Fv2 View-Changes-At: https://github.com/phillipwood/git/compare/1a8a4971c...98ad3de97 Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/stash-assume-push-with-dash-p/v2 Phillip Wood (2): stash: allow "git stash -p <pathspec>" to assume push again stash: allow "git stash [<options>] --patch <pathspec>" to assume push builtin/stash.c | 10 +++++++--- t/t3903-stash.sh | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) Range-diff against v1: 1: 6292feee7c4 ! 1: 2cd67f5cd85 stash: allow "git stash -p <pathspec>" to assume push again @@ Commit message 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 + push subcommand supports pathspecs, 9e140909f61 (stash: allow pathspecs 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 - 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. + Fix this by regression by checking argv[1] instead of argv[0] and add a + couple of tests to prevent future regressions. Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> @@ builtin/stash.c: static int push_stash(int argc, const char **argv, const char * if (argc) { - force_assume = !strcmp(argv[0], "-p"); -+ int flags = PARSE_OPT_KEEP_DASHDASH; -+ -+ if (push_assumed) -+ flags |= PARSE_OPT_STOP_AT_NON_OPTION; -+ ++ force_assume = argc > 1 && !strcmp(argv[1], "-p"); argc = parse_options(argc, argv, prefix, options, push_assumed ? git_stash_usage : -- git_stash_push_usage, -- PARSE_OPT_KEEP_DASHDASH); -+ git_stash_push_usage, flags); -+ force_assume |= patch_mode; - } - - if (argc) { + git_stash_push_usage, ## t/t3903-stash.sh ## @@ t/t3903-stash.sh: test_expect_success 'stash -- <pathspec> stashes and restores the file' ' test_path_is_file bar ' -+test_expect_success 'stash --patch <pathspec> stash and restores the file' ' ++test_expect_success 'stash -p <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 && ++ echo a | git stash -p file && + test_cmp expect-file file && + echo changed-other-file >expect && + test_cmp expect other-file && -: ----------- > 2: 98ad3de9770 stash: allow "git stash [<options>] --patch <pathspec>" to assume push -- 2.49.0.897.gfad3eb7d210
