Hi Junio, On Mon, 18 Nov 2019, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx> > writes: > > > ... there is so little overlap between the git add --patch part > > and the rest of git add --interactive that I could put the former > > into a totally different file: add-patch.c. > > We would want to call, after "add -i in C" stabilizes, directly into > "add -p" machinery from different subcommands such as "reset", > "checkout", etc., and bypass the rest of "add -i" when we do so. We > can credit the lack of overlap to the design that supports such > usage while it is still in a scripted Porcelain. > > Preparing the "add -p" machinery in such a way that "add -i" merely > is one of the users would make a lot of sense from organizational > point of view. If I understand you correctly, you mean that e.g. `git checkout -p` will call the code from `add-patch.c` directly? That is indeed the case. Already now, `git checkout -p` calls `run_add_interactive()` (see https://github.com/git/git/blob/v2.24.0/builtin/checkout.c#L463): if (opts->patch_mode) { const char *patch_mode; if (opts->checkout_index && opts->checkout_worktree) patch_mode = "--patch=checkout"; else if (opts->checkout_index && !opts->checkout_worktree) patch_mode = "--patch=reset"; else if (!opts->checkout_index && opts->checkout_worktree) patch_mode = "--patch=worktree"; else BUG("either flag must have been set, worktree=%d, index=%d", opts->checkout_worktree, opts->checkout_index); return run_add_interactive(revision, patch_mode, &opts->pathspec); } And with the changes in PR #174 (i.e. three patch series later, see https://github.com/gitgitgadget/git/pull/174/files#diff-1f1eb09e4703764f9f9b9f1f1e67eb97R205-R218 for details), this will call `run_add_p()`, i.e. the newly-added code in `add-patch.c` directly: if (!strcmp(patch_mode, "--patch")) mode = ADD_P_STAGE; else if (!strcmp(patch_mode, "--patch=stash")) mode = ADD_P_STASH; else if (!strcmp(patch_mode, "--patch=reset")) mode = ADD_P_RESET; else if (!strcmp(patch_mode, "--patch=checkout")) mode = ADD_P_CHECKOUT; else if (!strcmp(patch_mode, "--patch=worktree")) mode = ADD_P_WORKTREE; else die("'%s' not supported", patch_mode); return !!run_add_p(the_repository, mode, revision, pathspec); Now that I see it, it does not look the most elegant that the `patch_mode` is a string instead of that enum, but I think that will be relatively easy to fix once the legacy `git-add--interactive.perl` will have been decomissioned. Ciao, Dscho
