[PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Johannes Schindelin

What does the user most likely want with this command?

        $ git checkout --track origin/next

Exactly.  A branch called 'next', that tracks origin's branch 'next'.
Make it so.

Signed-off-by: Johannes Schindelin <[hidden email]>
---

        This comes in the wake for Shawn's call for more user-friendliness.

 Documentation/git-checkout.txt |   10 +++++++++-
 builtin-checkout.c             |   21 ++++++++++++++++++---
 t/t7201-co.sh                  |   11 +++++++++++
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 5aa69c0..43d4502 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -8,7 +8,7 @@ git-checkout - Checkout a branch or paths to the working tree
 SYNOPSIS
 --------
 [verse]
-'git checkout' [-q] [-f] [[--track | --no-track] -b <new_branch> [-l]] [-m] [<branch>]
+'git checkout' [-q] [-f] [--track | --no-track] [-b <new_branch> [-l]] [-m] [<branch>]
 'git checkout' [<tree-ish>] [--] <paths>...
 
 DESCRIPTION
@@ -21,6 +21,10 @@ specified, <new_branch>.  Using -b will cause <new_branch> to
 be created; in this case you can use the --track or --no-track
 options, which will be passed to `git branch`.
 
+As a convenience, --track will default to create a branch whose
+name is constructed from the specified branch name by stripping
+the first namespace level.
+
 When <paths> are given, this command does *not* switch
 branches.  It updates the named paths in the working tree from
 the index file (i.e. it runs `git checkout-index -f -u`), or
@@ -59,6 +63,10 @@ OPTIONS
  'git-checkout' and 'git-branch' to always behave as if '--no-track' were
  given. Set it to `always` if you want this behavior when the
  start-point is either a local or remote branch.
++
+If no '-b' option was given, a name will be made up for you, by stripping
+the part up to the first slash of the tracked branch.  For example, if you
+called 'git checkout --track origin/next', the branch name will be 'next'.
 
 --no-track::
  Ignore the branch.autosetupmerge configuration variable.
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 411cc51..e95eab9 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -437,13 +437,28 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
  git_config(git_default_config, NULL);
 
- opts.track = git_branch_track;
+ opts.track = -1;
 
  argc = parse_options(argc, argv, options, checkout_usage,
      PARSE_OPT_KEEP_DASHDASH);
 
- if (!opts.new_branch && (opts.track != git_branch_track))
- die("git checkout: --track and --no-track require -b");
+ /* --track without -b should DWIM */
+ if (opts.track && opts.track != -1 && !opts.new_branch) {
+ char *slash;
+ if (!argc || !strcmp(argv[0], "--"))
+ die ("--track needs a branch name");
+ slash = strchr(argv[0], '/');
+ if (slash && !prefixcmp(argv[0], "refs/"))
+ slash = strchr(slash + 1, '/');
+ if (slash && !prefixcmp(argv[0], "remotes/"))
+ slash = strchr(slash + 1, '/');
+ if (!slash || !slash[1])
+ die ("Missing branch name; try -b");
+ opts.new_branch = slash + 1;
+ }
+
+ if (opts.track == -1)
+ opts.track = git_branch_track;
 
  if (opts.force && opts.merge)
  die("git checkout: -f and -m are incompatible");
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 9ad5d63..943dd57 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -337,4 +337,15 @@ test_expect_success \
     test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
     test_must_fail git checkout --track -b track'
 
+test_expect_success \
+    'checkout with --track fakes a sensible -b <name>' '
+    git update-ref refs/remotes/origin/koala/bear renamer &&
+    git checkout --track origin/koala/bear &&
+    test "refs/heads/koala/bear" = "$(git symbolic-ref HEAD)" &&
+    test "$(git rev-parse HEAD)" = "$(git rev-parse renamer)"'
+
+test_expect_success \
+    'checkout with --track, but without -b, fails with too short tracked name' '
+    test_must_fail git checkout --track renamer'
+
 test_done
--
1.6.0.rc2.26.g0d7ea

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Junio C Hamano
Johannes Schindelin <[hidden email]> writes:

> What does the user most likely want with this command?
>
> $ git checkout --track origin/next
>
> Exactly.  A branch called 'next', that tracks origin's branch 'next'.

I like this.

An explicit --track request from the command line (as opposed to happening
to have "branch.autosetupmerge" configuration) is a very good cue that
what the user wants to do is not to take a peek on a detached HEAD but a
more permanent playpen created.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Junio C Hamano
Junio C Hamano <[hidden email]> writes:

> Johannes Schindelin <[hidden email]> writes:
>
>> What does the user most likely want with this command?
>>
>> $ git checkout --track origin/next
>>
>> Exactly.  A branch called 'next', that tracks origin's branch 'next'.
>
> I like this.
>
> An explicit --track request from the command line (as opposed to happening
> to have "branch.autosetupmerge" configuration) is a very good cue that
> what the user wants to do is not to take a peek on a detached HEAD but a
> more permanent playpen created.

A couple more thoughts.

(1) You may not necessarily are used to --track, but may still want this
    done.  It might not be a bad idea to associate this "local dwimming"
    to creation of a new branch.  In other words, all of these:

    $ git checkout -b origin/next
    $ git checkout -b --track origin/next
    $ git checkout --track origin/next

    may merit the same dwimming.

(2) If you work with somebody else, you might not want to have the name
    mapping to be "s|^[^/]*/||" (i.e. drop "origin/"):

    $ git remote add -f jeff $url_to_his_repository
    $ git checkout -b [--track] jeff-next jeff/next
    $ git checkout -b [--track] origin-next origin/next
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Johannes Schindelin
Hi,

On Sat, 9 Aug 2008, Junio C Hamano wrote:

> Junio C Hamano <[hidden email]> writes:
>
> > Johannes Schindelin <[hidden email]> writes:
> >
> >> What does the user most likely want with this command?
> >>
> >> $ git checkout --track origin/next
> >>
> >> Exactly.  A branch called 'next', that tracks origin's branch 'next'.
> >
> > I like this.
> >
> > An explicit --track request from the command line (as opposed to
> > happening to have "branch.autosetupmerge" configuration) is a very
> > good cue that what the user wants to do is not to take a peek on a
> > detached HEAD but a more permanent playpen created.
>
> A couple more thoughts.

At first, I liked the thoughts, but...

> (1) You may not necessarily are used to --track, but may still want this
>     done.  It might not be a bad idea to associate this "local dwimming"
>     to creation of a new branch.  In other words, all of these:
>
>     $ git checkout -b origin/next

This cannot be dwimmed, as it literally means "start a new branch called
'origin/next' from HEAD".

So it would change the current behavior would, breaking people's habits (I
do "git checkout -b bla" a lot when I realize that I want to have the
current changes on a new branch).

