[PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so

A user who has just cloned a remote repository and wishes to then work on a
branch other than master may not realize they first need to create the local
branch. e.g.:

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.

This commit teaches git to make a suggestion to the user:

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.
To create a local branch from the same named remote branch, use
  git checkout -b next origin/next

Motivated by http://article.gmane.org/gmane.comp.version-control.git/129528

Signed-off-by: Jay Soffian <redacted>
---
 builtin-checkout.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 41 insertions(+), 2 deletions(-)

I dunno, this seems like a lot of code just to make a suggestion to the
user. Is it worth it?

Also, I initially was going to use for_each_remote_ref and compare every
remote ref name to see if it tail matched what the user gave us, but it was
easier to use for_each_remote and build up the remote ref name and then check
for its existence. Not sure if either approach is preferable.

Thoughts/comments?

diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..7f2e215 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c

@@ -145,6 +145,38 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
 	mm->size = size;
 }
 
+struct suggest_new_branch_name_data {
+	const char *name, *found;
+	int matches;
+};
+
+static int suggest_new_branch_name_compare(struct remote *remote, void *priv)
+{
+	struct suggest_new_branch_name_data *data = priv;
+	unsigned char sha1[20];
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_addf(&buf, "refs/remotes/%s/%s", remote->name, data->name);
+	if (resolve_ref(buf.buf, sha1, 1, NULL)) {
+		data->matches++;
+		if (data->found)
+			strbuf_release(&buf);
+		else
+			data->found = strbuf_detach(&buf, NULL);
+	}
+	return 0;
+}
+
+static void suggest_new_branch_name(const char *name)
+{
+	struct suggest_new_branch_name_data data;
+	data.name = name;
+	data.found = NULL;
+	data.matches = 0;
+	for_each_remote(suggest_new_branch_name_compare, &data);
+	if (data.matches == 1)
+		fprintf(stderr, "To create a local branch from the same named remote branch, use\n  git checkout -b %s %s\n", name, prettify_refname(data.found));
+}
+
 static int checkout_merged(int pos, struct checkout *state)
 {
 	struct cache_entry *ce = active_cache[pos];

@@ -231,8 +263,13 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 		match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
 	}
 
