On Tue, 20 May 2025 at 11:27, Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
>
> Fix this by regression by checking argv[1] instead of argv[0] and add a
> couple of tests to prevent future regressions.
> 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.
> +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
