Re: [PATCH v6 3/6] worktree: improve message when creating a new worktree

2018-04-08 Thread Eric Sunshine
On Sat, Mar 31, 2018 at 11:18 AM, Thomas Gummerer  wrote:
> [...]
> Fix these inconsistencies, and no longer show the identifier by making
> the 'git reset --hard' call not print the "HEAD is now at ..." line
> using the newly introduced flag from the previous commit, and printing
> the message directly from the builtin command instead.
>
> Signed-off-by: Thomas Gummerer 
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -322,12 +320,22 @@ static int add_worktree(const char *path, const char 
> *refname,
> argv_array_pushl(, "reset", "--hard", NULL);
> +   argv_array_push(, "--no-show-new-head-line");
> cp.env = child_env.argv;
> ret = run_command();
> if (ret)
> goto done;
> }
>
> +   fprintf(stderr, _("New worktree HEAD is now at %s"),
> +   find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
> +
> +   strbuf_reset();
> +   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
> +   if (sb.len > 0)
> +   fprintf(stderr, " %s", sb.buf);
> +   fputc('\n', stderr);
> +
> is_junk = 0;
> FREE_AND_NULL(junk_work_tree);
> FREE_AND_NULL(junk_git_dir);

Generally speaking, code such as this probably ought to be inserted
outside of the is_junk={1,0} context in order to keep that critical
section as small as possible. However, as mentioned in my response to
the v6 cover letter[1], I think this chunk of new code can just go
away entirely if git-reset is made to print the customized message on
git-worktree's behalf.

[1]: 
https://public-inbox.org/git/capig+cq8vzdycumo-qoexndbgqgegj2bpmpa-y0vhgct_br...@mail.gmail.com/


[PATCH v6 3/6] worktree: improve message when creating a new worktree

2018-03-31 Thread Thomas Gummerer
Currently 'git worktree add' produces output like the following, when
'--no-checkout' is not given:

Preparing foo (identifier foo)
HEAD is now at 26da330922 

where the first line is written to stderr, and the second line coming
from 'git reset --hard' is written to stdout, even though both lines are
supposed to tell the user what has happened.  In addition to someone not
familiar with 'git worktree', this might seem as if the current HEAD was
modified, not the HEAD in the new working tree.

If the '--no-checkout' flag is given, the output of 'git worktree add'
is just:

Preparing foo (identifier foo)

even though the HEAD is set to a commit, which is just not checked out.

The identifier is also not particularly relevant for the user at the
moment, as it's only used internally to distinguish between different
worktrees that have the same $(basename ).

Fix these inconsistencies, and no longer show the identifier by making
the 'git reset --hard' call not print the "HEAD is now at ..." line
using the newly introduced flag from the previous commit, and printing
the message directly from the builtin command instead.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4d96a21a45..cc94325886 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -301,8 +301,6 @@ static int add_worktree(const char *path, const char 
*refname,
strbuf_addf(, "%s/commondir", sb_repo.buf);
write_file(sb.buf, "../..");
 
-   fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
-
argv_array_pushf(_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
argv_array_pushf(_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
cp.git_cmd = 1;
@@ -322,12 +320,22 @@ static int add_worktree(const char *path, const char 
*refname,
cp.argv = NULL;
argv_array_clear();
argv_array_pushl(, "reset", "--hard", NULL);
+   argv_array_push(, "--no-show-new-head-line");
cp.env = child_env.argv;
ret = run_command();
if (ret)
goto done;
}
 
+   fprintf(stderr, _("New worktree HEAD is now at %s"),
+   find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
+
+   strbuf_reset();
+   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
+   if (sb.len > 0)
+   fprintf(stderr, " %s", sb.buf);
+   fputc('\n', stderr);
+
is_junk = 0;
FREE_AND_NULL(junk_work_tree);
FREE_AND_NULL(junk_git_dir);
-- 
2.16.1.78.ga2082135d8