git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v1 2/2] worktree: make add dwim

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Nov 15, 2017, 08:50 UTC
Message-ID
<20171115085039.GA32324@hank>
In-Reply-To
<CAPig+cTYC01Y9-EvyNo9hxQRbT60iqp8MqXEB_zWBi14kPV1Ng@mail.gmail.com>
On 11/14, Eric Sunshine wrote:
Show 38 quoted lines
> On Tue, Nov 14, 2017 at 3:45 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > On 11/13, Junio C Hamano wrote:
> >> If so, as long as the new DWIM kicks in ONLY when "topic" does not
> >> exist, I suspect that there is no backward compatibility worries.
> >> The command line
> >>
> >>     $ git worktree add ../a-new-worktree topic
> >>
> >> would just have failed because three was no 'topic' branch yet, no?
> >
> > Indeed, with this there would not be any backwards compatility
> > worries.
> >
> > Ideally I'd still like to make
> >
> >     $ git worktree add ../topic
> >
> > work as described above, to avoid having to type 'topic' twice (the
> > directory name matches the branch name for the vast majority of my
> > worktrees) but that should then come behind a flag/config option?
> > Following your reasoning above it should probably be called something
> > other than --guess.
> >
> > Maybe --guess-remote and worktree.guessRemote would do?  I'm quite bad
> > at naming though, so other suggestions are very welcome.
> 
> For my own edification...
> 
> git worktree add ../dir branch
> 
> * Checks out branch into ../dir if branch exists.
> 
> * Errors out if branch does not exist. However, if we consider the
> history of the implementation of worktrees[*1*], then this really
> ought to try the "origin/branch -> branch" DWIM before erroring-out.
> Implementing this DWIM could be considered a regression fix according
> to [*1*] and, as Junio points out, should not pose backward
> compatibility worries.

Agreed, I think it is not very controversial that this would be an improvement.

Show 19 quoted lines
> git worktree add ../topic
> 
> * Correctly errors out, refusing to create a new branch named "topic",
> if "topic" is already a branch.
> 
> * Creates a new branch named "topic" if no such local branch exists.
> 
> The desired new DWIMing would change the second bullet point to:
> 
> * If no local branch named "topic" exists, DWIM it from "origin/topic"
> if possible, else create a new local branch named "topic".
> 
> But that's a behavior change since, with the existing implementation,
> a newly created local "topic" has no relation to, and does not track,
> any origin/topic branch. The proposed --guess option is to avoid users
> being hit by this backward incompatibility, however, one could also
> view the proposed DWIMing as some sort of bug fix since, at least
> some, users would expect the DWIMing since it already takes place
> elsewhere.

I'm not sure we can call it a bug fix anymore, since as Junio pointed out the current behaviour of creating a new branch at HEAD is documented in the man page.

However git-worktree is also still marked as experimental in the man page, so we could allow ourselves to be a little bit more lax when it comes to backwards compatibility, especially because it is easy to take corrective action after the new DWIMing happened.

Show 17 quoted lines
> So, at least two options exist:
> 
> * Just make the new DWIMing the default behavior, accepting that it
> might bite a few people. Fallout can be mitigated somewhat by
> prominently announcing that the DWIMing took place, in which case the
> user can take corrective action (say, by choosing a different worktree
> name); nothing is lost and no damage done since this is happening only
> at worktree creation time.
> 
> * Add a new option to enable DWIMing; perhaps name it -t/--track,
> which is familiar terminology and still gives you a relatively short
> and sweet "git worktree add -t ../topic".
> 
> Given the mentioned mitigation factor and that some/many users likely
> would expect it to DWIM "origin/topic -> topic" anyhow, I'd lean in
> favor of the first option (but perhaps I'm too daring with other
> people's workflows).

Yeah, I'm leaning towards the first option as well, but I'm clearly biased as that's how I'd like it to behave, and others might want the other behaviour. Unfortunately I don't know many worktree users, so I can't tell what the general consensus on this would be.

I guess the second option would be the safer one, and we can still switch that to be the default at some point if we wish to do so later.

tl;dr I have no idea which of the options would be better :)
Show 16 quoted lines
> FOOTNOTES
> 
> [*1*]: When Duy first implemented worktree support, he incorporated it
> directly into the git-checkout command ("git checkout --to worktree
> ..."), which means that he got all the git-checkout features for free,
> including the "origin/branch -> branch" DWIM. When worktree support
> was later moved to git-worktree, it lost most of the features
> inherited implicitly from git-checkout, such as -b, -B, --detach, so
> those were added back to git-worktree explicitly. However, at that
> early stage, git-worktree was still piggy-backing atop git-checkout,
> thus likely was still getting the "origin/branch -> branch" DWIM for
> free. A final iteration converted git-worktree away from heavyweight
> git-checkout to lightweight git-reset, at which point he DWIMing was
> lost. If you take this history into account, then loss of
> "origin/branch -> branch" DWIMing is a regression, so restoring it
> could be considered a bug fix.
Previous: Thomas GummererNext: Eric Sunshine
Message 9 of 28 in “checkout: factor out functions to new lib file”
  1. 1/2 checkout: factor out functions to new lib fileThomas Gummerer, Nov 12, 2017
  2. 2/2 worktree: make add dwimThomas Gummerer, Nov 12, 2017
  3. Junio C HamanoNov 13, 2017
  4. Thomas GummererNov 14, 2017
  5. Eric SunshineNov 14, 2017
  6. Eric SunshineNov 14, 2017
  7. Thomas GummererNov 15, 2017
  8. Thomas GummererNov 18, 2017
  9. Thomas GummererNov 15, 2017
  10. Eric SunshineNov 15, 2017
  11. Junio C HamanoNov 13, 2017
  12. Thomas GummererNov 14, 2017
  13. 1/3 checkout: factor out functions to new lib fileThomas Gummerer, Nov 18, 2017
  14. 2/3 worktree: make add <path> <branch> dwimThomas Gummerer, Nov 18, 2017
  15. Thomas GummererNov 18, 2017
  16. 3/3 worktree: make add <path> dwimThomas Gummerer, Nov 18, 2017
  17. 0/3 make git worktree add dwim moreThomas Gummerer, Nov 18, 2017
  18. 2/3 worktree: make add <path> <branch> dwimThomas Gummerer, Nov 18, 2017
  19. Eric SunshineNov 19, 2017
  20. Thomas GummererNov 19, 2017
  21. 1/3 checkout: factor out functions to new lib fileThomas Gummerer, Nov 18, 2017
  22. 3/3 worktree: make add <path> dwimThomas Gummerer, Nov 18, 2017
  23. Eric SunshineNov 19, 2017
  24. Eric SunshineNov 19, 2017
  25. Junio C HamanoNov 20, 2017
  26. Thomas GummererNov 21, 2017
  27. Junio C HamanoNov 22, 2017
  28. Thomas GummererNov 22, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.