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

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

[Jay Soffian]

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 <jaysoffian@gmail.com>
---
 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

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

[Sverre Rabbelier]

Heya,

On Mon, Oct 5, 2009 at 22:46, Jay Soffian <jaysoffian@gmail.com> 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

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

[Johannes Schindelin]

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

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

[Sverre Rabbelier]

Heya,

On Mon, Oct 5, 2009 at 23:17, Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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

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

[Jay Soffian]

On Mon, Oct 5, 2009 at 5:26 PM, Sverre Rabbelier <srabbelier@gmail.com> 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.

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

[Jay Soffian]

On Mon, Oct 5, 2009 at 5:17 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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.

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

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 598 bytes --]

Hi,

On Mon, 5 Oct 2009, Jay Soffian wrote:

> On Mon, Oct 5, 2009 at 5:17 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> 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?

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

Ciao,
Dscho

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

[Jeff King]

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

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

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

[Thomas Rast]

Jeff King wrote:
> On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:
> 
> > > $ 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

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

[Johannes Schindelin]

Hi,

On Tue, 6 Oct 2009, Thomas Rast wrote:

> Jeff King wrote:
> > On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:
> > 
> > > > $ 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

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

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

[Johannes Schindelin]

Hi,

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

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> 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

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

I do not think it was a waste of time; earlier you said that you were not
proposing to include a patch that does that DWIMery, and we need to
discuss the downsides and upsides until we can figure out if it does more
good than harm.

And I think we reasonably established that this does more harm than good,
so I am Ok if you want to stop here.

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

[Johannes Schindelin]

Hi,

On Mon, 5 Oct 2009, Jeff King wrote:

> On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:
> 
> > > $ 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

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

[Matthieu Moy]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Mon, 5 Oct 2009, Jeff King wrote:
>
>> 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.

One problem with this approach is that if users get used to the
behavior, the command will have great probability to end up in a
user's script, then the script will "work" as long as there is no
ambiguity, and cease to work afterwards. And for the user of the
script, this will sound like "WTF, it was working yesterday and it's
broken now".

So, the good thing with being strict, even if giving advice in case of
failure, is that it teaches the user the reliable way to do.

All that said, I'm not sure how serious this is, but we're in a
"devil's advocate" session, so I'm still allowed to speak ;-).


The other fear I have is to create confusion. Today, it's quite clear
that "next" is not the same as "origin/next". With some DWIMery on top
of this, a naive user may think they are more or less the same, and
then not understand what "git fetch" does and why it's not the same as
"git pull".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

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

[Mikael Magnusson]

2009/10/6 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Hi,
>>
>> On Mon, 5 Oct 2009, Jeff King wrote:
>>
>>> 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.
>
> One problem with this approach is that if users get used to the
> behavior, the command will have great probability to end up in a
> user's script, then the script will "work" as long as there is no
> ambiguity, and cease to work afterwards. And for the user of the
> script, this will sound like "WTF, it was working yesterday and it's
> broken now".
>
> So, the good thing with being strict, even if giving advice in case of
> failure, is that it teaches the user the reliable way to do.

I can imagine this happening:
% git clone git://git.git git
% git checkout next
do you want to checkout origin/next? y
# a few days later
% git fetch
% git checkout next
[freenode] /join #git
[#git] i did git checkout next but my files are still the same?

-- 
Mikael Magnusson

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

[Johannes Schindelin]

Hi,

On Tue, 6 Oct 2009, Mikael Magnusson wrote:

> I can imagine this happening:
> % git clone git://git.git git
> % git checkout next
> do you want to checkout origin/next? y
> # a few days later
> % git fetch
> % git checkout next
> [freenode] /join #git
> [#git] i did git checkout next but my files are still the same?

I imagined more something like this:

$ git clone git://git.git git
$ git checkout next
Automatically checking out local branch 'next' tracking 'origin/next'.
Please update it with 'git pull'.

Ciao,
Dscho

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

[Eugene Sajine]

It seams that my first email was eaten by the server for some
reason... Sorry, if it will be a dupe.

On Tue, Oct 6, 2009 at 12:18 PM,  <Euguess@gmail.com> wrote:
 Hi,

 If i may have a word:

> On Oct 6, 2009 5:41am, Mikael Magnusson <mikachu@gmail.com> wrote:
>> I can imagine this happening:
>>
>> % git clone git://git.git git
>>
>> % git checkout next
>>
>> do you want to checkout origin/next? y
>>
>> # a few days later
>>
>> % git fetch
>>
>> % git checkout next
>>
>> [freenode] /join #git
>>
>> [#git] i did git checkout next but my files are still the same?
>>


 I'm a new user of git and I don't think i will ever have a commit in
 git.git, because I'm not a programmer (I'm QA). I was reading this topic as
 carefully as i could and I think that this makes a lot of sense to address
 this issue. As i understand when somebody fetches from remote repo in order
 to be able to start working on the code from this remote repo you should
 create tracking branch for one of the branches from remote and only then you
 should do your changes or perform merges.
 in case if you didn't do that and you try to checkout you will end up having
 detached HEAD which is quite scary;) for non-experienced user and as i see
 might lead to some unnecessary questions in this list or on IRC channel...
 As for the solution i would choose the "simplest thing that will work" - so
 i think that we just have to notify user about his suicide attempt to
 checkout nonlocal branch and offer him a correct syntax to go with.
 Something like below should work:

 % git clone git://git.git git
 % git checkout next
 You're attempting to checkout to non-local branch. This will lead to your
 HEAD being detached (our team is on its way!).
 Do you want to check out local branch 'next' tracking 'origin/next' instead?
 y/n

 if yes, then:
 Created branch "next" tracking "origin/next"
 You can update it with 'git pull'.

 If no - abort or continue with checkout to nonlocal branch? ('m not sure if
 detaching HEAD can provide some benefits if done on purpose)

 I hope I'm not missing anything...

 Thanks,
 Eugene

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

[Junio C Hamano]

Eugene Sajine <euguess@gmail.com> writes:

>  As for the solution i would choose the "simplest thing that will work" - so
>  i think that we just have to notify user about his suicide attempt to
>  checkout nonlocal branch and offer him a correct syntax to go with.

We already do that, without going interactive, for warning unintended
detachment:

    $ git checkout origin/next
    Note: moving to 'origin/next' which isn't a local branch
    If you want to create a new branch from this checkout, you may do so
    (now or later) by using -b with the checkout command again. Example:
      git checkout -b <new_branch_name>
    ...

As to Mikael's scenario:

>>> I can imagine this happening:
>>> % git clone git://git.git git
>>> % git checkout next
>>> do you want to checkout origin/next? y
>>> # a few days later
>>> % git fetch
>>> % git checkout next
>>> [freenode] /join #git
>>> [#git] i did git checkout next but my files are still the same?

No amount of sugarcoating the checkout syntax changes the fact that in the
user's repository there _are_ two distinct refs, origin/next and next, and
the "fetch few days later" updates only the former but never the latter.
It can only be fixed by injecting a bit of clue to the user, in a way
Dscho suggested in the thread.

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

[Johannes Schindelin]

Hi,

On Tue, 6 Oct 2009, Euguess@gmail.com wrote:

> I'ma new user of git and I don't think i will ever have a commit in 
> git.git, because I'm not a programmer (I'm QA).

Welcome!

Let me take this opportunity to express my deep sadness that your first 
input to this list was brushed off so carelessly.

I sincerely hope that you give us another chance, and that you let us 
benefit from your fresh and unbiased view of the usability issues in Git 
(some of us use Git for so long, they think that Git has no usability 
issues anymore).

> I was reading this topic as carefully as i could and I think that this 
> makes a lot of sense to address this issue. As i understand when 
> somebody fetches from remote repo in order to be able to start working 
> on the code from this remote repo you should create tracking branch for 
> one of the branches from remote and only then you should do your changes 
> or perform merges.
>
> in case if you didn't do that and you try to checkout you will end up 
> having detached HEAD which is quite scary;) for non-experienced user and 
> as i see might lead to some unnecessary questions in this list or on IRC 
> channel...

Right.  We see that type of confusion in #git everyday, and blaming the 
user would be a violation of http://c2.com/cgi/wiki?BlameTheRightThing

> As for the solution i would choose the "simplest thing that will work" - 
> so i think that we just have to notify user about his suicide attempt to 
> checkout nonlocal branch and offer him a correct syntax to go with.
>
> Something like below should work:
> 
> % git clone git://git.git git
> % git checkout next
> You're attempting to checkout to non-local branch. This will lead to your HEAD
> being detached (our team is on its way!).
> Do you want to check out local branch 'next' tracking 'origin/next' instead?
> y/n
> 
> if yes, then:
> Created branch "next" tracking "origin/next"
> You can update it with 'git pull'.
> 
> If no - abort or continue with checkout to nonlocal branch? ('m not sure if
> detaching HEAD can provide some benefits if done on purpose)
> 
> I hope I'm not missing anything...

No, I think that is something perfectly fine to expect in a software whose 
UI complexity is unfortunately pretty much in disagreement with its 
internal complexity.

One thing one might add for the technically inclined folks (i.e. those who 
need to implement, and to see that Git is in dear need of some 
user-friendliness first): "git checkout" is a porcelain (i.e. a program 
meant for end-user consumption), and as such should not have a problem to 
react to isatty(0) (i.e. "is the input coming directly from the 
console?").

So yes, even if I was on the verge of giving up on this thread, I have 
been encouraged enough to get this uphill battle going again, and to try 
to overturn some stubborn resistance.

Ciao,
Dscho

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

[Björn Steinbrink]

On 2009.10.12 09:49:50 +0200, Johannes Schindelin wrote:
> On Tue, 6 Oct 2009, Euguess@gmail.com wrote:
> > I'ma new user of git and I don't think i will ever have a commit in 
> > git.git, because I'm not a programmer (I'm QA).
[...]
> > As for the solution i would choose the "simplest thing that will work" - 
> > so i think that we just have to notify user about his suicide attempt to 
> > checkout nonlocal branch and offer him a correct syntax to go with.
> >
> > Something like below should work:
> > 
> > % git clone git://git.git git
> > % git checkout next
> > You're attempting to checkout to non-local branch. This will lead to your HEAD
> > being detached (our team is on its way!).
> > Do you want to check out local branch 'next' tracking 'origin/next' instead?
> > y/n
> > 
> > if yes, then:
> > Created branch "next" tracking "origin/next"
> > You can update it with 'git pull'.
> > 
> > If no - abort or continue with checkout to nonlocal branch? ('m not sure if
> > detaching HEAD can provide some benefits if done on purpose)
> > 
> > I hope I'm not missing anything...
> 
> No, I think that is something perfectly fine to expect in a software whose 
> UI complexity is unfortunately pretty much in disagreement with its 
> internal complexity.
> 
> One thing one might add for the technically inclined folks (i.e. those who 
> need to implement, and to see that Git is in dear need of some 
> user-friendliness first): "git checkout" is a porcelain (i.e. a program 
> meant for end-user consumption), and as such should not have a problem to 
> react to isatty(0) (i.e. "is the input coming directly from the 
> console?").

So I didn't mean to chime in, but anyway... A few days ago, uau on #git
said that he thinks that "git clone" shouldn't create any branch heads
at all. Instead, git should learn to do something like "svn up", when
the user checked out a remote tracking branch. That was specifically
meant for users that _don't_ commit, like, say, QA guys ;-)

I didn't quite agree on the idea (feel free to tell me that I just blank
out UI problems :-p), but anyway, I felt like coming up with some hack
that achieves said functionality. The result was inspired by "git
checkout -" and looks at HEAD's reflog to figure out whether the user
has checked out a remote tracking branch the last time he used checkout
to switch branches. I dared to call it "git-up" in my $HOME/bin ;-)

#!/bin/bash
MODE=${1:---merge}

RTB=$(git rev-parse --symbolic-full-name $(git reflog | grep 'checkout: moving from .* to' | head -1 | sed -e 's/.* to //'))

if [ ${RTB:0:13} != "refs/remotes/" ]
then
	echo "You're not on a remote tracking branch"
	exit 1
fi

