git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH v1 2/2] worktree: make add dwim
Date: Tue, 14 Nov 2017 08:45:17 +0000	[thread overview]
Message-ID: <20171114084517.GA12097@hank> (raw)
In-Reply-To: <xmqq1sl2c21g.fsf@gitster.mtv.corp.google.com>

On 11/13, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > I'm a bit torn about hiding his behind an additional flag in git
> > worktree add or not.  I would like to have the feature without the
> > additional flag, but it might break some peoples expectations, so
> > dunno.
> 
> Yeah, I agree with the sentiment.  But what expectation would we be
> breaking, I wonder (see below)?
> 
> At the conceptual level, it is even more unfortunate that "git
> worktree --help" says this for "git worktree add <path> [<branch>]":
> 
>     If `<branch>` 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.
> 
> which means that it already does a DWIM, namely "since you didn't
> say what branch to create to track what other branch, we'll fork one
> for you from the HEAD, and give a name to it".  That may be a useful
> DWIM for some existing users sometimes, and you may even find it
> useful some of the time but not always.  Different people mean
> different things in different situations, and there is no single
> definition for DWIMming that would fit all of them.
> 
> Which in turn means that the new variable name DWIM_NEW_BRANCH and
> the new option name GUESS are way underspecified.  You'd need to
> name them after the "kind" of dwim; otherwise, not only the future
> additions for new (third) kind of dwim would become cumbersome, but
> readers of the code would be confused if they are about the existing
> dwim (fork a new branch from HEAD and give it a name guessed by the
> pathname) or your new dwim.

Yeah I definitely agree with that.  I was mainly thinking about my
personal use case with --guess, and didn't fully think through that it
might mean different things to other people.

> This also needs a documentation update.  Unlike the existing DWIM,
> it is totally unclear when you are dwimming in absence of which
> option.

Sorry about that, I totally forgot about the documentation for this.
Will add in the next round.

> I am guessing that, when you do not have a branch "topic" but your
> upstream does, saying
> 
>     $ git worktree add ../a-new-worktree topic
> 
> would create "refs/heads/topic" to build on top of
> "refs/remotes/origin/topic" just like "git checkout topic" would.
> 
> IOW, when fully spelled out:
> 
>     $ git branch -t -b topic origin/topic
>     $ git worktree add ../a-new-worktree topic
> 
> is what your DWIM does?  Am I following you correctly?

It's not quite what my DWIM does/what I originally intended my DWIM to
do.  What it does is when

    $ git worktree add ../topic

and omitting the branch argument, it would behave the way you spelled
out, i.e.

    $ git branch -t -b topic origin/topic
    $ git worktree add ../topic topic

instead of just checking it out from HEAD.  I must confess I missed
the branch argument, so that still behaves the old way with my code,
while what you have above would make a lot more sense.

> 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.

  reply	other threads:[~2017-11-14  8:43 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 [this message]
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
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=20171114084517.GA12097@hank \
    --to=t.gummerer@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).