On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <[email protected]> wrote:
> Make use of the new ref aliases to pass refs from another worktree
> around and access them from the current ref store instead. This does
> not change any functionality, but when a problem shows up, we would
> report something like
>From this description, I had a hard time grasping that the first
example output is the desired one. Not necessarily worth a re-roll,
but had it said instead something like this:
...but when a problem arises, we would like the reported
messages to mention full ref aliases, like this:
it would have been more obvious.
More below...
> fatal: bad object worktrees/ztemp/HEAD
> warning: reflog of 'main/HEAD' references pruned commits
>
> instead of
>
> fatal: bad object HEAD
> warning: reflog of 'HEAD' references pruned commits
>
> which does not really tell where the refs are from.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
> +void strbuf_worktree_ref(const struct worktree *wt,
> + struct strbuf *sb,
> + const char *refname)
> +{
> + if (wt && !wt->is_current) {
> + if (is_main_worktree(wt))
> + strbuf_addstr(sb, "main/");
> + else
> + strbuf_addf(sb, "worktrees/%s/", wt->id);
> + }
> + strbuf_addstr(sb, refname);
> +}
Seeing this use worktree->id to construct "worktrees/<id>/<refname>"
makes me wonder how the user is going to know the ID of a worktree in
order to specify such refs in the first place. For example, how is the
user going to know the <id> in "git rev-parse worktrees/<id>/HEAD"? I
don't think we print the worktree ID anywhere, do we? Certainly, "git
worktree list" doesn't show it, and "git worktree add" stopped showing
it as of 2c27002a0a (worktree: improve message when creating a new
worktree, 2018-04-24).
> diff --git a/worktree.h b/worktree.h
> @@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct
> worktree *wt,
> +/*
> + * Return a refname suitable for access from the current ref
> + * store. The result may be destroyed at the next call.
> + */
If you re-roll, perhaps: s/may/will/