Re: [PATCH v7 2/4] worktree: improve message when creating a new worktree

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 16, 2018, 02:09 UTC
Message-ID: <xmqq604rzytx.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180415202917.4360-3-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 9 quoted lines

> Currently 'git worktree add' produces output like the following:
>
>     Preparing ../foo (identifier foo)
>     HEAD is now at 26da330922 <title>
>
> The '../foo' is the path where the worktree is created, which the user
> has just given on the command line.  The identifier is an internal
> implementation detail, which is not particularly relevant for the user
> and indeed isn't mentioned explicitly anywhere in the man page.

OK. Maybe there once was a place or two that took the identifier as an input to name a specific worktree to work on (perhaps "prune"?), but if we no longer do that (which makes sense, as we should be able to uniquely identify a worktree by the path to it), then it makes perfect sense to prevent it from appearing in the UI.

> Instead of this message, print a message that gives the user a bit more
> detail of what exactly 'git worktree' is doing.  There are various dwim
> modes which are perform some magic under the hood, which should be

s/are perform/perform/, I think (no need to reroll, only to fix this).

Show 6 quoted lines

> Additionally currently the "Preparing ..." line is printed to stderr,
> while the "HEAD is now at ..." line is printed to stdout by 'git reset
> --hard', which is used internally by 'git worktree add'.  Fix this
> inconsistency by printing the "Preparing ..." message to stdout as
> well.  As "Preparing ..." is not an error, stdout also seems like the
> more appropriate output stream.

I think it is a bug for reset to give this kind of informational message to the standard output stream. A rule of thumb I use is "is this something that a user who wants quiet operation would wish to squelch with --quiet option?" and if the answer is yes, it should go to the standard error output, so info and progress should go to the standard output in general.

But I am OK to unify to the standard output with this patch. We may want to come back and correct _both_ this new message and what reset says, but that is outside the scope of this topic.

Thanks.