{"thread":{"id":"32911","subject":"[PATCH] contrib/subtree: remove contradicting use options on echo wrapper","startedAt":"2013-02-15T22:20:13Z","lastAt":"2013-02-15T23:18:42Z","messageCount":3,"participants":["Paul Campbell","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"209561","messageId":"CALeLG_=p9k2B6AmTG0iKf9GpGB=_6kcECmCdDV1nmruJ4bdGcw@mail.gmail.com","threadId":"32911","inReplyTo":null,"subject":"[PATCH] contrib/subtree: remove contradicting use options on echo wrapper","fromName":"Paul Campbell","fromEmail":"pcampbell@kemitix.net","sentAt":"2013-02-15T22:20:13Z","receivedAt":"2013-02-15T22:20:13Z","isPatch":true,"sender":{"key":"pcampbell@kemitix.net","avatar":"https://gravatar.com/avatar/57584e05501b694929004e43fcd7308f4ad64df2eb0474cd8c7e5f93662bb0f1?d=mp&s=160"},"body":"Remove redundant -n option and raw ^M in call to echo.\n\nCall to 'say' function, a wrapper of 'echo', passed the parameter -n, then\nincluded a raw ^M newline in the end of the last parameter. Yet the -n option\nis meant to suppress the addition of new line by echo.\n\nSigned-off-by: Paul Campbell <pcampbell@kemitix.net>\n---\n contrib/subtree/git-subtree.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 8a23f58..51146bd 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -592,7 +592,7 @@ cmd_split()\n \teval \"$grl\" |\n \twhile read rev parents; do\n \t\trevcount=$(($revcount + 1))\n-\t\tsay -n \"$revcount/$revmax ($createcount)\n\"\n+\t\tsay \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n \t\texists=$(cache_get $rev)\n \t\tif [ -n \"$exists\" ]; then\n-- \n1.8.1.3.605.g02339dd\n"},{"id":"209563","messageId":"7vtxpdfbhx.fsf@alter.siamese.dyndns.org","threadId":"32911","inReplyTo":"CALeLG_=p9k2B6AmTG0iKf9GpGB=_6kcECmCdDV1nmruJ4bdGcw@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: remove contradicting use options on echo wrapper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-15T22:39:06Z","receivedAt":"2013-02-15T22:39:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Campbell <pcampbell@kemitix.net> writes:\n\n> Remove redundant -n option and raw ^M in call to echo.\n>\n> Call to 'say' function, a wrapper of 'echo', passed the parameter -n, then\n> included a raw ^M newline in the end of the last parameter. Yet the -n option\n> is meant to suppress the addition of new line by echo.\n>\n> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>\n\nI generally do not comment on comment on contrib/ material, and I am\nnot familiar with subtree myself, but\n\n\tfor count in $(seq 0 $total)\n        do\n\t\techo -n \"$count/$total^M\"\n                ... do heavy lifting ...\n\tdone\n        echo \"Done                  \"\n\nis an idiomatic way to implement a progress meter without scrolling\nmore important message you gave earlier to the user before entering\nthe loop away.  The message appears, carrige-return moves the cursor\nto the beginning of the line without going to the next line, and the\nnext iteration overwrites the previous count.  Finally, the progress\nmeter is overwritten with the \"Done\" message.  Alternatively you can\nwrap it up with\n\n\techo\n        echo Done\n\nif you want to leave the final progress \"100/100\" before saying \"Done.\"\n\nIsn't that what this piece of code trying to do?\n\n> ---\n>  contrib/subtree/git-subtree.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n> index 8a23f58..51146bd 100755\n> --- a/contrib/subtree/git-subtree.sh\n> +++ b/contrib/subtree/git-subtree.sh\n> @@ -592,7 +592,7 @@ cmd_split()\n>  \teval \"$grl\" |\n>  \twhile read rev parents; do\n>  \t\trevcount=$(($revcount + 1))\n> -\t\tsay -n \"$revcount/$revmax ($createcount)\n> \"\n> +\t\tsay \"$revcount/$revmax ($createcount)\"\n>  \t\tdebug \"Processing commit: $rev\"\n>  \t\texists=$(cache_get $rev)\n>  \t\tif [ -n \"$exists\" ]; then\n"},{"id":"209569","messageId":"CALeLG_mzBj7q2Vd6vLPTQWexKQB2bEDvh2NT9i9_m8atuiZFcA@mail.gmail.com","threadId":"32911","inReplyTo":"7vtxpdfbhx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] contrib/subtree: remove contradicting use options on echo wrapper","fromName":"Paul Campbell","fromEmail":"pcampbell@kemitix.net","sentAt":"2013-02-15T23:18:42Z","receivedAt":"2013-02-15T23:18:42Z","isPatch":true,"sender":{"key":"pcampbell@kemitix.net","avatar":"https://gravatar.com/avatar/57584e05501b694929004e43fcd7308f4ad64df2eb0474cd8c7e5f93662bb0f1?d=mp&s=160"},"body":"On Fri, Feb 15, 2013 at 10:39 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Campbell <pcampbell@kemitix.net> writes:\n>\n>> Remove redundant -n option and raw ^M in call to echo.\n>>\n>> Call to 'say' function, a wrapper of 'echo', passed the parameter -n, then\n>> included a raw ^M newline in the end of the last parameter. Yet the -n option\n>> is meant to suppress the addition of new line by echo.\n>>\n>> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>\n>\n> I generally do not comment on comment on contrib/ material, and I am\n> not familiar with subtree myself, but\n>\n>         for count in $(seq 0 $total)\n>         do\n>                 echo -n \"$count/$total^M\"\n>                 ... do heavy lifting ...\n>         done\n>         echo \"Done                  \"\n>\n> is an idiomatic way to implement a progress meter without scrolling\n> more important message you gave earlier to the user before entering\n> the loop away.  The message appears, carrige-return moves the cursor\n> to the beginning of the line without going to the next line, and the\n> next iteration overwrites the previous count.  Finally, the progress\n> meter is overwritten with the \"Done\" message.  Alternatively you can\n> wrap it up with\n>\n>         echo\n>         echo Done\n>\n> if you want to leave the final progress \"100/100\" before saying \"Done.\"\n>\n> Isn't that what this piece of code trying to do?\n>\n>> ---\n>>  contrib/subtree/git-subtree.sh | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n>> index 8a23f58..51146bd 100755\n>> --- a/contrib/subtree/git-subtree.sh\n>> +++ b/contrib/subtree/git-subtree.sh\n>> @@ -592,7 +592,7 @@ cmd_split()\n>>       eval \"$grl\" |\n>>       while read rev parents; do\n>>               revcount=$(($revcount + 1))\n>> -             say -n \"$revcount/$revmax ($createcount)\n>> \"\n>> +             say \"$revcount/$revmax ($createcount)\"\n>>               debug \"Processing commit: $rev\"\n>>               exists=$(cache_get $rev)\n>>               if [ -n \"$exists\" ]; then\n\n[Apologies for resending this Junio. Forgot to hit reply all.]\n\nAh. I've not seen that done in shell before. In other languages I've\nseen and used '\\r'  for this purpose, rather than a raw ^M.\n\nI was getting frustrated with it as my apparently braindead text\neditor was converting it to a normal unix newline, which would then\nkeep getting picked up by git diff.\n\nPlease ignore my patch.\n\n-- \nPaul [W] Campbell\n"}]}