Show 277 quoted lines
> I still need
> to think about whether `--patch` is worth supporting for `stash create`,
> even though it is primarily aimed at scripts.
> >> I wonder if we really need pathspec support, or if we do is
>> "--pathspec-from-file" sufficient?
> > I agree that positional pathspec support probably isn't necessary.
> Since `create` is primarily aimed at scripts, `--pathspec-from-file`
> seems sufficient. It also avoids giving positional arguments another
> meaning while we are already dealing with the message ambiguity.
> >> I think it is fairly unlikely that the message is going to start with
>> '-' so using PARSE_OPT_STOP_AT_NON_OPTION seems like a reasonable way
>> forward to me. Adding "-m/--message" to match other commands that take
>> a message would certainly make sense.
> > Thanks for confirming those points.
> > Thanks,
> Kazumasa
> > On Mon, 5 Oct 2026 17:38:07 +0100, Phillip Wood
> <phillip.wood123@gmail.com> wrote:
>> Hi Kazumasa
>>
>> On 05/10/2026 06:55, 重田一聖 wrote:
>>>
>>> From that perspective, I can see three possible directions.
>>>
>>> 1. Keep extending `stash create`.
>>>
>>> We could expose more of the existing `do_create_stash()`
>>> functionality through `stash create`, following the conventions of
>>> `stash push` for the creation-related options they have in common.
>>>
>>> This seems implementable, but even with
>>> `PARSE_OPT_STOP_AT_NON_OPTION` it would change the handling of
>>> messages that begin with an option-like argument. Those would need
>>> explicit disambiguation, such as `--`.
>>>
>>> There is also the pathspec question. If positional arguments
>>> continue to be joined to form the message, pathspecs need some other
>>> way to be distinguished from that message.
>>
>> It is worth thinking about which options from "push" make sense with
>> "create" as the latter is really aimed at scripts rather than users. I
>> can see a script wanting to stash untracked files, but it may not make
>> sense to add interactive options like "--patch" which sometimes [1]
>> fails to clear the stashed changes from the worktree, that would be
>> problematic for scripts. I wonder if we really need pathspec support, or
>> if we do is "--pathspec-from-file" sufficient? I think it is fairly
>> unlikely that the message is going to start with '-' so using
>> PARSE_OPT_STOP_AT_NON_OPTION seems like a reasonable way forward to me.
>> Adding "-m/--message" to match other commands that take a message would
>> certainly make sense.
>>
>> Thanks
>>
>> Phillip
>>
>> [1] This happens when a user edits a hunk that looks like
>> @@ -1 +1,4 @@
>> -A
>> +a
>> +b
>> +c
>> +d
>>
>> to
>>
>> @@ -1 +1,3 @@
>> -A
>> +a
>> +b
>> +d
>>
>> To clear the stashed changes, we apply the hunk in reverse, so we
>> try to apply
>>
>> @@ -1,3 +1 @@
>> -a
>> -b
>> -d
>> +A
>>
>> to a file that looks like
>>
>> a
>> b
>> c
>> d
>>
>> which fails because the '-' lines do not match the content of the
>> file.
>>
>>>
>>> 2. Add a new stash subcommand for the creation functionality.
>>>
>>> This would leave the existing `stash create <message>` contract
>>> unchanged. Because the new command would not inherit `create`'s
>>> positional message grammar, its creation-related options and
>>> pathspec handling could follow conventions similar to `stash push`.
>>>
>>> This preserves the existing `create` grammar while avoiding the need
>>> to fit additional creation capabilities into it. The trade-off is
>>> adding another public stash subcommand and its long-term maintenance
>>> cost.
>>>
>>> 3. Add something like `--create-only` to `git stash push`.
>>>
>>> This would reuse the existing `push` option grammar without adding
>>> another subcommand.
>>>
>>> I also read the 2019 discussion around `git stash push --snapshot`.
>>> One concern there was that approximately the same end state could
>>> already be obtained with `git stash push && git stash apply`.
>>>
>>> I do not think that particular concern carries over directly here.
>>> `git stash create` already stops at object creation, but its public
>>> interface does not expose more of the creation capabilities already
>>> available in `do_create_stash()`. There is currently no public stash
>>> command that exposes those capabilities while retaining that
>>> create-only boundary.
>>>
>>> That does not mean a similar result cannot be constructed by other
>>> means. The missing piece is a public interface to the existing stash
>>> creation machinery at that boundary.
>>>
>>> Even so, there is still the separate question of whether `push` is
>>> the right place for a creation-only operation in the first place.
>>> The push-specific work around `do_create_stash()` would also need to
>>> be separated carefully.
>>>
>>> All three seem substantially broader than the original `-u` / `-a`
>>> patch.
>>>
>>> If this is worth pursuing further, which of these directions seems the
>>> most plausible? Also, is this the right thread to continue that design
>>> discussion, or would it be better to discuss it separately?
>>>
>>> Thanks again for the guidance,
>>> Kazumasa Shigeta
>>>
>>> On Fri, 2 Oct 2026 05:04:26 -0400, "重田一聖" <kazumasa.shigeta@kanamei.com> wrote:
>>>> Hi Junio,
>>>>
>>>>> we would prefer to hear what the user visible implication of
>>>>> "passing 0" is more than what mechanically is happening inside a
>>>>> program.
>>>>
>>>> The user-visible effect is that stash create cannot currently include
>>>> untracked or ignored files in the stash entry. If those are the only
>>>> changes, it creates no entry at all, while stash push and save can
>>>> include them with -u or -a as appropriate. I should have described that
>>>> difference directly instead of starting from the include_untracked
>>>> implementation detail.
>>>>
>>>>> ... was what you wanted to say, but I am not sure.
>>>>
>>>> Yes, exactly. I'll explain the backward-compatibility reason rather
>>>> than the mechanics of parse_options().
>>>>
>>>>> You already said that with "does not update, reset, or clean".
>>>>
>>>> I'll drop that paragraph.
>>>>
>>>>> if you did not make a breaking change to the established convention,
>>>>> is it worth saying?
>>>>
>>>> I don't think it adds anything here. I'll remove the exit-status
>>>> discussion from the commit message as well.
>>>>
>>>>> adding tests for comprehensive coverage is not something to boast
>>>>> about. Is it worth saying?
>>>>
>>>> I'll remove the test details from the commit message.
>>>>
>>>>> Why are we singling out only these two?
>>>>
>>>> I started by looking at the missing -u and -a support in create, and I
>>>> think that led me to focus too narrowly on those two when considering
>>>> the scope. I need to think more about whether this patch should remain
>>>> limited to those two.
>>>>
>>>> Thanks,
>>>> Kazumasa Shigeta
>>>>
>>>> On Thu, 01 Oct 2026 10:03:13 -0700, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Kazumasa Shigeta <kazumasa.shigeta@kanamei.com> writes:
>>>>>
>>>>>> `git stash create` always passes zero for the include_untracked parameter
>>>>>> of do_create_stash(), even though that helper already supports untracked
>>>>>> and ignored files and stash push/save expose those modes as
>>>>>> -u/--include-untracked and -a/--all.
>>>>>
>>>>> There may be no lies in what the above says, but we would prefer to
>>>>> hear what the user visible implication of "passing 0" is more than
>>>>> what mechanically is happening inside a program. For example:
>>>>>
>>>>> "git stash create", "git stash push", and "git stash save" are
>>>>> commands that create a new stash entry. The latter two are also
>>>>> responsible for storing the resulting stash entry to the reflog
>>>>> of the "refs/stash" ref, but have options to control what is
>>>>> included in the stash entry. Among these options, "create" only
>>>>> supports the equivalent of "-m <message." to record in the stash
>>>>> entry. Most notably, "-u" and "-a" options are missing.
>>>>>
>>>>>> Teach create to accept the same options and pass the existing mode
>>>>>> through. Unlike push/save, create continues to only create objects: it
>>>>>> does not update refs/stash, reset the index, or clean the working tree.
>>>>>
>>>>> Sure. It is a very concise and good description of what we want to
>>>>> do.
>>>>>
>>>>>> Use parse_options() for the new options and stop parsing at the first
>>>>>> non-option message word. This keeps option-like tokens after the message
>>>>>> as message text, while leading option-like arguments now follow Git's
>>>>>> normal option parsing. In particular, unknown or malformed leading
>>>>>> options are rejected instead of silently becoming a message, short
>>>>>> options may be combined, and `--` can be used when a message itself
>>>>>> begins with a dash.
>>>>>
>>>>> Why do we need to go into such a detail in the log message? What is
>>>>> the above paragraph designed to convey to the reader? Again, it may
>>>>> not be telling any lies, but it misses the point by being inconsiderate
>>>>> to your readers. What you need to tell them is _WHY_ you chose to
>>>>> use parse_options() in such a way. What were you trying to achieve?
>>>>>
>>>>> I am guessing that something along this line ...
>>>>>
>>>>> "git stash create" traditionally treated the rest of the command
>>>>> line as a message. For example,
>>>>>
>>>>> $ git stash create adding -u option
>>>>>
>>>>> has always been a request to create a stash entry with the
>>>>> string "adding -u option" as its message. We should not make it
>>>>> trigger the "-u" (include untracked) behavior for backward
>>>>> compatibility, by using parse_options() with stop-at-the-non-option
>>>>> mode to forbid it from reordering the command line arguments.
>>>>>
>>>>> ... was what you wanted to say, but I am not sure.
>>>>>
>>>>> How much of all these verbiage was written by AI by the way? You'd
>>>>> need to spend effort to make it readable to humans.
>>>>>
>>>>>> Keep create's existing no-change behavior: detect the usual no-change
>>>>>> case before do_create_stash() refreshes and writes the index, and return
>>>>>> success without printing an object name. If do_create_stash() still
>>>>>> reports its internal "nothing to create" result, map that to create's
>>>>>> public success status.
>>>>>
>>>>> You already said that with "does not update, reset, or clean".
>>>>>
>>>>>> This follows the stash subcommand exit-status convention established by
>>>>>> 786fc390465f (stash: reserve exit status 1 for conflicts, 2026-09-03):
>>>>>> subcommands return 0 on success, negative values on failure, and status 1
>>>>>> when applying a stash results in conflicts. cmd_stash() maps negative
>>>>>> subcommand failures to 128.
>>>>>
>>>>> Again, there may not be lies in here, but if you did not make a
>>>>> breaking change to the established convention, is it worth saying?
>>>>>
>>>>>> 9ca6326dff29 (stash: refactor stash_create, 2017-02-19) added the
>>>>>> internal include-untracked path while intentionally leaving the user
>>>>>> interface for "git stash create" unchanged. Reuse that machinery and
>>>>>> the existing INCLUDE_ALL_FILES mode rather than adding a separate stash
>>>>>> creation path.
>>>>>>
>>>>>> Add coverage for short and long aliases, combined short options, the
>>>>>> untracked/ignored boundary including an ignored-only worktree, option
>>>>>> parsing and dash-leading messages, no-change behavior, and preservation
>>>>>> of refs/stash, the index state, and the working tree.
>>>>>
>>>>> Again, adding tests for comprehensive coverage is not something to
>>>>> boast about. Is it worth saying?
>>>>>
>>>>> Aren't -p/-S/-k/-q and pathspec support all about the creating half
>>>>> of "git stash push" that are not available to "git stash create",
>>>>> not just "-u" and "-a"? Why are we singling out only these two? It
>>>>> may be more worthwhile to explain the rationale behind such a design
>>>>> decision.