Re: [PATCH v6 09/11] add-patch: add support for in-memory index patching
- From
Elijah Newren <newren@gmail.com>
- Date
- Nov 20, 2025, 07:04 UTC
- Message-ID
- <CABPp-BGRnx7+qvFcDeWCZEZm1aRn=kRezZ2KZA0E=8hji9Vjiw@mail.gmail.com>
- In-Reply-To
- <20251027-b4-pks-history-builtin-v6-9-407dd3f57ad3@pks.im>
On Mon, Oct 27, 2025 at 4:34 AM Patrick Steinhardt <ps@pks.im> wrote:
Show 95 quoted lines
> +int run_add_p_index(struct repository *r,
> + struct index_state *index,
> + const char *index_file,
> + struct interactive_options *opts,
> + const char *revision,
> + const struct pathspec *ps)
> +{
> + struct patch_mode mode = {
> + .apply_args = { "--cached", NULL },
> + .apply_check_args = { "--cached", NULL },
> + .prompt_mode = {
> + N_("Stage mode change [y,n,q,a,d%s,?]? "),
> + N_("Stage deletion [y,n,q,a,d%s,?]? "),
> + N_("Stage addition [y,n,q,a,d%s,?]? "),
> + N_("Stage this hunk [y,n,q,a,d%s,?]? ")
> + },
> + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk "
> + "will immediately be marked for staging."),
> + .help_patch_text =
> + N_("y - stage this hunk\n"
> + "n - do not stage this hunk\n"
> + "q - quit; do not stage this hunk or any of the remaining "
> + "ones\n"
> + "a - stage this hunk and all later hunks in the file\n"
> + "d - do not stage this hunk or any of the later hunks in "
> + "the file\n"),
> + .index_only = 1,
> + };
> + struct add_p_state s = {
> + .r = r,
> + .index = index,
> + .index_file = index_file,
> + .answer = STRBUF_INIT,
> + .buf = STRBUF_INIT,
> + .plain = STRBUF_INIT,
> + .colored = STRBUF_INIT,
> + .mode = &mode,
> + .revision = revision,
> + };
> + struct strbuf parent_revision = STRBUF_INIT;
> + char parent_tree_oid[GIT_MAX_HEXSZ + 1];
> + size_t binary_count = 0;
> + struct commit *commit;
> + int ret;
> +
> + commit = lookup_commit_reference_by_name(revision);
> + if (!commit) {
> + err(&s, _("Revision does not refer to a commit"));
> + ret = -1;
> + goto out;
> + }
> +
> + if (commit->parents)
> + oid_to_hex_r(parent_tree_oid, get_commit_tree_oid(commit->parents->item));
> + else
> + oid_to_hex_r(parent_tree_oid, r->hash_algo->empty_tree);
> +
> + strbuf_addf(&parent_revision, "%s~", revision);
> + mode.diff_cmd[0] = "diff-tree";
> + mode.diff_cmd[1] = "-r";
> + mode.diff_cmd[2] = parent_tree_oid;
> +
> + interactive_config_init(&s.cfg, r, opts);
> +
> + if (parse_diff(&s, ps) < 0) {
> + ret = -1;
> + goto out;
> + }
> +
> + for (size_t i = 0; i < s.file_diff_nr; i++) {
> + if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr)
> + binary_count++;
> + else if (patch_update_file(&s, s.file_diff + i))
> + break;
> + }
> +
> + if (s.file_diff_nr == 0) {
> + err(&s, _("No changes."));
> + ret = -1;
> + goto out;
> + }
> +
> + if (binary_count == s.file_diff_nr) {
> + err(&s, _("Only binary files changed."));
> + ret = -1;
> + goto out;
> + }
> +
> + ret = 0;
> +
> +out:
> + strbuf_release(&parent_revision);
> + add_p_state_clear(&s);
> + return ret;
> +}I'm totally unfamiliar with add-patch.[ch] beyond what I've been reviewing in this series, so this may be a dumb/naive question, but why add a sibling run_add_p_index() to run_add_p() via copy+paste+modify? (Or is it not copy+paste+modify in some interesting way?) I'm worried the two will drift, and I'm curious whether run_add_p() should just be calling run_add_p_index() and just passing r->index for the index field. Is there a reason that doesn't work?