From: Patrick Steinhardt <ps@pks.im>
To: James Liu <james@jamesliu.io>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] advice: add "all" option to disable all hints
Date: Wed, 24 Apr 2024 07:29:04 +0200 [thread overview]
Message-ID: <ZiiYoOKJpqq4XWq-@tanuki> (raw)
In-Reply-To: <20240424035857.84583-3-james@jamesliu.io>
[-- Attachment #1: Type: text/plain, Size: 3207 bytes --]
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-04-24 5:29 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-24 3:58 [PATCH 0/2] advice: add "all" option to disable all hints James Liu
2024-04-24 3:58 ` [PATCH 1/2] advice: allow advice type to be provided in tests James Liu
2024-04-24 5:28 ` Patrick Steinhardt
2024-04-24 3:58 ` [PATCH 2/2] advice: add "all" option to disable all hints James Liu
2024-04-24 5:29 ` Patrick Steinhardt [this message]
2024-04-24 6:28 ` [PATCH 0/2] " Junio C Hamano
2024-04-24 6:48 ` Patrick Steinhardt
2024-04-24 13:52 ` Phillip Wood
2024-04-24 14:07 ` Patrick Steinhardt
2024-04-24 14:59 ` Junio C Hamano
2024-04-25 6:46 ` Patrick Steinhardt
2024-04-25 16:18 ` Junio C Hamano
2024-04-24 16:14 ` Dragan Simic
2024-04-24 16:21 ` Dragan Simic
2024-04-24 14:31 ` Junio C Hamano
2024-04-25 6:42 ` Patrick Steinhardt
2024-04-24 7:37 ` Dragan Simic
2024-04-29 1:09 ` [PATCH v2 0/1] advice: add --no-advice global option James Liu
2024-04-29 1:09 ` [PATCH v2 1/1] " James Liu
2024-04-29 4:15 ` Dragan Simic
2024-04-29 5:01 ` James Liu
2024-04-29 5:36 ` Dragan Simic
2024-04-29 5:59 ` Dragan Simic
2024-04-29 6:04 ` Eric Sunshine
2024-04-29 6:12 ` Dragan Simic
2024-04-29 6:40 ` Jeff King
2024-04-29 6:55 ` Dragan Simic
2024-04-29 13:50 ` Junio C Hamano
2024-04-30 0:56 ` James Liu
2024-04-29 13:48 ` Junio C Hamano
2024-04-29 17:05 ` Rubén Justo
2024-04-29 17:54 ` Dragan Simic
2024-04-30 1:47 ` [PATCH v3 0/1] " James Liu
2024-04-30 1:47 ` [PATCH v3 1/1] " James Liu
2024-04-30 5:18 ` Patrick Steinhardt
2024-04-30 6:24 ` Dragan Simic
2024-04-30 16:29 ` Junio C Hamano
2024-05-03 7:17 ` [PATCH v4 0/3] advice: add "all" option to disable all hints James Liu
2024-05-03 7:17 ` [PATCH v4 1/3] doc: clean up usage documentation for --no-* opts James Liu
2024-05-03 17:30 ` Junio C Hamano
2024-05-06 1:39 ` James Liu
2024-05-03 7:17 ` [PATCH v4 2/3] doc: add spacing around paginate options James Liu
2024-05-03 14:32 ` Karthik Nayak
2024-05-03 17:36 ` Junio C Hamano
2024-05-03 7:17 ` [PATCH v4 3/3] advice: add --no-advice global option James Liu
2024-05-08 0:40 ` [PATCH v4 4/3] t0018: two small fixes Junio C Hamano
2024-05-03 7:31 ` [PATCH v4 0/3] advice: add "all" option to disable all hints Dragan Simic
2024-05-03 18:00 ` Re* " Junio C Hamano
2024-05-03 19:26 ` Eric Sunshine
2024-05-03 19:48 ` Junio C Hamano
2024-05-03 20:08 ` Junio C Hamano
2024-05-03 21:24 ` Eric Sunshine
2024-05-05 11:03 ` Johannes Schindelin
2024-05-06 16:40 ` Re* " Junio C Hamano
2024-05-07 0:11 ` Dragan Simic
2024-05-07 0:21 ` Junio C Hamano
2024-05-07 4:45 ` Dragan Simic
2024-05-07 0:01 ` Dragan Simic
2024-05-03 14:35 ` Karthik Nayak
2024-05-05 23:17 ` James Liu
2024-05-03 17:25 ` Junio C Hamano
2024-05-05 23:20 ` James Liu
2024-05-06 1:10 ` James Liu
2024-05-06 16:47 ` Junio C Hamano
2024-05-06 23:08 ` James Liu
2024-05-07 6:23 ` Karthik Nayak
2024-05-06 16:41 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZiiYoOKJpqq4XWq-@tanuki \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=james@jamesliu.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).