From: Derrick Stolee <derrickstolee@github.com>
To: Jeff King <peff@peff.net>,
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com, johannes.schindelin@gmx.de
Subject: Re: [PATCH] add: check color.ui for interactive add
Date: Tue, 6 Jun 2023 09:32:13 -0400 [thread overview]
Message-ID: <e6c9b1a4-4700-1205-9606-86f611a7f9c2@github.com> (raw)
In-Reply-To: <20230606021349.GC89456@coredump.intra.peff.net>
On 6/5/2023 10:13 PM, Jeff King wrote:
> 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
next prev parent reply other threads:[~2023-06-06 13:32 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-05 19:42 [PATCH] add: check color.ui for interactive add Derrick Stolee via GitGitGadget
2023-06-06 1:01 ` Junio C Hamano
2023-06-06 1:05 ` Junio C Hamano
2023-06-06 2:13 ` Jeff King
2023-06-06 13:32 ` Derrick Stolee [this message]
2023-06-06 14:20 ` [PATCH v2] " Derrick Stolee via GitGitGadget
2023-06-07 11:09 ` Phillip Wood
2023-06-07 13:33 ` Derrick Stolee
2023-06-12 17:09 ` Junio C Hamano
2023-06-07 13:44 ` [Patch v2 2/2] add: test use of brackets when color is disabled Derrick Stolee
2023-06-07 15:31 ` Phillip Wood
2023-06-12 17:13 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e6c9b1a4-4700-1205-9606-86f611a7f9c2@github.com \
--to=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).