From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 06 Feb 2026 19:02:02 GMT
Subject: Re: [PATCH v3 3/3] add-patch: Allow proper 'git apply' when using the --rework-with-file flag
Message-ID: <xmqqqzqxelw5.fsf@gitster.g>
In-Reply-To: <10c0a4cb36534f5ed1ebed783b37d03a56007f97.1770390576.git.abrahamadekunle50@gmail.com>

Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> writes:

> Subject: Re: [PATCH v3 3/3] add-patch: Allow proper 'git apply' when using the --rework-with-file flag

Style.  Downcase "Allow".  Applies to [2/3].

Avoid "proper" as it is not obvious to everybody what you find
proper and why you find it proper.  Applies to any value-judgement
adjective.

    Subject: [PATCH v3 3/3] add-patch: allow all-or-none application of a patch

or something?

> +static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)
> +{
> +	struct child_process cp = CHILD_PROCESS_INIT;
> +	size_t j;
> +
> +		/* Any hunk to be used? */

Funny indentaion?

> +	for (j = 0; j < file_diff->hunk_nr; j++)
> +		if (file_diff->hunk[j].use == USE_HUNK)
> +			break;
> +
> +	if (j < file_diff->hunk_nr ||
> +		(!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {
> +		/* At least one hunk selected: apply */
> +		strbuf_reset(&s->buf);
> +		reassemble_patch(s, file_diff, 0, &s->buf);
> +
> +		discard_index(s->s.r->index);
> +		if (s->mode->apply_for_checkout)
> +			apply_for_checkout(s, &s->buf,
> +					s->mode->is_reverse);
> +		else {
> +			setup_child_process(s, &cp, "apply", NULL);
> +			strvec_pushv(&cp.args, s->mode->apply_args);
> +			if (pipe_command(&cp, s->buf.buf, s->buf.len,
> +					NULL, 0, NULL, 0))
> +				error(_("'git apply' failed"));
> +		}
> +		if (repo_read_index(s->s.r) >= 0)
> +			repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0,
> +							1, NULL, NULL, NULL);
> +	}
> +
> +}

I suspect that the extraction of this helper function out of its
original place in patch_update_file() should be done in its own
patch.

Do we need new tests to cover this new feature?