SRTB=${RTB#refs/remotes/}
REMOTE=${SRTB%/*}
git fetch $REMOTE
git reset $MODE $RTB


It's obviously basically just "git reset" on crack, happily dropping
local commits. A "real" implementation would likely have to have more
checks to ensure that the user is using it in an expected way (like
checking that refs/remotes/whatever..HEAD is empty). And it could be
made to work with regular branch heads as well then, as a "fast-forward
only" way of updating (think "git merge --ff-only", but in a less
illogical way, as "--ff-only" actually means "don't create a merge",
which is kinda weird, at least to me).

As I said, I don't really agree on the idea of not creating any branch
heads on "clone", but maybe it's because I'm not a "don't commit, just
watch" person. And the theoretical "git up" command might be handy for
guys that just want to follow things, and thus don't really need branch
heads. At the moment, I don't have any intentions to improve the hack
(also due to lack of time), but if it seems worthwhile to anyone, feel
free to pick it up.

Björn, -ENOPATCH ;-)

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

[Thomas Rast]

Johannes Schindelin wrote:
> On Tue, 6 Oct 2009, Euguess@gmail.com wrote:
> > in case if you didn't do that and you try to checkout you will end up 
> > having detached HEAD which is quite scary;) for non-experienced user and 
> > as i see might lead to some unnecessary questions in this list or on IRC 
> > channel...
[...]
> One thing one might add for the technically inclined folks (i.e. those who 
> need to implement, and to see that Git is in dear need of some 
> user-friendliness first): "git checkout" is a porcelain (i.e. a program 
> meant for end-user consumption), and as such should not have a problem to 
> react to isatty(0) (i.e. "is the input coming directly from the 
> console?").

Sadly git-checkout seems to be stuck between being declared a
porcelain, but at the same time being an extremely important command
for scripts all over.  (There are probably others in the same place:
reset comes to mind.)

Your idea is also a backwards incompatible change, so we can just as
well implement the original suggestion and force scripts (or us) to
use some other means when they want to detach.  Say, why not just
invent an option along the lines of

  git checkout {-d|--detach} $ref

to make it explicit.  We have to resort to more arcane means to
*reliably* detach anyway, like 'git checkout master^0'.  Then in some
future release, git-checkout will start making DWIM branches if the -d
is not given.

And while we're there, --attach would be a nice complement to force
refs/heads/foo to attach.

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

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

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Your idea is also a backwards incompatible change, so we can just as
> well implement the original suggestion and force scripts (or us) to
> use some other means when they want to detach.  Say, why not just
> invent an option along the lines of
>
>   git checkout {-d|--detach} $ref
>
> to make it explicit.

Or can't you go the other way, say

	git checkout -t $remote_tracking

to create a local branch forking from the named remote tracking branch?

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

[Thomas Rast]

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > Your idea is also a backwards incompatible change, so we can just as
> > well implement the original suggestion and force scripts (or us) to
> > use some other means when they want to detach.  Say, why not just
> > invent an option along the lines of
> >
> >   git checkout {-d|--detach} $ref
> >
> > to make it explicit.
> 
> Or can't you go the other way, say
> 
> 	git checkout -t $remote_tracking
> 
> to create a local branch forking from the named remote tracking branch?

Sure, but we already have that and we still failed to fix the users,
so FWIW, I think Dscho's right and we should try fixing the UI next.

[I've also seen several users shoot themselves with detached HEADs to
the point where I explain the concept before even mentioning
checkout.]

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

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

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

>> Or can't you go the other way, say
>> 
>> 	git checkout -t $remote_tracking
>> 
>> to create a local branch forking from the named remote tracking branch?
>
> Sure, but we already have that and we still failed to fix the users,
> so FWIW, I think Dscho's right and we should try fixing the UI next.

What it means is that -t was a broken attempt to help the users at the UI
level, and I can surely see that.

So we need the set of new rules, say, for 1.7.0 release.  A strawman?

Assume that these are the only refs that exist:

    refs/remotes/origin/{master,next,nitfol}
    refs/remotes/xyzzy/{frotz,nitfol}
    refs/heads/master
    refs/tags/v1.0.0

#0. These will stay as is:

 $ git checkout mine               ;# switches to the branch
 $ git checkout $any_committish^0  ;# detaches

#1. These used to detach, but will create a local branch

 $ git checkout origin/next        ;# as if with -t
 $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

#2. These are allowed only when unambiguous and there is no local branch yet.

 $ git checkout next               ;# ok
 $ git checkout frotz              ;# ok (origin is not special)
 $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)

#3. These used to detach, but what should we do?

 $ git checkout v1.0.0             ;# detach, or refuse???
 $ git checkout origin/master      ;# detach, or refuse???

I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
started refusing to detach in case #3, as people who want to detach can
always suffix ^0 or ~0 to make it a general committish.

Did I cover all cases?

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> So we need the set of new rules, say, for 1.7.0 release.  A strawman?
>
> Assume that these are the only refs that exist:
>
>     refs/remotes/origin/{master,next,nitfol}
>     refs/remotes/xyzzy/{frotz,nitfol}
>     refs/heads/master

Sorry, I had this as refs/heads/{master,mine} in my initial draft but
removed the 'mine' branch by mistake; the first item in #0 does not make
sense without it.

>     refs/tags/v1.0.0
>
> #0. These will stay as is:
>
>  $ git checkout mine               ;# switches to the branch
>  $ git checkout $any_committish^0  ;# detaches
>
> #1. These used to detach, but will create a local branch
>
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
>
> #2. These are allowed only when unambiguous and there is no local branch yet.
>
>  $ git checkout next               ;# ok
>  $ git checkout frotz              ;# ok (origin is not special)
>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)
>
> #3. These used to detach, but what should we do?
>
>  $ git checkout v1.0.0             ;# detach, or refuse???
>  $ git checkout origin/master      ;# detach, or refuse???
>
> I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
> started refusing to detach in case #3, as people who want to detach can
> always suffix ^0 or ~0 to make it a general committish.
>
> Did I cover all cases?

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

[Thomas Rast]

Junio C Hamano wrote:
> 
> What it means is that -t was a broken attempt to help the users at the UI
> level, and I can surely see that.
> 
> So we need the set of new rules, say, for 1.7.0 release.  A strawman?

I feel somewhat uneasy commenting on this because I have a history of
writing just-barely-workable UIs.  That being said:

> Assume that these are the only refs that exist:
> 
>     refs/remotes/origin/{master,next,nitfol}
>     refs/remotes/xyzzy/{frotz,nitfol}
>     refs/heads/master
>     refs/tags/v1.0.0
> 
> #0. These will stay as is:
> 
>  $ git checkout mine               ;# switches to the branch
>  $ git checkout $any_committish^0  ;# detaches
> 
> #1. These used to detach, but will create a local branch
> 
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

Agreed, though I'm still in favour of a cleaner syntax for explicit
detaching.  (Cleaner in the sense that ^0 is documented as having a
completely different purpose and only works by accident.)

> #2. These are allowed only when unambiguous and there is no local branch yet.
> 
>  $ git checkout next               ;# ok
>  $ git checkout frotz              ;# ok (origin is not special)
>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)

I'm weakly leaning towards refusing all three, as the user should be
required to explicitly say a remote branch should be involved.

(Weakly because there's also a certain DWIM advantage to 'git checkout
sometopic'...)

> #3. These used to detach, but what should we do?
> 
>  $ git checkout v1.0.0             ;# detach, or refuse???

Refuse, on the grounds that the main goal here is not detaching unless
specifically told to.  (Having a branch called v1.0.0 is worse, as it
would just cause a lot of confusion and/or a refusal at the next
checkout.)

>  $ git checkout origin/master      ;# detach, or refuse???

This seems to be the trickiest of them.  Maybe check out 'master', to
make the process repeatable.  Imagine, in your setting,

  git checkout origin/next           ;# creates 'next' as with -t
  git checkout -                     ;# back
  git checkout origin/next           ;# should go to 'next' again

Then again, that would trade the confusion of detaching for the
confusion of not checking out the exact commit that the user
specified.  Worse, 'next' could conceivably be tracking (as per
branch.next.merge) some entirely different branch, making the "Your
branch is behind..." message misleading.

> I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
> started refusing to detach in case #3, as people who want to detach can
> always suffix ^0 or ~0 to make it a general committish.
> 
> Did I cover all cases?

Some that come to mind:

#3a. Other refs apart from tags that currently detach:

  git fetch origin master            ;# or even sillier, 'git fetch . master'
  git checkout FETCH_HEAD            ;# used to detach; refuse?

#3b. Full specifiers that currently detach:

  git checkout refs/heads/master     ;# could eventually attach
  git checkout heads/master          ;# same

#0a. Should probably detach if the previous checkout was detached:

  git checkout -                     ;# detach if previous was detached?
  git checkout @{-1}                 ;# same

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

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

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

>> #0. These will stay as is:
>> 
>>  $ git checkout mine               ;# switches to the branch
>>  $ git checkout $any_committish^0  ;# detaches
>> 
>> #1. These used to detach, but will create a local branch
>> 
>>  $ git checkout origin/next        ;# as if with -t
>>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
>
> Agreed, though I'm still in favour of a cleaner syntax for explicit
> detaching.  (Cleaner in the sense that ^0 is documented as having a
> completely different purpose and only works by accident.)

Oh, ^0 was just one way to make sure a committish is not a refname.  If
you have an abbreviated hexadecimal commit object name, that would also
detach, which should fall into category #0.  Sorry for the omission.

>> #2. These are allowed only when unambiguous and there is no local branch yet.
>> 
>>  $ git checkout next               ;# ok
>>  $ git checkout frotz              ;# ok (origin is not special)
>>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)
>
> I'm weakly leaning towards refusing all three, as the user should be
> required to explicitly say a remote branch should be involved.
>
> (Weakly because there's also a certain DWIM advantage to 'git checkout
> sometopic'...)

I thought this was the primary point of what Dscho has been advocating.

>> #3. These used to detach, but what should we do?
>> 
>>  $ git checkout v1.0.0             ;# detach, or refuse???
>
> Refuse, on the grounds that the main goal here is not detaching unless
> specifically told to.  (Having a branch called v1.0.0 is worse, as it
> would just cause a lot of confusion and/or a refusal at the next
> checkout.)
>
>>  $ git checkout origin/master      ;# detach, or refuse???
>
> This seems to be the trickiest of them.  Maybe check out 'master', to
> make the process repeatable.  Imagine, in your setting,
>
>   git checkout origin/next           ;# creates 'next' as with -t
>   git checkout -                     ;# back
>   git checkout origin/next           ;# should go to 'next' again
>
> Then again, that would trade the confusion of detaching for the
> confusion of not checking out the exact commit that the user
> specified.  Worse, 'next' could conceivably be tracking (as per
> branch.next.merge) some entirely different branch, making the "Your
> branch is behind..." message misleading.

As I said already in the thread, I think that is a misguided attempt to
half-hide the fact that there are origin/next (tracking branch) and next
(a fork of it), that are two separate entities.  It is misguided because
the user needs to understand and take advantage of the distinction to do
anything; in other words, it is not even an unnecessary complexity.

So I am very doubtful about the benefit of checking out 'master' when
the user explicitly tells us to check out 'origin/master', only because
the former forked from the latter.

> Some that come to mind:
>
> #3a. Other refs apart from tags that currently detach:
>
>   git fetch origin master            ;# or even sillier, 'git fetch . master'
>   git checkout FETCH_HEAD            ;# used to detach; refuse?

> #3b. Full specifiers that currently detach:
>
>   git checkout refs/heads/master     ;# could eventually attach
>   git checkout heads/master          ;# same

I'd throw both of these into category #3.

Anything that is valid "ref" (i.e. what dwim_ref() groks) that is not
a remote tracking branch (which creates a corresponding local branch)
can refuse to avoid unintended detachment by newbies.

> #0a. Should probably detach if the previous checkout was detached:
>
>   git checkout -                     ;# detach if previous was detached?
>   git checkout @{-1}                 ;# same

Perhaps.

So to recap, "git checkout $token" would:

 * If dwim_ref() groks $token, and

   - if it resolves to refs/heads/*, that is checking out a local branch;

   - if it resolves to refs/remotes/*, and if there is no corresponding
     local branch, create one forked from there, as if -t was given;

   - everything else we used to detach, but we refuse in 1.7.0, to make it
     harder for newbies to detach.

 * If check_ref_format() is happy with $token, get_sha1() does not grok
   $token, and there is only one ref of the form refs/remotes/$o/$token
   then we pretend as if -t $o/$token was given and create a local branch
   $token forked from it.

 * Otherwise, we always detach.

Note that "checkout -" and "checkout @{-4}" are part of dwim_ref() family.

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

[Johannes Schindelin]

Hi,

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

> Thomas Rast <trast@student.ethz.ch> writes:
> 
> [this was probably quoted from Junio, Dscho doesn't have time to go back 
>  and check, but then, this was not specified in the quoted mail]
>
> >> #2. These are allowed only when unambiguous and there is no local branch yet.
> >> 
> >>  $ git checkout next               ;# ok
> >>  $ git checkout frotz              ;# ok (origin is not special)
> >>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)
> >
> > I'm weakly leaning towards refusing all three, as the user should be
> > required to explicitly say a remote branch should be involved.
> >
> > (Weakly because there's also a certain DWIM advantage to 'git checkout
> > sometopic'...)
> 
> I thought this was the primary point of what Dscho has been advocating.

To be honest, I was not advocating anything except being more open to 
users' problems, because we _did_ grow a large user base, way beyond the 
Linux developers (whom we can always harrass and tell to RTFM).

Just to re-add my well-known stance: consistency is a good thing.  So if 
things are ambiguous, we can be consistent in saying so and refusing to 
DWIM.  And if things are _not_ ambiguous, we can be consistent in just 
DWIMming what the user most probably meant.

If the user just typed random things in the hope that it works, we cannot 
do anything about it anyway.

So in my opinion, we should DWIM "git checkout $X" to mean "git checkout 
-b $X refs/remotes/$REMOTE/$X" when there is no ref $X, refs/heads/$X and 
no other refs/remotes/$OTHER/$X.

Likewise "git checkout $REMOTE/$X".

But, in my opinion, if there is refs/heads/$X and refs/remotes/origin/$X, 
and the user says "git checkout origin/$X", we should tell the user that 
there are the options to checkout $X and origin/$X^0 (the latter only if 
the user really intended to detach her HEAD), but not try to DWIM 
anything.

IMHO it is obvious that Hannes' suggestion to fast-forward $X and check it 
out in said scenario has some benefits in certain situations, but dramatic 
downsides in others.

But I need to drive some very important point home in this thread: 1.7.0 
was announced to break some old-time habits in favor of a better 
user-interface.  We _need_ to use this opportunity fully.

Even if that means that a few fingers have to be retrained.  Because 
retraining a few for the benefit of an easier time with the many others 
is Just Worth It.

Or in other words: logic clearly dictates that the needs of the many 
outweigh the needs of the few.

Ciao,
Dscho

P.S.: In case certain persons, ahem, think that I am applying the "Many 
Outweigh Few" principle to the time involved in top-posting and 
"forgetting" to cut quoted text to what is actually addressed: yes, you 
could not be more correct.  And I no longer believe that this goes without 
saying.

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> So in my opinion, we should DWIM "git checkout $X" to mean "git checkout 
> -b $X refs/remotes/$REMOTE/$X" when there is no ref $X, refs/heads/$X and 
> no other refs/remotes/$OTHER/$X.
>
> Likewise "git checkout $REMOTE/$X".
>
> But, in my opinion, if there is refs/heads/$X and refs/remotes/origin/$X, 
> and the user says "git checkout origin/$X", we should tell the user that 
> there are the options to checkout $X and origin/$X^0 (the latter only if 
> the user really intended to detach her HEAD), but not try to DWIM 
> anything.

I am somewhat unhappy with that kind of inconsistent DWIMery.

Naively I would agree that it would be nice if "git checkout origin/next"
(or "next" when no other remotes/*/next exists) were DWIMmed as "git
checkout -t -b next origin/next".  But the way we _define_ that particular
DWIMmery and the way it appears to an uninitiated would be different.

We define this DWIMmery as s|^(.*)/([^/]*)$|-t -b $2 $1/$2|; iow, when the
user types "origin/next" and other coniditions hold, we pretend as if the
user typed "-b next origin/next".  But it would give an incorrect
impression to an end user "Ah, when my upstream project has next branch, I
can check it out with origin/next (or next)."  But when the user wants to
work further on 'next' by running "git checkout origin/next" the next day,
we say "Uh oh, that is ambiguous and we won't DWIM," which is technically
and implementation wise correct, but breaks the misconception the user
formed with your earlier DWIMmery.  I suspect that the user will be better
off if we do not give a wrong impression in the first place.  If any
DWIMmery gave a conception different from the following four points, that
DWIMmery is actively hurting the users:

 * You clone and get copies of where the other end has its branches;

 * You do all your work on your local branches;

 * You may incorporate what the other end further did by merging from the
   tracking branch from it;

 * You update the other end by pushing what you did on your local branches.

Now, the conclusion of the above embodied in the _current_ UI is:

 * To start your branch to build on what the other end did, you fork your
   local branch at the commit the other end left off, and make sure it builds
   on that tracking branch, with

        git checkout -t -b next origin/next

 * Since "-t -b $2 $1/$2" often appears as a pattern, you can say "-t $1/$2"
   and we DWIM as if you said "-t -b $2 $1/$2".

I do not think loosening the DWIMmery so that "$1/$2" is DWIMmed to the
above would help users.  If the current DWIM is not helping the users
understand the first four points and instead encouraging an incorrect
picture of how the world works, the new DWIMmery would be just as bad, if
not worse.

> IMHO it is obvious that Hannes' suggestion to fast-forward $X and check it 
> out in said scenario has some benefits in certain situations, but dramatic 
> downsides in others.

Yes.

> But I need to drive some very important point home in this thread: 1.7.0 
> was announced to break some old-time habits in favor of a better 
> user-interface.  We _need_ to use this opportunity fully.
>
> Even if that means that a few fingers have to be retrained.  Because 
> retraining a few for the benefit of an easier time with the many others 
> is Just Worth It.

Absolutely.  My point is that this particular DWIMmery would _NOT_ be a
better user interface.  Not for 1.7.0, not for any other release.  It
would not help the users to form a clear world model git offers and that
actively hurts them.

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

[Jeff King]

On Tue, Oct 13, 2009 at 11:20:28PM +0200, Johannes Schindelin wrote:

> So in my opinion, we should DWIM "git checkout $X" to mean "git checkout 
> -b $X refs/remotes/$REMOTE/$X" when there is no ref $X, refs/heads/$X and 
> no other refs/remotes/$OTHER/$X.

The similar suggestion that is less magical is to say something like
"there is no $X; maybe you meant $REMOTE/$X?".  Is there a reason not to
phase in the behavior, to make sure it is not doing unexpected things?
In other words:

  1. In v1.6.6, find all error-correcting candidates and print them as
     a suggestion (similar to what we do with "git foo").

  2. Then, if we all agree that it seems to be producing sane results,
     the next step is to turn the unambiguous cases into a DWIM (and
     leave the ambiguous ones with the "did you mean?" message).

Because right now I think there are a lot of hypothetical "maybe it
would be less convenient or more confusing in this instance", but we
don't have any data on how often those instances occur, or how actual
users might react. So doing step (1) would be a way of collecting some
of that data (will users say "stupid git, if you knew what I wanted, why
didn't you just do it?" or "stupid git, your suggestion is just
confusing me!").

-Peff

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

[Johannes Schindelin]

Hi,

On Tue, 13 Oct 2009, Jeff King wrote:

> On Tue, Oct 13, 2009 at 11:20:28PM +0200, Johannes Schindelin wrote:
> 
> > So in my opinion, we should DWIM "git checkout $X" to mean "git checkout 
> > -b $X refs/remotes/$REMOTE/$X" when there is no ref $X, refs/heads/$X and 
> > no other refs/remotes/$OTHER/$X.
> 
> The similar suggestion that is less magical is to say something like
> "there is no $X; maybe you meant $REMOTE/$X?".

At some point, trying to educate the user is not helpful but annoying.  If 
Git already knows what I want, why does it not do it already?  _That_ is 
the question I already hear in my ears.

> Is there a reason not to phase in the behavior, to make sure it is not 
> doing unexpected things?

Sure, I have nothing against that.  But just insisting on the current 
behavior, or on some behavior that is not helpful at all, well, is not 
really clever.

Note that I am fully aware that my "git checkout -t origin/master" DWIMery 
backfired quite badly.  So I am in the same boat.

> In other words:
> 
>   1. In v1.6.6, find all error-correcting candidates and print them as
>      a suggestion (similar to what we do with "git foo").
> 
>   2. Then, if we all agree that it seems to be producing sane results,
>      the next step is to turn the unambiguous cases into a DWIM (and
>      leave the ambiguous ones with the "did you mean?" message).
> 
> Because right now I think there are a lot of hypothetical "maybe it
> would be less convenient or more confusing in this instance", but we
> don't have any data on how often those instances occur, or how actual
> users might react.

Oh, I do not want to spam the list with user experiences.  But I do have 
not only a faint idea how users react.  Thankyouverymuch.

> So doing step (1) would be a way of collecting some of that data (will 
> users say "stupid git, if you knew what I wanted, why didn't you just do 
> it?" or "stupid git, your suggestion is just confusing me!").

I disagree.  It is not about collecting data.  We will not get any 
feedback from the affected people.  You know that, I know that.

The step (1) would help in the way that it is a smoother transition.

Ciao,
Dscho

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

[Jay Soffian]

On Tue, Oct 13, 2009 at 7:22 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> At some point, trying to educate the user is not helpful but annoying.  If
> Git already knows what I want, why does it not do it already?  _That_ is
> the question I already hear in my ears.

Modify checkout so that the first commit while detached automatically
creates a branch. Perhaps the name is derived from the branch point,
or the user is prompted for a name.

This doesn't help with the original problem, which was that a user
attempted to checkout refs/remotes/origin/<name> by just saying 'git
checkout <name>' which I happen to think should work. A lot of what I
keep hearing in this thread seems to be in the vein of the perfect
being the enemy of the good.

That rambled a bit. Sorry.

j.

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

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> This doesn't help with the original problem, which was that a user
> attempted to checkout refs/remotes/origin/<name> by just saying 'git
> checkout <name>' which I happen to think should work. A lot of what I
> keep hearing in this thread seems to be in the vein of the perfect
> being the enemy of the good.

I do not think there is "perfect" nor "good" anywhere in this.  It is just
the proposals were either not well thought out, were not presented well,
or were misunderstood, or a bit of all.

When you do not have local "frotz" branch, and do have cloned/fetched from
the origin that has "frotz" branch, I am actually Ok with this

    $ git checkout frotz [--]

to do an equivalent of:

    $ git checkout -t -b frotz origin/frotz

I do not have problem with this _particular_ DWIMmery.  It will not break
people's expectations, other than "asking to check out non-existing ref
should fail".  That expectation might be logical, but I do not think it is
useful.

Another reason I won't have problem with this one is that perhaps after
creating a few more commits, the next day when the user does the same

    $ git checkout frotz

what will be shown is the _local_ frotz branch.  Nowhere in this sequence
there is any room to mistake that you somehow checked out a branch owned
by somebody else (namely, origin).  You started by auto-creating your
local branch, worked on it, and checked it out again the next day.  In
other words, this is really about a shorthand to create a new local branch
called "frotz" when the commit that the branch should start from is
clearly unambiguous.

I have trouble with yours, on the other hand, which is to make

    $ git checkout origin/frotz
    $ git checkout v1.5.5

into

    $ git checkout -b frotz-47 origin/frotz
    $ git checkout -b v1.5.5-47 v1.5.5

(replace -47 with whatever random string you would come up with to make it
unique), as it _will_ break people's expectations, and the expected
behaviour to detach without polluting the local branch namespace for
the purpose of sightseeing happens to be a useful one.

I also have issues with turning

    $ git checkout origin/frotz

into

    $ git checkout -b frotz origin/frotz

only when frotz does not exist locally.  This will cause the "next day"
problem, and also by naming the remote tracking branch, gives a wrong
impression that this is about a remote branch.  It should not be.

Perhaps without touching the "detached" case at all, if we limit the scope
of the change that comes out of this discussion to only one case, it might
result in a good trouble-free enhancement [*1*].

The new rule would be:

    "git checkout $name", when all of the following holds:

    - $name is a good name for a local branch (i.e. check-ref-format is
      happy);

    - No local branch of that name exists;

    - There is exactly one remote $remote that has $name branch; and

    - $name itself is not a good commit name (i.e. get_sha1() barfs)

    is a request to create a local branch $name, and the branch tracks the
    remote tracking branch found in the third condition [*2*].

The important point here is that this exception is _not_ about remote
tracking branch but is about a rule to allow omitting -b to create and
checkout a local branch when the user's intent is clear that (1) he wants
to create a new one named $name, and (2) he wants to create it starting at
the commit $remote/$name.

Such a change feels quite safe and I wouldn't be opposed to it.

We _could_ discuss extending the $name in the above rule to other kinds
(tags and even arbitrary committish that may not even have a direct ref
pointing at it), but I think they are much more problematic.

[Footnote]

*1* Yes, I know I won't try to come with a strawman.

*2* The fourth condition is to avoid taking "origin/frotz" when "origin"
remote has "frotz" branch _and_ "other" remote has "origin/frotz" branch.

The remote chosen by the third condition would be "other" (because
"origin" remote only has "frotz", and not "origin/frotz", the name is
unique in the sense of the third condition).  The fourth condition
prevents this from happening, and forbids an explicit request to detach
HEAD at one point (i.e. "origin/frotz") from triggering.

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

[Jay Soffian]

On Tue, Oct 13, 2009 at 11:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> When you do not have local "frotz" branch, and do have cloned/fetched from
> the origin that has "frotz" branch, I am actually Ok with this
>
>    $ git checkout frotz [--]
>
> to do an equivalent of:
>
>    $ git checkout -t -b frotz origin/frotz
>
> I do not have problem with this _particular_ DWIMmery.  It will not break
> people's expectations, other than "asking to check out non-existing ref
> should fail".  That expectation might be logical, but I do not think it is
> useful.
>
> Another reason I won't have problem with this one is that perhaps after
> creating a few more commits, the next day when the user does the same
>
>    $ git checkout frotz
>
> what will be shown is the _local_ frotz branch.  Nowhere in this sequence
> there is any room to mistake that you somehow checked out a branch owned
> by somebody else (namely, origin).  You started by auto-creating your
> local branch, worked on it, and checked it out again the next day.  In
> other words, this is really about a shorthand to create a new local branch
> called "frotz" when the commit that the branch should start from is
> clearly unambiguous.

Okay, this is good, and I can work up a patch if no one beats me to the punch.

> I have trouble with yours, on the other hand, which is to make
>
>    $ git checkout origin/frotz
>    $ git checkout v1.5.5
>
> into
>
>    $ git checkout -b frotz-47 origin/frotz
>    $ git checkout -b v1.5.5-47 v1.5.5

I suggested no such thing, at least, I don't think I did. What I said was:

---snip---
Modify checkout so that the first commit while detached automatically
creates a branch. Perhaps the name is derived from the branch point,
or the user is prompted for a name.
---snip---

So we'd only automatically create a new branch at commit time. But
never mind that, it was just a suggestion and I don't like it.

What if instead we do something like this:

$ git checkout v1.5.5
Note: moving to 'v1.5.5' which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
  git checkout -b <new_branch_name>
HEAD is now at 1d2375d... GIT 1.5.5
$ [edit foo.c]
$ git add foo.c
$ git commit -m "edited some file"
Cannot commit to v1.5.5. Please use git commit -b <branch> to specify
the name of a new branch to commit to, or use git commit --detach to
force a detached commit.

So we modify git to, by default, no longer allow creating a commit
while detached or on a branch that cannot be committed to.

j.

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

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> What if instead we do something like this:
>
> $ git checkout v1.5.5
> Note: moving to 'v1.5.5' which isn't a local branch
> If you want to create a new branch from this checkout, you may do so
> (now or later) by using -b with the checkout command again. Example:
>   git checkout -b <new_branch_name>
> HEAD is now at 1d2375d... GIT 1.5.5
> $ [edit foo.c]
> $ git add foo.c
> $ git commit -m "edited some file"
> Cannot commit to v1.5.5. Please use git commit -b <branch> to specify
> the name of a new branch to commit to, or use git commit --detach to
> force a detached commit.
>
> So we modify git to, by default, no longer allow creating a commit
> while detached or on a branch that cannot be committed to.

I'd probably object to such a change if there is no easy way to turn it
off per session (that means "expert mode" configuration variable, or a
command line option per "git commit" invocation, are not a viable escape
hatch), as I do it all the time, while reworking on an existing series.
E.g.

    $ git checkout $(git merge-base master topic)
    $ work on redoing topic
      - cherry-picking parts from topic~$n
      - editing
      - committing
    $ git show-branch HEAD topic
    $ git branch -f topic

is a very common sequence of how I personally work, at day-job and also
while maintaining git itself.

I probably would not mind such a change if I can say "I am detaching now
in order to build on a non-branch.  Do not bother me with unwarranted and
misguided helpfulness until I am done" once upfront when I perform the
first checkout.

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

[Uri Okrent]

Junio C Hamano wrote:
  > When you do not have local "frotz" branch, and do have cloned/fetched from
> the origin that has "frotz" branch, I am actually Ok with this
> 
>     $ git checkout frotz [--]
> 
> to do an equivalent of:
> 
>     $ git checkout -t -b frotz origin/frotz
> 
> I do not have problem with this _particular_ DWIMmery.  It will not break
> people's expectations, other than "asking to check out non-existing ref
> should fail".  That expectation might be logical, but I do not think it is
> useful.

FWIW most of the people we train (and we do spend time explaining the
distinction between origin/foo and foo) still find it annoying to have
to create local branches that track branches from the cloned repo.  I
think this probably has something to do with the fact that master is
automatically checked out when you clone--people expect other upstream
branches to just 'exist' locally in a similar fashion.

I think the above suggestion is simple enough and would provide the most
bang-for-your-buck in terms of usability.
-- 
    Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/

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

[Jeff King]

On Wed, Oct 14, 2009 at 01:22:26AM +0200, Johannes Schindelin wrote:

> At some point, trying to educate the user is not helpful but annoying.  If 
> Git already knows what I want, why does it not do it already?  _That_ is 
> the question I already hear in my ears.

I am not entirely convinced that the suggested behaviors will result in
that user response, or a different one (like "why does git keep giving
me bad advice?"). Which is why I suggested data collection.

> > So doing step (1) would be a way of collecting some of that data (will 
> > users say "stupid git, if you knew what I wanted, why didn't you just do 
> > it?" or "stupid git, your suggestion is just confusing me!").
> 
> I disagree.  It is not about collecting data.  We will not get any 
> feedback from the affected people.  You know that, I know that.

I don't agree. You are already talking about users complaining about
git's interface. Isn't that feedback? How do you hear those complaints
now?

I don't think they will come on the list and talk about it, but if we
release a version of git that has differing behavior and give it some
time to be used in the wild, we _will_ get feedback in the form of
blogs, complaints on other lists, word-of-mouth, etc.

Now maybe that is not a good idea in this instance, because that sort of
feedback may take several versions to appear, and we are talking about a
potential timetable of v1.7.0, which is probalby only two versions away.

-Peff

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

[Thomas Rast]

Thomas Rast wrote:
> Junio C Hamano wrote:
> > 
> > #1. These used to detach, but will create a local branch
> > 
> >  $ git checkout origin/next        ;# as if with -t
> >  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
> 
> Agreed, though I'm still in favour of a cleaner syntax for explicit
> detaching.  (Cleaner in the sense that ^0 is documented as having a
> completely different purpose and only works by accident.)

Not sure if it's too late in the thread, but after sleeping over it
and re-reading (and the other developments in the thread) I'm not
happy with my earlier opinion any more.  I think the DWIM part of it
is a bad idea because of this:

> >  $ git checkout origin/master      ;# detach, or refuse???
> 
> This seems to be the trickiest of them.  Maybe check out 'master', to
> make the process repeatable.  Imagine, in your setting,
> 
>   git checkout origin/next           ;# creates 'next' as with -t
>   git checkout -                     ;# back
>   git checkout origin/next           ;# should go to 'next' again
> 
> Then again, that would trade the confusion of detaching for the
> confusion of not checking out the exact commit that the user
> specified.  Worse, 'next' could conceivably be tracking (as per
> branch.next.merge) some entirely different branch, making the "Your
> branch is behind..." message misleading.

So I think we're now mixing up two different goals in this thread:
a) Stopping the users from hurting themselves by inadvertent detaching
b) Helping the users by DWIMming local branches for them

I'm all for (a), but (b) is much harder.

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

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

[Jakub Narebski]

Thomas Rast <trast@student.ethz.ch> writes:

> So I think we're now mixing up two different goals in this thread:
> a) Stopping the users from hurting themselves by inadvertent detaching
> b) Helping the users by DWIMming local branches for them
> 
> I'm all for (a), but (b) is much harder.

Perhaps (b) should be protected by branch.autocreatelocal (similar to
branch.autosetupmerge and branch.autosetuprebase).

Also we should always print a message if we DWIM creating or checking
out local branch equivalent to remote-tracking branch.


Also, why interactive checkout (checkout --interactive?) idea was
abandoned?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

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

[Johannes Sixt]

Junio C Hamano schrieb:
> #1. These used to detach, but will create a local branch
> 
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)

