{"thread":{"id":"36163","subject":"[PATCH] test-lib.sh: use printf instead of echo","startedAt":"2014-03-14T23:57:36Z","lastAt":"2014-03-20T16:42:00Z","messageCount":14,"participants":["Uwe Storbeck","Jonathan Nieder","Johannes Sixt","Junio C Hamano","David Kastrup"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236763","messageId":"20140314235735.GA6959@ibr.ch","threadId":"36163","inReplyTo":null,"subject":"[PATCH] test-lib.sh: use printf instead of echo","fromName":"Uwe Storbeck","fromEmail":"uwe@ibr.ch","sentAt":"2014-03-14T23:57:36Z","receivedAt":"2014-03-14T23:57:36Z","isPatch":true,"sender":{"key":"uwe@ibr.ch","avatar":null},"body":"when variables may contain backslash sequences.\n\nBackslash sequences are interpreted as control characters\nby the echo command of some shells (e.g. dash).\n\nSigned-off-by: Uwe Storbeck <uwe@ibr.ch>\n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1531c24..8209204 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n \n if test \"$help\" = \"t\"\n then\n-\techo \"$test_description\"\n+\tprintf '%s\\n' \"$test_description\"\n \texit 0\n fi\n \n@@ -328,7 +328,7 @@ test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n-\techo \"$@\" | sed -e 's/^/#\t/'\n+\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n }\n \n-- \n1.9.0\n"},{"id":"236766","messageId":"20140315001855.GK15625@google.com","threadId":"36163","inReplyTo":"20140314235735.GA6959@ibr.ch","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-03-15T00:18:55Z","receivedAt":"2014-03-15T00:18:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Uwe Storbeck wrote:\n\n> Backslash sequences are interpreted as control characters\n> by the echo command of some shells (e.g. dash).\n\nThis has bothered me for a while but never enough to do anything about\nit.  Thanks for fixing it.\n\n> Signed-off-by: Uwe Storbeck <uwe@ibr.ch>\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\n(patch left unsnipped for reference)\n> ---\n>  t/test-lib.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1531c24..8209204 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n>  \n>  if test \"$help\" = \"t\"\n>  then\n> -\techo \"$test_description\"\n> +\tprintf '%s\\n' \"$test_description\"\n>  \texit 0\n>  fi\n>  \n> @@ -328,7 +328,7 @@ test_failure_ () {\n>  \ttest_failure=$(($test_failure + 1))\n>  \tsay_color error \"not ok $test_count - $1\"\n>  \tshift\n> -\techo \"$@\" | sed -e 's/^/#\t/'\n> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n>  \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n>  }\n>  \n"},{"id":"236787","messageId":"53241688.2050604@kdbg.org","threadId":"36163","inReplyTo":"20140314235735.GA6959@ibr.ch","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-03-15T08:59:52Z","receivedAt":"2014-03-15T08:59:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 15.03.2014 00:57, schrieb Uwe Storbeck:\n> when variables may contain backslash sequences.\n> \n> Backslash sequences are interpreted as control characters\n> by the echo command of some shells (e.g. dash).\n> \n> Signed-off-by: Uwe Storbeck <uwe@ibr.ch>\n> ---\n>  t/test-lib.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1531c24..8209204 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n>  \n>  if test \"$help\" = \"t\"\n>  then\n> -\techo \"$test_description\"\n> +\tprintf '%s\\n' \"$test_description\"\n>  \texit 0\n>  fi\n>  \n> @@ -328,7 +328,7 @@ test_failure_ () {\n>  \ttest_failure=$(($test_failure + 1))\n>  \tsay_color error \"not ok $test_count - $1\"\n>  \tshift\n> -\techo \"$@\" | sed -e 's/^/#\t/'\n> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n\nThis should be\n\n\tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n\n>  \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n>  }\n>  \n> \n"},{"id":"236893","messageId":"20140317183807.GB1971@ibr.ch","threadId":"36163","inReplyTo":"53241688.2050604@kdbg.org","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Uwe Storbeck","fromEmail":"uwe@ibr.ch","sentAt":"2014-03-17T18:38:08Z","receivedAt":"2014-03-17T18:38:08Z","isPatch":true,"sender":{"key":"uwe@ibr.ch","avatar":null},"body":"On Mar 15, Johannes Sixt wrote:\n> > -\techo \"$@\" | sed -e 's/^/#\t/'\n> > +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n> \n> This should be\n> \n> \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n\nRight, that should be $* to always be one argument for the format\npattern.\n\nThanks\n\nUwe\n"},{"id":"236894","messageId":"20140317183917.GA28675@ibr.ch","threadId":"36163","inReplyTo":"20140314235735.GA6959@ibr.ch","subject":"[PATCH v2] test-lib.sh: use printf instead of echo","fromName":"Uwe Storbeck","fromEmail":"uwe@ibr.ch","sentAt":"2014-03-17T18:39:17Z","receivedAt":"2014-03-17T18:39:17Z","isPatch":true,"sender":{"key":"uwe@ibr.ch","avatar":null},"body":"when variables may contain backslash sequences.\n\nBackslash sequences are interpreted as control characters\nby the echo command of some shells (e.g. dash).\n\nSigned-off-by: Uwe Storbeck <uwe@ibr.ch>\n---\n\nChanged $@ to $* in printf.\nThanks to Johannes Sixt to point that out.\n\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1531c24..3c7cb1d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n \n if test \"$help\" = \"t\"\n then\n-\techo \"$test_description\"\n+\tprintf '%s\\n' \"$test_description\"\n \texit 0\n fi\n \n@@ -328,7 +328,7 @@ test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n-\techo \"$@\" | sed -e 's/^/#\t/'\n+\tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n }\n \n-- \n1.9.0\n"},{"id":"236902","messageId":"xmqq61nceidp.fsf@gitster.dls.corp.google.com","threadId":"36163","inReplyTo":"20140315001855.GK15625@google.com","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T19:26:42Z","receivedAt":"2014-03-17T19:26:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Uwe Storbeck wrote:\n>\n>> Backslash sequences are interpreted as control characters\n>> by the echo command of some shells (e.g. dash).\n>\n> This has bothered me for a while but never enough to do anything about\n> it.  Thanks for fixing it.\n>\n>> Signed-off-by: Uwe Storbeck <uwe@ibr.ch>\n>\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n>\n> (patch left unsnipped for reference)\n>> ---\n>>  t/test-lib.sh | 4 ++--\n>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> index 1531c24..8209204 100644\n>> --- a/t/test-lib.sh\n>> +++ b/t/test-lib.sh\n>> @@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n>>  \n>>  if test \"$help\" = \"t\"\n>>  then\n>> -\techo \"$test_description\"\n>> +\tprintf '%s\\n' \"$test_description\"\n>>  \texit 0\n>>  fi\n>>  \n>> @@ -328,7 +328,7 @@ test_failure_ () {\n>>  \ttest_failure=$(($test_failure + 1))\n>>  \tsay_color error \"not ok $test_count - $1\"\n>>  \tshift\n>> -\techo \"$@\" | sed -e 's/^/#\t/'\n>> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n\nThis is wrong, isn't it?  Why do we want one line per item here?\n\n>>  \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n>>  }\n>>  \n"},{"id":"236945","messageId":"20140318001410.GA4813@ibr.ch","threadId":"36163","inReplyTo":"20140314235735.GA6959@ibr.ch","subject":"[PATCH v3] test-lib.sh: do not \"echo\" externally supplied strings","fromName":"Uwe Storbeck","fromEmail":"uwe@ibr.ch","sentAt":"2014-03-18T00:14:11Z","receivedAt":"2014-03-18T00:14:11Z","isPatch":true,"sender":{"key":"uwe@ibr.ch","avatar":null},"body":"In some places we \"echo\" a string that is supplied by the calling\ntest script and may contain backslash sequences. The echo command\nof some shells, most notably \"dash\", interprets these backslash\nsequences (POSIX.1 allows this) which may scramble the test\noutput.\n\nSigned-off-by: Uwe Storbeck <uwe@ibr.ch>\n---\n\nCommit message rewritten to avoid title continuation in the body.\nThanks Junio C Hamano for the hint.\n\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1531c24..3c7cb1d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n \n if test \"$help\" = \"t\"\n then\n-\techo \"$test_description\"\n+\tprintf '%s\\n' \"$test_description\"\n \texit 0\n fi\n \n@@ -328,7 +328,7 @@ test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n-\techo \"$@\" | sed -e 's/^/#\t/'\n+\tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n }\n \n-- \n1.9.0\n"},{"id":"236996","messageId":"xmqqvbvb8ht7.fsf@gitster.dls.corp.google.com","threadId":"36163","inReplyTo":"20140318001410.GA4813@ibr.ch","subject":"Re: [PATCH v3] test-lib.sh: do not \"echo\" externally supplied strings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-18T18:47:48Z","receivedAt":"2014-03-18T18:47:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Storbeck <uwe@ibr.ch> writes:\n\n> In some places we \"echo\" a string that is supplied by the calling\n> test script and may contain backslash sequences. The echo command\n> of some shells, most notably \"dash\", interprets these backslash\n> sequences (POSIX.1 allows this) which may scramble the test\n> output.\n>\n> Signed-off-by: Uwe Storbeck <uwe@ibr.ch>\n> ---\n>\n> Commit message rewritten to avoid title continuation in the body.\n> Thanks Junio C Hamano for the hint.\n\nHere is what I queued yesterday.  I was wrong to say \"control\ncharacters\"; a backslash sequence is not necessarily a control\ncharacter (e.g. \\c at the end that suppresses the final LF), so I'll\nreplace it with your version.\n\nThe test titles are not externally supplied but under our control,\nso it is less problematic than the \"rebase -i\" case where we do get\nbitten by user supplied commit title string.  Still it is a good\nidea to apply this change to protect ourselves.\n\nThanks.  \n\ncommit 215cd588caebe86fe77115bdda97930b4659aecd\nAuthor: Uwe Storbeck <uwe@ibr.ch>\nDate:   Sat Mar 15 00:57:36 2014 +0100\n\n    test-lib.sh: do not \"echo\" test titles\n    \n    The test title could be a string with backslash in it, and can be\n    interpreted as control characters by the echo command of some shells\n    (e.g. dash).\n    \n    Signed-off-by: Uwe Storbeck <uwe@ibr.ch>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1531c24..3c7cb1d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -277,7 +277,7 @@ error \"Test script did not set test_description.\"\n \n if test \"$help\" = \"t\"\n then\n-\techo \"$test_description\"\n+\tprintf '%s\\n' \"$test_description\"\n \texit 0\n fi\n \n@@ -328,7 +328,7 @@ test_failure_ () {\n \ttest_failure=$(($test_failure + 1))\n \tsay_color error \"not ok $test_count - $1\"\n \tshift\n-\techo \"$@\" | sed -e 's/^/#\t/'\n+\tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \ttest \"$immediate\" = \"\" || { GIT_EXIT_OK=t; exit 1; }\n }\n"},{"id":"237017","messageId":"20140318221844.GA828@google.com","threadId":"36163","inReplyTo":"xmqq61nceidp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-03-18T22:18:44Z","receivedAt":"2014-03-18T22:18:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n>> Uwe Storbeck wrote:\n\n>>> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n>\n> This is wrong, isn't it?  Why do we want one line per item here?\n\nYes, Hannes caught the same, too.  Sorry for the sloppiness.\n\nWe currently use \"echo\" all over the place (e.g., 'echo \"$path\"' in\ngit-sh-setup), and every time we fix it there is a chance of making\nmistakes.  I wonder if it would make sense to add a helper to make the\necho calls easier to replace:\n\n-- >8 --\nSubject: git-sh-setup: introduce sane_echo helper\n\nUsing 'echo' with arguments that might contain backslashes or \"-e\" or\n\"-n\" can produce confusing results that vary from platform to platform\n(e.g., dash always interprets \\ escape sequences and echoes \"-e\"\nverbatim, whereas bash does not interpret \\ escapes unless \"-e\" is\npassed as an argument to echo and suppresses the \"-e\" from its\noutput).\n\nInstead, we should use printf, which is more predictable:\n\n\tprintf '%s\\n' \"$foo\"; # Just prints $foo, on all platforms.\n\nBlindly replacing echo with \"printf '%s\\n'\" would not be good enough\nbecause that printf prints each argument on its own line.  Provide a\nsane_echo helper that prints its arguments, space-delimited, on a\nsingle line, to make this easier to remember, and tweak 'say'\nand 'die_with_status' to illustrate how it is used.\n\nNo functional change intended.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n git-sh-setup.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git i/git-sh-setup.sh w/git-sh-setup.sh\nindex 256c89a..f35b5b9 100644\n--- i/git-sh-setup.sh\n+++ w/git-sh-setup.sh\n@@ -43,6 +43,10 @@ git_broken_path_fix () {\n \n # @@BROKEN_PATH_FIX@@\n \n+sane_echo () {\n+\tprintf '%s\\n' \"$*\"\n+}\n+\n die () {\n \tdie_with_status 1 \"$@\"\n }\n@@ -50,7 +54,7 @@ die () {\n die_with_status () {\n \tstatus=$1\n \tshift\n-\tprintf >&2 '%s\\n' \"$*\"\n+\tsane_echo >&2 \"$*\"\n \texit \"$status\"\n }\n \n@@ -59,7 +63,7 @@ GIT_QUIET=\n say () {\n \tif test -z \"$GIT_QUIET\"\n \tthen\n-\t\tprintf '%s\\n' \"$*\"\n+\t\tsane_echo \"$*\"\n \tfi\n }\n \n"},{"id":"237085","messageId":"xmqqfvme5cql.fsf@gitster.dls.corp.google.com","threadId":"36163","inReplyTo":"20140318221844.GA828@google.com","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T17:17:54Z","receivedAt":"2014-03-19T17:17:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>>> Uwe Storbeck wrote:\n>\n>>>> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n>>\n>> This is wrong, isn't it?  Why do we want one line per item here?\n>\n> Yes, Hannes caught the same, too.  Sorry for the sloppiness.\n>\n> We currently use \"echo\" all over the place (e.g., 'echo \"$path\"' in\n> git-sh-setup), and every time we fix it there is a chance of making\n> mistakes.  I wonder if it would make sense to add a helper to make the\n> echo calls easier to replace:\n\nI agree that we would benefit from having a helper to print a single\nline, which we very often do, without having to worry about the\nboilerplate '%s\\n' of printf or the portability gotcha of echo.\n\nI am a bit reluctant to name the helper \"sane_echo\" to declare \"echo\nthat interprets backslashes in the string is insane\", though.  For\nthese \"print a single line\" uses, we are only interested in using a\nsubset of the features offered by 'echo', but that does not mean the\nother features we do not want to trigger in our use is of no use to\nany sane person.  It very different from \"sane_unset\" that works\naround \"unset\" on an unset variable that can trigger an error when\nnobody sane is interested in that error condition.  If somebody is\ninterested if a variable is not yet set and behave differently,\nthere are more direct ways to see the \"set-ness\" of a variable, and\nasking \"unset\" for that information is insane, hence I think the\nname \"sane_unset\" is justified.  I do not feel the same way for\n\"sane_echo\".\n\nI would have called it \"say\" if the name weren't taken.\n\n> -- >8 --\n> Subject: git-sh-setup: introduce sane_echo helper\n>\n> Using 'echo' with arguments that might contain backslashes or \"-e\" or\n> \"-n\" can produce confusing results that vary from platform to platform\n> (e.g., dash always interprets \\ escape sequences and echoes \"-e\"\n> verbatim, whereas bash does not interpret \\ escapes unless \"-e\" is\n> passed as an argument to echo and suppresses the \"-e\" from its\n> output).\n>\n> Instead, we should use printf, which is more predictable:\n>\n> \tprintf '%s\\n' \"$foo\"; # Just prints $foo, on all platforms.\n>\n> Blindly replacing echo with \"printf '%s\\n'\" would not be good enough\n> because that printf prints each argument on its own line.  Provide a\n> sane_echo helper that prints its arguments, space-delimited, on a\n> single line, to make this easier to remember, and tweak 'say'\n> and 'die_with_status' to illustrate how it is used.\n>\n> No functional change intended.\n>\n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> ---\n>  git-sh-setup.sh | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git i/git-sh-setup.sh w/git-sh-setup.sh\n> index 256c89a..f35b5b9 100644\n> --- i/git-sh-setup.sh\n> +++ w/git-sh-setup.sh\n> @@ -43,6 +43,10 @@ git_broken_path_fix () {\n>  \n>  # @@BROKEN_PATH_FIX@@\n>  \n> +sane_echo () {\n> +\tprintf '%s\\n' \"$*\"\n> +}\n> +\n>  die () {\n>  \tdie_with_status 1 \"$@\"\n>  }\n> @@ -50,7 +54,7 @@ die () {\n>  die_with_status () {\n>  \tstatus=$1\n>  \tshift\n> -\tprintf >&2 '%s\\n' \"$*\"\n> +\tsane_echo >&2 \"$*\"\n>  \texit \"$status\"\n>  }\n>  \n> @@ -59,7 +63,7 @@ GIT_QUIET=\n>  say () {\n>  \tif test -z \"$GIT_QUIET\"\n>  \tthen\n> -\t\tprintf '%s\\n' \"$*\"\n> +\t\tsane_echo \"$*\"\n>  \tfi\n>  }\n>  \n"},{"id":"237089","messageId":"871txygl27.fsf@fencepost.gnu.org","threadId":"36163","inReplyTo":"xmqqfvme5cql.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-03-19T17:22:40Z","receivedAt":"2014-03-19T17:22:40Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>> Junio C Hamano wrote:\n>>>> Uwe Storbeck wrote:\n>>\n>>>>> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n>>>\n>>> This is wrong, isn't it?  Why do we want one line per item here?\n>>\n>> Yes, Hannes caught the same, too.  Sorry for the sloppiness.\n>>\n>> We currently use \"echo\" all over the place (e.g., 'echo \"$path\"' in\n>> git-sh-setup), and every time we fix it there is a chance of making\n>> mistakes.  I wonder if it would make sense to add a helper to make the\n>> echo calls easier to replace:\n>\n> I agree that we would benefit from having a helper to print a single\n> line, which we very often do, without having to worry about the\n> boilerplate '%s\\n' of printf or the portability gotcha of echo.\n>\n> I am a bit reluctant to name the helper \"sane_echo\" to declare \"echo\n> that interprets backslashes in the string is insane\", though.\n\nraw_echo\n\n-- \nDavid Kastrup\n"},{"id":"237106","messageId":"xmqqob122dq0.fsf@gitster.dls.corp.google.com","threadId":"36163","inReplyTo":"871txygl27.fsf@fencepost.gnu.org","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T19:24:55Z","receivedAt":"2014-03-19T19:24:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup <dak@gnu.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>\n>>> Junio C Hamano wrote:\n>>>>> Uwe Storbeck wrote:\n>>>\n>>>>>> +\tprintf '%s\\n' \"$@\" | sed -e 's/^/#\t/'\n>>>>\n>>>> This is wrong, isn't it?  Why do we want one line per item here?\n>>>\n>>> Yes, Hannes caught the same, too.  Sorry for the sloppiness.\n>>>\n>>> We currently use \"echo\" all over the place (e.g., 'echo \"$path\"' in\n>>> git-sh-setup), and every time we fix it there is a chance of making\n>>> mistakes.  I wonder if it would make sense to add a helper to make the\n>>> echo calls easier to replace:\n>>\n>> I agree that we would benefit from having a helper to print a single\n>> line, which we very often do, without having to worry about the\n>> boilerplate '%s\\n' of printf or the portability gotcha of echo.\n>>\n>> I am a bit reluctant to name the helper \"sane_echo\" to declare \"echo\n>> that interprets backslashes in the string is insane\", though.\n>\n> raw_echo\n\nYeah, but the thing is, this is not even \"raw\" if you view it from\nthe direction of knowing what \"echo\" does.  That is why I repeated\n\"helper to print a single line\", which is a viewpoint from the user\nside.  \"We do not care how it is implemented, we just want a single\nline printed\" is what we want to express, which \"say\" is perfectly\nin line with.  \"We use a subset semantics of 'echo' to implement it\"\nis of secondary concern.\n"},{"id":"237135","messageId":"20140320001718.GM15625@google.com","threadId":"36163","inReplyTo":"xmqqfvme5cql.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-03-20T00:17:18Z","receivedAt":"2014-03-20T00:17:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> We currently use \"echo\" all over the place (e.g., 'echo \"$path\"' in\n>> git-sh-setup), and every time we fix it there is a chance of making\n>> mistakes.  I wonder if it would make sense to add a helper to make the\n>> echo calls easier to replace:\n>\n> I agree that we would benefit from having a helper to print a single\n> line, which we very often do, without having to worry about the\n> boilerplate '%s\\n' of printf or the portability gotcha of echo.\n>\n> I am a bit reluctant to name the helper \"sane_echo\" to declare \"echo\n> that interprets backslashes in the string is insane\", though.  For\n> these \"print a single line\" uses, we are only interested in using a\n> subset of the features offered by 'echo', but that does not mean the\n> other features we do not want to trigger in our use is of no use to\n> any sane person.\n\nIn a portable script, uncareful use of 'echo' is always insane.\n\nIn a script tailored to an environment where echo behaves consistently\nit is perfectly reasonable to use 'echo', but that's a different\nstory.  In the context of git, saying \"Here is the thing you should\nalways use instead of echo\" is a good thing, in my opinion.\n\nHoping that clarifies,\nJonathan\n"},{"id":"237178","messageId":"xmqq4n2s255z.fsf@gitster.dls.corp.google.com","threadId":"36163","inReplyTo":"20140320001718.GM15625@google.com","subject":"Re: [PATCH] test-lib.sh: use printf instead of echo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-20T16:42:00Z","receivedAt":"2014-03-20T16:42:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Junio C Hamano wrote:\n> ...\n>> I am a bit reluctant to name the helper \"sane_echo\" to declare \"echo\n>> that interprets backslashes in the string is insane\", though.  For\n>> these \"print a single line\" uses, we are only interested in using a\n>> subset of the features offered by 'echo', but that does not mean the\n>> other features we do not want to trigger in our use is of no use to\n>> any sane person.\n>\n> In a portable script, uncareful use of 'echo' is always insane.\n\nI agree that makes sense and I actually think that it is a bit\nstronger than that.  If a script is meant to be portable, there is\nno way to use \"echo\" on a string whose contents is unknown sanely.\nThere is no \"careful use is OK\".\n\n> In a script tailored to an environment where echo behaves consistently\n> it is perfectly reasonable to use 'echo', but that's a different\n> story.  In the context of git, saying \"Here is the thing you should\n> always use instead of echo\" is a good thing, in my opinion.\n\nThat is true in my opinion, but that thing is also what you should\nalways use instead of \"printf '%s\\n'\".  A guideline more useful for\nthe users is \"Here is the thing you should always use when literally\nemitting a single line.\", isn't it?\n"}]}