On Tue, Nov 01 2022, Taylor Blau wrote: > 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.[...] To be clear, I'm asking if there's cases where we think one method or the other produces different results, which I understood Jeff hinting at. > 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".[...] All > other things *aren't* equal here, since we're not lying to > sub-processes about configuration values set by the user. The user is asking us to warn about storing certain things in config, we know we already warned, so we're looking to flip that value to the "quiet" setting. If you consider that overriding I don't get how doing so via a different environment variable changes anything. It would be doing the same thing, just making it less obvious.