If I did 'git checkout origin/next' last week, I will already have a
branch next. What should happen if I do it again today?

I think that it should DWIM: If last week's next fast-fowards to this
week's origin/next (*and* next is the branch that tracks origin/next),
then the fast foward should happen. Otherwise 'git checkout origin/next'
should fail.

This way, if I built on last week's next, I will be notified; but if I
only want to browse history, then I won't be impeded by the existence of next.

-- Hannes

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

[Daniel Barkalow]

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

> Thomas Rast <trast@student.ethz.ch> writes:
> 
> >> Or can't you go the other way, say
> >> 
> >> 	git checkout -t $remote_tracking
> >> 
> >> to create a local branch forking from the named remote tracking branch?
> >
> > Sure, but we already have that and we still failed to fix the users,
> > so FWIW, I think Dscho's right and we should try fixing the UI next.
> 
> What it means is that -t was a broken attempt to help the users at the UI
> level, and I can surely see that.
> 
> So we need the set of new rules, say, for 1.7.0 release.  A strawman?
> 
> Assume that these are the only refs that exist:
> 
>     refs/remotes/origin/{master,next,nitfol}
>     refs/remotes/xyzzy/{frotz,nitfol}
>     refs/heads/master
>     refs/tags/v1.0.0
> 
> #0. These will stay as is:
> 
>  $ git checkout mine               ;# switches to the branch
>  $ git checkout $any_committish^0  ;# detaches
> 
> #1. These used to detach, but will create a local branch
> 
>  $ git checkout origin/next        ;# as if with -t
>  $ git checkout xyzzy/frotz        ;# as if with -t (origin is not special)
>
> #2. These are allowed only when unambiguous and there is no local branch yet.
> 
>  $ git checkout next               ;# ok
>  $ git checkout frotz              ;# ok (origin is not special)
>  $ git checkout nitfol             ;# not ok (ambiguous and origin is not special)
> 
> #3. These used to detach, but what should we do?
> 
>  $ git checkout v1.0.0             ;# detach, or refuse???
>  $ git checkout origin/master      ;# detach, or refuse???
> 
> I can buy 0, 1, and 2, and I think it is a minor inconvenience if we
> started refusing to detach in case #3, as people who want to detach can
> always suffix ^0 or ~0 to make it a general committish.

