Re: [PATCH v3] worktree add: improve message for ambiguous remote branch name
Junio C Hamano <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
"Yoichi NAKAYAMA via GitGitGadget" <[email protected]> writes: > From: Yoichi NAKAYAMA <[email protected]> > > When the user runs 'git worktree add x y' command that does not > exactly say which remote they want to work with, and there is no local > branch named y, we try to guess which remote by passing y then create > a new branch named y which tracks the remote branch. I used x and y as placeholders. The readers would be helped if you used a more plausible sounding names, e.g., naming directory as something like foo-dir (the point being 'dir' somewhere in its name) and naming a branch as something like bar-topic. If this were 'git worktree add', it is probably more than likely that the destination directory would begin with ../ to have the new worktree next to the primary repository we are running in, no? > If there are multiple remotes that have branch named y, we silently > gave up, leaving the variable branch intact. This later causes > creating local branch and worktree not happen, and we end up with > passing an non-existing branch to lookup_commit_reference_by_name(), > triggering "invalid reference" error and die. "This later causes" part still seems a bit too sketchy to help a totally new reader, even though I've stared at this code long enough so it would be sufficient for me personally. But these logs are not about helping me, but helping other developers, so... > +#define WORKTREE_ADD_AMBIGUOUS_REMOTE_BRANCH_NAME_HINT_TEXT \ > + _("Matched multiple remote tracking branches, you can list them by:\n" \ > + "\n" \ > + " git branch -r --list \"*/%s\"\n" \ > + "\n" \ > + "If you meant to create a worktree from a remote tracking branch on,\n" \ > + "e.g. 'origin', you can do so by:\n" \ > + "\n" \ > + " git worktree add -b %s %s origin/%s\n" \ > + "\n" \ > + "If you'd like to always prefer some remote, e.g. 'origin',\n" \ > + "consider setting checkout.defaultRemote=origin in your config.") Instead of throwing the problem back to the user with four extra lines of message telling them how to run 'git branch', I would have expected this patch to teach unique_tracking_name() to optionally return the list of remotes with that branch name, and to use that result in this message. However, if the goal is simply to provide something better than 'invalid reference', we do not even need to go that far. Just stating that branch 'y' appears on multiple remotes and asking them to clarify which one they mean might be a sufficient improvement. Could the original request be aiming to create a new worktree with the HEAD detached at the commit pointed at by the remote-tracking branch, instead of creating a local branch forked from it? I am just wondering if "-b %s" is too specific to one possible interpretation that may contradict to what the user actually wanted to do. Thanks.