On Tue, Nov 01, 2022 at 02:07:39PM +0100, Ævar Arnfjörð Bjarmason wrote: > > You can also put it lower in the function, when we actually warn, which > > saves adding to the environment when there is nothing to warn about > > (though this way, we avoid doing the redundant work). > > > > Compared to munging the config, this seems shorter and simpler, and > > there's no chance of us ever getting confused between the fake > > "suppress" value and something the user actually asked for. > > Sure, we can do it with an environment variable, in the end that's all > git_config_push_parameter() is doing too. It's just setting things in > "GIT_CONFIG_PARAMETERS". > > And yes, we can set this in the low-level function instead of with > git_config_push_parameter() in builtin/*.c as I did. I was aiming for > something demonstrably narrow, at the cost of some verbosity. > > But I don't get how other things being equal you think sticking this in > "GIT_CHECKED_CREDENTIALS_IN_URL" instead of "GIT_CONFIG_PARAMETERS" > helps. I vaguely prefer calling this GIT_CHECKED_CREDENTIALS_IN_URL instead of stuffing it in the configuration. All other things *aren't* equal here, since we're not lying to sub-processes about configuration values set by the user. Instead, we're saying: "regardless of what value the user assigned transfer.credentialsInUrl, we can avoid looking at it because we have already checked for the presence of credentials in the URL". Thanks, Taylor
