From: Thomas Gummerer <firstname.lastname@example.org> To: Eric Sunshine <email@example.com> Cc: "Junio C Hamano" <firstname.lastname@example.org>, "Git List" <email@example.com>, "Nguyễn Thái Ngọc Duy" <firstname.lastname@example.org> Subject: Re: [PATCH v1 2/2] worktree: make add dwim Date: Wed, 15 Nov 2017 08:50:39 +0000 [thread overview] Message-ID: <20171115085039.GA32324@hank> (raw) In-Reply-To: <CAPig+cTYC01Y9-EvyNo9hxQRbT60iqp8MqXEB_zWBi14kPV1Ng@mail.gmail.com> On 11/14, Eric Sunshine wrote: > On Tue, Nov 14, 2017 at 3:45 AM, Thomas Gummerer <email@example.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. > 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. > 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 :) > 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.
next prev parent reply other threads:[~2017-11-15 8:49 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-12 13:43 [PATCH v1 1/2] checkout: factor out functions to new lib file Thomas Gummerer 2017-11-12 13:43 ` [PATCH v1 2/2] worktree: make add dwim Thomas Gummerer 2017-11-13 3:04 ` Junio C Hamano 2017-11-14 8:45 ` Thomas Gummerer 2017-11-14 20:14 ` Eric Sunshine 2017-11-14 20:29 ` Eric Sunshine 2017-11-15 8:52 ` Thomas Gummerer 2017-11-18 18:13 ` Thomas Gummerer 2017-11-15 8:50 ` Thomas Gummerer [this message] 2017-11-15 9:12 ` Eric Sunshine 2017-11-13 2:41 ` [PATCH v1 1/2] checkout: factor out functions to new lib file Junio C Hamano 2017-11-14 8:46 ` Thomas Gummerer 2017-11-18 18:11 ` [PATCH v2 1/3] " Thomas Gummerer 2017-11-18 18:11 ` [PATCH v2 2/3] worktree: make add <path> <branch> dwim Thomas Gummerer 2017-11-18 22:18 ` Thomas Gummerer 2017-11-18 18:11 ` [PATCH v2 3/3] worktree: make add <path> dwim Thomas Gummerer 2017-11-18 22:47 ` [PATCH v3 0/3] make git worktree add dwim more Thomas Gummerer 2017-11-18 22:47 ` [PATCH v3 1/3] checkout: factor out functions to new lib file Thomas Gummerer 2017-11-18 22:47 ` [PATCH v3 2/3] worktree: make add <path> <branch> dwim Thomas Gummerer 2017-11-19 8:31 ` Eric Sunshine 2017-11-19 17:43 ` Thomas Gummerer 2017-11-18 22:47 ` [PATCH v3 3/3] worktree: make add <path> dwim Thomas Gummerer 2017-11-19 19:04 ` Eric Sunshine 2017-11-19 20:20 ` Eric Sunshine 2017-11-20 0:39 ` Junio C Hamano 2017-11-21 22:13 ` Thomas Gummerer 2017-11-22 1:20 ` Junio C Hamano 2017-11-22 19:49 ` Thomas Gummerer
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171115085039.GA32324@hank \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH v1 2/2] worktree: make add dwim' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).