Re: [PATCH v4 1/4] worktree: improve message when creating a new worktree

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 20, 2018, 06:37 UTC
Message-ID: <CAPig+cT8i9L9kbhx=b0sG4_QYNdoEDPW-1xypM9xzBqPmqR__Q@mail.gmail.com>
In-Reply-To: <20180317222219.4940-2-t.gummerer@gmail.com>

On Sat, Mar 17, 2018 at 6:22 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:

Show 13 quoted lines

> [...]
> Fix these inconsistencies, and no longer show the identifier by making
> the 'git reset --hard' call quiet, and printing the message directly
> from the builtin command instead.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -303,8 +303,6 @@ static int add_worktree(const char *path, const char *refname,
>         strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
>         write_file(sb.buf, "../..");
>
> -       fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);

A minor regression with this change is that error messages from git-update-ref or git-symbolic-ref -- which could be emitted after this point but before the new "worktree HEAD is now at..." message -- are now somewhat orphaned. I'm not sure that it matters, though.

Show 6 quoted lines

>         argv_array_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
>         argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
>         cp.git_cmd = 1;
> @@ -320,10 +318,19 @@ static int add_worktree(const char *path, const char *refname,
> +       fprintf(stderr, _("worktree HEAD is now at %s"),
> +               find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));

I wonder if this should say "new worktree HEAD is now at..." to clearly indicate that it is talking about HEAD in the _new_ worktree, not HEAD in the current worktree.

Show 5 quoted lines

> +       strbuf_reset(&sb);
> +       pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
> +       if (sb.len > 0)
> +               fprintf(stderr, " %s", sb.buf);
> +       fputc('\n', stderr);