>     $ git checkout -b --track origin/next

This is a clear syntax error.  By the same reasoning, we would have to
allow "git add remote origin git://..."

I think allowing this would confuse the syntax, and foster unreasonable
expectations.

>     $ git checkout --track origin/next

That is what my patch does.

> (2) If you work with somebody else, you might not want to have the name
>     mapping to be "s|^[^/]*/||" (i.e. drop "origin/"):
>
>     $ git remote add -f jeff $url_to_his_repository
>     $ git checkout -b [--track] jeff-next jeff/next
>     $ git checkout -b [--track] origin-next origin/next

As I said, I think you must not allow switching around the options -b and
--track.

And the rest of what you describe is already supported.

Ciao,
Dscho

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Junio C Hamano
Johannes Schindelin <[hidden email]> writes:

> On Sat, 9 Aug 2008, Junio C Hamano wrote:
> ...
> At first, I liked the thoughts, but...
>
>> (1) You may not necessarily are used to --track, but may still want this
>>     done.  It might not be a bad idea to associate this "local dwimming"
>>     to creation of a new branch.  In other words, all of these:
>>
>>     $ git checkout -b origin/next
>
> This cannot be dwimmed, as it literally means "start a new branch called
> 'origin/next' from HEAD".

Right.  Forget this part.

