Subject: Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree

From: Thomas Gummerer <[email-masked]>
Date: Mon, 5 Feb 2018 20:13:49 +0000

On 02/05, Duy Nguyen wrote:
> 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 ...".

Good point, I didn't think about the sentence structure other
languages could require here. 

> 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?
> 
> 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.
> 
> Is it a bit too long to print everything in one line though?
> CMIT_FMT_ONELINE could already fill 70 columns easily.

Yeah, it does get a bit long.  Maybe just splitting the sentences here
would be the best solution, then we also won't have to worry about the
split not being good for translation.  I'll do that.

> > +	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
> >