On Mon, Apr 9, 2018 at 3:30 PM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> On 04/08, Eric Sunshine wrote:
>> As with Junio, I'm fine with this hidden option (for now), however, I
>> think you can take this a step further. Rather than having a (hidden)
>> git-reset option which suppresses "HEAD is now at...", instead have a
>> (hidden) option which augments the message. For example,
>> --new-head-desc="New worktree" would make it output "New worktree HEAD
>> is now at...". Changes to builtin/reset.c to support this would hardly
>> be larger than the changes you already made.
>
> Something else I just noticed that may make this a worse solution is
> that this breaks the sentence in two pieces for translators. I guess
> we could somehow get the "New worktree" part of the option translated,
> but that still means that if some language would require to move parts
> of the sentence around that would be less than ideal for translation.
Good point.
One solution would be to have the new hidden option replace the string
entirely: --new-head-msg="New worktree HEAD is now at %s", which would
allow translators to deal with the entire sentence. Even clearer would
be to drop "now", as in "New worktree HEAD is at %s". (Default in
reset.c would still be "HEAD is now at %s", of course.)
Another solution would be not to augment the "HEAD is now at..."
message at all. I realize that that augmentation was one of the
original motivations for this patch series, but with the upcoming
restoration of the "Preparing worktree" message:
Preparing worktree (_branch disposition_)
HEAD is now at ...
it seems clear enough (at least to me) from the context introduced by
the "Preparing..." message that "HEAD is now at..." refers to HEAD in
the worktree. (But that's just my opinion.)
> Would factoring out what we have in 'print_new_head_line()' into some
> common code, maybe in 'pretty.c', and still doing the printing from
> here be a reasonable tradeoff?
Isn't that getting uglier again? Not only would you have to publish
that function, but you'd still need the hidden git-reset
--show-new-head-line option.
Also, you'd end up calling that function from within low-level worker
worktree.c:add_worktree(), thus making it take on UI duties, which
would be nice to avoid if possible. (Unfortunately, the alternate idea
of having worktree.c:add() handle this UI task doesn't quite work
since add_worktree() doesn't return sufficient information for add()
to know whether or not it should print "HEAD is now at..."; however,
add_worktree() could be augmented to return more information.)
So, I don't presently have a good answer. I'm partial to the idea of
not augmenting "HEAD is now at...", partly because context makes it
relatively clear that it applies to the new worktree and partly
because it's simpler (less implementation work for you) to leave it as
is. If that choice isn't desirable, then next best would be for
--new-head-msg= to replace the entire "HEAD is now at..." string
rather than augmenting it.
