{"thread":{"id":"39266","subject":"Re: [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split()","startedAt":"2015-05-07T03:39:47Z","lastAt":"2015-05-08T18:55:17Z","messageCount":10,"participants":["Danny Lin","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"260700","messageId":"CAMbsUu66AJ1hC-nDrHSojMibYp-rh=zSpEwC3hCaG-1yU71GZw@mail.gmail.com","threadId":"39266","inReplyTo":null,"subject":"Re: [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split()","fromName":"Danny Lin","fromEmail":"danny0838@gmail.com","sentAt":"2015-05-07T03:39:47Z","receivedAt":"2015-05-07T03:39:47Z","isPatch":true,"sender":{"key":"danny0838@gmail.com","avatar":"https://avatars.githubusercontent.com/u/531417?v=4"},"body":"Replace all echo using printf for better portability.\n\nAlso re-wrap previous 'say -n \"$str<CR>\"' using a new\nfunction state() so to prevent CR chars included in\nthe source code, which could be mal-processed on some\nshells (e.g. MsysGit trims CR before executing a shell\nscript file in order to make it work right on Windows\neven if it uses CRLF as linefeeds.\n\nSigned-off-by: Danny Lin <danny0838@gmail.com>\n---\n contrib/subtree/git-subtree.sh | 56 +++++++++++++++++++++++++-----------------\n 1 file changed, 34 insertions(+), 22 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex fa1a583..2da1433 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -29,7 +29,7 @@ rejoin        merge the new branch back into HEAD\n  options for 'add', 'merge', 'pull' and 'push'\n squash        merge subtree changes as a single commit\n \"\n-eval \"$(echo \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" || echo exit $?)\"\n+eval \"$(printf %s \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" ||\nprintf %s \"exit $?\")\"\n\n PATH=$PATH:$(git --exec-path)\n . git-sh-setup\n@@ -51,17 +51,29 @@ prefix=\n debug()\n {\n     if [ -n \"$debug\" ]; then\n-        echo \"$@\" >&2\n+        printf \"%s\\n\" \"$*\" >&2\n     fi\n }\n\n say()\n {\n     if [ -z \"$quiet\" ]; then\n-        echo \"$@\" >&2\n+        printf \"%s\\n\" \"$*\" >&2\n     fi\n }\n\n+state()\n+{\n+    if [ -z \"$quiet\" ]; then\n+        printf \"%s\\r\" \"$*\" >&2\n+    fi\n+}\n+\n+log()\n+{\n+    printf \"%s\\n\" \"$*\"\n+}\n+\n assert()\n {\n     if \"$@\"; then\n@@ -72,7 +84,7 @@ assert()\n }\n\n\n-#echo \"Options: $*\"\n+#log \"Options: $*\"\n\n while [ $# -gt 0 ]; do\n     opt=\"$1\"\n@@ -149,7 +161,7 @@ cache_get()\n     for oldrev in $*; do\n         if [ -r \"$cachedir/$oldrev\" ]; then\n             read newrev <\"$cachedir/$oldrev\"\n-            echo $newrev\n+            log $newrev\n         fi\n     done\n }\n@@ -158,7 +170,7 @@ cache_miss()\n {\n     for oldrev in $*; do\n         if [ ! -r \"$cachedir/$oldrev\" ]; then\n-            echo $oldrev\n+            log $oldrev\n         fi\n     done\n }\n@@ -175,7 +187,7 @@ check_parents()\n\n set_notree()\n {\n-    echo \"1\" > \"$cachedir/notree/$1\"\n+    log \"1\" > \"$cachedir/notree/$1\"\n }\n\n cache_set()\n@@ -187,7 +199,7 @@ cache_set()\n          -a -e \"$cachedir/$oldrev\" ]; then\n         die \"cache for $oldrev already exists!\"\n     fi\n-    echo \"$newrev\" >\"$cachedir/$oldrev\"\n+    log \"$newrev\" >\"$cachedir/$oldrev\"\n }\n\n rev_exists()\n@@ -219,7 +231,7 @@ rev_is_descendant_of_branch()\n try_remove_previous()\n {\n     if rev_exists \"$1^\"; then\n-        echo \"^$1^\"\n+        log \"^$1^\"\n     fi\n }\n\n@@ -247,7 +259,7 @@ find_latest_squash()\n                         sq=\"$sub\"\n                     fi\n                     debug \"Squash found: $sq $sub\"\n-                    echo \"$sq\" \"$sub\"\n+                    log \"$sq\" \"$sub\"\n                     break\n                 fi\n                 sq=\n@@ -339,9 +351,9 @@ add_msg()\n add_squashed_msg()\n {\n     if [ -n \"$message\" ]; then\n-        echo \"$message\"\n+        log \"$message\"\n     else\n-        echo \"Merge commit '$1' as '$2'\"\n+        log \"Merge commit '$1' as '$2'\"\n     fi\n }\n\n@@ -373,17 +385,17 @@ squash_msg()\n\n     if [ -n \"$oldsub\" ]; then\n         oldsub_short=$(git rev-parse --short \"$oldsub\")\n-        echo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n+        log \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n         echo\n         git log --pretty=tformat:'%h %s' \"$oldsub..$newsub\"\n         git log --pretty=tformat:'REVERT: %h %s' \"$newsub..$oldsub\"\n     else\n-        echo \"Squashed '$dir/' content from commit $newsub_short\"\n+        log \"Squashed '$dir/' content from commit $newsub_short\"\n     fi\n\n     echo\n-    echo \"git-subtree-dir: $dir\"\n-    echo \"git-subtree-split: $newsub\"\n+    log \"git-subtree-dir: $dir\"\n+    log \"git-subtree-split: $newsub\"\n }\n\n toptree_for_commit()\n@@ -401,7 +413,7 @@ subtree_for_commit()\n         assert [ \"$name\" = \"$dir\" ]\n         assert [ \"$type\" = \"tree\" -o \"$type\" = \"commit\" ]\n         [ \"$type\" = \"commit\" ] && continue  # ignore submodules\n-        echo $tree\n+        log $tree\n         break\n     done\n }\n@@ -474,7 +486,7 @@ copy_or_skip()\n     done\n\n     if [ -n \"$identical\" ]; then\n-        echo $identical\n+        log $identical\n     else\n         copy_commit $rev $tree \"$p\" || exit $?\n     fi\n@@ -526,7 +538,7 @@ cmd_add()\n\n cmd_add_repository()\n {\n-    echo \"git fetch\" \"$@\"\n+    log \"git fetch\" \"$@\"\n     repository=$1\n     refspec=$2\n     git fetch \"$@\" || exit $?\n@@ -599,7 +611,7 @@ cmd_split()\n     eval \"$grl\" |\n     while read rev parents; do\n         revcount=$(($revcount + 1))\n-        say -n \"$revcount/$revmax ($createcount)\n\"\n+        state \"$revcount/$revmax ($createcount)\"\n         debug \"Processing commit: $rev\"\n         exists=$(cache_get $rev)\n         if [ -n \"$exists\" ]; then\n@@ -656,7 +668,7 @@ cmd_split()\n         git update-ref -m 'subtree split' \"refs/heads/$branch\"\n$latest_new || exit $?\n         say \"$action branch '$branch'\"\n     fi\n-    echo $latest_new\n+    log $latest_new\n     exit 0\n }\n\n@@ -726,7 +738,7 @@ cmd_push()\n     if [ -e \"$dir\" ]; then\n         repository=$1\n         refspec=$2\n-        echo \"git push using: \" $repository $refspec\n+        log \"git push using: \" $repository $refspec\n         localrev=$(git subtree split --prefix=\"$prefix\") || die\n         git push $repository $localrev:refs/heads/$refspec\n     else\n-- \n2.3.7.windows.1\n\n\n\n2015-05-07 3:58 GMT+08:00 Eric Sunshine <sunshine@sunshineco.com>:\n> On Wed, May 6, 2015 at 3:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Danny Lin <danny0838@gmail.com> writes:\n>>\n>>> cmd_split() prints a CR char by assigning a variable\n>>> with a literal CR in the source code, which could be\n>>> trimmed or mis-processed in some terminals. Replace\n>>> with $(printf '\\r') to fix it.\n>\n> For future readers of the patch who haven't followed the email\n> discussion, it might be a good idea to explain the problem in more\n> detail. Saying merely \"could be trimmed or mis-processed in some\n> terminals\" doesn't give much for people to latch onto if they want to\n> understand the specific problem. Concrete information would help.\n>\nAdded related information.\n\n>>> Signed-off-by: Danny Lin <danny0838@gmail.com>\n>>> ---\n>>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\n>>> index fa1a583..3a581fc 100755\n>>> --- a/contrib/subtree/git-subtree.sh\n>>> +++ b/contrib/subtree/git-subtree.sh\n>>> @@ -596,10 +596,11 @@ cmd_split()\n>>>      revmax=$(eval \"$grl\" | wc -l)\n>>>      revcount=0\n>>>      createcount=0\n>>> +    CR=$(printf '\\r')\n>>>      eval \"$grl\" |\n>>>      while read rev parents; do\n>>>          revcount=$(($revcount + 1))\n>>> -        say -n \"$revcount/$revmax ($createcount)\n>>> \"\n>>> +        say -n \"$revcount/$revmax ($createcount)$CR\"\n>>\n>> Interesting.  I would have expected, especially this is a portability-fix\n>> change, that the change would be a single liner\n>>\n>> -       say -n ...\n>> +       printf \"%s\\r\" \"$revcount/$revmax ($createcount)\"\n>>\n>> that does not touch any other line.\n>\n> Unfortunately, that solution does not respect the $quiet flag like\n> say() does. I had envisioned the patch as reimplementing say() using\n> printf rather than echo, and having say() itself either recognizing\n> the -n flag or just update callers to specify \\n when they want it\n> (which is probably the cleaner of the two approaches).\n>\nIf a more thorough portability fix is desired, I'd prefer a work like this\n(see the patch above).\n"},{"id":"260701","messageId":"CAMbsUu7vkS4D2z_gNFsujVsyHjRiXseTLGCaic=841V=HZyb_g@mail.gmail.com","threadId":"39266","inReplyTo":"CAMbsUu66AJ1hC-nDrHSojMibYp-rh=zSpEwC3hCaG-1yU71GZw@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split()","fromName":"Danny Lin","fromEmail":"danny0838@gmail.com","sentAt":"2015-05-07T03:43:15Z","receivedAt":"2015-05-07T03:43:15Z","isPatch":true,"sender":{"key":"danny0838@gmail.com","avatar":"https://avatars.githubusercontent.com/u/531417?v=4"},"body":"Subject: [PATCH] contrib/subtree: portability fix for string printing\n\nReplace all echo using printf for better portability.\n\nAlso re-wrap previous 'say -n \"$str<CR>\"' using a new\nfunction state() to prevent CR chars included in the\nsource code, which could be mal-processed on some\nshells. For example, MsysGit trims CR before executing\na shell script file in order to make it work right on\nWindows even if it uses CRLF as linefeeds.\n\nSigned-off-by: Danny Lin <danny0838@gmail.com>\n---\n contrib/subtree/git-subtree.sh | 56 +++++++++++++++++++++++++-----------------\n 1 file changed, 34 insertions(+), 22 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex fa1a583..2da1433 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -29,7 +29,7 @@ rejoin        merge the new branch back into HEAD\n  options for 'add', 'merge', 'pull' and 'push'\n squash        merge subtree changes as a single commit\n \"\n-eval \"$(echo \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" || echo exit $?)\"\n+eval \"$(printf %s \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" ||\nprintf %s \"exit $?\")\"\n\n PATH=$PATH:$(git --exec-path)\n . git-sh-setup\n@@ -51,17 +51,29 @@ prefix=\n debug()\n {\n     if [ -n \"$debug\" ]; then\n-        echo \"$@\" >&2\n+        printf \"%s\\n\" \"$*\" >&2\n     fi\n }\n\n say()\n {\n     if [ -z \"$quiet\" ]; then\n-        echo \"$@\" >&2\n+        printf \"%s\\n\" \"$*\" >&2\n     fi\n }\n\n+state()\n+{\n+    if [ -z \"$quiet\" ]; then\n+        printf \"%s\\r\" \"$*\" >&2\n+    fi\n+}\n+\n+log()\n+{\n+    printf \"%s\\n\" \"$*\"\n+}\n+\n assert()\n {\n     if \"$@\"; then\n@@ -72,7 +84,7 @@ assert()\n }\n\n\n-#echo \"Options: $*\"\n+#log \"Options: $*\"\n\n while [ $# -gt 0 ]; do\n     opt=\"$1\"\n@@ -149,7 +161,7 @@ cache_get()\n     for oldrev in $*; do\n         if [ -r \"$cachedir/$oldrev\" ]; then\n             read newrev <\"$cachedir/$oldrev\"\n-            echo $newrev\n+            log $newrev\n         fi\n     done\n }\n@@ -158,7 +170,7 @@ cache_miss()\n {\n     for oldrev in $*; do\n         if [ ! -r \"$cachedir/$oldrev\" ]; then\n-            echo $oldrev\n+            log $oldrev\n         fi\n     done\n }\n@@ -175,7 +187,7 @@ check_parents()\n\n set_notree()\n {\n-    echo \"1\" > \"$cachedir/notree/$1\"\n+    log \"1\" > \"$cachedir/notree/$1\"\n }\n\n cache_set()\n@@ -187,7 +199,7 @@ cache_set()\n          -a -e \"$cachedir/$oldrev\" ]; then\n         die \"cache for $oldrev already exists!\"\n     fi\n-    echo \"$newrev\" >\"$cachedir/$oldrev\"\n+    log \"$newrev\" >\"$cachedir/$oldrev\"\n }\n\n rev_exists()\n@@ -219,7 +231,7 @@ rev_is_descendant_of_branch()\n try_remove_previous()\n {\n     if rev_exists \"$1^\"; then\n-        echo \"^$1^\"\n+        log \"^$1^\"\n     fi\n }\n\n@@ -247,7 +259,7 @@ find_latest_squash()\n                         sq=\"$sub\"\n                     fi\n                     debug \"Squash found: $sq $sub\"\n-                    echo \"$sq\" \"$sub\"\n+                    log \"$sq\" \"$sub\"\n                     break\n                 fi\n                 sq=\n@@ -339,9 +351,9 @@ add_msg()\n add_squashed_msg()\n {\n     if [ -n \"$message\" ]; then\n-        echo \"$message\"\n+        log \"$message\"\n     else\n-        echo \"Merge commit '$1' as '$2'\"\n+        log \"Merge commit '$1' as '$2'\"\n     fi\n }\n\n@@ -373,17 +385,17 @@ squash_msg()\n\n     if [ -n \"$oldsub\" ]; then\n         oldsub_short=$(git rev-parse --short \"$oldsub\")\n-        echo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n+        log \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n         echo\n         git log --pretty=tformat:'%h %s' \"$oldsub..$newsub\"\n         git log --pretty=tformat:'REVERT: %h %s' \"$newsub..$oldsub\"\n     else\n-        echo \"Squashed '$dir/' content from commit $newsub_short\"\n+        log \"Squashed '$dir/' content from commit $newsub_short\"\n     fi\n\n     echo\n-    echo \"git-subtree-dir: $dir\"\n-    echo \"git-subtree-split: $newsub\"\n+    log \"git-subtree-dir: $dir\"\n+    log \"git-subtree-split: $newsub\"\n }\n\n toptree_for_commit()\n@@ -401,7 +413,7 @@ subtree_for_commit()\n         assert [ \"$name\" = \"$dir\" ]\n         assert [ \"$type\" = \"tree\" -o \"$type\" = \"commit\" ]\n         [ \"$type\" = \"commit\" ] && continue  # ignore submodules\n-        echo $tree\n+        log $tree\n         break\n     done\n }\n@@ -474,7 +486,7 @@ copy_or_skip()\n     done\n\n     if [ -n \"$identical\" ]; then\n-        echo $identical\n+        log $identical\n     else\n         copy_commit $rev $tree \"$p\" || exit $?\n     fi\n@@ -526,7 +538,7 @@ cmd_add()\n\n cmd_add_repository()\n {\n-    echo \"git fetch\" \"$@\"\n+    log \"git fetch\" \"$@\"\n     repository=$1\n     refspec=$2\n     git fetch \"$@\" || exit $?\n@@ -599,7 +611,7 @@ cmd_split()\n     eval \"$grl\" |\n     while read rev parents; do\n         revcount=$(($revcount + 1))\n-        say -n \"$revcount/$revmax ($createcount)\n\"\n+        state \"$revcount/$revmax ($createcount)\"\n         debug \"Processing commit: $rev\"\n         exists=$(cache_get $rev)\n         if [ -n \"$exists\" ]; then\n@@ -656,7 +668,7 @@ cmd_split()\n         git update-ref -m 'subtree split' \"refs/heads/$branch\"\n$latest_new || exit $?\n         say \"$action branch '$branch'\"\n     fi\n-    echo $latest_new\n+    log $latest_new\n     exit 0\n }\n\n@@ -726,7 +738,7 @@ cmd_push()\n     if [ -e \"$dir\" ]; then\n         repository=$1\n         refspec=$2\n-        echo \"git push using: \" $repository $refspec\n+        log \"git push using: \" $repository $refspec\n         localrev=$(git subtree split --prefix=\"$prefix\") || die\n         git push $repository $localrev:refs/heads/$refspec\n     else\n-- \n2.3.7.windows.1\n\n\nTypo fix for previous patch.\n"},{"id":"260702","messageId":"CAMbsUu6XT4hB0L0PjgJKniyBJ9svkQoJqnxiYaRWo9EZUXnNhg@mail.gmail.com","threadId":"39266","inReplyTo":"CAMbsUu7vkS4D2z_gNFsujVsyHjRiXseTLGCaic=841V=HZyb_g@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split()","fromName":"Danny Lin","fromEmail":"danny0838@gmail.com","sentAt":"2015-05-07T05:10:09Z","receivedAt":"2015-05-07T05:10:09Z","isPatch":true,"sender":{"key":"danny0838@gmail.com","avatar":"https://avatars.githubusercontent.com/u/531417?v=4"},"body":"I'm sorry that I cannot get git send-email work currently.\nI'd submit the updated patch with an attachment until it's\nworking right. ><\n\n\nFrom f36d4eab24565894cfe3abba0131f01668a5934b Mon Sep 17 00:00:00 2001\nFrom: Danny Lin <danny0838@gmail.com>\nDate: Thu, 7 May 2015 10:51:31 +0800\nSubject: [PATCH] contrib/subtree: portability fix for string printing\n\nReplace all echo using printf for better portability.\n\nAlso re-wrap previous 'say -n \"$str<CR>\"' using a new\nfunction state() to prevent CR chars included in the\nsource code, which could be mal-processed in some\nshells. For example, MsysGit trims CR before executing\na shell script file in order to make it work right on\nWindows even if it uses CRLF as linefeeds.\n\nSigned-off-by: Danny Lin <danny0838@gmail.com>\n---\n contrib/subtree/git-subtree.sh | 56 +++++++++++++++++++++++++-----------------\n 1 file changed, 34 insertions(+), 22 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex fa1a583..2da1433 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -29,7 +29,7 @@ rejoin        merge the new branch back into HEAD\n  options for 'add', 'merge', 'pull' and 'push'\n squash        merge subtree changes as a single commit\n \"\n-eval \"$(echo \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" || echo exit $?)\"\n+eval \"$(printf %s \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" || printf %s \"exit $?\")\"\n \n PATH=$PATH:$(git --exec-path)\n . git-sh-setup\n@@ -51,17 +51,29 @@ prefix=\n debug()\n {\n \tif [ -n \"$debug\" ]; then\n-\t\techo \"$@\" >&2\n+\t\tprintf \"%s\\n\" \"$*\" >&2\n \tfi\n }\n \n say()\n {\n \tif [ -z \"$quiet\" ]; then\n-\t\techo \"$@\" >&2\n+\t\tprintf \"%s\\n\" \"$*\" >&2\n \tfi\n }\n \n+state()\n+{\n+\tif [ -z \"$quiet\" ]; then\n+\t\tprintf \"%s\\r\" \"$*\" >&2\n+\tfi\n+}\n+\n+log()\n+{\n+\tprintf \"%s\\n\" \"$*\"\n+}\n+\n assert()\n {\n \tif \"$@\"; then\n@@ -72,7 +84,7 @@ assert()\n }\n \n \n-#echo \"Options: $*\"\n+#log \"Options: $*\"\n \n while [ $# -gt 0 ]; do\n \topt=\"$1\"\n@@ -149,7 +161,7 @@ cache_get()\n \tfor oldrev in $*; do\n \t\tif [ -r \"$cachedir/$oldrev\" ]; then\n \t\t\tread newrev <\"$cachedir/$oldrev\"\n-\t\t\techo $newrev\n+\t\t\tlog $newrev\n \t\tfi\n \tdone\n }\n@@ -158,7 +170,7 @@ cache_miss()\n {\n \tfor oldrev in $*; do\n \t\tif [ ! -r \"$cachedir/$oldrev\" ]; then\n-\t\t\techo $oldrev\n+\t\t\tlog $oldrev\n \t\tfi\n \tdone\n }\n@@ -175,7 +187,7 @@ check_parents()\n \n set_notree()\n {\n-\techo \"1\" > \"$cachedir/notree/$1\"\n+\tlog \"1\" > \"$cachedir/notree/$1\"\n }\n \n cache_set()\n@@ -187,7 +199,7 @@ cache_set()\n \t     -a -e \"$cachedir/$oldrev\" ]; then\n \t\tdie \"cache for $oldrev already exists!\"\n \tfi\n-\techo \"$newrev\" >\"$cachedir/$oldrev\"\n+\tlog \"$newrev\" >\"$cachedir/$oldrev\"\n }\n \n rev_exists()\n@@ -219,7 +231,7 @@ rev_is_descendant_of_branch()\n try_remove_previous()\n {\n \tif rev_exists \"$1^\"; then\n-\t\techo \"^$1^\"\n+\t\tlog \"^$1^\"\n \tfi\n }\n \n@@ -247,7 +259,7 @@ find_latest_squash()\n \t\t\t\t\t\tsq=\"$sub\"\n \t\t\t\t\tfi\n \t\t\t\t\tdebug \"Squash found: $sq $sub\"\n-\t\t\t\t\techo \"$sq\" \"$sub\"\n+\t\t\t\t\tlog \"$sq\" \"$sub\"\n \t\t\t\t\tbreak\n \t\t\t\tfi\n \t\t\t\tsq=\n@@ -339,9 +351,9 @@ add_msg()\n add_squashed_msg()\n {\n \tif [ -n \"$message\" ]; then\n-\t\techo \"$message\"\n+\t\tlog \"$message\"\n \telse\n-\t\techo \"Merge commit '$1' as '$2'\"\n+\t\tlog \"Merge commit '$1' as '$2'\"\n \tfi\n }\n \n@@ -373,17 +385,17 @@ squash_msg()\n \t\n \tif [ -n \"$oldsub\" ]; then\n \t\toldsub_short=$(git rev-parse --short \"$oldsub\")\n-\t\techo \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n+\t\tlog \"Squashed '$dir/' changes from $oldsub_short..$newsub_short\"\n \t\techo\n \t\tgit log --pretty=tformat:'%h %s' \"$oldsub..$newsub\"\n \t\tgit log --pretty=tformat:'REVERT: %h %s' \"$newsub..$oldsub\"\n \telse\n-\t\techo \"Squashed '$dir/' content from commit $newsub_short\"\n+\t\tlog \"Squashed '$dir/' content from commit $newsub_short\"\n \tfi\n \t\n \techo\n-\techo \"git-subtree-dir: $dir\"\n-\techo \"git-subtree-split: $newsub\"\n+\tlog \"git-subtree-dir: $dir\"\n+\tlog \"git-subtree-split: $newsub\"\n }\n \n toptree_for_commit()\n@@ -401,7 +413,7 @@ subtree_for_commit()\n \t\tassert [ \"$name\" = \"$dir\" ]\n \t\tassert [ \"$type\" = \"tree\" -o \"$type\" = \"commit\" ]\n \t\t[ \"$type\" = \"commit\" ] && continue  # ignore submodules\n-\t\techo $tree\n+\t\tlog $tree\n \t\tbreak\n \tdone\n }\n@@ -474,7 +486,7 @@ copy_or_skip()\n \tdone\n \t\n \tif [ -n \"$identical\" ]; then\n-\t\techo $identical\n+\t\tlog $identical\n \telse\n \t\tcopy_commit $rev $tree \"$p\" || exit $?\n \tfi\n@@ -526,7 +538,7 @@ cmd_add()\n \n cmd_add_repository()\n {\n-\techo \"git fetch\" \"$@\"\n+\tlog \"git fetch\" \"$@\"\n \trepository=$1\n \trefspec=$2\n \tgit fetch \"$@\" || exit $?\n@@ -599,7 +611,7 @@ cmd_split()\n \teval \"$grl\" |\n \twhile read rev parents; do\n \t\trevcount=$(($revcount + 1))\n-\t\tsay -n \"$revcount/$revmax ($createcount)\r\"\n+\t\tstate \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n \t\texists=$(cache_get $rev)\n \t\tif [ -n \"$exists\" ]; then\n@@ -656,7 +668,7 @@ cmd_split()\n \t\tgit update-ref -m 'subtree split' \"refs/heads/$branch\" $latest_new || exit $?\n \t\tsay \"$action branch '$branch'\"\n \tfi\n-\techo $latest_new\n+\tlog $latest_new\n \texit 0\n }\n \n@@ -726,7 +738,7 @@ cmd_push()\n \tif [ -e \"$dir\" ]; then\n \t    repository=$1\n \t    refspec=$2\n-\t    echo \"git push using: \" $repository $refspec\n+\t    log \"git push using: \" $repository $refspec\n \t    localrev=$(git subtree split --prefix=\"$prefix\") || die\n \t    git push $repository $localrev:refs/heads/$refspec\n \telse\n-- \n2.3.7.windows.1\n\n"},{"id":"260767","messageId":"xmqqmw1gp7aa.fsf@gitster.dls.corp.google.com","threadId":"39266","inReplyTo":"CAMbsUu6XT4hB0L0PjgJKniyBJ9svkQoJqnxiYaRWo9EZUXnNhg@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-07T18:33:01Z","receivedAt":"2015-05-07T18:33:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danny Lin <danny0838@gmail.com> writes:\n\n> Replace all echo using printf for better portability.\n\nI doubt this change is sensible.\n\nIt is not like \"echo is bad, don't use it\".  It is more about \"some\nfeatures of 'echo', like 'echo -n $msg' vs 'echo $msg\\c' are not\nportable\".\n\n>  \"\n> -eval \"$(echo \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" || echo exit $?)\"\n> +eval \"$(printf %s \"$OPTS_SPEC\" | git rev-parse --parseopt -- \"$@\" || printf %s \"exit $?\")\"\n\nI do not think we want this.\n\n>  PATH=$PATH:$(git --exec-path)\n>  . git-sh-setup\n> @@ -51,17 +51,29 @@ prefix=\n>  debug()\n>  {\n>  \tif [ -n \"$debug\" ]; then\n> -\t\techo \"$@\" >&2\n> +\t\tprintf \"%s\\n\" \"$*\" >&2\n>  \tfi\n>  }\n>  \n>  say()\n>  {\n>  \tif [ -z \"$quiet\" ]; then\n> -\t\techo \"$@\" >&2\n> +\t\tprintf \"%s\\n\" \"$*\" >&2\n>  \tfi\n>  }\n\nThese are OK.\n\n> +state()\n> +{\n> +\tif [ -z \"$quiet\" ]; then\n> +\t\tprintf \"%s\\r\" \"$*\" >&2\n> +\tfi\n> +}\n\nThis is good, but I think it is misnamed.  \"progress\" might be more\nappropriate.\n\n> +\n> +log()\n> +{\n> +\tprintf \"%s\\n\" \"$*\"\n> +}\n\nI do not think we need this.\n\n> @@ -72,7 +84,7 @@ assert()\n>  }\n>  \n>  \n> -#echo \"Options: $*\"\n> +#log \"Options: $*\"\n\nDefinitely not.\n\n>  while [ $# -gt 0 ]; do\n>  \topt=\"$1\"\n> @@ -149,7 +161,7 @@ cache_get()\n>  \tfor oldrev in $*; do\n>  \t\tif [ -r \"$cachedir/$oldrev\" ]; then\n>  \t\t\tread newrev <\"$cachedir/$oldrev\"\n> -\t\t\techo $newrev\n> +\t\t\tlog $newrev\n\nWe know this is 40-hex, and there is no magic, don't we?\n\n> @@ -158,7 +170,7 @@ cache_miss()\n>  {\n>  \tfor oldrev in $*; do\n>  \t\tif [ ! -r \"$cachedir/$oldrev\" ]; then\n> -\t\t\techo $oldrev\n> +\t\t\tlog $oldrev\n\nLikewise.\n\nAnd I'll stop saying \"Likewise\" at this point.\n\n> @@ -599,7 +611,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> +\t\tstate \"$revcount/$revmax ($createcount)\"\n>  \t\tdebug \"Processing commit: $rev\"\n>  \t\texists=$(cache_get $rev)\n>  \t\tif [ -n \"$exists\" ]; then\n\nGood.\n\nIf we wanted to make \"state\" (or \"progress\") to be usable in a wider\ncontext, we may want to change its implementation a little bit, but\nthat is a separate topic.  It only has a single caller, and it only\nfeeds ever growing string, so the \"print and then carriage-return\"\nis sufficient for now.\n\nThanks.\n"},{"id":"260789","messageId":"1431046299-5984-1-git-send-email-danny0838@gmail.com","threadId":"39266","inReplyTo":"xmqqmw1gp7aa.fsf@gitster.dls.corp.google.com","subject":"[PATCH] contrib/subtree: portability fix for string printing","fromName":"Danny Lin","fromEmail":"danny0838@gmail.com","sentAt":"2015-05-08T00:51:39Z","receivedAt":"2015-05-08T00:51:39Z","isPatch":true,"sender":{"key":"danny0838@gmail.com","avatar":"https://avatars.githubusercontent.com/u/531417?v=4"},"body":"Replace echo using printf in debug() and say() for\nbetter portability.\n\nAlso re-wrap previous 'say -n \"$str<CR>\"' using a new\nfunction progress() to prevent CR chars included in the\nsource code, which could be mal-processed in some shells.\nFor example, MsysGit trims CR before executing a shell\nscript file in order to make it work right on Windows\neven if it uses CRLF as linefeeds.\n\nSigned-off-by: Danny Lin <danny0838@gmail.com>\n---\n contrib/subtree/git-subtree.sh | 15 +++++++++++----\n 1 file changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex fa1a583..6f6ddbe 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -51,14 +51,21 @@ prefix=\n debug()\n {\n \tif [ -n \"$debug\" ]; then\n-\t\techo \"$@\" >&2\n+\t\tprintf \"%s\\n\" \"$*\" >&2\n \tfi\n }\n \n say()\n {\n \tif [ -z \"$quiet\" ]; then\n-\t\techo \"$@\" >&2\n+\t\tprintf \"%s\\n\" \"$*\" >&2\n+\tfi\n+}\n+\n+progress()\n+{\n+\tif [ -z \"$quiet\" ]; then\n+\t\tprintf \"%s\\r\" \"$*\" >&2\n \tfi\n }\n \n@@ -247,7 +254,7 @@ find_latest_squash()\n \t\t\t\t\t\tsq=\"$sub\"\n \t\t\t\t\tfi\n \t\t\t\t\tdebug \"Squash found: $sq $sub\"\n-\t\t\t\t\techo \"$sq\" \"$sub\"\n+\t\t\t\t\tlog \"$sq\" \"$sub\"\n \t\t\t\t\tbreak\n \t\t\t\tfi\n \t\t\t\tsq=\n@@ -599,7 +606,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\tprogress \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n \t\texists=$(cache_get $rev)\n \t\tif [ -n \"$exists\" ]; then\n-- \n2.3.7.windows.1\n"},{"id":"260790","messageId":"1431046619-2340-1-git-send-email-danny0838@gmail.com","threadId":"39266","inReplyTo":"xmqqmw1gp7aa.fsf@gitster.dls.corp.google.com","subject":"[PATCH] contrib/subtree: portability fix for string printing","fromName":"Danny Lin","fromEmail":"danny0838@gmail.com","sentAt":"2015-05-08T00:56:59Z","receivedAt":"2015-05-08T00:56:59Z","isPatch":true,"sender":{"key":"danny0838@gmail.com","avatar":"https://avatars.githubusercontent.com/u/531417?v=4"},"body":"Replace echo using printf in debug() and say() for\nbetter portability.\n\nAlso re-wrap previous 'say -n \"$str<CR>\"' using a new\nfunction progress() to prevent CR chars included in the\nsource code, which could be mal-processed in some shells.\nFor example, MsysGit trims CR before executing a shell\nscript file in order to make it work right on Windows\neven if it uses CRLF as linefeeds.\n\nSigned-off-by: Danny Lin <danny0838@gmail.com>\n---\n contrib/subtree/git-subtree.sh | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex fa1a583..d4dae7a 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -51,14 +51,21 @@ prefix=\n debug()\n {\n \tif [ -n \"$debug\" ]; then\n-\t\techo \"$@\" >&2\n+\t\tprintf \"%s\\n\" \"$*\" >&2\n \tfi\n }\n \n say()\n {\n \tif [ -z \"$quiet\" ]; then\n-\t\techo \"$@\" >&2\n+\t\tprintf \"%s\\n\" \"$*\" >&2\n+\tfi\n+}\n+\n+progress()\n+{\n+\tif [ -z \"$quiet\" ]; then\n+\t\tprintf \"%s\\r\" \"$*\" >&2\n \tfi\n }\n \n@@ -599,7 +606,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\tprogress \"$revcount/$revmax ($createcount)\"\n \t\tdebug \"Processing commit: $rev\"\n \t\texists=$(cache_get $rev)\n \t\tif [ -n \"$exists\" ]; then\n-- \n2.3.7.windows.1\n\nPrevious patch had a flaw, revised.\n"},{"id":"260828","messageId":"xmqqy4kzklhp.fsf@gitster.dls.corp.google.com","threadId":"39266","inReplyTo":"1431046619-2340-1-git-send-email-danny0838@gmail.com","subject":"Re: [PATCH] contrib/subtree: portability fix for string printing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-08T17:49:38Z","receivedAt":"2015-05-08T17:49:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danny Lin <danny0838@gmail.com> writes:\n\n> Replace echo using printf in debug() and say() for\n> better portability.\n>\n> Also re-wrap previous 'say -n \"$str<CR>\"' using a new\n> function progress() to prevent CR chars included in the\n> source code, which could be mal-processed in some shells.\n> For example, MsysGit trims CR before executing a shell\n> script file in order to make it work right on Windows\n> even if it uses CRLF as linefeeds.\n>\n> Signed-off-by: Danny Lin <danny0838@gmail.com>\n> ---\n\nThanks, this looks good.  Will apply with a little bit of tweak in\nthe log message.\n\nJust for future reference, when shooting many iterations of the same\npatch in a short timeframe, please be aware that the recipient may\nnot get the messages in the order you sent, and that it may not be\napparent to the recipients what changed between the iterations.\nWhat we commonly do around here to address these issues is to\nmention what changed from the previous one below the \"---\" line\nbefore the diffstat.  I would have done something like this if I\nwere doing this patch, for example:\n\n        ...\n        even if it uses CRLF as linefeeds.\n\n        Signed-off-by: Danny Lin <danny0838@gmail.com>\n        ---\n\n        * The previous one still used \"log\" helper by mistake even\n          though I removed the implementation of it and decided to\n          use \"echo\" for non-tricky cases.  This fixes it.\n\n          contrib/subtree/git-subtree.sh | 13 ++++++++++---\n        ...\n"},{"id":"260829","messageId":"CAPig+cQQSrQiSzp7Jat8LYH+RqYdpJ2XCXweAtrYE_QoLzSznQ@mail.gmail.com","threadId":"39266","inReplyTo":"xmqqy4kzklhp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/subtree: portability fix for string printing","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-08T17:56:34Z","receivedAt":"2015-05-08T17:56:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 8, 2015 at 1:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Danny Lin <danny0838@gmail.com> writes:\n>\n>> Replace echo using printf in debug() and say() for\n>> better portability.\n>>\n>> Also re-wrap previous 'say -n \"$str<CR>\"' using a new\n>> function progress() to prevent CR chars included in the\n>> source code, which could be mal-processed in some shells.\n>> For example, MsysGit trims CR before executing a shell\n>> script file in order to make it work right on Windows\n>> even if it uses CRLF as linefeeds.\n>>\n>> Signed-off-by: Danny Lin <danny0838@gmail.com>\n>> ---\n>\n> Thanks, this looks good.  Will apply with a little bit of tweak in\n> the log message.\n\nHmm, I would say that the changes to debug() and say() should either\nbe dropped or moved to a separate patch (along with the first\nparagraph of the commit message). With the introduction of the\nprogress() abstraction, there is no longer any need for changes to\nsay(), and the \"better portability\" rationale for changing say() and\ndebug() is never properly explained, and is thus nebulous at best.\n"},{"id":"260838","messageId":"xmqqpp6alxiw.fsf@gitster.dls.corp.google.com","threadId":"39266","inReplyTo":"CAPig+cQQSrQiSzp7Jat8LYH+RqYdpJ2XCXweAtrYE_QoLzSznQ@mail.gmail.com","subject":"Re: [PATCH] contrib/subtree: portability fix for string printing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-08T18:44:23Z","receivedAt":"2015-05-08T18:44:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Fri, May 8, 2015 at 1:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Danny Lin <danny0838@gmail.com> writes:\n>>\n>>> Replace echo using printf in debug() and say() for\n>>> better portability.\n>>>\n>>> Also re-wrap previous 'say -n \"$str<CR>\"' using a new\n>>> function progress() to prevent CR chars included in the\n>>> source code, which could be mal-processed in some shells.\n>>> For example, MsysGit trims CR before executing a shell\n>>> script file in order to make it work right on Windows\n>>> even if it uses CRLF as linefeeds.\n>>>\n>>> Signed-off-by: Danny Lin <danny0838@gmail.com>\n>>> ---\n>>\n>> Thanks, this looks good.  Will apply with a little bit of tweak in\n>> the log message.\n>\n> Hmm, I would say that the changes to debug() and say() should either\n> be dropped or moved to a separate patch (along with the first\n> paragraph of the commit message). With the introduction of the\n> progress() abstraction, there is no longer any need for changes to\n> say(), and the \"better portability\" rationale for changing say() and\n> debug() is never properly explained, and is thus nebulous at best.\n\nI justified them in this way.\n\n    contrib/subtree: portability fix for string printing\n    \n    'echo -n' is not portable, but this script used it as a way to give\n    a string followed by a carriage return for progress messages.\n    Introduce a new helper shell function \"progress\" and use printf as a\n    more portable way to do this.  As a side effect, this makes it\n    unnecessary to have a raw CR in our source, which can be munged in\n    some shells.  For example, MsysGit trims CR before executing a shell\n    script file in order to make it work right on Windows even if it\n    uses CRLF as linefeeds.\n    \n    While at it, replace \"echo\" using printf in debug() and say() to\n    avoid tempting people introducing the same bug.\n    \n    Signed-off-by: Danny Lin <danny0838@gmail.com>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"260840","messageId":"CAPig+cQVpYseCs7V_zHbUhEbWitdNZx1UJgHSdd2svowBOxsYg@mail.gmail.com","threadId":"39266","inReplyTo":"xmqqpp6alxiw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/subtree: portability fix for string printing","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-08T18:55:17Z","receivedAt":"2015-05-08T18:55:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 8, 2015 at 2:44 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>> On Fri, May 8, 2015 at 1:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Thanks, this looks good.  Will apply with a little bit of tweak in\n>>> the log message.\n>>\n>> Hmm, I would say that the changes to debug() and say() should either\n>> be dropped or moved to a separate patch (along with the first\n>> paragraph of the commit message). With the introduction of the\n>> progress() abstraction, there is no longer any need for changes to\n>> say(), and the \"better portability\" rationale for changing say() and\n>> debug() is never properly explained, and is thus nebulous at best.\n>\n> I justified them in this way.\n>\n>     contrib/subtree: portability fix for string printing\n>\n>     'echo -n' is not portable, but this script used it as a way to give\n>     a string followed by a carriage return for progress messages.\n>     Introduce a new helper shell function \"progress\" and use printf as a\n>     more portable way to do this.  As a side effect, this makes it\n>     unnecessary to have a raw CR in our source, which can be munged in\n>     some shells.  For example, MsysGit trims CR before executing a shell\n>     script file in order to make it work right on Windows even if it\n>     uses CRLF as linefeeds.\n\nVery nicely explained.\n\n>     While at it, replace \"echo\" using printf in debug() and say() to\n>     avoid tempting people introducing the same bug.\n\nOkay, this works as reasonable justification for including those\nchanges in the same patch.\n\nIt might read a bit more fluidly if rephrased something like this:\n\n    While at it, replace 'echo' with 'printf' in debug() and say() to\n    eliminate the temptation of reintroducing the same bug.\n"}]}