From: Samuel Abraham <abrahamadekunle50@gmail.com>
Date: Fri, 06 Feb 2026 20:39:19 GMT
Subject: Re: [PATCH v3 3/3] add-patch: Allow proper 'git apply' when using the --rework-with-file flag
Message-ID: <CADYq+fb3zP0KiPSGGnnbHsX86wc0fWGjbv8s4BNaKPsO+T7znA@mail.gmail.com>
In-Reply-To: <xmqqqzqxelw5.fsf@gitster.g>

On Fri, Feb 6, 2026 at 8:02 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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?

Okay

>
> > +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?

Sorry

>
> > +     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.

Okay

>
> Do we need new tests to cover this new feature?

Yes I will include the tests in the next version.
Thanks

Abraham