Rubén Justo <rjusto@xxxxxxxxx> writes:
> Using advise_if_enabled() to display an advice will automatically
> include instructions on how to disable the advice, along with the
> main advice:
>
> hint: use --reapply-cherry-picks to include skipped commits
> hint: Disable this message with "git config advice.skippedCherryPicks false"
>
> This can become distracting or noisy over time, while the user may
> still want to receive the main advice.
>
> Let's have a switch to allow disabling this automatic advice.
>
> Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx>
> ---
> advice.c | 3 ++-
> advice.h | 3 ++-
> t/t0018-advice.sh | 8 ++++++++
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/advice.c b/advice.c
> index 50c79443ba..fa203f8806 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -79,6 +79,7 @@ static struct {
> [ADVICE_UPDATE_SPARSE_PATH] = { "updateSparsePath", 1 },
> [ADVICE_WAITING_FOR_EDITOR] = { "waitingForEditor", 1 },
> [ADVICE_WORKTREE_ADD_ORPHAN] = { "worktreeAddOrphan", 1 },
> + [ADVICE_ADVICE_OFF] = { "adviceOff", 1 },
> };
>
> static const char turn_off_instructions[] =
> @@ -93,7 +94,7 @@ static void vadvise(const char *advice, int display_instructions,
>
> strbuf_vaddf(&buf, advice, params);
>
> - if (display_instructions)
> + if (display_instructions && advice_enabled(ADVICE_ADVICE_OFF))
> strbuf_addf(&buf, turn_off_instructions, key);
>
> for (cp = buf.buf; *cp; cp = np) {
> diff --git a/advice.h b/advice.h
> index 2affbe1426..1f2eef034e 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -10,7 +10,7 @@ struct string_list;
> * Add the new config variable to Documentation/config/advice.txt.
> * Call advise_if_enabled to print your advice.
> */
> - enum advice_type {
> +enum advice_type {
> ADVICE_ADD_EMBEDDED_REPO,
> ADVICE_ADD_EMPTY_PATHSPEC,
> ADVICE_ADD_IGNORED_FILE,
> @@ -50,6 +50,7 @@ struct string_list;
> ADVICE_WAITING_FOR_EDITOR,
> ADVICE_SKIPPED_CHERRY_PICKS,
> ADVICE_WORKTREE_ADD_ORPHAN,
> + ADVICE_ADVICE_OFF,
> };
>
> int git_default_advice_config(const char *var, const char *value);
> diff --git a/t/t0018-advice.sh b/t/t0018-advice.sh
> index c13057a4ca..0b6a8b4a10 100755
> --- a/t/t0018-advice.sh
> +++ b/t/t0018-advice.sh
> @@ -30,4 +30,12 @@ test_expect_success 'advice should not be printed when config variable is set to
> test_must_be_empty actual
> '
>
> +test_expect_success 'advice without the instructions to disable it' '
> + cat >expect <<-\EOF &&
> + hint: This is a piece of advice
> + EOF
> + test-tool -c advice.adviceOff=0 advise "This is a piece of advice" 2>actual &&
> + test_cmp expect actual
> +'
This is testing the right thing but in a "showing off a shiny new
toy" way. We want to make sure we will catch regressions in the
future by testing with a bit more conditions perturbed. For
example, with the new "-c var=val" mechanism, we could
* set advice.nestedtag to off (which would disable the whole
advice)
* set advice.adviceoff to on (which should be the same as not
setting it explicitly at all).
to test different combinations that we were unable to test before
[2/3] invented the mechanism.
Thanks.
