[v3,0/1] advice: add --no-advice global option

Message

James Liu April 30, 2024, 1:47 a.m. UTC

Hi,

This is v3 of the patch to add a global --no-advice option for silencing
all advice hints. The environment variable has been renamed to
GIT_ADVICE and marked for internal use only, and the conditional in the
advice_enabled() helper has been adjusted to use git_env_bool().

I explored the idea of adding another test to ensure the configuration
is propagated to subprocesses correctly. This would use `git fetch --all`
as the trigger, however it appears that advice is only printed when
`fetch_one()` is invoked, which I don't think spawns any child
processes. With that said, since this is a common pattern, I believe
the existing additional test case is sufficient.

Cheers,
James

James Liu (1):
  advice: add --no-advice global option

 Documentation/git.txt |  5 ++++-
 advice.c              |  8 +++++++-
 environment.h         |  7 +++++++
 git.c                 |  6 +++++-
 t/t0018-advice.sh     | 20 ++++++++++++++++++++
 5 files changed, 43 insertions(+), 3 deletions(-)

Range-diff against v2:
1:  0f2ecb7862 ! 1:  55d5559586 advice: add --no-advice global option
    @@ Commit message
     
         Add a --no-advice global option to disable all advice hints from being
         displayed. This is independent of the toggles for individual advice
    -    hints.
    +    hints. Use an internal environment variable (GIT_ADVICE) to ensure this
    +    configuration is propagated to the usage site, even if it executes in a
    +    subprocess.
     
         Signed-off-by: James Liu <[e-mail]>
     
    @@ advice.c: void advise(const char *advice, ...)
     -	int enabled = advice_setting[type].level != ADVICE_LEVEL_DISABLED;
     +	int enabled;
     +
    -+	if (getenv(GIT_NO_ADVICE))
    ++	if (!git_env_bool(GIT_ADVICE, 1))
     +		return 0;
     +
     +	enabled = advice_setting[type].level != ADVICE_LEVEL_DISABLED;
    @@ advice.c: void advise(const char *advice, ...)
     
      ## environment.h ##
     @@ environment.h: const char *getenv_safe(struct strvec *argv, const char *name);
    - #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
      #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
      #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
    -+#define GIT_NO_ADVICE "GIT_NO_ADVICE"
      
    ++/*
    ++ * Environment variable used to propagate the --no-advice global option to the
    ++ * advice_enabled() helper, even when run in a subprocess.
    ++ * This is an internal variable that should not be set by the user.
    ++ */
    ++#define GIT_ADVICE "GIT_ADVICE"
    ++
      /*
       * Environment variable used in handshaking the wire protocol.
    +  * Contains a colon ':' separated list of keys with optional values
     
      ## git.c ##
     @@ git.c: const char git_usage_string[] =
    @@ git.c: static int handle_options(const char ***argv, int *argc, int *envchanged)
      			if (envchanged)
      				*envchanged = 1;
     +		} else if (!strcmp(cmd, "--no-advice")) {
    -+			setenv(GIT_NO_ADVICE, "1", 1);
    ++			setenv(GIT_ADVICE, "0", 1);
     +			if (envchanged)
     +				*envchanged = 1;
      		} else {