From: Duy Nguyen <pclouds@gmail.com> To: Thomas Gummerer <t.gummerer@gmail.com> Cc: Git Mailing List <git@vger.kernel.org>, Eric Sunshine <sunshine@sunshineco.com> Subject: Re: [PATCH] worktree: teach "add" to check out existing branches Date: Mon, 22 Jan 2018 18:18:57 +0700 Message-ID: <CACsJy8D9LS7e=cVE3Fq2qOnxK5++nFg2vjuhkNtRO-Bx0X1j6w@mail.gmail.com> (raw) In-Reply-To: <20180121120208.12760-1-t.gummerer@gmail.com> On Sun, Jan 21, 2018 at 7:02 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote: > Currently 'git worktree add <path>' creates a new branch named after the > basename of the path by default. If a branch with that name already > exists, the command refuses to do anything, unless the '--force' option > is given. > > However we can do a little better than that, and check the branch out if > it is not checked out anywhere else. This will help users who just want > to check an existing branch out into a new worktree, and save a few > keystrokes. > > As the current behaviour is to simply 'die()' when a brach with the name > of the basename of the path already exists, there are no backwards > compatibility worries here. > > We will still 'die()' if the branch is checked out in another worktree, > unless the --force flag is passed. > > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> > --- > > This is a follow-up to > https://public-inbox.org/git/20171118181345.GC32324@hank/, where this > was first suggested, but I didn't want to do it as part of that > series. Now I finally got around to implementing it. > > Documentation/git-worktree.txt | 9 +++++++-- > builtin/worktree.c | 22 +++++++++++++++------- > t/t2025-worktree-add.sh | 15 ++++++++++++--- > 3 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt > index 41585f535d..98731b71a7 100644 > --- a/Documentation/git-worktree.txt > +++ b/Documentation/git-worktree.txt > @@ -61,8 +61,13 @@ $ git worktree add --track -b <branch> <path> <remote>/<branch> > ------------ > + > If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used, > -then, as a convenience, a new branch based at HEAD is created automatically, > -as if `-b $(basename <path>)` was specified. > +then, as a convenience, a worktree with a branch named after > +`$(basename <path>)` (call it `<branch>`) is created. If `<branch>` > +doesn't exist, a new branch based on HEAD is automatically created as > +if `-b <branch>` was given. If `<branch>` exists in the repository, > +it will be checked out in the new worktree, if it's not checked out > +anywhere else, otherwise the command will refuse to create the > +worktree. It starts getting a bit too magical to me. We probably should print something "switching to branch..." or "creating new branch ..." to let people know what decision was taken, unless --quiet is given. Yeah I know --quiet does not exist. You don't need to add it now either since nobody has asked for it. More or less related to this, there was a question [1] whether we could do better than $(basename <path>) for determining branch name. Since you're doing start to check if a branch exists, people may start asking to check for branch "foo/bar" from the path abc/foo/bar instead of just branch "bar". [1] https://github.com/git-for-windows/git/issues/1390 > > list:: > > diff --git a/builtin/worktree.c b/builtin/worktree.c > index 7cef5b120b..148a864bb9 100644 > --- a/builtin/worktree.c > +++ b/builtin/worktree.c > @@ -411,13 +411,21 @@ static int add(int ac, const char **av, const char *prefix) > if (ac < 2 && !opts.new_branch && !opts.detach) { > int n; > const char *s = worktree_basename(path, &n); > - opts.new_branch = xstrndup(s, n); > - if (guess_remote) { > - struct object_id oid; > - const char *remote = > - unique_tracking_name(opts.new_branch, &oid); > - if (remote) > - branch = remote; > + const char *branchname = xstrndup(s, n); > + struct strbuf ref = STRBUF_INIT; Perhaps a blank line after this to separate var declarations and the rest. > + if (!strbuf_check_branch_ref(&ref, branchname) && > + ref_exists(ref.buf)) { > + branch = branchname; Hmm.. do we need UNLEAK(branch);? Maybe you should try valgrind, I'm not sure. > + opts.checkout = 1; > + } else { > + opts.new_branch = branchname; > + if (guess_remote) { > + struct object_id oid; > + const char *remote = > + unique_tracking_name(opts.new_branch, &oid); > + if (remote) > + branch = remote; > + } > } > } > > diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh > index 2b95944973..721b0e4c26 100755 > --- a/t/t2025-worktree-add.sh > +++ b/t/t2025-worktree-add.sh > @@ -198,13 +198,22 @@ test_expect_success '"add" with <branch> omitted' ' > test_cmp_rev HEAD bat > ' > > -test_expect_success '"add" auto-vivify does not clobber existing branch' ' > +test_expect_success '"add" auto-vivify checks out existing branch' ' > test_commit c1 && > test_commit c2 && > git branch precious HEAD~1 && > - test_must_fail git worktree add precious && > + git worktree add precious && > test_cmp_rev HEAD~1 precious && > - test_path_is_missing precious > + ( > + cd precious && > + test_cmp_rev precious HEAD > + ) > +' > + > +test_expect_success '"add" auto-vivify fails with checked out branch' ' > + git checkout -b test-branch && > + test_must_fail git worktree add test-branch && > + test_path_is_missing test-branch > ' > > test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' ' > -- > 2.16.0.312.g896df04e46 > -- Duy
next prev parent reply other threads:[~2018-01-22 11:19 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-01-21 12:02 Thomas Gummerer 2018-01-21 12:02 ` Robert P. J. Day 2018-01-22 11:18 ` Duy Nguyen [this message] 2018-01-22 20:17 ` Thomas Gummerer 2018-02-04 22:13 ` [PATCH v2 0/3] " Thomas Gummerer 2018-02-04 22:13 ` [PATCH v2 1/3] worktree: improve message when creating a new worktree Thomas Gummerer 2018-02-05 2:12 ` Duy Nguyen 2018-02-05 20:13 ` Thomas Gummerer 2018-02-05 20:15 ` Junio C Hamano 2018-02-07 8:51 ` Eric Sunshine 2018-02-09 11:27 ` Thomas Gummerer 2018-02-09 12:08 ` Duy Nguyen 2018-02-10 11:20 ` Duy Nguyen 2018-02-04 22:13 ` [PATCH v2 2/3] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer 2018-02-04 22:13 ` [PATCH v2 3/3] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-02-05 2:18 ` Duy Nguyen 2018-02-05 20:20 ` Junio C Hamano 2018-02-05 20:23 ` Thomas Gummerer 2018-02-06 11:53 ` Duy Nguyen 2018-02-09 11:04 ` Thomas Gummerer 2018-03-17 22:08 ` [PATCH v3 0/4] " Thomas Gummerer 2018-03-17 22:08 ` [PATCH v3 1/4] worktree: improve message when creating a new worktree Thomas Gummerer 2018-03-17 22:08 ` [PATCH v3 2/4] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer 2018-03-17 22:08 ` [PATCH v3 3/4] worktree: factor out dwim_branch function Thomas Gummerer 2018-03-17 22:08 ` [PATCH v3 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-03-17 22:22 ` [PATCH v4 0/4] " Thomas Gummerer 2018-03-17 22:22 ` [PATCH v4 1/4] worktree: improve message when creating a new worktree Thomas Gummerer 2018-03-19 17:11 ` Duy Nguyen 2018-03-19 18:09 ` Junio C Hamano 2018-03-20 6:37 ` Eric Sunshine 2018-03-24 20:34 ` Thomas Gummerer 2018-03-17 22:22 ` [PATCH v4 2/4] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer 2018-03-20 6:40 ` Eric Sunshine 2018-03-20 7:26 ` Eric Sunshine 2018-03-20 7:32 ` Eric Sunshine 2018-03-24 20:35 ` Thomas Gummerer 2018-03-17 22:22 ` [PATCH v4 3/4] worktree: factor out dwim_branch function Thomas Gummerer 2018-03-17 22:22 ` [PATCH v4 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-03-20 8:02 ` Eric Sunshine 2018-03-24 21:00 ` Thomas Gummerer 2018-03-25 13:49 ` [PATCH v5 0/6] " Thomas Gummerer 2018-03-25 13:49 ` [PATCH v5 1/6] worktree: improve message when creating a new worktree Thomas Gummerer 2018-03-25 13:49 ` [PATCH v5 2/6] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer 2018-03-27 8:59 ` Eric Sunshine 2018-03-30 13:53 ` Thomas Gummerer 2018-03-25 13:49 ` [PATCH v5 3/6] worktree: remove force_new_branch from struct add_opts Thomas Gummerer 2018-03-27 9:00 ` Eric Sunshine 2018-03-30 13:55 ` Thomas Gummerer 2018-03-25 13:49 ` [PATCH v5 4/6] worktree: factor out dwim_branch function Thomas Gummerer 2018-03-27 9:01 ` Eric Sunshine 2018-03-25 13:49 ` [PATCH v5 5/6] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-03-27 9:04 ` Eric Sunshine 2018-03-30 14:04 ` Thomas Gummerer 2018-03-25 13:49 ` [PATCH v5 6/6] t2025: rename now outdated branch name Thomas Gummerer 2018-03-27 8:58 ` [PATCH v5 0/6] worktree: teach "add" to check out existing branches Eric Sunshine 2018-03-30 14:08 ` Thomas Gummerer 2018-03-31 15:17 ` [PATCH v6 " Thomas Gummerer 2018-03-31 15:17 ` [PATCH v6 1/6] worktree: remove extra members from struct add_opts Thomas Gummerer 2018-03-31 15:18 ` [PATCH v6 2/6] reset: introduce show-new-head-line option Thomas Gummerer 2018-04-02 20:29 ` Junio C Hamano 2018-04-02 22:07 ` Thomas Gummerer 2018-04-02 22:20 ` Thomas Gummerer 2018-04-02 20:34 ` Junio C Hamano 2018-04-02 22:09 ` Thomas Gummerer 2018-03-31 15:18 ` [PATCH v6 3/6] worktree: improve message when creating a new worktree Thomas Gummerer 2018-04-08 9:27 ` Eric Sunshine 2018-03-31 15:18 ` [PATCH v6 4/6] worktree: be clearer when "add" dwim-ery kicks in Thomas Gummerer 2018-03-31 15:18 ` [PATCH v6 5/6] worktree: factor out dwim_branch function Thomas Gummerer 2018-03-31 15:18 ` [PATCH v6 6/6] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-04-01 13:11 ` [PATCH v6 6.5/6] fixup! " Thomas Gummerer 2018-04-09 0:23 ` Eric Sunshine 2018-04-09 19:44 ` Thomas Gummerer 2018-04-09 21:35 ` Eric Sunshine 2018-04-08 10:09 ` [PATCH v6 6/6] " Eric Sunshine 2018-04-08 14:30 ` Thomas Gummerer 2018-04-08 9:08 ` [PATCH v6 0/6] " Eric Sunshine 2018-04-08 14:24 ` Thomas Gummerer 2018-04-09 0:38 ` Eric Sunshine 2018-04-09 19:47 ` Thomas Gummerer 2018-04-09 19:30 ` Thomas Gummerer 2018-04-09 22:06 ` Eric Sunshine 2018-04-11 20:09 ` Thomas Gummerer 2018-04-11 20:48 ` Eric Sunshine 2018-04-11 20:50 ` Thomas Gummerer 2018-04-11 21:14 ` Eric Sunshine 2018-04-15 20:29 ` [PATCH v7 0/4] " Thomas Gummerer 2018-04-15 20:29 ` [PATCH v7 1/4] worktree: remove extra members from struct add_opts Thomas Gummerer 2018-04-15 20:29 ` [PATCH v7 2/4] worktree: improve message when creating a new worktree Thomas Gummerer 2018-04-16 2:09 ` Junio C Hamano 2018-04-23 18:55 ` Thomas Gummerer 2018-04-23 4:27 ` Eric Sunshine 2018-04-23 18:50 ` Thomas Gummerer 2018-04-15 20:29 ` [PATCH v7 3/4] worktree: factor out dwim_branch function Thomas Gummerer 2018-04-15 20:29 ` [PATCH v7 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-04-23 4:52 ` [PATCH v7 0/4] " Eric Sunshine 2018-04-23 19:38 ` [PATCH v8 " Thomas Gummerer 2018-04-23 19:38 ` [PATCH v8 1/4] worktree: remove extra members from struct add_opts Thomas Gummerer 2018-04-24 3:26 ` Eric Sunshine 2018-04-23 19:38 ` [PATCH v8 2/4] worktree: improve message when creating a new worktree Thomas Gummerer 2018-04-24 3:58 ` Eric Sunshine 2018-04-23 19:38 ` [PATCH v8 3/4] worktree: factor out dwim_branch function Thomas Gummerer 2018-04-23 19:38 ` [PATCH v8 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-04-24 4:25 ` Eric Sunshine 2018-04-24 21:56 ` [PATCH v9 0/4] " Thomas Gummerer 2018-04-24 21:56 ` [PATCH v9 1/4] worktree: remove extra members from struct add_opts Thomas Gummerer 2018-04-24 21:56 ` [PATCH v9 2/4] worktree: improve message when creating a new worktree Thomas Gummerer 2018-04-24 21:56 ` [PATCH v9 3/4] worktree: factor out dwim_branch function Thomas Gummerer 2018-04-24 21:56 ` [PATCH v9 4/4] worktree: teach "add" to check out existing branches Thomas Gummerer 2018-04-27 7:36 ` [PATCH v9 0/4] " Eric Sunshine 2018-04-28 16:09 ` Thomas Gummerer 2018-04-30 0:07 ` Junio C Hamano 2018-03-18 0:24 ` [PATCH v3 " Junio C Hamano
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='CACsJy8D9LS7e=cVE3Fq2qOnxK5++nFg2vjuhkNtRO-Bx0X1j6w@mail.gmail.com' \ --to=pclouds@gmail.com \ --cc=git@vger.kernel.org \ --cc=sunshine@sunshineco.com \ --cc=t.gummerer@gmail.com \ /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
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git