>> (2) If you work with somebody else, you might not want to have the name
>>     mapping to be "s|^[^/]*/||" (i.e. drop "origin/"):
>>
>>     $ git remote add -f jeff $url_to_his_repository
>>     $ git checkout -b [--track] jeff-next jeff/next
>>     $ git checkout -b [--track] origin-next origin/next
>
> As I said, I think you must not allow switching around the options -b and
> --track.

Oh, that was a typo.  "git checkout [--track] -b" was what I meant, but
the point was that with your patch "git checkout --track jeff/next" and
"git checkout --track origin/next" would create 'next' branch which will
not be useful for people who work with more than one repository.

Yes, you can of course explicitly name what you want to create with -b,
but that argument goes directly against the "usability enhancement" theme
of your patch.

Don't mistake this comment as "I oppose to the patch".  I was hoping
people who care, not necessarily you, might come up with a clean UI and
mechanism to let users affect how this dwimmery would work depending on
how the users want to work, by raising this point as something to ponder
on.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Pieter de Bie-2

On Aug 9, 2008, at 11:11 PM, Junio C Hamano wrote:

>>> (1) You may not necessarily are used to --track, but may still  
>>> want this
>>>    done.  It might not be a bad idea to associate this "local  
>>> dwimming"
>>>    to creation of a new branch.  In other words, all of these:
>>>
>>>    $ git checkout -b origin/next
>>
>> This cannot be dwimmed, as it literally means "start a new branch  
>> called
>> 'origin/next' from HEAD".
>
> Right.  Forget this part.

This is too bad. I often see people make mistakes like

        git checkout -b origin/master
or
        git checkout -b origin/master origin/master

which should be

        git checkout -b origin/master master

I think both forms should at least be an error if the remote branch  
"origin/master" already exists, as then suddenly they aren't reachable  
anymore by using "origin/master".

Changing the behaviour to mean "git checkout -b origin/master master"  
will change the meaning, but who uses -b with an existing remote  
branch anyway? I think the current behaviour leads to more confusion  
in every case and should at least error out ("Error: creating a local  
repository with the same name as the remote is not allowed") or do  
what it's meant to do, which is create a local repository with the  
trailing part.

Just my .02,

- Pieter

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Tarmigan Casebolt
In reply to this post by Johannes Schindelin
On Sat, Aug 9, 2008 at 2:08 PM, Johannes Schindelin
<[hidden email]> wrote:

> Hi,
>
> On Sat, 9 Aug 2008, Junio C Hamano wrote:
>
>> Junio C Hamano <[hidden email]> writes:
>>
>> > Johannes Schindelin <[hidden email]> writes:
>> >
>> >> What does the user most likely want with this command?
>> >>
>> >>    $ git checkout --track origin/next
>> >>
>> >> Exactly.  A branch called 'next', that tracks origin's branch 'next'.
>> >
>> > I like this.
>> >
>> > An explicit --track request from the command line (as opposed to
>> > happening to have "branch.autosetupmerge" configuration) is a very
>> > good cue that what the user wants to do is not to take a peek on a
>> > detached HEAD but a more permanent playpen created.
>>
>> A couple more thoughts.
>
> At first, I liked the thoughts, but...
>
>> (1) You may not necessarily are used to --track, but may still want this
>>     done.  It might not be a bad idea to associate this "local dwimming"
>>     to creation of a new branch.  In other words, all of these:
>>
>>     $ git checkout -b origin/next
>
> This cannot be dwimmed, as it literally means "start a new branch called
> 'origin/next' from HEAD".

Could we check whether there is already a remote called "origin" with
a branch called "next"?  If refs/remotes/origin/next exists it could
be confusing to create refs/heads/origin/next anyway, so it this dwim
might eliminate a problem as well as be nicer.  In fact, git soon
complains (rightly) about ambigiuos references in this case.

> So it would change the current behavior would, breaking people's habits (I
> do "git checkout -b bla" a lot when I realize that I want to have the
> current changes on a new branch).