I suspect that a very common pattern for people who follow trees for 
testing and such or who only develop in topic branches is:

$ git clone ...
$ git checkout origin/next
$ git fetch origin
$ git checkout origin/next

For people who use topic branches extensively:

$ git fetch origin
$ git checkout origin/next
(test, find issues, maybe make changes)
$ git checkout -b topic
$ git commit
(send changes)

Some people (IIRC, including Linus):

$ git checkout origin/next
(work)
$ git commit
$ git checkout -b topic

In all of these cases, the user will get a misleading "next" local branch; 
in Linus's case, this branch ends up with commits from a topic branch.

For that matter, even the intended user would have problems with your 
suggestion:

$ git clone ...

$ git checkout origin/next
(do some next stuff)
$ git checkout origin/master
(do some master stuff)
$ git checkout origin/next

On the second cycle, either git refuses or does something actively 
confusing to this user, and the user has to learn the difference between 
local branches and remote branches on the *second* cycle. IMHO, it's much 
better to make users learn things at the point when they don't think they 
know how to use the system, rather than when they think they understand it 
and are just trying to get things done.

	-Daniel
*This .sig left intentionally blank*

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

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> I suspect that a very common pattern for people who follow trees for 
> testing and such or who only develop in topic branches is:
> ...
> << many issues with this kind of DWIM omitted >>
> ...
> On the second cycle, either git refuses or does something actively 
> confusing to this user, and the user has to learn the difference between 
> local branches and remote branches on the *second* cycle. IMHO, it's much 
> better to make users learn things at the point when they don't think they 
> know how to use the system, rather than when they think they understand it 
> and are just trying to get things done.

