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

Abraham Samuel Adekunle [off-list ref] 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?