{"thread":{"id":"15113","subject":"[PATCH] Extend \"checkout --track\" DWIM to support more cases","startedAt":"2008-08-20T18:50:28Z","lastAt":"2008-08-22T23:33:44Z","messageCount":11,"participants":["Alex Riesen","Johannes Schindelin","Junio C Hamano","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"87845","messageId":"20080820185028.GA16626@blimp.local","threadId":"15113","inReplyTo":null,"subject":"[PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-08-20T18:50:28Z","receivedAt":"2008-08-20T18:50:28Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The code handles additionally \"refs/remotes//name\",\n\"remotes//name\", and \"refs//name\".\nTest cases included.\n\nSigned-off-by: Alex Riesen \n---\n\nJohannes has likable ideas :)\n\n builtin-checkout.c | 20 +++++++++++++++-----\n t/t7201-co.sh | 23 ++++++++++++++++++++++-\n 2 files changed, 37 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex e95eab9..20466e2 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -447,11 +447,21 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tchar *slash;\n \t\tif (!argc || !strcmp(argv[0], \"--\"))\n \t\t\tdie (\"--track needs a branch name\");\n-\t\tslash = strchr(argv[0], '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n-\t\t\tslash = strchr(slash + 1, '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n-\t\t\tslash = strchr(slash + 1, '/');\n+\t\tif (!prefixcmp(argv[0], \"remotes/\"))\n+\t\t\t/* skip the name of a remote */\n+\t\t\tslash = strchr(argv[0] + 8, '/');\n+\t\telse if (!prefixcmp(argv[0], \"refs/\")) {\n+\t\t\t/* skip namespaces, try use the names of their\n+\t\t\t * branches, but for the target namespace\n+\t\t\t * (heads) demand a new name. Also skip the\n+\t\t\t * first element in \"remotes\" namespace */\n+\t\t\tconst char *ns = argv[0] + 5;\n+\t\t\tslash = !prefixcmp(ns, \"heads/\") ? NULL:\n+\t\t\t\t!prefixcmp(ns, \"remotes/\") ?\n+\t\t\t\tstrchr(ns + 8, '/'): strchr(ns, '/');\n+\t\t} else\n+\t\t\t/* otherwise - just skip the first element */\n+\t\t\tslash = strchr(argv[0], '/');\n \t\tif (!slash || !slash[1])\n \t\t\tdie (\"Missing branch name; try -b\");\n \t\topts.new_branch = slash + 1;\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 943dd57..1dff84d 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -340,9 +340,30 @@ test_expect_success \\\n test_expect_success \\\n 'checkout with --track fakes a sensible -b ' '\n git update-ref refs/remotes/origin/koala/bear renamer &&\n+ git update-ref refs/new/koala/bear renamer &&\n+\n git checkout --track origin/koala/bear &&\n test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n- test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\"'\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\" &&\n+\n+ git checkout master && git branch -D koala/bear &&\n+\n+ git checkout --track refs/remotes/origin/koala/bear &&\n+ test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\" &&\n+\n+ git checkout master && git branch -D koala/bear &&\n+\n+ git checkout --track remotes/origin/koala/bear &&\n+ test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\" &&\n+\n+ git checkout master && git branch -D koala/bear &&\n+\n+ git checkout --track refs/new/koala/bear &&\n+ test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\"\n+'\n \n test_expect_success \\\n 'checkout with --track, but without -b, fails with too short tracked name' '\n-- \n1.6.0.22.g09248\n"},{"id":"87852","messageId":"alpine.DEB.1.00.0808202151320.24820@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15113","inReplyTo":"20080820185028.GA16626@blimp.local","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-20T19:52:23Z","receivedAt":"2008-08-20T19:52:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Aug 2008, Alex Riesen wrote:\n\n> The code handles additionally \"refs/remotes//name\",\n> \"remotes//name\", and \"refs//name\".\n> Test cases included.\n> \n> Signed-off-by: Alex Riesen \n> ---\n> \n> Johannes has likable ideas :)\n> \n> builtin-checkout.c | 20 +++++++++++++++-----\n> t/t7201-co.sh | 23 ++++++++++++++++++++++-\n> 2 files changed, 37 insertions(+), 6 deletions(-)\n> \n> diff --git a/builtin-checkout.c b/builtin-checkout.c\n> index e95eab9..20466e2 100644\n> --- a/builtin-checkout.c\n> +++ b/builtin-checkout.c\n> @@ -447,11 +447,21 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> \t\tchar *slash;\n> \t\tif (!argc || !strcmp(argv[0], \"--\"))\n> \t\t\tdie (\"--track needs a branch name\");\n> -\t\tslash = strchr(argv[0], '/');\n> -\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n> -\t\t\tslash = strchr(slash + 1, '/');\n> -\t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n> -\t\t\tslash = strchr(slash + 1, '/');\n\nWhy is this not enough? It strips refs/ if there is one, and remotes/ if \nthere is one (possibly after stripping refs/). No?\n\nPuzzled,\nDscho\n"},{"id":"87859","messageId":"20080820200440.GF16626@blimp.local","threadId":"15113","inReplyTo":"alpine.DEB.1.00.0808202151320.24820@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-08-20T20:04:40Z","receivedAt":"2008-08-20T20:04:40Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Johannes Schindelin, Wed, Aug 20, 2008 21:52:23 +0200:\n> On Wed, 20 Aug 2008, Alex Riesen wrote:\n> > -\t\tslash = strchr(argv[0], '/');\n> > -\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n> > -\t\t\tslash = strchr(slash + 1, '/');\n> > -\t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n> > -\t\t\tslash = strchr(slash + 1, '/');\n> \n> Why is this not enough? It strips refs/ if there is one, and remotes/ if \n> there is one (possibly after stripping refs/). No?\n> \n\nNo. It strips refs/ OR remotes/ (because of prefixcmp with argv[0]).\nAnd I still wanted refs//something...\n"},{"id":"87861","messageId":"alpine.DEB.1.00.0808202213340.24820@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"15113","inReplyTo":"20080820200440.GF16626@blimp.local","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-20T20:16:19Z","receivedAt":"2008-08-20T20:16:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Aug 2008, Alex Riesen wrote:\n\n> Johannes Schindelin, Wed, Aug 20, 2008 21:52:23 +0200:\n> > On Wed, 20 Aug 2008, Alex Riesen wrote:\n> > > -\t\tslash = strchr(argv[0], '/');\n> > > -\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n> > > -\t\t\tslash = strchr(slash + 1, '/');\n> > > -\t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n> > > -\t\t\tslash = strchr(slash + 1, '/');\n> > \n> > Why is this not enough? It strips refs/ if there is one, and remotes/ if \n> > there is one (possibly after stripping refs/). No?\n> > \n> \n> No. It strips refs/ OR remotes/ (because of prefixcmp with argv[0]).\n> And I still wanted refs//something...\n\nYes, you are correct. However, to fix my thinko, I deem this preferable:\n\n-- snipsnap --\n\n builtin-checkout.c | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex e95eab9..2a076cf 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -448,8 +448,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tif (!argc || !strcmp(argv[0], \"--\"))\n \t\t\tdie (\"--track needs a branch name\");\n \t\tslash = strchr(argv[0], '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n-\t\t\tslash = strchr(slash + 1, '/');\n+\t\tif (slash && !prefixcmp(argv[0], \"refs/\")) {\n+\t\t\targv[0] = slash + 1;\n+\t\t\tslash = strchr(argv[0], '/');\n+\t\t}\n \t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n \t\t\tslash = strchr(slash + 1, '/');\n \t\tif (!slash || !slash[1])\n"},{"id":"87869","messageId":"20080820202952.GH16626@blimp.local","threadId":"15113","inReplyTo":"alpine.DEB.1.00.0808202213340.24820@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-08-20T20:29:52Z","receivedAt":"2008-08-20T20:29:52Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Johannes Schindelin, Wed, Aug 20, 2008 22:16:19 +0200:\n> > \n> > No. It strips refs/ OR remotes/ (because of prefixcmp with argv[0]).\n> > And I still wanted refs//something...\n> \n> Yes, you are correct. However, to fix my thinko, I deem this preferable:\n> \n> -- snipsnap --\n> \n> builtin-checkout.c | 6 ++++--\n> 1 files changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin-checkout.c b/builtin-checkout.c\n> index e95eab9..2a076cf 100644\n> --- a/builtin-checkout.c\n> +++ b/builtin-checkout.c\n> @@ -448,8 +448,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> \t\tif (!argc || !strcmp(argv[0], \"--\"))\n> \t\t\tdie (\"--track needs a branch name\");\n> \t\tslash = strchr(argv[0], '/');\n> -\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n> -\t\t\tslash = strchr(slash + 1, '/');\n> +\t\tif (slash && !prefixcmp(argv[0], \"refs/\")) {\n> +\t\t\targv[0] = slash + 1;\n> +\t\t\tslash = strchr(argv[0], '/');\n> +\t\t}\n\nYes, I agree (and its shorter). The git-checkout manpage can be\nimproved, too (no DWIM is obvious, except may be for the implementor).\n"},{"id":"87896","messageId":"7vd4k3nx7m.fsf@gitster.siamese.dyndns.org","threadId":"15113","inReplyTo":"20080820202952.GH16626@blimp.local","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-20T22:22:21Z","receivedAt":"2008-08-20T22:22:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Johannes Schindelin, Wed, Aug 20, 2008 22:16:19 +0200:\n>> > \n>> > No. It strips refs/ OR remotes/ (because of prefixcmp with argv[0]).\n>> > And I still wanted refs//something...\n>> \n>> Yes, you are correct. However, to fix my thinko, I deem this preferable:\n>> \n>> -- snipsnap --\n>> \n>> builtin-checkout.c | 6 ++++--\n>> 1 files changed, 4 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/builtin-checkout.c b/builtin-checkout.c\n>> index e95eab9..2a076cf 100644\n>> --- a/builtin-checkout.c\n>> +++ b/builtin-checkout.c\n>> @@ -448,8 +448,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>> \t\tif (!argc || !strcmp(argv[0], \"--\"))\n>> \t\t\tdie (\"--track needs a branch name\");\n>> \t\tslash = strchr(argv[0], '/');\n>> -\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n>> -\t\t\tslash = strchr(slash + 1, '/');\n>> +\t\tif (slash && !prefixcmp(argv[0], \"refs/\")) {\n>> +\t\t\targv[0] = slash + 1;\n>> +\t\t\tslash = strchr(argv[0], '/');\n>> +\t\t}\n>\n> Yes, I agree (and its shorter). The git-checkout manpage can be\n> improved, too (no DWIM is obvious, except may be for the implementor).\n\nI think that makes sense. Care to send an appliable patch with\ndocumentation updates, please?\n"},{"id":"88011","messageId":"20080821172320.GA5119@blimp.local","threadId":"15113","inReplyTo":"7vd4k3nx7m.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-08-21T17:23:20Z","receivedAt":"2008-08-21T17:23:20Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The code handles additionally \"refs/remotes//name\",\n\"remotes//name\", and \"refs//name\".\n\nSigned-off-by: Alex Riesen \n---\nJunio C Hamano, Thu, Aug 21, 2008 00:22:21 +0200:\n> Alex Riesen writes:\n> > Johannes Schindelin, Wed, Aug 20, 2008 22:16:19 +0200:\n> >> > No. It strips refs/ OR remotes/ (because of prefixcmp with argv[0]).\n> >> > And I still wanted refs//something...\n> >> \n> >> Yes, you are correct. However, to fix my thinko, I deem this preferable:\n> >> \n> >\n> > Yes, I agree (and its shorter). The git-checkout manpage can be\n> > improved, too (no DWIM is obvious, except may be for the implementor).\n> \n> I think that makes sense. Care to send an appliable patch with\n> documentation updates, please?\n\nI just have another quiet evening, so I did that. Johannes, I changed\nyour fix a bit: I see that argv[0] is used later (or if I'm blind and\nit actually isn't, it may be used in future: I have a feeling that\nbuiltint-checkout.c will be popular place).\n\n Documentation/git-checkout.txt | 18 +++++++++++++++---\n builtin-checkout.c | 13 ++++++++-----\n t/t7201-co.sh | 23 ++++++++++++++++++++++-\n 3 files changed, 45 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 43d4502..d93a18f 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -64,9 +64,21 @@ OPTIONS\n \tgiven. Set it to `always` if you want this behavior when the\n \tstart-point is either a local or remote branch.\n +\n-If no '-b' option was given, a name will be made up for you, by stripping\n-the part up to the first slash of the tracked branch. For example, if you\n-called 'git checkout --track origin/next', the branch name will be 'next'.\n+If no '-b' option was given, the name of the new branch will be\n+derived from the remote branch, by attempting to guess the name of the\n+branch on remote system and use it. The algorithm will remove a\n+prefixed refs/ and the part after refs/ up to a slash (that part\n+usually being a branch namespace, which makes it confusing to use for\n+branch names). If the part after refs/ was remotes/, than a part past\n+it is stripped too (it is expected to be the name of remote system).\n+Otherwise, the part up to the first slash is removed.\n+The algorithm aborts either if the given name has no slashes in it or\n+if the part left after stripping the prefixes is empty. In this case,\n+the name should be provided with '-b'.\n++\n+For example, if you called 'git checkout --track origin/next', the\n+branch name will be 'next', as it will for 'git checkout --track\n+remotes/origin/next'.\n \n --no-track::\n \tIgnore the branch.autosetupmerge configuration variable.\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex e95eab9..214b0b2 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -445,12 +445,15 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t/* --track without -b should DWIM */\n \tif (opts.track && opts.track != -1 && !opts.new_branch) {\n \t\tchar *slash;\n-\t\tif (!argc || !strcmp(argv[0], \"--\"))\n+\t\tchar *argv0 = argv[0];\n+\t\tif (!argc || !strcmp(argv0, \"--\"))\n \t\t\tdie (\"--track needs a branch name\");\n-\t\tslash = strchr(argv[0], '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n-\t\t\tslash = strchr(slash + 1, '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n+\t\tslash = strchr(argv0, '/');\n+\t\tif (slash && !prefixcmp(argv0, \"refs/\")) {\n+\t\t\targv0 = slash + 1;\n+\t\t\tslash = strchr(argv0, '/');\n+\t\t}\n+\t\tif (slash && !prefixcmp(argv0, \"remotes/\"))\n \t\t\tslash = strchr(slash + 1, '/');\n \t\tif (!slash || !slash[1])\n \t\t\tdie (\"Missing branch name; try -b\");\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 943dd57..1dff84d 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -340,9 +340,30 @@ test_expect_success \\\n test_expect_success \\\n 'checkout with --track fakes a sensible -b ' '\n git update-ref refs/remotes/origin/koala/bear renamer &&\n+ git update-ref refs/new/koala/bear renamer &&\n+\n git checkout --track origin/koala/bear &&\n test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n- test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\"'\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\" &&\n+\n+ git checkout master && git branch -D koala/bear &&\n+\n+ git checkout --track refs/remotes/origin/koala/bear &&\n+ test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\" &&\n+\n+ git checkout master && git branch -D koala/bear &&\n+\n+ git checkout --track remotes/origin/koala/bear &&\n+ test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\" &&\n+\n+ git checkout master && git branch -D koala/bear &&\n+\n+ git checkout --track refs/new/koala/bear &&\n+ test \"refs/heads/koala/bear\" = \"$(git symbolic-ref HEAD)\" &&\n+ test \"$(git rev-parse HEAD)\" = \"$(git rev-parse renamer)\"\n+'\n \n test_expect_success \\\n 'checkout with --track, but without -b, fails with too short tracked name' '\n-- \n1.6.0.59.g8ae62\n"},{"id":"88087","messageId":"7vy72pd278.fsf@gitster.siamese.dyndns.org","threadId":"15113","inReplyTo":"20080821172320.GA5119@blimp.local","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-22T05:54:35Z","receivedAt":"2008-08-22T05:54:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> The code handles additionally \"refs/remotes//name\",\n> \"remotes//name\", and \"refs//name\".\n>\n> Signed-off-by: Alex Riesen \n> ---\n> ...\n> I just have another quiet evening, so I did that. Johannes, I changed\n> your fix a bit: I see that argv[0] is used later (or if I'm blind and\n> it actually isn't, it may be used in future: I have a feeling that\n> builtint-checkout.c will be popular place).\n\nThanks; the documentation looks good to me.\n\n> +\t\tchar *argv0 = argv[0];\n> +\t\tif (!argc || !strcmp(argv0, \"--\"))\n> \t\t\tdie (\"--track needs a branch name\");\n> +\t\tslash = strchr(argv0, '/');\n> +\t\tif (slash && !prefixcmp(argv0, \"refs/\")) {\n> +\t\t\targv0 = slash + 1;\n> +\t\t\tslash = strchr(argv0, '/');\n> +\t\t}\n> +\t\tif (slash && !prefixcmp(argv0, \"remotes/\"))\n> \t\t\tslash = strchr(slash + 1, '/');\n> \t\tif (!slash || !slash[1])\n> \t\t\tdie (\"Missing branch name; try -b\");\n\nI however wonder if this is clearer.\n\n * \"enum branch_track\" was unsigned; comparing equality with -1 was Ok but\n we couldn't say 0 < opts.track;\n\n * argv[] is an array of constant strings; cannot point into it with\n opts.newbranch without making the latter also a constant string.\n\n * the logic is to strip \"refs/\" if there is one, \"remotes/\" if there is\n one after that, and then strip one level after that unconditionally.\n No need to look explicitly for a slash while doing the first two steps.\n\n cache.h | 1 +\n builtin-checkout.c | 26 +++++++++++++-------------\n 2 files changed, 14 insertions(+), 13 deletions(-)\n\ndiff --git i/cache.h w/cache.h\nindex 928ae9f..a097a95 100644\n--- i/cache.h\n+++ w/cache.h\n@@ -451,6 +451,7 @@ enum safe_crlf {\n extern enum safe_crlf safe_crlf;\n \n enum branch_track {\n+\tBRANCH_TRACK_UNSPECIFIED = -1,\n \tBRANCH_TRACK_NEVER = 0,\n \tBRANCH_TRACK_REMOTE,\n \tBRANCH_TRACK_ALWAYS,\ndiff --git i/builtin-checkout.c w/builtin-checkout.c\nindex e95eab9..b380ad6 100644\n--- i/builtin-checkout.c\n+++ w/builtin-checkout.c\n@@ -157,7 +157,7 @@ struct checkout_opts {\n \tint force;\n \tint writeout_error;\n \n-\tchar *new_branch;\n+\tconst char *new_branch;\n \tint new_branch_log;\n \tenum branch_track track;\n };\n@@ -437,27 +437,27 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_default_config, NULL);\n \n-\topts.track = -1;\n+\topts.track = BRANCH_TRACK_UNSPECIFIED;\n \n \targc = parse_options(argc, argv, options, checkout_usage,\n \t\t\t PARSE_OPT_KEEP_DASHDASH);\n \n \t/* --track without -b should DWIM */\n-\tif (opts.track && opts.track != -1 && !opts.new_branch) {\n-\t\tchar *slash;\n-\t\tif (!argc || !strcmp(argv[0], \"--\"))\n+\tif (0 < opts.track && !opts.new_branch) {\n+\t\tconst char *argv0 = argv[0];\n+\t\tif (!argc || !strcmp(argv0, \"--\"))\n \t\t\tdie (\"--track needs a branch name\");\n-\t\tslash = strchr(argv[0], '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"refs/\"))\n-\t\t\tslash = strchr(slash + 1, '/');\n-\t\tif (slash && !prefixcmp(argv[0], \"remotes/\"))\n-\t\t\tslash = strchr(slash + 1, '/');\n-\t\tif (!slash || !slash[1])\n+\t\tif (!prefixcmp(argv0, \"refs/\"))\n+\t\t\targv0 += 5;\n+\t\tif (!prefixcmp(argv0, \"remotes/\"))\n+\t\t\targv0 += 8;\n+\t\targv0 = strchr(argv0, '/');\n+\t\tif (!argv0 || !argv0[1])\n \t\t\tdie (\"Missing branch name; try -b\");\n-\t\topts.new_branch = slash + 1;\n+\t\topts.new_branch = argv0 + 1;\n \t}\n \n-\tif (opts.track == -1)\n+\tif (opts.track == BRANCH_TRACK_UNSPECIFIED)\n \t\topts.track = git_branch_track;\n \n \tif (opts.force && opts.merge)\n"},{"id":"88110","messageId":"81b0412b0808220208i5de03604rec37081f0bc1cdd4@mail.gmail.com","threadId":"15113","inReplyTo":"7vy72pd278.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-08-22T09:08:01Z","receivedAt":"2008-08-22T09:08:01Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2008/8/22 Junio C Hamano :\n> I however wonder if this is clearer.\n\nIt is :)\n\n> * \"enum branch_track\" was unsigned; comparing equality with -1 was Ok but\n> we couldn't say 0 < opts.track;\n>\n> * argv[] is an array of constant strings; cannot point into it with\n> opts.newbranch without making the latter also a constant string.\n\nCleanup, but it is unrelated, isn't it?\n\n> * the logic is to strip \"refs/\" if there is one, \"remotes/\" if there is\n> one after that, and then strip one level after that unconditionally.\n> No need to look explicitly for a slash while doing the first two steps.\n\nMaybe that should go in documentation instead of the piece I wrote\n"},{"id":"88203","messageId":"7v7ia892bp.fsf@gitster.siamese.dyndns.org","threadId":"15113","inReplyTo":"81b0412b0808220208i5de03604rec37081f0bc1cdd4@mail.gmail.com","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-22T21:17:46Z","receivedAt":"2008-08-22T21:17:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" writes:\n\n> 2008/8/22 Junio C Hamano :\n>> I however wonder if this is clearer.\n>\n> It is :)\n>\n>> * \"enum branch_track\" was unsigned; comparing equality with -1 was Ok but\n>> we couldn't say 0 < opts.track;\n>>\n>> * argv[] is an array of constant strings; cannot point into it with\n>> opts.newbranch without making the latter also a constant string.\n>\n> Cleanup, but it is unrelated, isn't it?\n\nThe code did not compile without it for me as I sometimes use -Werror.\n\n>> * the logic is to strip \"refs/\" if there is one, \"remotes/\" if there is\n>> one after that, and then strip one level after that unconditionally.\n>> No need to look explicitly for a slash while doing the first two steps.\n>\n> Maybe that should go in documentation instead of the piece I wrote\n\nOh I think what you wrote is fine. I tried to be more descriptive than\nsimply saying \"No need to look explicitly for a slash\" while explaining\nthe changes.\n"},{"id":"88234","messageId":"76718490808221633y1da9d790j53df67f5c968915e@mail.gmail.com","threadId":"15113","inReplyTo":"20080821172320.GA5119@blimp.local","subject":"Re: [PATCH] Extend \"checkout --track\" DWIM to support more cases","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2008-08-22T23:33:44Z","receivedAt":"2008-08-22T23:33:44Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Aug 21, 2008 at 1:23 PM, Alex Riesen wrote:\n> +If no '-b' option was given, the name of the new branch will be\n> +derived from the remote branch, by attempting to guess the name of the\n> +branch on remote system and use it. The algorithm will remove a\n> +prefixed refs/ and the part after refs/ up to a slash (that part\n> +usually being a branch namespace, which makes it confusing to use for\n> +branch names). If the part after refs/ was remotes/, than a part past\n\ns/than/then/\n\nj.\n"}]}