Duy Nguyen <pclouds@xxxxxxxxx> writes:
> On Sun, Feb 04, 2018 at 10:13:03PM +0000, Thomas Gummerer wrote:
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 7cef5b120b..d1549e441d 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -303,7 +303,7 @@ 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);
>> + fprintf(stderr, _("Preparing %s (identifier %s)"), path, name);
>>
>> 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);
>> @@ -320,10 +320,19 @@ static int add_worktree(const char *path, const char *refname,
>> if (ret)
>> goto done;
>>
>> + fprintf(stderr, _(", setting HEAD to %s"),
>
> As a former translator, I'm not thrilled to see a sentence broken into
> two pieces like this. I'm not a Japanese translator, but I think this
> sentence is translated differently when the translator sees the whole
> line "Preparing ..., setting ...".
>
> Since the code between the first fprintf and this one should not take
> long to execute, perhaps we can just delete the first printf and print
> a whole [*] sentence here?
Yeah, also it is a bit troubling that "Preparing" is an incomplete line,
and when update-ref/symblic-ref call fails, will stay to be incomplete.
> I think the purpose of "Preparing..." in the first place is to show
> something when git is busy checkout out the worktree. As long as we
> print it before git-reset, we should be good.
>
>> + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
>> +
>> + strbuf_reset(&sb);
>> + pp_commit_easy(CMIT_FMT_ONELINE, commit, &sb);
>> + if (sb.len > 0)
>> + fprintf(stderr, " %s", sb.buf);
>
> [*] Yes I know it's not "whole" because of this piece. But this one is
> more or less a separate sentence already so it's probably ok.
Doing all the additional fprintf/fputc at the same place (instead of
across update-ref/symbolic-ref) would make it easier to follow and
also allow it to make "whole" even with that piece. Prepare the
abbreviated commit->object.oid.hash and FMT_ONELINE in the same
strbuf and say that the HEAD is there. I think it also makes sense
to split it into two sentences, so from that point of view, just
dropping the change in the "Preparing" hunk and then saying "HEAD is
now at '9876fdea title of the commit'" after update/symbolic-ref may
also be a good change. That automatically removes the "what hapepns
to the rest of the incomplete line when run_command() fails?" issue.
> Is it a bit too long to print everything in one line though?
> CMIT_FMT_ONELINE could already fill 70 columns easily.
>
>> + fputc('\n', stderr);
>> +
>> if (opts->checkout) {
>> cp.argv = NULL;
>> argv_array_clear(&cp.args);
>> - argv_array_pushl(&cp.args, "reset", "--hard", NULL);
>> + argv_array_pushl(&cp.args, "reset", "--hard", "--quiet", NULL);
>> cp.env = child_env.argv;
>> ret = run_command(&cp);
>> if (ret)
>> --
>> 2.16.1.101.gde0f0111ea
>>