Yeah, and I think J6t pointed out the same issue.

I think it tells us something, after some of "the most trusted Git
contributors" thought "really long and hard, and making sure to take
user-friendliness into account at least as much as simplicity of
implementation", they are getting to the same conclusion that this
particular DWIMery is a misguided attempt to be helpful without really
helping but rather hurting the users.

I will stop trying to come up with a strawman for other people's itch that
I do not agree to begin with, at least for now.  I will still look at
concrete and workable proposals from other people, though.

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

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > I suspect that a very common pattern for people who follow trees for 
> > testing and such or who only develop in topic branches is:
> > ...
> > << many issues with this kind of DWIM omitted >>
> > ...
> > On the second cycle, either git refuses or does something actively 
> > confusing to this user, and the user has to learn the difference between 
> > local branches and remote branches on the *second* cycle. IMHO, it's much 
> > better to make users learn things at the point when they don't think they 
> > know how to use the system, rather than when they think they understand it 
> > and are just trying to get things done.
> 
> Yeah, and I think J6t pointed out the same issue.
> 
> I think it tells us something, after some of "the most trusted Git
> contributors" thought "really long and hard, and making sure to take
> user-friendliness into account at least as much as simplicity of
> implementation", they are getting to the same conclusion that this
> particular DWIMery is a misguided attempt to be helpful without really
> helping but rather hurting the users.
> 
> I will stop trying to come up with a strawman for other people's itch that
> I do not agree to begin with, at least for now.  I will still look at
> concrete and workable proposals from other people, though.

I personally think that the real issue is that our "detached HEAD" message 
is still too scary, and what we really want is to issue the scary message 
when using "git commit" to move a detached HEAD from what was checked out 
to a new commit. So:

