Re: [PATCH] add: check color.ui for interactive add

From: Derrick Stolee <derrickstolee@github.com>
Date: Jun 6, 2023, 13:32 UTC
Message-ID: <e6c9b1a4-4700-1205-9606-86f611a7f9c2@github.com>
In-Reply-To: <20230606021349.GC89456@coredump.intra.peff.net>

On 6/5/2023 10:13 PM, Jeff King wrote:

Show 17 quoted lines

> On Mon, Jun 05, 2023 at 07:42:43PM +0000, Derrick Stolee via GitGitGadget wrote:
> 
>> The fix is simple, to use git_color_default_config() as the fallback for
>> git_add_config(). A more robust change would instead encapsulate the
>> git_use_color_default global in methods that would check the config
>> setting if it has not been initialized yet. Some ideas are being
>> discussed on this front [1], but nothing has been finalized.
> 
> I think it should be OK to load the color config here, but note...
> 
>>     This is also a reoccurrence of the "config not loaded" bug from [3].
> 
> ...that this case is a little different than the core.replaceRefs one.
> One of the reasons we don't just load all config by default is that it
> was an intentional scheme that not all programs would respect all
> config, and color in particular was one of the things that wasn't meant
> to be supported everywhere.

...snipping valuable context...

> So all of this is a big digression from your patch. I think for "git
> add" it is probably OK to enable color by default. But I mostly want to
> point out that trying to roll this into a more elaborate fix may run
> afoul of all kinds of rabbit holes.

Thank you for this context, which I will keep in mind as an important feature in this space. The default config doesn't have this property, so I'll remain focused on those values in the "lazy load" work.

Just riffing: it's likely that we could load the color config in git.c based on the "porcelain" vs "plumbing" category of a builtin. We have that specification in command-list.txt, but _not_ in 'struct cmd_struct' so it would take some work to introduce that concept.

Thanks, -Stolee