Re: [PATCH 14/14] pull --rebase: error on no merge candidate cases

From: Stefan Beller <sbeller@google.com>
Date: May 19, 2015, 00:12 UTC
Message-ID: <CAGZ79kbhA2+AJ1Vtz6khX5uEBCCUBUuXhpBezZApZsC_txhewA@mail.gmail.com>
In-Reply-To: <1431961571-20370-15-git-send-email-pyokagan@gmail.com>

On Mon, May 18, 2015 at 8:06 AM, Paul Tan <pyokagan@gmail.com> wrote:

Show 23 quoted lines

> Tweak the error messages printed by die_no_merge_candidates() to take
> into account that we may be "rebasing against" rather than "merging
> with".
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>  builtin/pull.c | 42 +++++++++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index c8d673d..15b65a0 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -474,10 +474,12 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
>         const char *remote = curr_branch ? curr_branch->remote_name : NULL;
>
>         if (*refspecs) {
> -               fprintf(stderr,
> -                       _("There are no candidates for merging among the refs that you just fetched.\n"
> -                       "Generally this means that you provided a wildcard refspec which had no\n"
> -                       "matches on the remote end.\n"));
> +               if (opt_rebase)
> +                       fputs(_("There is no candidate for rebasing against among the refs that you just fetched."), stderr);

Is there a reason you switch to fputs, instead of fprintf? $grep -I -r fputs|wc -l 123 $ grep -I -r fprintf|wc -l 689

fputs seems to be used already, though I never came across these parts of the code myself, so I wondered if we had fputs in the code base already.

My next guess would have been personal preference, though there seem to be actual reasons[1] why to use fputs.

Maybe mention the switch in the commit message (As the formatting is unnecessary, switch to the lighter fputs function)?

[1] http://stackoverflow.com/questions/5690979/fputs-vs-fprintf

Show 57 quoted lines

> +               else
> +                       fputs(_("There are no candidates for merging among the refs that you just fetched."), stderr);
> +               fputs(_("Generally this means that you provided a wildcard refspec which had no\n"
> +                       "matches on the remote end."), stderr);
>         } else if (repo && curr_branch && (!remote || strcmp(repo, remote))) {
>                 fprintf(stderr,
>                         _("You asked to pull from the remote '%s', but did not specify\n"
> @@ -485,12 +487,15 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
>                         "for your current branch, you must specify a branch on the command line.\n"),
>                         repo);
>         } else if (!curr_branch) {
> -               fprintf(stderr,
> -                       _("You are not currently on a branch. Please specify which\n"
> -                       "branch you want to merge with. See git-pull(1) for details.\n"
> -                       "\n"
> -                       "    git pull <remote> <branch>\n"
> -                       "\n"));
> +               fputs(_("You are not currently on a branch."), stderr);
> +               if (opt_rebase)
> +                       fputs(_("Please specify which branch you want to rebase against."), stderr);
> +               else
> +                       fputs(_("Please specify which branch you want to merge with."), stderr);
> +               fputs(_("See git-pull(1) for details."), stderr);
> +               fputs("", stderr);
> +               fputs("    git pull <remote> <branch>", stderr);
> +               fputs("", stderr);
>         } else if (!curr_branch->merge_nr) {
>                 struct known_remote *remotes = NULL;
>                 const char *remote_name = "<remote>";
> @@ -499,14 +504,17 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
>                 if (remotes && !remotes->next)
>                         remote_name = remotes->remote->name;
>
> +               fputs(_("There is no tracking information for the current branch."), stderr);
> +               if (opt_rebase)
> +                       fputs(_("Please specify which branch you want to rebase against."), stderr);
> +               else
> +                       fputs(_("Please specify which branch you want to merge with."), stderr);
> +               fputs(_("See git-pull(1) for details."), stderr);
> +               fputs("", stderr);
> +               fputs("    git pull <remote> <branch>", stderr);
> +               fputs("", stderr);
>                 fprintf(stderr,
> -                       _("There is no tracking information for the current branch.\n"
> -                       "Please specify which branch you want to merge with.\n"
> -                       "See git-pull(1) for details.\n"
> -                       "\n"
> -                       "    git pull <remote> <branch>\n"
> -                       "\n"
> -                       "If you wish to set tracking information for this branch you can do so with:\n"
> +                       _("If you wish to set tracking information for this branch you can do so with:\n"
>                         "\n"
>                         "    git branch --set-upstream-to=%s/<branch> %s\n"
>                         "\n"),
> --
> 2.1.4
>