{"thread":{"id":"30188","subject":"[PATCH] rev-parse --show-prefix: add in trailing newline","startedAt":"2012-04-09T13:27:56Z","lastAt":"2012-04-10T22:51:44Z","messageCount":9,"participants":["Ross Lagerwall","Junio C Hamano","Jonathan Nieder","Andreas Schwab","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"188794","messageId":"1333978076-29968-1-git-send-email-rosslagerwall@gmail.com","threadId":"30188","inReplyTo":null,"subject":"[PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Ross Lagerwall","fromEmail":"rosslagerwall@gmail.com","sentAt":"2012-04-09T13:27:56Z","receivedAt":"2012-04-09T13:27:56Z","isPatch":true,"sender":{"key":"rosslagerwall@gmail.com","avatar":"https://avatars.githubusercontent.com/u/878055?v=4"},"body":"Print out a trailing newline when --show-prefix is run with cwd\nat the top level of the tree which results in an empty prefix.\nBehavior is now like --show-cdup.\n\nFixes an expected failure in t1501.\n\nSigned-off-by: Ross Lagerwall \n---\n builtin/rev-parse.c | 2 ++\n t/t1501-worktree.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 98d1cbe..733f626 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tif (!strcmp(arg, \"--show-prefix\")) {\n \t\t\t\tif (prefix)\n \t\t\t\t\tputs(prefix);\n+\t\t\t\telse\n+\t\t\t\t\tputchar('\\n');\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-cdup\")) {\ndiff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh\nindex e661147..8f36aa9 100755\n--- a/t/t1501-worktree.sh\n+++ b/t/t1501-worktree.sh\n@@ -68,7 +68,7 @@ test_expect_success 'inside work tree' '\n \t)\n '\n \n-test_expect_failure 'empty prefix is actually written out' '\n+test_expect_success 'empty prefix is actually written out' '\n \techo >expected &&\n \t(\n \t\tcd work &&\n-- \n1.7.10\n"},{"id":"188810","messageId":"7v1unwwwov.fsf@alter.siamese.dyndns.org","threadId":"30188","inReplyTo":"1333978076-29968-1-git-send-email-rosslagerwall@gmail.com","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-09T19:07:44Z","receivedAt":"2012-04-09T19:07:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ross Lagerwall writes:\n\n> diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh\n> index e661147..8f36aa9 100755\n> --- a/t/t1501-worktree.sh\n> +++ b/t/t1501-worktree.sh\n> @@ -68,7 +68,7 @@ test_expect_success 'inside work tree' '\n> \t)\n> '\n> \n> -test_expect_failure 'empty prefix is actually written out' '\n> +test_expect_success 'empty prefix is actually written out' '\n> \techo >expected &&\n> \t(\n> \t\tcd work &&\n\nfc68875 (t1501 (rev-parse): clarify, 2010-07-24) says it is a \"possible\nbug\", but I do not think the expectation of the new test added by that\ncommit is entirely correct.\n\nBecause the assignment will strip the trailing LF away adding an extra\nnewline at the end would not matter in practice in many scripts, e.g.\n\n\ta=$(git rev-parse --show-prefix)\n if test -z \"$a\"\n then\n\t\techo we are at toplevel\n\tfi\n\nor\n\n\tpfx=$(git rev-parse --show-prefix)\n\tcd_to_toplevel\n\tfor arg\n do\n\t\tcase \"$arg\" in\n /*)\n \tuser refers to \"$arg\" that is a full path ;;\n\t\t*)\n\t\t\tuser refers to \"$pfx$arg\" ;;\n\t\tesac\n\tdone\n\nBut it will break existing scripts if they expect the following to work:\n\n\tfile=\"$(git rev-parse --show-prefix)/$1\"\n cd_to_toplevel\n test -f \"$file\"\n"},{"id":"188836","messageId":"4F83C589.20609@gmail.com","threadId":"30188","inReplyTo":"7v1unwwwov.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Ross Lagerwall","fromEmail":"rosslagerwall@gmail.com","sentAt":"2012-04-10T05:30:49Z","receivedAt":"2012-04-10T05:30:49Z","isPatch":true,"sender":{"key":"rosslagerwall@gmail.com","avatar":"https://avatars.githubusercontent.com/u/878055?v=4"},"body":"On 2012/04/09 21:07, Junio C Hamano wrote:\n> fc68875 (t1501 (rev-parse): clarify, 2010-07-24) says it is a \"possible\n> bug\", but I do not think the expectation of the new test added by that\n> commit is entirely correct.\n>\n> Because the assignment will strip the trailing LF away adding an extra\n> newline at the end would not matter in practice in many scripts, e.g.\n>\n\nTrue, and the current behavior does exactly what the man page says:\n\"When the command is invoked from a subdirectory, show the path of the \ncurrent directory relative to the top-level directory.\"\n\nSo then perhaps the test should be removed?\n\nAlso, --show-cdup has a similar description for the man page but does \noutput a newline when invoked from the root dir. Should this behavior be \nmade consistent?\n\n-- \nRoss Lagerwall\n"},{"id":"188862","messageId":"20120410140421.GC2444@burratino","threadId":"30188","inReplyTo":"7v1unwwwov.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-10T14:04:21Z","receivedAt":"2012-04-10T14:04:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> But it will break existing scripts if they expect the following to work:\n>\n> \tfile=\"$(git rev-parse --show-prefix)/$1\"\n> cd_to_toplevel\n> test -f \"$file\"\n\nHuh? I tried this:\n\n\t$ foo=\"$(printf '\\n')/bar\"\n\t$ printf '%s\\n' \"$foo\"\n\t/bar\n\nIf that didn't work, it would mean that \"git rev-parse --show-prefix\"\nwould never work when the prefix is nonempty, because it is followed\nby a newline.\n"},{"id":"188863","messageId":"m2vcl7ir86.fsf@igel.home","threadId":"30188","inReplyTo":"7v1unwwwov.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-10T14:41:45Z","receivedAt":"2012-04-10T14:41:45Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Junio C Hamano writes:\n\n> Because the assignment will strip the trailing LF away\n\nNo, it doesn't. Command substitution does.\n\n> But it will break existing scripts if they expect the following to work:\n>\n> \tfile=\"$(git rev-parse --show-prefix)/$1\"\n> cd_to_toplevel\n> test -f \"$file\"\n\nThis is already broken because of the extra slash.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"188879","messageId":"7v8vi3sgx8.fsf@alter.siamese.dyndns.org","threadId":"30188","inReplyTo":"m2vcl7ir86.fsf@igel.home","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-10T16:14:11Z","receivedAt":"2012-04-10T16:14:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab writes:\n\n> This is already broken because of the extra slash.\n\nYeah, you are right.\n"},{"id":"188888","messageId":"20120410173616.GA4300@burratino","threadId":"30188","inReplyTo":"1333978076-29968-1-git-send-email-rosslagerwall@gmail.com","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-10T17:36:16Z","receivedAt":"2012-04-10T17:36:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(cc-ing Duy)\nHi,\n\nRoss Lagerwall wrote:\n\n> --- a/builtin/rev-parse.c\n> +++ b/builtin/rev-parse.c\n> @@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n> \t\t\tif (!strcmp(arg, \"--show-prefix\")) {\n> \t\t\t\tif (prefix)\n> \t\t\t\t\tputs(prefix);\n> +\t\t\t\telse\n> +\t\t\t\t\tputchar('\\n');\n> \t\t\t\tcontinue;\n\nThis makes the output more consistent but it is a little puzzling how\nit does that. Why is prefix NULL instead of \"\" when we are at the\ntoplevel of the worktree?\n\nThanks,\nJonathan\n"},{"id":"188890","messageId":"7vty0rpj2i.fsf@alter.siamese.dyndns.org","threadId":"30188","inReplyTo":"20120410173616.GA4300@burratino","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-10T17:56:05Z","receivedAt":"2012-04-10T17:56:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> (cc-ing Duy)\n> Hi,\n>\n> Ross Lagerwall wrote:\n>\n>> --- a/builtin/rev-parse.c\n>> +++ b/builtin/rev-parse.c\n>> @@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n>> \t\t\tif (!strcmp(arg, \"--show-prefix\")) {\n>> \t\t\t\tif (prefix)\n>> \t\t\t\t\tputs(prefix);\n>> +\t\t\t\telse\n>> +\t\t\t\t\tputchar('\\n');\n>> \t\t\t\tcontinue;\n>\n> This makes the output more consistent but it is a little puzzling how\n> it does that. Why is prefix NULL instead of \"\" when we are at the\n> toplevel of the worktree?\n\nInteresting point ;-)\n"},{"id":"188914","messageId":"CACsJy8Dc9xi=z7x4OfeAWHSeuUOFFVFf93GFOfauVB7c=_cZ1Q@mail.gmail.com","threadId":"30188","inReplyTo":"20120410173616.GA4300@burratino","subject":"Re: [PATCH] rev-parse --show-prefix: add in trailing newline","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-10T22:51:44Z","receivedAt":"2012-04-10T22:51:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2012/4/11 Jonathan Nieder :\n> (cc-ing Duy)\n> Hi,\n>\n> Ross Lagerwall wrote:\n>\n>> --- a/builtin/rev-parse.c\n>> +++ b/builtin/rev-parse.c\n>> @@ -634,6 +634,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n>>                       if (!strcmp(arg, \"--show-prefix\")) {\n>>                               if (prefix)\n>>                                       puts(prefix);\n>> +                             else\n>> +                                     putchar('\\n');\n>>                               continue;\n>\n> This makes the output more consistent but it is a little puzzling how\n> it does that.  Why is prefix NULL instead of \"\" when we are at the\n> toplevel of the worktree?\n\nIt dates back to the first version of setup_git_directory() in d288a70\n([PATCH] Make \"git diff\" work inside relative subdirectories -\n2005-08-16). If we get \"\", setup_git_.. converts it to NULL. Although\nI think Linus made a mistake in the first version of \"rev-parse\n--git-dir\" doing \"puts(prefix);\" unconditionally in the same commit.\nJunio fixed --git-dir in 4866ccf (Rationalize output selection in\nrev-parse. - 2005-08-24) with the \"if (prefix)\" clause and should have\nadded the \"else putchar('\\n');\" there too, I think.\n-- \nDuy\n"}]}