{"thread":{"id":"54674","subject":"[PATCH] pull: colorize the hint about setting `pull.rebase`","startedAt":"2020-11-19T10:22:55Z","lastAt":"2020-11-19T22:13:01Z","messageCount":2,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"410340","messageId":"pull.795.git.1605781349528.gitgitgadget@gmail.com","threadId":"54674","inReplyTo":null,"subject":"[PATCH] pull: colorize the hint about setting `pull.rebase`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-19T10:22:29Z","receivedAt":"2020-11-19T10:22:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn d18c950a69f (pull: warn if the user didn't say whether to rebase or\nto merge, 2020-03-09), a new hint was introduced to encourage users to\nmake a conscious decision about whether they want their pull to merge or\nto rebase by configuring the `pull.rebase` setting.\n\nThis warning was clearly intended to advise users, but as pointed out in\nhttps://lore.kernel.org/git/87ima2rdsm.fsf%40evledraar.gmail.com, it\nuses `warning()` instead of `advise()`.\n\nOne consequence is that the advice is not colorized in the same manner\nas other, similar messages. So let's use `advise()` instead.\n\nPointed-out-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    pull: colorize the hint about setting pull.rebase\n    \n    I see that Junio briefly wondered\n    [https://lore.kernel.org/git/xmqqeeuecngu.fsf@gitster-ct.c.googlers.com/] \n    about using an advice here, and concluded that it was not needed because\n    the warning is self-squelching (i.e. there is already a config setting\n    that will silence this warning). But that missed the fact that warnings\n    are not colorized, whereas advice is.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-795%2Fdscho%2Fcolorize-pull.rebase-advice-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-795/dscho/colorize-pull.rebase-advice-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/795\n\n builtin/pull.c               | 24 ++++++++++++------------\n t/t7601-merge-pull-config.sh |  7 +++++--\n 2 files changed, 17 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 17aa63cd35..1034372f8b 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -345,18 +345,18 @@ static enum rebase_type config_get_rebase(void)\n \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n \n \tif (opt_verbosity >= 0 && !opt_ff) {\n-\t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n-\t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n-\t\t\t\"commands sometime before your next pull:\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"  git config pull.rebase false  # merge (the default strategy)\\n\"\n-\t\t\t\"  git config pull.rebase true   # rebase\\n\"\n-\t\t\t\"  git config pull.ff only       # fast-forward only\\n\"\n-\t\t\t\"\\n\"\n-\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n-\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n-\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n-\t\t\t\"invocation.\\n\"));\n+\t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n+\t\t\t \"discouraged. You can squelch this message by running one of the following\\n\"\n+\t\t\t \"commands sometime before your next pull:\\n\"\n+\t\t\t \"\\n\"\n+\t\t\t \"  git config pull.rebase false  # merge (the default strategy)\\n\"\n+\t\t\t \"  git config pull.rebase true   # rebase\\n\"\n+\t\t\t \"  git config pull.ff only       # fast-forward only\\n\"\n+\t\t\t \"\\n\"\n+\t\t\t \"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n+\t\t\t \"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n+\t\t\t \"or --ff-only on the command line to override the configured default per\\n\"\n+\t\t\t \"invocation.\\n\"));\n \t}\n \n \treturn REBASE_FALSE;\ndiff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\nindex c5c4ea5fc0..6774e9d86f 100755\n--- a/t/t7601-merge-pull-config.sh\n+++ b/t/t7601-merge-pull-config.sh\n@@ -29,8 +29,11 @@ test_expect_success 'setup' '\n \n test_expect_success 'pull.rebase not set' '\n \tgit reset --hard c0 &&\n-\tgit pull . c1 2>err &&\n-\ttest_i18ngrep \"Pulling without specifying how to reconcile\" err\n+\tgit -c color.advice=always pull . c1 2>err &&\n+\ttest_decode_color <err >decoded &&\n+\ttest_i18ngrep \"<YELLOW>hint: \" decoded &&\n+\ttest_i18ngrep \"Pulling without specifying how to reconcile\" decoded\n+\n '\n \n test_expect_success 'pull.rebase not set and pull.ff=true' '\n\nbase-commit: faefdd61ec7c7f6f3c8c9907891465ac9a2a1475\n-- \ngitgitgadget\n"},{"id":"410387","messageId":"xmqqsg95vvvj.fsf@gitster.c.googlers.com","threadId":"54674","inReplyTo":"pull.795.git.1605781349528.gitgitgadget@gmail.com","subject":"Re: [PATCH] pull: colorize the hint about setting `pull.rebase`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-19T22:12:48Z","receivedAt":"2020-11-19T22:13:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> In d18c950a69f (pull: warn if the user didn't say whether to rebase or\n> to merge, 2020-03-09), a new hint was introduced to encourage users to\n> make a conscious decision about whether they want their pull to merge or\n> to rebase by configuring the `pull.rebase` setting.\n>\n> This warning was clearly intended to advise users, but as pointed out in\n> https://lore.kernel.org/git/87ima2rdsm.fsf%40evledraar.gmail.com, it\n> uses `warning()` instead of `advise()`.\n>\n> One consequence is that the advice is not colorized in the same manner\n> as other, similar messages. So let's use `advise()` instead.\n\nThe advise() gives unconditional output like warning(), so we still\ntake advantage of the variable's self-squelching nature.  I am OK\nwith the change.\n\nBesides coloring (which I personally find it distracting but not so\nstrongly to forbid others from using), advise() behaves better when\nshowing multi-line messages, and that would be another reason why\nwe want to use it over warning() here.\n\nWill queue.  Thanks.\n\n> Pointed-out-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>     pull: colorize the hint about setting pull.rebase\n>     \n>     I see that Junio briefly wondered\n>     [https://lore.kernel.org/git/xmqqeeuecngu.fsf@gitster-ct.c.googlers.com/] \n>     about using an advice here, and concluded that it was not needed because\n>     the warning is self-squelching (i.e. there is already a config setting\n>     that will silence this warning). But that missed the fact that warnings\n>     are not colorized, whereas advice is.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-795%2Fdscho%2Fcolorize-pull.rebase-advice-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-795/dscho/colorize-pull.rebase-advice-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/795\n>\n>  builtin/pull.c               | 24 ++++++++++++------------\n>  t/t7601-merge-pull-config.sh |  7 +++++--\n>  2 files changed, 17 insertions(+), 14 deletions(-)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 17aa63cd35..1034372f8b 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -345,18 +345,18 @@ static enum rebase_type config_get_rebase(void)\n>  \t\treturn parse_config_rebase(\"pull.rebase\", value, 1);\n>  \n>  \tif (opt_verbosity >= 0 && !opt_ff) {\n> -\t\twarning(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n> -\t\t\t\"discouraged. You can squelch this message by running one of the following\\n\"\n> -\t\t\t\"commands sometime before your next pull:\\n\"\n> -\t\t\t\"\\n\"\n> -\t\t\t\"  git config pull.rebase false  # merge (the default strategy)\\n\"\n> -\t\t\t\"  git config pull.rebase true   # rebase\\n\"\n> -\t\t\t\"  git config pull.ff only       # fast-forward only\\n\"\n> -\t\t\t\"\\n\"\n> -\t\t\t\"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n> -\t\t\t\"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n> -\t\t\t\"or --ff-only on the command line to override the configured default per\\n\"\n> -\t\t\t\"invocation.\\n\"));\n> +\t\tadvise(_(\"Pulling without specifying how to reconcile divergent branches is\\n\"\n> +\t\t\t \"discouraged. You can squelch this message by running one of the following\\n\"\n> +\t\t\t \"commands sometime before your next pull:\\n\"\n> +\t\t\t \"\\n\"\n> +\t\t\t \"  git config pull.rebase false  # merge (the default strategy)\\n\"\n> +\t\t\t \"  git config pull.rebase true   # rebase\\n\"\n> +\t\t\t \"  git config pull.ff only       # fast-forward only\\n\"\n> +\t\t\t \"\\n\"\n> +\t\t\t \"You can replace \\\"git config\\\" with \\\"git config --global\\\" to set a default\\n\"\n> +\t\t\t \"preference for all repositories. You can also pass --rebase, --no-rebase,\\n\"\n> +\t\t\t \"or --ff-only on the command line to override the configured default per\\n\"\n> +\t\t\t \"invocation.\\n\"));\n>  \t}\n>  \n>  \treturn REBASE_FALSE;\n> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh\n> index c5c4ea5fc0..6774e9d86f 100755\n> --- a/t/t7601-merge-pull-config.sh\n> +++ b/t/t7601-merge-pull-config.sh\n> @@ -29,8 +29,11 @@ test_expect_success 'setup' '\n>  \n>  test_expect_success 'pull.rebase not set' '\n>  \tgit reset --hard c0 &&\n> -\tgit pull . c1 2>err &&\n> -\ttest_i18ngrep \"Pulling without specifying how to reconcile\" err\n> +\tgit -c color.advice=always pull . c1 2>err &&\n> +\ttest_decode_color <err >decoded &&\n> +\ttest_i18ngrep \"<YELLOW>hint: \" decoded &&\n> +\ttest_i18ngrep \"Pulling without specifying how to reconcile\" decoded\n> +\n>  '\n>  \n>  test_expect_success 'pull.rebase not set and pull.ff=true' '\n>\n> base-commit: faefdd61ec7c7f6f3c8c9907891465ac9a2a1475\n"}]}