public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Phillip Wood <phillip.wood123@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] stash: allow "git stash -p <pathspec>" to assume push again
Date: Fri, 16 May 2025 12:10:41 -0700	[thread overview]
Message-ID: <xmqqtt5ktlqm.fsf@gitster.g> (raw)
In-Reply-To: <6292feee7c4347efad31e9fb2a1763779b7df133.1747407473.git.phillip.wood@dunelm.org.uk> (Phillip Wood's message of "Fri, 16 May 2025 15:58:29 +0100")

Phillip Wood <phillip.wood123@gmail•com> writes:

> 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.

> 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.

> 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.

> +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.

  reply	other threads:[~2025-05-16 19:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-16 14:58 [PATCH] stash: allow "git stash -p <pathspec>" to assume push again Phillip Wood
2025-05-16 19:10 ` Junio C Hamano [this message]
2025-05-20  9:21   ` Phillip Wood
2025-05-20  9:26 ` [PATCH v2 0/2] stash: fix and improve "git stash -p <pathspec>" Phillip Wood
2025-05-20  9:26   ` [PATCH v2 1/2] stash: allow "git stash -p <pathspec>" to assume push again Phillip Wood
2025-06-06 11:31     ` Martin Ågren
2025-06-06 15:26       ` Phillip Wood
2025-05-20  9:27   ` [PATCH v2 2/2] stash: allow "git stash [<options>] --patch <pathspec>" to assume push Phillip Wood
2025-06-06 11:32     ` Martin Ågren
2025-05-21 13:04   ` [PATCH v2 0/2] stash: fix and improve "git stash -p <pathspec>" Junio C Hamano
2025-06-03 22:11   ` Junio C Hamano
2025-06-06 11:39     ` Martin Ågren
2025-06-07  9:45 ` [PATCH v3 " Phillip Wood
2025-06-07  9:45   ` [PATCH v3 1/2] stash: allow "git stash -p <pathspec>" to assume push again Phillip Wood
2025-06-07  9:45   ` [PATCH v3 2/2] stash: allow "git stash [<options>] --patch <pathspec>" to assume push Phillip Wood
2025-06-07 12:56   ` [PATCH v3 0/2] stash: fix and improve "git stash -p <pathspec>" Martin Ågren
2025-06-09  9:42     ` Phillip Wood
2025-06-10  9:56       ` Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtt5ktlqm.fsf@gitster.g \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=phillip.wood123@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox