git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] advice: allow disabling the automatic hint in advise_if_enabled()

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 9, 2024, 18:23 UTC
Message-ID
<xmqqo7dugyd4.fsf@gitster.g>
In-Reply-To
<d6099d78-43c6-4709-9121-11f84228cf91@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
Show 76 quoted lines
> 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@gmail.com>
> ---
>  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.
Previous: Rubén JustoNext: Taylor Blau
Message 8 of 36 in “allow disabling the automatic hint in advise_if_enabled()”
  1. 0/3 allow disabling the automatic hint in advise_if_enabled()Rubén Justo, Jan 9, 2024
  2. 1/3 t/test-tool: usage descriptionRubén Justo, Jan 9, 2024
  3. Junio C HamanoJan 9, 2024
  4. 2/3 t/test-tool: handle -c <name>=<value> argumentsRubén Justo, Jan 9, 2024
  5. Junio C HamanoJan 9, 2024
  6. Taylor BlauJan 9, 2024
  7. 3/3 advice: allow disabling the automatic hint in advise_if_enabled()Rubén Justo, Jan 9, 2024
  8. Junio C HamanoJan 9, 2024
  9. Taylor BlauJan 9, 2024
  10. Junio C HamanoJan 9, 2024
  11. Rubén JustoJan 10, 2024
  12. Jeff KingJan 10, 2024
  13. Rubén JustoJan 10, 2024
  14. Dragan SimicJan 10, 2024
  15. Rubén JustoJan 10, 2024
  16. Dragan SimicJan 10, 2024
  17. Junio C HamanoJan 10, 2024
  18. Dragan SimicJan 10, 2024
  19. Jeff KingJan 11, 2024
  20. Dragan SimicJan 18, 2024
  21. Junio C HamanoJan 18, 2024
  22. Dragan SimicJan 18, 2024
  23. Junio C HamanoJan 18, 2024
  24. Dragan SimicJan 18, 2024
  25. Rubén JustoJan 20, 2024
  26. Dragan SimicJan 20, 2024
  27. Junio C HamanoJan 10, 2024
  28. Taylor BlauJan 9, 2024
  29. Junio C HamanoJan 9, 2024
  30. Rubén JustoJan 10, 2024
  31. advice: allow disabling the automatic hint in advise_if_enabled()Rubén Justo, Jan 12, 2024
  32. Junio C HamanoJan 12, 2024
  33. Jeff KingJan 13, 2024
  34. Junio C HamanoJan 16, 2024
  35. Rubén JustoJan 15, 2024
  36. advice: allow disabling the automatic hint in advise_if_enabled()Rubén Justo, Jan 15, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.