Subject: Re: [PATCH v6 07/11] add-patch: split out `struct interactive_options`

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 2 Dec 2025 19:48:32 +0100

On Thu, Nov 20, 2025 at 03:05:17PM +0000, Phillip Wood wrote:
> On 27/10/2025 11:33, Patrick Steinhardt wrote:
> > Refactor the code and split out a new `struct interactive_options` that
> > hosts common options used by both. These options are then applied to a
> > `struct interactive_config` that hosts common configuration.
> 
> I'm a little skeptical about renaming "sturct add_p_opt" as it only holds
> members that are relavent to run_add_p(). Also if we're trying to draw clear
> boundaries between the two subsystems hosting "struct interactive_options"
> and "struct interactive_config" in add-patch.c rather than add-interactive.c
> is potentially confusing.

I didn't want to add it to "add-interactive.c" though because of the
direction of the dependency: "add-interactive.c" will depend on
"add-patch.c", not the other way round.

We could of course split out the new options into a separate file
altogether. But that felt a bit heavy-handed to me.

> > This refactoring doesn't yet fully detangle the two subsystems from one
> > another, as we still end up calling `init_add_i_state()` in the "git add
> > -p" subsystem. This will be fixed in a subsequent commit.
> 
> I think the ultimate aim of not having to initalize a "struct add_i_state"
> in run_add_p() is a good idea. I'm not sure though that having to pass a
> "struct interactive_options" to run_add_p() is any less confusing than
> having to pass a "struct add_p_opt" to run_add_i().

I agree that the end result is still a bit confusing. But what I wanted
to achieve is that at least the two subsystems are clearly separated
from so that one doesn't have to wonder anymore which parts interact
with one another. It was extremely puzzling to me at first, and the end
result here is significantly easier to understand from my point of view.

Patrick