$ git checkout origin/next
(friendly message telling you you're browsing history)
$ git commit
(scary message telling you you're not on any branch)
$ git commit
(one line message like usual, except "detached HEAD" instead of branch 
name)

This still makes sure that you get the scary message before you could lose 
track of your work, but only gives it to you at the point where there's a 
commit that's in your HEAD and nowhere else.

The other thing that I think would be nice is:

$ git checkout origin/next
$ git fetch origin
$ git checkout !! (probably not a good syntax)

That is, expand "!!" to the string used to detach HEAD, and expand it 
again now. (Of course, something would have to be done if you did "git 
checkout HEAD^1" before, or "git checkout !!^1".) This is related in that 
I think the scary message should happen when "git commit" sees this stored 
string and clears it.

	-Daniel
*This .sig left intentionally blank*

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

[Jeff King]

On Tue, Oct 13, 2009 at 05:31:46PM -0400, Daniel Barkalow wrote:

> I personally think that the real issue is that our "detached HEAD" message 
> is still too scary, and what we really want is to issue the scary message 
> when using "git commit" to move a detached HEAD from what was checked out 
> to a new commit. So:

This has been discussed before (I happen to agree with you, but you
probably want to address other comments in the thread):

  http://thread.gmane.org/gmane.comp.version-control.git/38201/focus=38213

-Peff

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Oct 13, 2009 at 05:31:46PM -0400, Daniel Barkalow wrote:
>
>> I personally think that the real issue is that our "detached HEAD" message 
>> is still too scary, and what we really want is to issue the scary message 
>> when using "git commit" to move a detached HEAD from what was checked out 
>> to a new commit. So:
>
> This has been discussed before (I happen to agree with you, but you
> probably want to address other comments in the thread):
>
>   http://thread.gmane.org/gmane.comp.version-control.git/38201/focus=38213

I just re-read the discussion again (thanks for a useful pointers).  I
mostly agree with everything said in the thread and obviously agree with
its conclusion, but one thing I noticed that everybody (who _was_ a git
expert) in the thread was assuming bothered me somewhat.

In this sequence:

    1$ git checkout $commit_name_that_is_not_a_local_branch
    2$ git commit; hack; hack; hack;...
    3$ git checkout $branch_name

Step #1 is where the HEAD is detached.  It is correct to argue that
detached HEAD is a different state and we should inform unsuspecting
users, which we do.

Step #2 is where a commit that is not connected to any ref is made.

Step #3 is where the state built in the detached HEAD "branch" vanishes
into lost-found.

The experts argued that #3 is where it is dangerous, and while it is
technically correct, an unsuspecting non-expert would not even _know_ that
nothing dangerous is happening while in step #2.

If the commit name used in step #1 were "v1.0.0", and if the user while in
step #2 ran "gitk v1.0.0" (or "git log v1.0.0"), he will be confused by
not seeing the recent commits.  The distinction between "detached HEAD"
and being on a branch needs to be understood to appreciate this (and taken
advantage of, when running e.g. "git show-branch v1.0.0 HEAD").

Way before step #3, such a user, even though technically not in any danger
yet, would be confused and panic: "I wanted to fix something in the 1.0.0
release, but where did my fix go?"

The current message in step #1 reads like this:

    $ git checkout origin/next
    Note: moving to 'origin/next' which isn't a local branch
    If you want to create a new branch from this checkout, you may do so
    (now or later) by using -b with the checkout command again. Example:
      git checkout -b <new_branch_name>
    HEAD is now at 9ecb2a7... Merge branch 'maint'

And perhaps for people who do not understand the second point in the
four-point list [*1*] I showed earlier in the thread, "If you want to
create a new branch" may not be descriptive enough, as a sight-seer and an
occasional typofixer, the user does not know what branch is good for to
begin with, and would not be able to tell if s/he even "wants to create"
one.  Perhaps it would help more if we reworded three lines after "Note:"
with something like:

    To keep the history of commits you will build from now on in a branch,
    you may want to do "git checkout -b <new-branch-name>" now.

and customize the "in a branch" and <new-branch-name> part if the checkout
was given a remote tracking branch and the corresponding local branch does
not yet exist, e.g. in the above example:

    To keep the history of commits you will build from now on in 'next'
    branch, you may want to do "git checkout -b next" now.


[Footnote]

*1* The world model in which a git user works is:

 * You clone and get copies of where the other end has its branches;

 * You do all your work on your local branches;

 * You may incorporate what the other end further did by merging from the
   tracking branch from it;

 * You update the other end by pushing what you did on your local branches.

I do not think you can nor should hide them from the user [*2*].

*2* We had to repeat "don't hide but teach" many times until it finally
sank in for another essential thing in the git world model.  I hope we do
not have to do the same repeating for the above four points.  Luckily we
do not have to repeat "don't hide but teach" about the index anymore these
days.

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

[Johannes Schindelin]

Hi,

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

> Jeff King <peff@peff.net> writes:
> 
> > On Tue, Oct 13, 2009 at 05:31:46PM -0400, Daniel Barkalow wrote:
> >
> >> I personally think that the real issue is that our "detached HEAD" message 
> >> is still too scary, and what we really want is to issue the scary message 
> >> when using "git commit" to move a detached HEAD from what was checked out 
> >> to a new commit.
> >
> > This has been discussed before (I happen to agree with you, but you
> > probably want to address other comments in the thread):
> >
> >   http://thread.gmane.org/gmane.comp.version-control.git/38201/focus=38213
> 
> I just re-read the discussion again (thanks for a useful pointers).  I
> mostly agree with everything said in the thread and obviously agree with
> its conclusion, but one thing I noticed that everybody (who _was_ a git
> expert) in the thread was assuming bothered me somewhat.

We can of course continue to this public wanking session in our nice 
little circle here on git@vger, fully aware that real users will not dare 
to interrupt us.

In the alternative, we can go out into the world (you know, that thing 
behind the computer screen?) and ask somebody who has _not_ been exposed 
to Git for _4 years_ (like most of you!) just how hard it is to work with 
Git.

Let me tell you this from my experience: the least likely answer is "the 
messages are too scary".  Invariably, the answer I get is "it is totally 
unintuitive".  Often followed by "I tell Git to do something 
straight-forward, and it refuses to do it."

Maybe we should just admit that we are no user interface designers, so one 
cannot expect miracles from us in that respect.  And first and foremost, 
we should not pretend to ourselves that we are good at user interfaces, 
because we have a track record of sucking in that area.  Big time.

Ciao,
Dscho

P.S.: As somebody mentioned already, it is time to fix the tools, not our 
users.

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

[Thomas Rast]

Johannes Schindelin wrote:
> 
> Let me tell you this from my experience: the least likely answer is "the 
> messages are too scary".  Invariably, the answer I get is "it is totally 
> unintuitive".  Often followed by "I tell Git to do something 
> straight-forward, and it refuses to do it."

<aside>

Actually I had a rather insightful discussion yesterday (spawned by
this exact thread) with someone here at the institute.  He said
something to the effect that git's problem is mostly that it is unlike
everything else.  You cannot explain git in simple metaphors like
files, copies and such.  Any attempt to do so will just fall short
really soon.

[On the other hand, some users appear unwilling to learn something new
because they "just want to version control this" or "just need to make
a commit to this project".]

</aside>

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

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

[Johannes Schindelin]

Hi,

On Wed, 14 Oct 2009, Thomas Rast wrote:

> [On the other hand, some users appear unwilling to learn something new 
> because they "just want to version control this" or "just need to make a 
> commit to this project".]

Frankly, if the choice is between "I just want to make a commit to this 
project" and "Then I'll not use version control at all", I'd rather choose 
the former.

Which is exactly what I did the other day, having to write a non-trivial 
script to allow the user to do what he wants to do.

Ciao,
Dscho

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

[Thomas Rast]

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 14 Oct 2009, Thomas Rast wrote:
> 
> > [On the other hand, some users appear unwilling to learn something new 
> > because they "just want to version control this" or "just need to make a 
> > commit to this project".]
> 
> Frankly, if the choice is between "I just want to make a commit to this 
> project" and "Then I'll not use version control at all", I'd rather choose 
> the former.

Using your automatic gearbox analogy, I should point out that people
still spend significant amounts of time and money on learning how to
drive, despite the fact that learning the internals of the engine is
no longer required.

Yet for some reason, the same people want computers to read their
minds instead of learning how to operate (the more involved parts of)
it.

(Yeah, call me arrogant...)

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

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

[Uri Okrent]

Junio C Hamano wrote:
> In this sequence:
> 
>     1$ git checkout $commit_name_that_is_not_a_local_branch
>     2$ git commit; hack; hack; hack;...
>     3$ git checkout $branch_name
> [...]
> Step #3 is where the state built in the detached HEAD "branch" vanishes
> into lost-found.
> 
> The experts argued that #3 is where it is dangerous...

If step 3 is where the danger lies, wouldn't it then be most appropriate to put
the warning message there? I.e., warn or refuse to switch branches when
currently on a detached head containing new commits, kind of like branch -d's
cowardliness.
-- 
    Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/

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

[Junio C Hamano]

Uri Okrent <uokrent@gmail.com> writes:

> Junio C Hamano wrote:
>> In this sequence:
>>
>>     1$ git checkout $commit_name_that_is_not_a_local_branch
>>     2$ git commit; hack; hack; hack;...
>>     3$ git checkout $branch_name
>> [...]
>> Step #3 is where the state built in the detached HEAD "branch" vanishes
>> into lost-found.
>>
>> The experts argued that #3 is where it is dangerous...
>
> If step 3 is where the danger lies, wouldn't it then be most appropriate to put
> the warning message there?

You already get reminded that you were on a detached HEAD in step #3.

The primary point of the message you are replying to was that I do not
agree with the view that step #3 is the most problematic step.  The
existing reminder would help people who read it and are capable of
realizing "ah, I started it on a throw-away branch but ended up with
something I would rather keep" and doing "git branch topic HEAD@{1}".  

It will not help people who haven't got enough clue yet to know what a
detached HEAD is, or you can refer to your previous point with HEAD@{1}
notation.  We do give brief advice at step #1 to alleviate this issue.

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

[Björn Steinbrink]

On 2009.10.13 17:31:46 -0400, Daniel Barkalow wrote:
> The other thing that I think would be nice is:
> 
> $ git checkout origin/next
> $ git fetch origin
> $ git checkout !! (probably not a good syntax)
> 
> That is, expand "!!" to the string used to detach HEAD, and expand it 
> again now. (Of course, something would have to be done if you did "git 
> checkout HEAD^1" before, or "git checkout !!^1".) This is related in that 
> I think the scary message should happen when "git commit" sees this stored 
> string and clears it.

That sounds somewhat like the "git up" hack I've shown here:
http://article.gmane.org/gmane.comp.version-control.git/130050

In #git, Dscho even suggested that "git pull" could do that kind of
DWIMmery while on a detached HEAD that waas reached by checking out a remote
tracking branch. I'm undecided about that, because real merges/rebases
could make it easier to lose work, as opposed to the "fast-forward only"
behaviour I had in mind for that "git up" thing. Though of course, the
"git pull" DWIMmery for a detached HEAD could simply refuse to do
anything but a fast-forward.

Overall, I'm starting to think that improving the "work with a detached
HEAD" area might be more worthwhile than adding DWIMmery that tries to
completely avoid a detached HEAD.

This could include DWIMmery like the "git up"/"git pull" stuff, and
improved security checks, like checking that leaving a detached HEAD
doesn't "lose" any commits to the reflog.  So checkout could do
something like "git rev-list HEAD --not --all" (or does --all include
HEAD?) and complain if there's something to be "lost".

Björn

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.

[PATCH 1/3] check_filename(): make verify_filename() callable without dying

[Junio C Hamano]

Make it possible to invole the logic of verify_filename() to make sure the
pathname arguments are unambiguous without actually dying.  The caller may
want to do something different.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h |    1 +
 setup.c |   38 ++++++++++++++++++++------------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/cache.h b/cache.h
index 96840c7..71a731d 100644
--- a/cache.h
+++ b/cache.h
@@ -396,6 +396,7 @@ extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
 extern const char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
+extern int check_filename(const char *prefix, const char *name);
 extern void verify_filename(const char *prefix, const char *name);
 extern void verify_non_filename(const char *prefix, const char *name);
 
diff --git a/setup.c b/setup.c
index 029371e..f67250b 100644
--- a/setup.c
+++ b/setup.c
@@ -61,6 +61,19 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
 	return path;
 }
 
+int check_filename(const char *prefix, const char *arg)
+{
+	const char *name;
+	struct stat st;
+
+	name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
+	if (!lstat(name, &st))
+		return 1; /* file exists */
+	if (errno == ENOENT || errno == ENOTDIR)
+		return 0; /* file does not exist */
+	die_errno("failed to stat '%s'", arg);
+}
+
 /*
  * Verify a filename that we got as an argument for a pathspec
  * entry. Note that a filename that begins with "-" never verifies
@@ -70,18 +83,12 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
  */
 void verify_filename(const char *prefix, const char *arg)
 {
-	const char *name;
-	struct stat st;
-
 	if (*arg == '-')
 		die("bad flag '%s' used after filename", arg);
-	name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
-	if (!lstat(name, &st))
+	if (check_filename(prefix, arg))
 		return;
-	if (errno == ENOENT)
-		die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
-		    "Use '--' to separate paths from revisions", arg);
-	die_errno("failed to stat '%s'", arg);
+	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
+	    "Use '--' to separate paths from revisions", arg);
 }
 
 /*
@@ -91,19 +98,14 @@ void verify_filename(const char *prefix, const char *arg)
  */
 void verify_non_filename(const char *prefix, const char *arg)
 {
-	const char *name;
-	struct stat st;
-
 	if (!is_inside_work_tree() || is_inside_git_dir())
 		return;
 	if (*arg == '-')
 		return; /* flag */
-	name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
-	if (!lstat(name, &st))
-		die("ambiguous argument '%s': both revision and filename\n"
-		    "Use '--' to separate filenames from revisions", arg);
-	if (errno != ENOENT && errno != ENOTDIR)
-		die_errno("failed to stat '%s'", arg);
+	if (!check_filename(prefix, arg))
+		return;
+	die("ambiguous argument '%s': both revision and filename\n"
+	    "Use '--' to separate filenames from revisions", arg);
 }
 
 const char **get_pathspec(const char *prefix, const char **pathspec)
-- 
1.6.5.1.95.g09fbd

[PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"

[Junio C Hamano]

When 'frotz' is not a valid object name nor a tracked filename,
we used to complain and failed this command.  When there is only
one remote that has 'frotz' as one of its tracking branches, we can
DWIM it as a request to create a local branch 'frotz' forking from
the matching remote tracking branch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..fb7e68a 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -572,6 +572,40 @@ static int interactive_checkout(const char *revision, const char **pathspec,
 	return run_add_interactive(revision, "--patch=checkout", pathspec);
 }
 
+struct tracking_name_data {
+	const char *name;
+	char *remote;
+	int unique;
+};
+
+static int check_tracking_name(const char *refname, const unsigned char *sha1,
+			       int flags, void *cb_data)
+{
+	struct tracking_name_data *cb = cb_data;
+	const char *slash;
+
+	if (prefixcmp(refname, "refs/remotes/"))
+		return 0;
+	slash = strchr(refname + 13, '/');
+	if (!slash || strcmp(slash + 1, cb->name))
+		return 0;
+	if (cb->remote) {
+		cb->unique = 0;
+		return 0;
+	}
+	cb->remote = xstrdup(refname);
+	return 0;
+}
+
+static const char *unique_tracking_name(const char *name)
+{
+	struct tracking_name_data cb_data = { name, NULL, 1 };
+	for_each_ref(check_tracking_name, &cb_data);
+	if (cb_data.unique)
+		return cb_data.remote;
+	free(cb_data.remote);
+	return NULL;
+}
 
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
@@ -630,8 +664,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		opts.new_branch = argv0 + 1;
 	}
 
-	if (opts.track == BRANCH_TRACK_UNSPECIFIED)
-		opts.track = git_branch_track;
 	if (conflict_style) {
 		opts.merge = 1; /* implied */
 		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
@@ -655,6 +687,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	 *   With no paths, if <something> is a commit, that is to
 	 *   switch to the branch or detach HEAD at it.
 	 *
+	 *   With no paths, if <something> is _not_ a commit, no -t nor -b
+	 *   was given, and there is a tracking branch whose name is
+	 *   <something> in one and only one remote, then this is a short-hand
+	 *   to fork local <something> from that remote tracking branch.
+	 *
 	 *   Otherwise <something> shall not be ambiguous.
 	 *   - If it's *only* a reference, treat it like case (1).
 	 *   - If it's only a path, treat it like case (2).
@@ -677,7 +714,20 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
-			goto no_reference;          /* case (3 -> 2) */
+			if (!patch_mode &&
+			    opts.track == BRANCH_TRACK_UNSPECIFIED &&
+			    !opts.new_branch &&
+			    !check_filename(NULL, arg) &&
+			    argc == 1) {
+				const char *remote = unique_tracking_name(arg);
+				if (!remote || get_sha1(remote, rev))
+					goto no_reference;
+				opts.new_branch = arg;
+				arg = remote;
+				/* DWIMmed to create local branch */
+			}
+			else
+				goto no_reference;
 		}
 
 		/* we can't end up being in (2) anymore, eat the argument */
@@ -715,6 +765,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 no_reference:
+
+	if (opts.track == BRANCH_TRACK_UNSPECIFIED)
+		opts.track = git_branch_track;
+
 	if (argc) {
 		const char **pathspec = get_pathspec(prefix, argv);
 
-- 
1.6.5.1.95.g09fbd

Re: [PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"

[Nanako Shiraishi]

Quoting Junio C Hamano <gitster@pobox.com>

> When 'frotz' is not a valid object name nor a tracked filename,
> we used to complain and failed this command.  When there is only
> one remote that has 'frotz' as one of its tracking branches, we can
> DWIM it as a request to create a local branch 'frotz' forking from
> the matching remote tracking branch.

In the subject you used 'git checkout -b frotz origin/frotz'. Did you forget to say '-t'?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"

[Björn Steinbrink]

On 2009.10.18 19:34:48 +0900, Nanako Shiraishi wrote:
> Quoting Junio C Hamano <gitster@pobox.com>
> 
> > When 'frotz' is not a valid object name nor a tracked filename,
> > we used to complain and failed this command.  When there is only
> > one remote that has 'frotz' as one of its tracking branches, we can
> > DWIM it as a request to create a local branch 'frotz' forking from
> > the matching remote tracking branch.
> 
> In the subject you used 'git checkout -b frotz origin/frotz'. Did you
> forget to say '-t'?

Hm, the DWIMmery only triggers when opts.track is
BRANCH_TRACK_UNSPECIFIED, i.e. -t was not used. And it doesn't change
opts.track when it DWIMs, so it respects branch.autosetupmerge, which
would be overriden by -t. So it seems correct that -t is not in there.

Björn

Re: [PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"

[Nanako Shiraishi]

Quoting Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> On 2009.10.18 19:34:48 +0900, Nanako Shiraishi wrote:
>> Quoting Junio C Hamano <gitster@pobox.com>
>> 
>> > When 'frotz' is not a valid object name nor a tracked filename,
>> > we used to complain and failed this command.  When there is only
>> > one remote that has 'frotz' as one of its tracking branches, we can
>> > DWIM it as a request to create a local branch 'frotz' forking from
>> > the matching remote tracking branch.
>> 
>> In the subject you used 'git checkout -b frotz origin/frotz'. Did you
>> forget to say '-t'?
>
> Hm, the DWIMmery only triggers when opts.track is
> BRANCH_TRACK_UNSPECIFIED, i.e. -t was not used. And it doesn't change
> opts.track when it DWIMs, so it respects branch.autosetupmerge, which
> would be overriden by -t. So it seems correct that -t is not in there.

I see.

A user who always wants tracking can set the config option and use 
the new "git checkout frotz" shortcut, but a user who usually 
doesn't want tracking doesn't have the config option and when he 
wants tracking only for this new branch he can explicitly say "git 
checkout -t origin/frotz", right?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

>>> In the subject you used 'git checkout -b frotz origin/frotz'. Did you
>>> forget to say '-t'?
>>
>> Hm, the DWIMmery only triggers when opts.track is
>> BRANCH_TRACK_UNSPECIFIED, i.e. -t was not used. And it doesn't change
>> opts.track when it DWIMs, so it respects branch.autosetupmerge, which
>> would be overriden by -t. So it seems correct that -t is not in there.
>
> I see.
>
> A user who always wants tracking can set the config option and use 
> the new "git checkout frotz" shortcut, but a user who usually 
> doesn't want tracking doesn't have the config option and when he 
> wants tracking only for this new branch he can explicitly say "git 
> checkout -t origin/frotz", right?

Correct.

Re: [PATCH 2/3] DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"

[Björn Steinbrink]

On 2009.10.19 05:20:43 +0900, Nanako Shiraishi wrote:
> A user who always wants tracking can set the config option and use 
> the new "git checkout frotz" shortcut, but a user who usually 
> doesn't want tracking doesn't have the config option and when he 
> wants tracking only for this new branch he can explicitly say "git 
> checkout -t origin/frotz", right?

Well, branch.autosetupmerge has three possible values.
 - true: Do the upstream setup when starting from remote tracking
   branches
 - always: Also do the upstream setup when starting from a local branch
   head
 - false: Don't do any upstream setup

The default is "true", which should catch the "git checkout frotz"
shortcut, as that selects a remote tracking branch as the starting
point. So the user doesn't have to change any config setting to have
that act as if -t was given.

Only if we doesn't want "git checkout frotz" to not do the upstream
setup, he needs to set branch.autosetupmerge to false.

And falling back to "git checkout --track/--no-track origin/frotz" he
can override whatever config setting he has.

Björn

[PATCH 3/3] git checkout --nodwim

[Junio C Hamano]

Porcelains may want to make sure their calls to "git checkout" will
reliably fail regardless of the presense of random remote tracking
branches by the new DWIMmery introduced.

Luckily all existing in-tree callers have extra checks to make sure they
feed local branch name when they want to switch, or they explicitly ask
to detach HEAD at the given commit, so there is no need to add this option
for them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index fb7e68a..6ec9b83 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -616,6 +616,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
 	int patch_mode = 0;
+	int dwim_new_local_branch = 1;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
@@ -631,6 +632,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
+		OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
+			    "do not dwim local branch creation", 0),
 		OPT_END(),
 	};
 	int has_dash_dash;
@@ -715,6 +718,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
 			if (!patch_mode &&
+			    dwim_new_local_branch &&
 			    opts.track == BRANCH_TRACK_UNSPECIFIED &&
 			    !opts.new_branch &&
 			    !check_filename(NULL, arg) &&
-- 
1.6.5.1.95.g09fbd

Re: [PATCH 3/3] git checkout --nodwim

[Alex Riesen]

On Sun, Oct 18, 2009 at 10:01, Junio C Hamano <gitster@pobox.com> wrote:
> +               OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
> +                           "do not dwim local branch creation", 0),

Isn't there a special negation support for --no-something in parse-options?

Re: [PATCH 3/3] git checkout --nodwim

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> On Sun, Oct 18, 2009 at 10:01, Junio C Hamano <gitster@pobox.com> wrote:
>> +               OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
>> +                           "do not dwim local branch creation", 0),
>
> Isn't there a special negation support for --no-something in parse-options?

There probably is, but this is a whetherbaloon patch without documentation
and pretty much Porcelain only, so I took the lazy route.

Helping hands in polishing it up is very welcome.

[PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Alex Riesen]

The one which guesses local branch name from a remote reference.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Junio C Hamano, Sun, Oct 18, 2009 21:53:51 +0200:
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> > On Sun, Oct 18, 2009 at 10:01, Junio C Hamano <gitster@pobox.com> wrote:
> >> +               OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
> >> +                           "do not dwim local branch creation", 0),
> >
> > Isn't there a special negation support for --no-something in parse-options?
> 
> There probably is, but this is a whetherbaloon patch without documentation
> and pretty much Porcelain only, so I took the lazy route.
> 
> Helping hands in polishing it up is very welcome.

Maybe like this?

BTW, can parse-options take care of the " (default)" addition?

 builtin-checkout.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 6ec9b83..22b023b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -632,8 +632,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
-		OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
-			    "do not dwim local branch creation", 0),
+		OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
+			    "Guess local branch from remote reference (default)", 0),
 		OPT_END(),
 	};
 	int has_dash_dash;
