From: 重田一聖 Date: Fri, 02 Oct 2026 01:53:04 GMT Subject: Re: [PATCH v2] stash: expose untracked modes in create Message-ID: In-Reply-To: Hi Junio, Sorry for the crossed replies. As you also pointed out, I should have replied to Phillip before sending v2. After Patrick raised that point, I was writing my response to Phillip, and I did not notice that your messages had arrived while I was doing so. I ended up sending my reply to Phillip before seeing your comments. Thank you for the detailed review. I will go through your comments carefully before following up. I am not very quick at writing these emails, so it takes me quite a while to respond. Sorry about that. I will do my best to understand the points properly and improve the next round. Thanks, Kazumasa Shigeta On Thu, 01 Oct 2026 10:03:13 -0700, Junio C Hamano wrote: > Kazumasa Shigeta 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 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.