Re: [PATCH v2 1/1] advice: add --no-advice global option
- From
James Liu <james@jamesliu.io>
- Date
- Apr 30, 2024, 00:56 UTC
- Message-ID
- <D0X1RH1PK67K.34KDYBQM6XDDU@jamesliu.io>
- In-Reply-To
- <20240429064025.GA2905826@coredump.intra.peff.net>
On Mon Apr 29, 2024 at 4:40 PM AEST, Jeff King wrote:
Show 10 quoted lines
> You need an environment variable if you want the command-line option to > work consistently across commands that spawn external processes. E.g.: > > git --no-advice fetch --all > > is going to spawn fetch sub-processes under the hood. You'd want them to > respect --no-advice, too, so we either have to propagate the > command-line option or use the environment. And when you consider an > external script like git-foo that runs a bunch of underlying Git > commands, then propagating becomes too cumbersome and error-prone.
Thanks for the explanation Jeff! Makes sense why the pattern is so prevalent.
Show 11 quoted lines
> You should use git_env_bool() to avoid the confusing behavior that
> GIT_NO_ADVICE=false still turns off advice. ;)
>
> You can also drop the "NO", which helps avoid awkward double negation.
> For example, if you do:
>
> if (git_env_bool("GIT_ADVICE", 1))
> return 0;
>
> then leaving that variable unset will act as if it is set to "1", but
> you can still do GIT_ADVICE=0 to suppress it.Awesome. I'll apply this suggestion in the next version of this patch.
Cheers, James