If we did this dwim only for remotes, your desired behavior could
still work, right?.  The default is to --track for branches created
from remotes anyway, right?

Thanks,
Tarmigan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Johannes Schindelin
In reply to this post by Pieter de Bie-2
Hi,

On Sat, 9 Aug 2008, Pieter de Bie wrote:

> Changing the behaviour to mean "git checkout -b origin/master master"
> will change the meaning, but who uses -b with an existing remote branch
> anyway?

FWIW I am totally opposed to this kind of reasoning.  I consider it sloppy
to make assumptions that might or might not be true, and to force
a change that might be convenient to you, but is likely to hurt others.

Besides, your suggestion completely breaks consistency.  If somebody asks
to name a new branch "origin/master" (and "-b origin/master" is _just_
_that_), then it is not Git's job to fix the user's mistake.  Just like it
is not Git's job to fix when somebody said "git commit", but meant "git
push".

After all, I might _want_ to create a local branch "origin/master", and
you would just break the valid assumption that "-b origin/master" would do
that for me.

Hth,
Dscho

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Johannes Schindelin
In reply to this post by Junio C Hamano
Hi,

On Sat, 9 Aug 2008, Junio C Hamano wrote:

> Johannes Schindelin <[hidden email]> writes:
>
> > On Sat, 9 Aug 2008, Junio C Hamano wrote:
> > ...
> >> (2) If you work with somebody else, you might not want to have the name
> >>     mapping to be "s|^[^/]*/||" (i.e. drop "origin/"):
> >>
> >>     $ git remote add -f jeff $url_to_his_repository
> >>     $ git checkout -b [--track] jeff-next jeff/next
> >>     $ git checkout -b [--track] origin-next origin/next
> >
> > As I said, I think you must not allow switching around the options -b
> > and --track.
>
> Oh, that was a typo.  "git checkout [--track] -b" was what I meant, but
> the point was that with your patch "git checkout --track jeff/next" and
> "git checkout --track origin/next" would create 'next' branch which will
> not be useful for people who work with more than one repository.
>
> Yes, you can of course explicitly name what you want to create with -b,
> but that argument goes directly against the "usability enhancement"
> theme of your patch.

Not necessarily:

$ git checkout --track jeff/next
Switched to a new branch "next"

[do a lot of work, even on that 'next' branch]

[weeks, months or centuries later, decide to do something on origin/next]

$ git checkout --track origin/next
fatal: A branch named 'next' already exists.

[Ah! Slap your head, remembering that 'next' tracks jeff's 'next']
$ git branch -m next jeff-next
Branch: next renamed to jeff-next
$ git checkout --track -b origin-next origin/next


Concluding, I do not see how the DWIMing of the normal case impacts the
non-normal case negatively.

Don't get me wrong.  I do not need that patch in git.git desperately.  
But if it is rejected, I want it to be rejected for reasons I understand.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] checkout --track: make up a sensible branch name if '-b' was omitted

Junio C Hamano
Johannes Schindelin <[hidden email]> writes:

> Don't get me wrong.  I do not need that patch in git.git desperately.  
> But if it is rejected, I want it to be rejected for reasons I understand.

You are the second person in the past few days to talk about rejection
after my comments.  I'll try to do better in the future, but if it was not
clear, I thought the original is good enough for inclusion as-is.

My comments were about potential improvements on top of what was
presented, and I say "potential" not in the sense that yours is inferior
than the ideal because it lacks such improvements, but in the sense that
the suggested line of thought might not be even an improvement (iow, I do
not mean "the code your patch brings is has potential to improve and not
good enough as it stands", but "I think we might also want to do these on
top of it, but I may well have overlooked downsides in my suggestion hence
I am bringing it up for discussion").
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html