-- 
1.6.5.1.50.g84e6e

Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> The one which guesses local branch name from a remote reference.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
>
> Maybe like this?
>
> BTW, can parse-options take care of the " (default)" addition?
>
>  builtin-checkout.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index 6ec9b83..22b023b 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -632,8 +632,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		OPT_STRING(0, "conflict", &conflict_style, "style",
>  			   "conflict style (merge or diff3)"),
>  		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
> -		OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
> -			    "do not dwim local branch creation", 0),
> +		OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
> +			    "Guess local branch from remote reference (default)", 0),

Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
with --no-dwim?

>  		OPT_END(),
>  	};
>  	int has_dash_dash;
> -- 
> 1.6.5.1.50.g84e6e

Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Alex Riesen]

On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>> +             OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>> +                         "Guess local branch from remote reference (default)", 0),
>
> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
> with --no-dwim?

It seems to do, though (I checked before sending).

Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Alex Riesen]

On Mon, Oct 19, 2009 at 08:07, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>>> +             OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>>> +                         "Guess local branch from remote reference (default)", 0),
>>
>> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
>> with --no-dwim?
>
> It seems to do, though (I checked before sending).
>

Right, just looked at the parse-options: it is defined for all types.

parse-options.c +/get_value

	const int unset = flags & OPT_UNSET;
...
	case OPTION_SET_INT:
		*(int *)opt->value = unset ? 0 : opt->defval;
		return 0;

Very useful.

Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> On Mon, Oct 19, 2009 at 08:07, Alex Riesen <raa.lkml@gmail.com> wrote:
>> On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>>> Alex Riesen <raa.lkml@gmail.com> writes:
>>>> +             OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>>>> +                         "Guess local branch from remote reference (default)", 0),
>>>
>>> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
>>> with --no-dwim?
>>
>> It seems to do, though (I checked before sending).
>>
>
> Right, just looked at the parse-options: it is defined for all types.
>
> parse-options.c +/get_value
>
> 	const int unset = flags & OPT_UNSET;
> ...
> 	case OPTION_SET_INT:
> 		*(int *)opt->value = unset ? 0 : opt->defval;
> 		return 0;
>
> Very useful.

Ah, did you mean to change the default value to 1 as well?

Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Alex Riesen]

On Mon, Oct 19, 2009 at 08:16, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> On Mon, Oct 19, 2009 at 08:07, Alex Riesen <raa.lkml@gmail.com> wrote:
>>> On Mon, Oct 19, 2009 at 00:49, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Alex Riesen <raa.lkml@gmail.com> writes:
>>>>> +             OPT_SET_INT(0, "dwim", &dwim_new_local_branch,
>>>>> +                         "Guess local branch from remote reference (default)", 0),
>>>>
>>>> Humph, how does SET_INT know to set it to 1 with --dwim and set it to 0
>>>> with --no-dwim?
>>>
>>> It seems to do, though (I checked before sending).
>>>
>>
>> Right, just looked at the parse-options: it is defined for all types.
>>
>> parse-options.c +/get_value
>>
>>       const int unset = flags & OPT_UNSET;
>> ...
>>       case OPTION_SET_INT:
>>               *(int *)opt->value = unset ? 0 : opt->defval;
>>               return 0;
>>
>> Very useful.
>
> Ah, did you mean to change the default value to 1 as well?
>

Err... yes. I (wrongly) assumed that the current value in the
storage is the default. Now, having looked at struct option
I see that It isn't (and the default is in defval).

BTW, why is the option an ...INT? Where a future extension planned?

Re: [PATCH] Use "--no-" prefix to switch off some of checkout dwimmery

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> BTW, why is the option an ...INT? Where a future extension planned?

No, in my original I wanted to default to 1 and an option to set it to
zero, only because I did not want a variable with negative name.

Re: [PATCH 3/3] git checkout --nodwim

[Avery Pennarun]

On Sun, Oct 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
>> On Sun, Oct 18, 2009 at 10:01, Junio C Hamano <gitster@pobox.com> wrote:
>>> +               OPT_SET_INT(0, "nodwim", &dwim_new_local_branch,
>>> +                           "do not dwim local branch creation", 0),
>>
>> Isn't there a special negation support for --no-something in parse-options?
>
> There probably is, but this is a whetherbaloon patch without documentation
> and pretty much Porcelain only, so I took the lazy route.
>
> Helping hands in polishing it up is very welcome.

I find the idea of an option for "don't do what I mean" to be pretty
entertaining.  Or maybe just misleading :)

Have fun,

Avery

Re: [PATCH 3/3] git checkout --nodwim

[Nanako Shiraishi]

Quoting Avery Pennarun <apenwarr@gmail.com>

> On Sun, Oct 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Helping hands in polishing it up is very welcome.
>
> I find the idea of an option for "don't do what I mean" to be pretty
> entertaining.  Or maybe just misleading :)
>
> Have fun,
>
> Avery

As Junio asked for helping hands, let's try to be helpful and constructive.

Maybe "don't second-guess" explains it better?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH 3/3] git checkout --nodwim

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> As Junio asked for helping hands, let's try to be helpful and constructive.
>
> Maybe "don't second-guess" explains it better?

Perhaps --no-guess, as --no-second-guess is rather hard to read even in scripts.

[PATCH] git checkout --no-guess

[Junio C Hamano]

Porcelains may want to make sure their calls to "git checkout" will
reliably fail regardless of the presense of random remote tracking
branches by the new DWIMmery introduced.

Luckily all existing in-tree callers have extra checks to make sure they
feed local branch name when they want to switch, or they explicitly ask to
detach HEAD at the given commit, so there is no need to add this option
for them.

As this is strictly script-only option, do not even bother to document it,
and do bother to hide it from "git checkout -h".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Junio C Hamano <gitster@pobox.com> writes:

 > Nanako Shiraishi <nanako3@lavabit.com> writes:
 >
 >> As Junio asked for helping hands, let's try to be helpful and constructive.
 >>
 >> Maybe "don't second-guess" explains it better?
 >
 > Perhaps --no-guess, as --no-second-guess is rather hard to read even in scripts.

 builtin-checkout.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index fb7e68a..da04eed 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -616,6 +616,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct tree *source_tree = NULL;
 	char *conflict_style = NULL;
 	int patch_mode = 0;
+	int dwim_new_local_branch = 1;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
@@ -631,6 +632,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
+		{ OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
+		  "second guess 'git checkout no-such-branch'",
+		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
 		OPT_END(),
 	};
 	int has_dash_dash;
@@ -715,6 +719,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
 			if (!patch_mode &&
+			    dwim_new_local_branch &&
 			    opts.track == BRANCH_TRACK_UNSPECIFIED &&
 			    !opts.new_branch &&
 			    !check_filename(NULL, arg) &&
-- 
1.6.5.1.107.gba912

Re: [PATCH] git checkout --no-guess

[Avery Pennarun]

On Wed, Oct 21, 2009 at 6:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> As this is strictly script-only option, do not even bother to document it,
> and do bother to hide it from "git checkout -h".

Is it a standard git policy to not document script-only options?  As a
person who writes scripts that use git, we will need to discover these
options somehow...

Just curious.  (And now wondering how many other wonderful options are
in there but undocumented...)

Thanks,

Avery

Re: [PATCH] git checkout --no-guess

[Jay Soffian]

On Wed, Oct 21, 2009 at 3:51 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
> On Wed, Oct 21, 2009 at 6:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> As this is strictly script-only option, do not even bother to document it,
>> and do bother to hide it from "git checkout -h".
>
> Is it a standard git policy to not document script-only options?  As a
> person who writes scripts that use git, we will need to discover these
> options somehow...
>
> Just curious.  (And now wondering how many other wonderful options are
> in there but undocumented...)

 *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
 *                     shown only in the full usage.

Which translates to --help-all:

--help-all
           Some git commands take options that are only used for
plumbing or that are deprecated, and such options are hidden from the
default usage. This option gives the full list of options.

 So git checkout --help-all should show it.

j.

Re: [PATCH] git checkout --no-guess

[Avery Pennarun]

On Mon, Oct 26, 2009 at 2:17 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Wed, Oct 21, 2009 at 3:51 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
>> Just curious.  (And now wondering how many other wonderful options are
>> in there but undocumented...)
>
>  *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
>  *                     shown only in the full usage.
>
> Which translates to --help-all:
>
> --help-all
>           Some git commands take options that are only used for
> plumbing or that are deprecated, and such options are hidden from the
> default usage. This option gives the full list of options.
>
>  So git checkout --help-all should show it.

Thanks!  I had no idea about --help-all.

Avery

Re: [PATCH 3/3] git checkout --nodwim

[Johannes Schindelin]

Hi,

On Thu, 22 Oct 2009, Nanako Shiraishi wrote:

> Quoting Avery Pennarun <apenwarr@gmail.com>
> 
> > On Sun, Oct 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Helping hands in polishing it up is very welcome.
> >
> > I find the idea of an option for "don't do what I mean" to be pretty
> > entertaining.  Or maybe just misleading :)
> >
> > Have fun,
> >
> > Avery
> 
> As Junio asked for helping hands, let's try to be helpful and constructive.
> 
> Maybe "don't second-guess" explains it better?

