{"thread":{"id":"42456","subject":"[PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","startedAt":"2016-05-26T11:54:22Z","lastAt":"2016-05-27T18:01:31Z","messageCount":7,"participants":["Jordan DE GEA","Junio C Hamano","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287568","messageId":"1464263662-1290-1-git-send-email-jordan.de-gea@grenoble-inp.org","threadId":"42456","inReplyTo":"vpqshx5cb51.fsf@anie.imag.fr","subject":"[PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Jordan DE GEA","fromEmail":"jordan.de-gea@grenoble-inp.org","sentAt":"2016-05-26T11:54:22Z","receivedAt":"2016-05-26T11:54:22Z","isPatch":true,"sender":{"key":"jordan.de-gea@grenoble-inp.org","avatar":null},"body":"From: Jordan DE GEA \n\nSince `git worktree add` uses `git checkout` when `[]` is used,\nand `git checkout -` is already supported, it makes sense to allow the\nsame shortcut in `git worktree add`.\n\nSigned-off-by: Matthieu Moy \nSigned-off-by: Jordan DE GEA \n---\n Documentation/git-worktree.txt | 3 ++-\n builtin/worktree.c | 3 +++\n t/t2025-worktree-add.sh | 18 ++++++++++++++++++\n 3 files changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex c622345..48e5fdf 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -48,7 +48,8 @@ add []::\n \n Create `` and checkout `` into it. The new working directory\n is linked to the current repository, sharing everything except working\n-directory specific files such as HEAD, index, etc.\n+directory specific files such as HEAD, index, etc. The last branch you \n+were on can be specify with `-` as `` which is synonymous with `\"@{-1}\"`.\n +\n If `` is omitted and neither `-b` nor `-B` nor `--detached` used,\n then, as a convenience, a new branch based at HEAD is created automatically,\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d8e3795..d800d47 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -340,6 +340,9 @@ static int add(int ac, const char **av, const char *prefix)\n \tpath = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];\n \tbranch = ac < 2 ? \"HEAD\" : av[1];\n \n+\tif (!strcmp(branch, \"-\"))\n+\t\tbranch = \"@{-1}\";\n+\n \topts.force_new_branch = !!new_branch_force;\n \tif (opts.force_new_branch) {\n \t\tstruct strbuf symref = STRBUF_INIT;\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 3acb992..b713efb 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -18,6 +18,24 @@ test_expect_success '\"add\" an existing empty worktree' '\n \tgit worktree add --detach existing_empty master\n '\n \n+test_expect_success '\"add\" using shorthand - fails when no previous branch' '\n+\ttest_must_fail git worktree add existing -\n+'\n+\n+test_expect_success '\"add\" using - shorthand' '\n+\tgit checkout -b newbranch &&\n+\techo hello >myworld &&\n+\tgit add myworld &&\n+\tgit commit -m myworld &&\n+\tgit checkout master &&\n+\tgit worktree add short-hand - &&\n+\tcd short-hand &&\n+\ttest $(git rev-parse --symbolic-full-name HEAD) = \"refs/heads/newbranch\"\n+\tbranch=$(cd short-hand && git rev-parse --symbolic-full-name HEAD) &&\n+\ttest \"$branch\" = refs/heads/newbranch &&\n+\tcd ..\n+'\n+\n test_expect_success '\"add\" refuses to checkout locked branch' '\n \ttest_must_fail git worktree add zere master &&\n \t! test -d zere &&\n-- \n2.7.4 (Apple Git-66)\n"},{"id":"287606","messageId":"xmqqshx4vedr.fsf@gitster.mtv.corp.google.com","threadId":"42456","inReplyTo":"1464263662-1290-1-git-send-email-jordan.de-gea@grenoble-inp.org","subject":"Re: [PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-26T18:54:40Z","receivedAt":"2016-05-26T18:54:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jordan DE GEA writes:\n\n> From: Jordan DE GEA \n>\n> Since `git worktree add` uses `git checkout` when `[]` is used,\n> and `git checkout -` is already supported, it makes sense to allow the\n> same shortcut in `git worktree add`.\n\nOK.\n\n>\n> Signed-off-by: Matthieu Moy \n> Signed-off-by: Jordan DE GEA \n> ---\n> Documentation/git-worktree.txt | 3 ++-\n> builtin/worktree.c | 3 +++\n> t/t2025-worktree-add.sh | 18 ++++++++++++++++++\n> 3 files changed, 23 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n> index c622345..48e5fdf 100644\n> --- a/Documentation/git-worktree.txt\n> +++ b/Documentation/git-worktree.txt\n> @@ -48,7 +48,8 @@ add []::\n> \n> Create `` and checkout `` into it. The new working directory\n> is linked to the current repository, sharing everything except working\n> -directory specific files such as HEAD, index, etc.\n> +directory specific files such as HEAD, index, etc. The last branch you \n> +were on can be specify with `-` as `` which is synonymous with `\"@{-1}\"`.\n\nYou meant \"can be specified\", I think. Fixing it would make the\nline a bit too long, so fold it around the word \"synonymous\".\n\n> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n> index 3acb992..b713efb 100755\n> --- a/t/t2025-worktree-add.sh\n> +++ b/t/t2025-worktree-add.sh\n> @@ -18,6 +18,24 @@ test_expect_success '\"add\" an existing empty worktree' '\n> \tgit worktree add --detach existing_empty master\n> '\n> \n> +test_expect_success '\"add\" using shorthand - fails when no previous branch' '\n> +\ttest_must_fail git worktree add existing -\n> +'\n\nJust an observation, but the error message we would see here might\nbe interesting.\n\n> +test_expect_success '\"add\" using - shorthand' '\n> +\tgit checkout -b newbranch &&\n> +\techo hello >myworld &&\n> +\tgit add myworld &&\n> +\tgit commit -m myworld &&\n> +\tgit checkout master &&\n> +\tgit worktree add short-hand - &&\n\n\n> +\tcd short-hand &&\n> +\ttest $(git rev-parse --symbolic-full-name HEAD) = \"refs/heads/newbranch\"\n\nBroken &&-chain.\n\n> +\tbranch=$(cd short-hand && git rev-parse --symbolic-full-name HEAD) &&\n> +\ttest \"$branch\" = refs/heads/newbranch &&\n> +\tcd ..\n\nIf any of the command between \"cd short-hand\" and \"cd ..\" failed,\nafter correcting the broken &&-chain, the next test will end up\nrunning in short-hand directory, which it is not expecting. A\ncanonical way to avoid this problem is to replace the above with:\n\n\t...\n git worktree add short-hand - &&\n (\n\t\tcd short-hand &&\n ...\n test \"$branch\" = refs/heads/newbranch\n\t)\n\nIn this particular case, alternatively, you could also do something\nlike this:\n\n git worktree add short-hand - &&\n\techo refs/heads/newbranch >expect &&\n\tgit -C short-hand rev-parse --symbolic-full-name HEAD >actual &&\n\ttest_cmp expect actual\n\nand it would be sufficient.\n\nIt is not immediately obvious to me why you have two copies of the\nsame test in your patch to see where HEAD points at. If the reason\nis because you suspect that \"git -C $there\" form may give subtly\ndifferent behaviour and wanted to test both, then you could do\nsomething like this:\n\n git worktree add short-hand - &&\n\techo refs/heads/newbranch >expect &&\n\tgit -C short-hand rev-parse --symbolic-full-name HEAD >actual &&\n\ttest_cmp expect actual &&\n\t(cd short-hand && git rev-parse --symbolic-full-name HEAD) >actual &&\n\ttest_cmp expect actual\n\nbut I do not think that is necessary. This test is not about \"does\nrev-parse --symbolic-full-name work correctly with 'git -C $there'?\"\n\nThanks.\n"},{"id":"287665","messageId":"vpqvb1zrh8s.fsf@anie.imag.fr","threadId":"42456","inReplyTo":"xmqqshx4vedr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-27T09:18:43Z","receivedAt":"2016-05-27T09:18:43Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Jordan DE GEA writes:\n>\n>> +\tbranch=$(cd short-hand && git rev-parse --symbolic-full-name HEAD) &&\n>> +\ttest \"$branch\" = refs/heads/newbranch &&\n>> +\tcd ..\n>\n> If any of the command between \"cd short-hand\" and \"cd ..\" failed,\n> after correcting the broken &&-chain, the next test will end up\n> running in short-hand directory, which it is not expecting. A\n> canonical way to avoid this problem is to replace the above with:\n>\n> \t...\n> git worktree add short-hand - &&\n> (\n> \t\tcd short-hand &&\n> ...\n> test \"$branch\" = refs/heads/newbranch\n> \t)\n\nActually, $(...) implicitly does a subshell, so the \"cd ..\" was just\nuseless.\n\n> \tgit -C short-hand rev-parse --symbolic-full-name HEAD >actual &&\n\nIndeed, git -C is an even better way to say \"cd .. && git ...\"\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287669","messageId":"9975E0C6-7BCC-4D80-BBC6-2C0BAB13D3E6@grenoble-inp.org","threadId":"42456","inReplyTo":"xmqqshx4vedr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Jordan DE GEA","fromEmail":"jordan.de-gea@grenoble-inp.org","sentAt":"2016-05-27T12:43:47Z","receivedAt":"2016-05-27T12:43:47Z","isPatch":true,"sender":{"key":"jordan.de-gea@grenoble-inp.org","avatar":null},"body":">> +test_expect_success '\"add\" using shorthand - fails when no previous branch' '\n>> +\ttest_must_fail git worktree add existing -\n>> +'\n> \n> Just an observation, but the error message we would see here might\n> be interesting.\n\nOf course, that’s useful to be sure of the error, I will do in next preroll.\n\n> \n>> +\tbranch=$(cd short-hand && git rev-parse --symbolic-full-name HEAD) &&\n>> +\ttest \"$branch\" = refs/heads/newbranch &&\n>> +\tcd ..\n> \n> If any of the command between \"cd short-hand\" and \"cd ..\" failed,\n> after correcting the broken &&-chain, the next test will end up\n> running in short-hand directory, which it is not expecting. A\n> canonical way to avoid this problem is to replace the above with:\n> \n> \t...\n> git worktree add short-hand - &&\n> (\n> \t\tcd short-hand &&\n> ...\n> test \"$branch\" = refs/heads/newbranch\n> \t)\n> \n> In this particular case, alternatively, you could also do something\n> like this:\n> \n> git worktree add short-hand - &&\n> \techo refs/heads/newbranch >expect &&\n> \tgit -C short-hand rev-parse --symbolic-full-name HEAD >actual &&\n> \ttest_cmp expect actual\nYes, that’s a good idea. I take these lines. \n"},{"id":"287670","messageId":"1464355028-10574-1-git-send-email-jordan.de-gea@grenoble-inp.org","threadId":"42456","inReplyTo":"9975E0C6-7BCC-4D80-BBC6-2C0BAB13D3E6@grenoble-inp.org","subject":"[PATCH v2] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Jordan DE GEA","fromEmail":"jordan.de-gea@grenoble-inp.org","sentAt":"2016-05-27T13:17:08Z","receivedAt":"2016-05-27T13:17:08Z","isPatch":true,"sender":{"key":"jordan.de-gea@grenoble-inp.org","avatar":null},"body":"Since `git worktree add` uses `git checkout` when `[]` is used,\nand `git checkout -` is already supported, it makes sense to allow the\nsame shortcut in `git worktree add`.\n\nSigned-off-by: Matthieu Moy \nSigned-off-by: Jordan DE GEA \n---\nChanges since v1:\n - improved tests.\n - improved documentation.\n\n Documentation/git-worktree.txt | 3 ++-\n builtin/worktree.c | 3 +++\n t/t2025-worktree-add.sh | 18 ++++++++++++++++++\n 3 files changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex c622345..8358a3e 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -48,7 +48,8 @@ add []::\n \n Create `` and checkout `` into it. The new working directory\n is linked to the current repository, sharing everything except working\n-directory specific files such as HEAD, index, etc.\n+directory specific files such as HEAD, index, etc. `-` may also be \n+specified as ``; it is synonymous with `@{-1}`.\n +\n If `` is omitted and neither `-b` nor `-B` nor `--detached` used,\n then, as a convenience, a new branch based at HEAD is created automatically,\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d8e3795..d800d47 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -340,6 +340,9 @@ static int add(int ac, const char **av, const char *prefix)\n \tpath = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];\n \tbranch = ac < 2 ? \"HEAD\" : av[1];\n \n+\tif (!strcmp(branch, \"-\"))\n+\t\tbranch = \"@{-1}\";\n+\n \topts.force_new_branch = !!new_branch_force;\n \tif (opts.force_new_branch) {\n \t\tstruct strbuf symref = STRBUF_INIT;\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 3acb992..c4f5177 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -18,6 +18,24 @@ test_expect_success '\"add\" an existing empty worktree' '\n \tgit worktree add --detach existing_empty master\n '\n \n+test_expect_success '\"add\" using shorthand - fails when no previous branch' '\n+\techo \"fatal: invalid reference: @{-1}\" >expect &&\n+\ttest_must_fail git worktree add existing_short - 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '\"add\" using - shorthand' '\n+\tgit checkout -b newbranch &&\n+\techo hello >myworld &&\n+\tgit add myworld &&\n+\tgit commit -m myworld &&\n+\tgit checkout master &&\n+\tgit worktree add short-hand - &&\n+\techo refs/heads/newbranch >expect &&\n+\tgit -C short-hand rev-parse --symbolic-full-name HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '\"add\" refuses to checkout locked branch' '\n \ttest_must_fail git worktree add zere master &&\n \t! test -d zere &&\n-- \n2.7.4 (Apple Git-66)\n"},{"id":"287699","messageId":"xmqqvb1zqtgp.fsf@gitster.mtv.corp.google.com","threadId":"42456","inReplyTo":"vpqvb1zrh8s.fsf@anie.imag.fr","subject":"Re: [PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-27T17:52:22Z","receivedAt":"2016-05-27T17:52:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> Jordan DE GEA writes:\n>>\n>>> +\tbranch=$(cd short-hand && git rev-parse --symbolic-full-name HEAD) &&\n>>> +\ttest \"$branch\" = refs/heads/newbranch &&\n>>> +\tcd ..\n>>\n>> If any of the command between \"cd short-hand\" and \"cd ..\" failed,\n>> after correcting the broken &&-chain, the next test will end up\n>> running in short-hand directory, which it is not expecting. A\n>> canonical way to avoid this problem is to replace the above with:\n>>\n>> \t...\n>> git worktree add short-hand - &&\n>> (\n>> \t\tcd short-hand &&\n>> ...\n>> test \"$branch\" = refs/heads/newbranch\n>> \t)\n>\n> Actually, $(...) implicitly does a subshell, so the \"cd ..\" was just\n> useless.\n\nYou trimmed my message a bit too aggressively while composing your\nresponse, and I think that is what ended up confusing you. \n\nHere is what I wrote:\n\n| > +\tcd short-hand &&\n| > +\ttest $(git rev-parse --symbolic-full-name HEAD) = \"refs/heads/newbranch\"\n| \n| Broken &&-chain.\n| \n| > +\tbranch=$(cd short-hand && git rev-parse --symbolic-full-name HEAD) &&\n| > +\ttest \"$branch\" = refs/heads/newbranch &&\n| > +\tcd ..\n| \n| If any of the command between \"cd short-hand\" and \"cd ..\" failed,\n| ...\n\nThe problematic \"cd short-hand\" is the one a few lines above where\nyou started quoting, not the one you saw in the $().\n"},{"id":"287701","messageId":"xmqqr3cnqt1g.fsf@gitster.mtv.corp.google.com","threadId":"42456","inReplyTo":"9975E0C6-7BCC-4D80-BBC6-2C0BAB13D3E6@grenoble-inp.org","subject":"Re: [PATCH] worktree: allow \"-\" short-hand for @{-1} in add command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-27T18:01:31Z","receivedAt":"2016-05-27T18:01:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jordan DE GEA writes:\n\n>>> +test_expect_success '\"add\" using shorthand - fails when no previous branch' '\n>>> +\ttest_must_fail git worktree add existing -\n>>> +'\n>> \n>> Just an observation, but the error message we would see here might\n>> be interesting.\n>\n> Of course, that’s useful to be sure of the error, I will do in next preroll.\n\nThat was not what I meant. The exit status being non-zero is what\nwe primarily care about. The exact phrasing of the error message is\nmuch less important and in general we shouldn't enforce \"the error\nmessage must remain so\" in the test.\n\nIf you observe the error message from this test, e.g. by running it\nwith \"-v\", I suspect that you would see the message would complain\nabout \"@{-1}\".\n\nI just wanted to make sure that you saw it and thought about its\nramifications.\n\nIt is perfectly fine by me (others might disagree, though) if your\nconclusion after thinking about it is \"Even though the user may be\nsurprised to get complaints for \"@{-1}\" that she never gave to the\ncommand (she gave \"-\"), because we clearly document that \"-\" is a\nmere synonym/short-hand for @{-1}, it is OK\". I still want to see\nthat behaviour justified in the proposed commit log message.\n\nAnd that is why I said it \"might be interesting\".\n"}]}