On Wed, Apr 24, 2024 at 01:58:57PM +1000, James Liu wrote:
[snip]
> --- a/advice.c
> +++ b/advice.c
> @@ -43,6 +43,7 @@ static struct {
> const char *key;
> enum advice_level level;
> } advice_setting[] = {
> + [ADVICE_ALL] = { "all" },
> [ADVICE_ADD_EMBEDDED_REPO] = { "addEmbeddedRepo" },
> [ADVICE_ADD_EMPTY_PATHSPEC] = { "addEmptyPathspec" },
> [ADVICE_ADD_IGNORED_FILE] = { "addIgnoredFile" },
> @@ -132,6 +133,13 @@ int advice_enabled(enum advice_type type)
> return enabled &&
> advice_enabled(ADVICE_PUSH_UPDATE_REJECTED_ALIAS);
>
> + /*
> + * We still allow for specific advice hints to be enabled if
> + * advice.all == false.
> + */
> + if (advice_setting[ADVICE_ALL].level == ADVICE_LEVEL_DISABLED)
> + return advice_setting[type].level == ADVICE_LEVEL_ENABLED;
Makes sense. When the advice is unset we don't need to handle it at all,
and if it is enabled we basically want to behave the same as before.
> return enabled;
> }
>
> diff --git a/advice.h b/advice.h
> index c8d29f97f3..b5ac99a645 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -11,6 +11,7 @@ struct string_list;
> * Call advise_if_enabled to print your advice.
> */
> enum advice_type {
> + ADVICE_ALL,
> ADVICE_ADD_EMBEDDED_REPO,
> ADVICE_ADD_EMPTY_PATHSPEC,
> ADVICE_ADD_IGNORED_FILE,
> diff --git a/t/t0018-advice.sh b/t/t0018-advice.sh
> index 8010796e1f..19318cc9bb 100755
> --- a/t/t0018-advice.sh
> +++ b/t/t0018-advice.sh
> @@ -53,4 +53,37 @@ test_expect_success 'advice should be printed when advice.pushUpdateRejected is
> test_cmp expect actual
> '
>
> +test_expect_success 'advice should not be printed when advice.all is set to false' '
> + test_config advice.all false &&
> + test-tool advise nestedTag "This is a piece of advice" 2>actual &&
> + test_must_be_empty actual
> +'
> +
> +test_expect_success 'advice should not be printed for pushUpdateRejected when advice.all is set to false' '
> + test_config advice.all false &&
> + test-tool advise pushUpdateRejected "This is a piece of advice" 2>actual &&
> + test_must_be_empty actual
> +'
> +
> +test_expect_success 'advice should be printed when advice.all is set to false, but specific advice is set to true' '
> + cat >expect <<-\EOF &&
> + hint: This is a piece of advice
> + EOF
> + test_config advice.all false &&
> + test_config advice.nestedTag true &&
> + test-tool advise nestedTag "This is a piece of advice" 2>actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'advice should be printed when advice.all is set to false, but advice.pushUpdateRejected and its alias are set to true' '
> + cat >expect <<-\EOF &&
> + hint: This is a piece of advice
> + EOF
> + test_config advice.all false &&
> + test_config advice.pushUpdateRejected true &&
> + test_config advice.pushNonFastForward true &&
> + test-tool advise pushUpdateRejected "This is a piece of advice" 2>actual &&
> + test_cmp expect actual
> +'
> test_done
Do we actually have to enable both advices here? If not, then this
should probably be two separate tests that check whether each of the
keys does its thing.
Patrick
Attachment:
signature.asc
Description: PGP signature
