Re: [PATCH v2] stash: expose untracked modes in create

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