My take on it:

1) --no-porcelain

2) we all are bike-shedding, not being constructive at all

Ciao,
Dscho

Re: [PATCH 3/3] git checkout --nodwim

[Erik Faye-Lund]

On Thu, Oct 22, 2009 at 2:27 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> My take on it:
>
> 1) --no-porcelain
>
> 2) we all are bike-shedding, not being constructive at all

In that case, I propose "--yellow".

-- 
Erik "kusma" Faye-Lund

Re: [PATCH 3/3] git checkout --nodwim

[Michael J Gruber]

Johannes Schindelin venit, vidit, dixit 22.10.2009 02:27:
> Hi,
> 
> On Thu, 22 Oct 2009, Nanako Shiraishi wrote:
> 
>> Quoting Avery Pennarun <apenwarr@gmail.com>
>>
>>> On Sun, Oct 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Helping hands in polishing it up is very welcome.
>>>
>>> I find the idea of an option for "don't do what I mean" to be pretty
>>> entertaining.  Or maybe just misleading :)
>>>
>>> Have fun,
>>>
>>> Avery
>>
>> As Junio asked for helping hands, let's try to be helpful and constructive.
>>
>> Maybe "don't second-guess" explains it better?
> 
> My take on it:
> 
> 1) --no-porcelain

Between --no-dwim and --no-porcelain, maybe --no-wimp is a good compromise?

> 2) we all are bike-shedding, not being constructive at all

That's the fun part!

Michael

Re: [PATCH 3/3] git checkout --nodwim

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Thu, 22 Oct 2009, Nanako Shiraishi wrote:
>
>> Quoting Avery Pennarun <apenwarr@gmail.com>
>> 
>> > On Sun, Oct 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> >> Helping hands in polishing it up is very welcome.
>> >
>> > I find the idea of an option for "don't do what I mean" to be pretty
>> > entertaining.  Or maybe just misleading :)
>> >
>> > Have fun,
>> >
>> > Avery
>> 
>> As Junio asked for helping hands, let's try to be helpful and constructive.
>> 
>> Maybe "don't second-guess" explains it better?
>
> My take on it:
>
> 1) --no-porcelain
>
> 2) we all are bike-shedding, not being constructive at all

You are right about (2), regarding the option name. I've queued one that
uses --no-guess.

Regarding the correct use of parse_options(), I had to figure it out
myself, and helping hand would have, eh, helped me.

Re: [PATCH 3/3] git checkout --nodwim

[David Roundy]

On Sat, Oct 24, 2009 at 2:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> My take on it:
>>
>> 1) --no-porcelain
>>
>> 2) we all are bike-shedding, not being constructive at all
>
> You are right about (2), regarding the option name. I've queued one that
> uses --no-guess.

Perhaps a universal --plumbing flag would be handy? It'd be nice to be
able to pass any command --plumbing and it'll either behave in a
stable, predictable, plumbing-like way, or die with an error message
stating that it isn't a plumbing command.  Right now, it's sort of
hard to figure out what is plumbing and what is porcelain.  Some
commands are clear, but other commands labelled as plumbing in git(1)
are deprecated in favor of commands labelled as porcelain.

David

Re: [PATCH 3/3] git checkout --nodwim

[Junio C Hamano]

David Roundy <roundyd@physics.oregonstate.edu> writes:

> Perhaps a universal --plumbing flag would be handy?

Yes in general but it is unclear what aspect of its behaviour we will
be casting in stone with a generic --plumbing option in this case.  I
also think that "checkout" Porcelain is not yet mature enough for us
to do this right now. For example, I am reasonably sure that somebody
motivated enough will teach it to touch submodule trees when switching
to another branch by default, and it is unclear if we should turn off
these expected additions when --plumbing is seen.

Re: [PATCH 3/3] git checkout --nodwim

[Johannes Schindelin]

Hi,

On Sat, 24 Oct 2009, David Roundy wrote:

> On Sat, Oct 24, 2009 at 2:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> My take on it:
> >>
> >> 1) --no-porcelain
> >>
> >> 2) we all are bike-shedding, not being constructive at all
> >
> > You are right about (2), regarding the option name. I've queued one that
> > uses --no-guess.
> 
> Perhaps a universal --plumbing flag would be handy?

No.  Older Git versions do not know about it, so you cannot Just Modify 
Your Scripts.  So the benefit of --plumbing is dubitable.

FWIW the same goes for --no-porcelain.

Ciao,
Dscho

Re: [PATCH 3/3] git checkout --nodwim

[Avery Pennarun]

On Mon, Oct 26, 2009 at 4:12 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Sat, 24 Oct 2009, David Roundy wrote:
>> Perhaps a universal --plumbing flag would be handy?
>
> No.  Older Git versions do not know about it, so you cannot Just Modify
> Your Scripts.  So the benefit of --plumbing is dubitable.
>
> FWIW the same goes for --no-porcelain.

I suppose that, three years down the road, the existence of such an
option would be useful.  Until then, any change at all to any
command's interface seems to have the same problem as you describe.

That said, as a person who maintains a bunch of git-wrapping scripts
at work, it seems more straightforward to me to continue the
separation between plumbing vs. porcelain commands, rather than giving
each command two subtly incompatible modes.  It's much easier for me
to remember "don't use git checkout" than to remember "when you call
git checkout, make sure to use --plumbing, even though *today* it
works just fine without it."

I don't think there's actually a plumbing alternative to git-checkout,
however.  My git-subtree script (and another script at work) have
already had some bugs because of this (specifically, the differing
behaviour of git-checkout with and without a path specified).  Is
there something else I should be using in my scripts to be maximally
safe?

Have fun,

Avery

Re: [PATCH 3/3] git checkout --nodwim

[Jeff King]

On Mon, Oct 26, 2009 at 04:40:41PM -0400, Avery Pennarun wrote:

> I don't think there's actually a plumbing alternative to git-checkout,
> however.  My git-subtree script (and another script at work) have
> already had some bugs because of this (specifically, the differing
> behaviour of git-checkout with and without a path specified).  Is
> there something else I should be using in my scripts to be maximally
> safe?

It's git-update-ref. Which highlights one problem with the
porcelain/plumbing distinction. Our plumbing building blocks work at a
very low level, but often when scripting you want to use higher level
building blocks. So porcelain gets used in scripts, and gets an
ambiguous state. Consider "git commit", for example. Does anyone
actually script around "write-tree" and "commit-tree" these days, or do
they just script around "git commit"?

-Peff

Re: [PATCH 3/3] git checkout --nodwim

[Avery Pennarun]

On Mon, Oct 26, 2009 at 5:26 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 26, 2009 at 04:40:41PM -0400, Avery Pennarun wrote:
>> I don't think there's actually a plumbing alternative to git-checkout,
>> however.  My git-subtree script (and another script at work) have
>> already had some bugs because of this (specifically, the differing
>> behaviour of git-checkout with and without a path specified).  Is
>> there something else I should be using in my scripts to be maximally
>> safe?
>
> It's git-update-ref.

That would be similar to git commit, not git checkout, right?  Oh
wait, I see the confusion: git checkout does two things.  It switches
branches, and it checks out files from the index into the work tree.
I meant the latter meaning.

> Consider "git commit", for example. Does anyone
> actually script around "write-tree" and "commit-tree" these days, or do
> they just script around "git commit"?

Oh, I use those all the time.  They're awesome!  It allows you to
create commits without having a working tree, which lets me do very
interesting tricks.  git-subtree uses this heavily.

I'm probably a weirdo, though.

Have fun,

Avery

Re: [PATCH 3/3] git checkout --nodwim

[Jeff King]

On Mon, Oct 26, 2009 at 06:01:29PM -0400, Avery Pennarun wrote:

> > It's git-update-ref.
> 
> That would be similar to git commit, not git checkout, right?  Oh
> wait, I see the confusion: git checkout does two things.  It switches
> branches, and it checks out files from the index into the work tree.
> I meant the latter meaning.

Er, sorry, yes. It should be "git symbolic-ref", of course, to change
HEAD, and then probably read-tree and checkout-index. I was just not
thinking when I wrote the other message (hopefully I am doing so now).

> > Consider "git commit", for example. Does anyone
> > actually script around "write-tree" and "commit-tree" these days, or do
> > they just script around "git commit"?
> 
> Oh, I use those all the time.  They're awesome!  It allows you to
> create commits without having a working tree, which lets me do very
> interesting tricks.  git-subtree uses this heavily.
> 
> I'm probably a weirdo, though.

OK, I should have phrased my statement differently (see, I told you I
wasn't thinking). Yes, there are reasons to script around low-level
building blocks, when you don't want the assumptions associated with the
higher level. But I'm sure there are tons of scripts that munge some
files in a worktree, followed by "git add -A; git commit -m 'automagic
update'". And in that case, nobody would script around "commit-tree"
because it's a lot more work.

-Peff

Re: [PATCH 3/3] git checkout --nodwim

[Avery Pennarun]

On Mon, Oct 26, 2009 at 6:14 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 26, 2009 at 06:01:29PM -0400, Avery Pennarun wrote:
>> > It's git-update-ref.
>>
>> That would be similar to git commit, not git checkout, right?  Oh
>> wait, I see the confusion: git checkout does two things.  It switches
>> branches, and it checks out files from the index into the work tree.
>> I meant the latter meaning.
>
> Er, sorry, yes. It should be "git symbolic-ref", of course, to change
> HEAD, and then probably read-tree and checkout-index. I was just not
> thinking when I wrote the other message (hopefully I am doing so now).

Wow, I've browsed through the git manpages repeatedly and never found
checkout-index.  It was exactly the missing building block I was
looking for.  Thanks!

>> > Consider "git commit", for example. Does anyone
>> > actually script around "write-tree" and "commit-tree" these days, or do
>> > they just script around "git commit"?
>>
>> Oh, I use those all the time.  They're awesome!  It allows you to
>> create commits without having a working tree, which lets me do very
>> interesting tricks.  git-subtree uses this heavily.
>>
>> I'm probably a weirdo, though.
>
> OK, I should have phrased my statement differently (see, I told you I
> wasn't thinking). Yes, there are reasons to script around low-level
> building blocks, when you don't want the assumptions associated with the
> higher level. But I'm sure there are tons of scripts that munge some
> files in a worktree, followed by "git add -A; git commit -m 'automagic
> update'". And in that case, nobody would script around "commit-tree"
> because it's a lot more work.

Unfortunately this is pretty tricky to get perfect; perhaps there's no
way to do it.

In git-subtree, for example, I *mostly* use write-tree and
commit-tree, but when I do the final merge operation (to take the
synthetic history and merge it into your "real" history) I use commit.
 This is because I wanted the default merge handling, commit message,
etc for that part.  Unfortunately, it's possible that this dragged in
a bunch of stuff I *didn't* want.  It also makes git-subtree, which
otherwise could be used as plumbing, effectively into a porcelain.

I don't really know what to do about that.  You could introduce an
abstraction level somewhere between commit-tree and commit, but surely
someone would eventually find a case where that abstraction level is
still not right.  To bring this around to the original topic of this
thread, such an extra level of abstraction is equivalent to the
suggested --plumbing (or whatever) option, whether it's presented as
an option or a separate command.

Have fun,

Avery

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

[Jeff King]

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