Re: [PATCH v2 1/2] stash: allow "git stash -p <pathspec>" to assume push again
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Jun 6, 2025, 11:31 UTC
- Message-ID
- <CAN0heSpGtLW8B-wtoMgW7gunMMeVTL1jhk8xN1LBbeeG4f1Fxw@mail.gmail.com>
- In-Reply-To
- <2cd67f5cd85af03ae99a2760a76e9df5a7edfd95.1747733203.git.phillip.wood@dunelm.org.uk>
On Tue, 20 May 2025 at 11:27, Phillip Wood <phillip.wood123@gmail.com> wrote:
> > Fix this by regression by checking argv[1] instead of argv[0] and add a > couple of tests to prevent future regressions.
Show 6 quoted lines
> if (argc) {
> - force_assume = !strcmp(argv[0], "-p");
> + force_assume = argc > 1 && !strcmp(argv[1], "-p");
> argc = parse_options(argc, argv, prefix, options,
> push_assumed ? git_stash_usage :
> git_stash_push_usage,After reading up on 8c3713cede (stash: eliminate crude option parsing, 2020-02-17), I share your analysis. This fix looks correct to me.
Show 13 quoted lines
> +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 -p 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 > +'
This only exercises the patch machinery fairly trivially: all hunks are added. The implementation under test could miss `-p` completely and behave as `git stash push -- file` or some variant of it, and this test would continue to pass. (Confirmed by editing the test to not use `-p` and seeing it run successfully.)
It might be worthwhile to set up some more elaborate scenario where you pick only some hunks, e.g., this (whitespace-damaged) diff:
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index d24559a328..3b28504126 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1178,16 +1178,19 @@ test_expect_success 'stash -- <pathspec> stashes and restores the file' ' ' test_expect_success 'stash -p <pathspec> stash and restores the file' ' - cat file >expect-file && - echo changed-file >file && + test_write_lines b c >file && + git commit -m "a few lines" -- file && + test_write_lines a b c d >file && + test_write_lines b c d >expect-file && echo changed-other-file >other-file && - echo a | git stash -p file && + test_write_lines s y n | git stash -p file && test_cmp expect-file file && echo changed-other-file >expect && test_cmp expect other-file && + test_write_lines b c >file && git stash pop && test_cmp expect other-file && - echo changed-file >expect && + test_write_lines a b c >expect && test_cmp expect file ' Martin