Junio C Hamano wrote:
> Refactor the advise() call that teaches users how they can choose
> between merge and rebase into a helper function. This revealed that
> the caller's logic needs to be further clarified to allow future
> actions (like "erroring out" instead of the current "go ahead and
> merge anyway") that should happen whether the advice message is
> squelched out.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> builtin/pull.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 2976b8e5cb..1b87ea95eb 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -925,6 +925,22 @@ static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_
> return ret;
> }
>
> +static void show_advice_pull_non_ff(void)
> +{
> + advise(_("Pulling without specifying how to reconcile divergent branches is\n"
> + "discouraged. You can squelch this message by running one of the following\n"
> + "commands sometime before your next pull:\n"
> + "\n"
> + " git config pull.rebase false # merge (the default strategy)\n"
> + " git config pull.rebase true # rebase\n"
> + " git config pull.ff only # fast-forward only\n"
> + "\n"
> + "You can replace \"git config\" with \"git config --global\" to set a default\n"
> + "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> + "or --ff-only on the command line to override the configured default per\n"
> + "invocation.\n"));
> +}
While this change is good, I see a lot of advice code setting up a
separate constant message, like:
static const char message_advice_pull_non_ff[] =
N_("...");
> int cmd_pull(int argc, const char **argv, const char *prefix)
> {
> const char *repo, **refspecs;
> @@ -1028,19 +1044,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> if (opt_rebase && merge_heads.nr > 1)
> die(_("Cannot rebase onto multiple branches."));
>
> - if (rebase_unspecified && opt_verbosity >= 0 && !opt_ff) {
> - advise(_("Pulling without specifying how to reconcile divergent branches is\n"
> - "discouraged. You can squelch this message by running one of the following\n"
> - "commands sometime before your next pull:\n"
> - "\n"
> - " git config pull.rebase false # merge (the default strategy)\n"
> - " git config pull.rebase true # rebase\n"
> - " git config pull.ff only # fast-forward only\n"
> - "\n"
> - "You can replace \"git config\" with \"git config --global\" to set a default\n"
> - "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> - "or --ff-only on the command line to override the configured default per\n"
> - "invocation.\n"));
> + if (rebase_unspecified && !opt_ff) {
> + if (opt_verbosity >= 0)
> + show_advice_pull_non_ff();
Then, we could just do:
if (opt_verbosity >= 0)
advise(_(message_advice_pull_non_ff)).
Or even better:
if (opt_verbosity >= 0)
advise_if_enabled(ADVICE_PULL_NON_FF, _(message_advice_pull_non_ff));
I'm not familiar with the advise code, I don't know if there's any good
reason to setup these separate messages, so feel free to disregard this
suggestion.
Cheers.
--
Felipe Contreras