-	if (report_path_error(ps_matched, pathspec, 0))
+	if (report_path_error(ps_matched, pathspec, 0)) {
+		for (pos = 0; pathspec[pos]; pos++)
+			;
+		if (pos == 1)
+			suggest_new_branch_name(pathspec[0]);
 		return 1;
+	}
 
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {

@@ -675,8 +712,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			arg = "@{-1}";
 
 		if (get_sha1(arg, rev)) {
-			if (has_dash_dash)          /* case (1) */
+			if (has_dash_dash) {         /* case (1) */
+				suggest_new_branch_name(arg);
 				die("invalid reference: %s", arg);
+			}
 			goto no_reference;          /* case (3 -> 2) */
 		}
 

-- 
1.6.4.2

Heya,

On Mon, Oct 5, 2009 at 22:46, Jay Soffian [off-list ref] wrote:

To create a local branch from the same named remote branch, use
 git checkout -b next origin/next

Since Dscho added the most useful "-t" option to git checkout, why not
suggest that?

$ git checkout -t origin/next # instant win

-- 
Cheers,

Sverre Rabbelier

Hi,

On Mon, 5 Oct 2009, Jay Soffian wrote:

A user who has just cloned a remote repository and wishes to then work on a
branch other than master may not realize they first need to create the local
branch. e.g.:

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.

This commit teaches git to make a suggestion to the user:

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.
To create a local branch from the same named remote branch, use
  git checkout -b next origin/next

Motivated by http://article.gmane.org/gmane.comp.version-control.git/129528

Actually, we should really think long and hard why we should not 
automatically check out the local branch "next" in that case.  I mean, 
really long and hard, and making sure to take user-friendliness into 
account at least as much as simplicity of implementation.

Ciao,
Dscho

Heya,

On Mon, Oct 5, 2009 at 23:17, Johannes Schindelin
[off-list ref] wrote:

Actually, we should really think long and hard why we should not
automatically check out the local branch "next" in that case.  I mean,
really long and hard, and making sure to take user-friendliness into
account at least as much as simplicity of implementation.

If git was a little more interactive I'd say prompt the user, problem solved?

$ git checkout next
No such branch 'next', do you want to check out a local branch for
'origin/next' instead? [Y/n]

@jay: you assume that if there is more than one matching remote the
user is experienced (as they have multiple remotes) enough to know
what to do?

-- 
Cheers,

Sverre Rabbelier

On Mon, Oct 5, 2009 at 5:26 PM, Sverre Rabbelier [off-list ref] wrote:

@jay: you assume that if there is more than one matching remote the
user is experienced (as they have multiple remotes) enough to know
what to do?

That and it was just an RFC patch, so I just decided to ignore that
case initially.

j.

On Mon, Oct 5, 2009 at 5:17 PM, Johannes Schindelin
[off-list ref] wrote:

Actually, we should really think long and hard why we should not
automatically check out the local branch "next" in that case.  I mean,
really long and hard, and making sure to take user-friendliness into
account at least as much as simplicity of implementation.

Sure, why not? Are you asking for a patch, or just soliciting conversation?

j.

Hi,

On Mon, 5 Oct 2009, Jay Soffian wrote:

On Mon, Oct 5, 2009 at 5:17 PM, Johannes Schindelin
[off-list ref] wrote:

quoted

Actually, we should really think long and hard why we should not
automatically check out the local branch "next" in that case.  I mean,
really long and hard, and making sure to take user-friendliness into
account at least as much as simplicity of implementation.

Sure, why not? Are you asking for a patch, or just soliciting 
conversation?

I am asking for thoughtful arguments for and against my (shyly implied) 
proposal.

Ciao,
Dscho

On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:

quoted

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.
To create a local branch from the same named remote branch, use
  git checkout -b next origin/next

Motivated by http://article.gmane.org/gmane.comp.version-control.git/129528

Actually, we should really think long and hard why we should not 
automatically check out the local branch "next" in that case.  I mean, 
really long and hard, and making sure to take user-friendliness into 
account at least as much as simplicity of implementation.

Some devil's advocate questions:

  1. How do we find "origin/next" given "next"? What are the exact
     lookup rules? Do they cover every case? Do they avoid surprising
     the user?

  2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
     "foobar/next" both exist)?

  3. If our lookup does have ambiguities or corner cases, is it better
     to simply be suggesting to the user, rather than proceeding with an
     action?

-Peff

On Mon, Oct 05, 2009 at 04:46:23PM -0400, Jay Soffian wrote:

+static int suggest_new_branch_name_compare(struct remote *remote, void *priv)
+{
+	struct suggest_new_branch_name_data *data = priv;
+	unsigned char sha1[20];
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_addf(&buf, "refs/remotes/%s/%s", remote->name, data->name);
+	if (resolve_ref(buf.buf, sha1, 1, NULL)) {
+		data->matches++;
+		if (data->found)
+			strbuf_release(&buf);
+		else
+			data->found = strbuf_detach(&buf, NULL);
+	}
+	return 0;
+}

This assumes that remote X always has its tracking branches in
refs/remotes/X/*. But that is really dependent on how the fetch refspec
is set up. True, it will be like that for remotes set up by "git remote"
or "git clone", but it isn't universal (and we have tried not to make
that assumption elsewhere, like when finding upstream branches to merge
from).  Doing it right would mean interpreting the refspecs in
remote.*.fetch.

But this is not necessarily about actual remotes, I don't think. It is
really about the names of refs we have, and that you could reference,
but that are not actual tracking branches. It's just that refs/remotes
is the obvious hierarchy there.

But I wonder if what you should do instead is to iterate through each
ref, removing refs/heads/* and refs/tags/* (which are uninteresting, as
they are already part of the normal ref lookup), and then suffix-match.
So looking for "next" would find "refs/remotes/origin/next", or even
"refs/foobar/next" if you had some "foobar" hierarchy.

It would also match "foo" to "refs/remotes/origin/jk/foo". I'm not sure
if that is a feature or a bug, though.


Aside from that, I can't think of anything wrong with the idea.
Personally I find it more chatty than I would want, because I know what
I'm doing. So I would suggest adding an advice.suggestBranchName config
option to voluntarily suppress it.

-Peff

Jeff King wrote:

On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:

quoted

quoted

$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.

Actually, we should really think long and hard why we should not 
automatically check out the local branch "next" in that case.  I mean, 
really long and hard, and making sure to take user-friendliness into 
account at least as much as simplicity of implementation.

Some devil's advocate questions:

  1. How do we find "origin/next" given "next"? What are the exact
     lookup rules? Do they cover every case? Do they avoid surprising
     the user?

  2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
     "foobar/next" both exist)?

  3. If our lookup does have ambiguities or corner cases, is it better
     to simply be suggesting to the user, rather than proceeding with an
     action?

If I may add another:

4. Are there any (scripted?) use-cases where git-checkout should fail
   because it was given an invalid branch name?

The following gives a hint, though they could of course be fixed and
the ^0 case doesn't really count:

  $ git grep 'git checkout .*||' -- "*.sh"
  git-bisect.sh:          git checkout "$start_head" -- || exit
  git-rebase--interactive.sh:                     output git checkout $first_parent 2> /dev/null ||
  git-rebase--interactive.sh:                     output git checkout "$1" ||
  git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
  t/t2007-checkout-symlink.sh:git checkout -f master || exit

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Hi,

On Mon, 5 Oct 2009, Jeff King wrote:

On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:

quoted

quoted

$ git clone git://git.kernel.org/pub/scm/git/git.git
$ cd git
$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.
To create a local branch from the same named remote branch, use
  git checkout -b next origin/next

Motivated by http://article.gmane.org/gmane.comp.version-control.git/129528

Actually, we should really think long and hard why we should not 
automatically check out the local branch "next" in that case.  I mean, 
really long and hard, and making sure to take user-friendliness into 
account at least as much as simplicity of implementation.

Some devil's advocate questions:

  1. How do we find "origin/next" given "next"? What are the exact
     lookup rules? Do they cover every case? Do they avoid surprising
     the user?

I am sure your strategy would be the same as mine: enumerate all remote 
branches, strip the remote nickname, and compare.  If there are 
ambiguities, tell the user and stop.

  2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
     "foobar/next" both exist)?

See above.

  3. If our lookup does have ambiguities or corner cases, is it better
     to simply be suggesting to the user, rather than proceeding with an
     action?

See above.

Ciao,
Dscho

Hi,

On Tue, 6 Oct 2009, Thomas Rast wrote:

Jeff King wrote:

quoted

On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:

quoted

quoted

$ git checkout next
error: pathspec 'next' did not match any file(s) known to git.

Actually, we should really think long and hard why we should not 
automatically check out the local branch "next" in that case.  I mean, 
really long and hard, and making sure to take user-friendliness into 
account at least as much as simplicity of implementation.

Some devil's advocate questions:

  1. How do we find "origin/next" given "next"? What are the exact
     lookup rules? Do they cover every case? Do they avoid surprising
     the user?

  2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
     "foobar/next" both exist)?

  3. If our lookup does have ambiguities or corner cases, is it better
     to simply be suggesting to the user, rather than proceeding with an
     action?

If I may add another:

4. Are there any (scripted?) use-cases where git-checkout should fail
   because it was given an invalid branch name?

The following gives a hint, though they could of course be fixed and
the ^0 case doesn't really count:

  $ git grep 'git checkout .*||' -- "*.sh"
  git-bisect.sh:          git checkout "$start_head" -- || exit
  git-rebase--interactive.sh:                     output git checkout $first_parent 2> /dev/null ||
  git-rebase--interactive.sh:                     output git checkout "$1" ||
  git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
  t/t2007-checkout-symlink.sh:git checkout -f master || exit

Actually, in said cases (with exception of the test case, which should be 
fine, however, having no remote branches), I would expect the user to be 
grateful if the DWIMery would happen.

I have to clarify something here: I am not proposing to include a patch 
that does that DWIMery.  We need to discuss the downsides and upsides 
until we can be pretty certain that it does more good than harm.

Unfortunately, this list does not seem to be very inviting to pure users, 
who I hoped would chime in on this issue.

Ciao,
Dscho

Johannes Schindelin [off-list ref] writes:

quoted

4. Are there any (scripted?) use-cases where git-checkout should fail
   because it was given an invalid branch name?

The following gives a hint, though they could of course be fixed and
the ^0 case doesn't really count:

  $ git grep 'git checkout .*||' -- "*.sh"
  git-bisect.sh:        git checkout "$start_head" -- || exit
  git-rebase--interactive.sh:  output git checkout $first_parent 2> /dev/null ||
  git-rebase--interactive.sh:  output git checkout "$1" ||
  git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
  t/t2007-checkout-symlink.sh:git checkout -f master || exit

Actually, in said cases (with exception of the test case, which should be 
fine, however, having no remote branches), I would expect the user to be 
grateful if the DWIMery would happen.

Did you check the context before making that assertion?

 - The one in git-bisect switches to (or detaches at) what was earlier
   written in BISECT_START, which is either a branch name or a commit
   object name, so the user definitely does not want DWIMery if it could
   check out something else --- I do not think DWIMery hurts as long as
   the user does not delete the original branch while bisecting, though.

 - The first one in "rebase -i" is always fed a commit object name;
   DWIMery is not needed (and it would not hurt).

 - The second one in "rebase -i" is about switching to the branch being
   rebased, and it has an explicit check to see if "$1" is a branch name;
   DWIMery is not needed (and it would not hurt because of the check
   before it).

 - The one in "rebase" proper, as Thomas pointed out, is an explicit
   request to detach, so DWIMery won't happen.

The first three cases that could trigger DWIMery fall into "DWIMery does
not hurt because it happens to be a no-op in the way it is used" category,
not "In this case, the users would actively appreciate DWIMery".  IOW,
this does not look particularly a good argument to support DWIMery to me.

About the second one in "rebase -i", and also the corresponding one in
"rebase", which is:

	test -z "$switch_to" || git checkout "$switch_to"

If the command did DWIM, you would fork a local branch from the remote and
immediately rebase it.  Any good git tutorial teaches not to rebase work
by others, and keeping the result of such a rebase on a local branch goes
directly against it [*1*]; the script needs to be updated to protect
itself from DWIMery if we were to change "checkout" in these cases.


[Footnote]

*1* It is quite useful to temporarily rebase others work, e.g. in order to
compare what got changed in the newer version of series, so I wouldn't
object if the user did

    git checkout origin/topic
    git rebase $(git merge-base origin/topic@{1} origin/topic)
    git show-branch origin/topic@{1} HEAD

but notice that it all happens on detached HEAD, not to be kept.

Hi,

On Tue, 6 Oct 2009, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

quoted

4. Are there any (scripted?) use-cases where git-checkout should fail
   because it was given an invalid branch name?

The following gives a hint, though they could of course be fixed and
the ^0 case doesn't really count:

  $ git grep 'git checkout .*||' -- "*.sh"
  git-bisect.sh:        git checkout "$start_head" -- || exit
  git-rebase--interactive.sh:  output git checkout $first_parent 2> /dev/null ||
  git-rebase--interactive.sh:  output git checkout "$1" ||
  git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
  t/t2007-checkout-symlink.sh:git checkout -f master || exit

Actually, in said cases (with exception of the test case, which should be 
fine, however, having no remote branches), I would expect the user to be 
grateful if the DWIMery would happen.

Did you check the context before making that assertion?

No, but I checked the _names_ of the scripts.

In case of bisect, if I know upstream is good, I might indeed say "git 
bisect good next", even if I haven't checked myself earlier.

In case of "rebase", about the same happens: if I say "git rebase next", 
and there is no "next", but an "origin/next", and no other remote branch 
"*/next", it is pretty clear what I mean, too.

In any case, it seems pretty clear to me that this DWIMery, while I am 
pretty certain would be useful for actual users without commits in 
git.git, will not make it into git.git.

So I'll stop wasting my time with this discussion.

Ciao,
Dscho

Johannes Schindelin [off-list ref] writes:

Actually, we should really think long and hard why we should not 
automatically check out the local branch "next" in that case.

While people were thinking long and hard, I've spent some quality time
having fun with these patches, and realized that if we limit the scope of
the change to make sure that we only change the behaviour of a case where
we refused to do anything, this is not even something we need to think
long nor hard after all.

At least from the maintainer's point of view, that is.

I on the other hand do agree that we need to think long and hard when it
comes to the matter of explaining this to the users, though.  I couldn't
come up with a good (re-)ordering of the documentation to fit this new
"short-cut" into the manpage.

A three-patch series will follow shortly.