{"thread":{"id":"40273","subject":"[PATCH] filter-branch: add passed/remaining seconds on progress","startedAt":"2015-09-04T15:16:38Z","lastAt":"2015-09-23T20:48:55Z","messageCount":19,"participants":["Gábor Bernát","Junio C Hamano","Eric Sunshine","Gabor Bernat","Ramsay Jones","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"269425","messageId":"1441379798-15453-1-git-send-email-bernat@primeranks.net","threadId":"40273","inReplyTo":null,"subject":"[PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gábor Bernát","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-04T15:16:38Z","receivedAt":"2015-09-04T15:16:38Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"From: Gabor Bernat \n\nadds seconds progress and estimated seconds time if getting the current\ntimestamp is supported by the date %+s command\n\nSigned-off-by: Gabor Bernat \n---\n\nI've submitted this first to this list as a feature request, however\nin the meantime with the help of Jeff King , Junio C\nHamano , Eric Sunshine \nand Mikael Magnusson came up with solution, so now\nI submit it as a revised patch.\n\nThe current solution updates the progress for all commits until 1\nsecond time is elapsed. Afterwards updates it at most once a second.\n---\n git-filter-branch.sh | 36 +++++++++++++++++++++++++++++++++++-\n 1 file changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5b3f63d..5e9ae0f 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -277,9 +277,43 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n # Rewrite the commits\n \n git_filter_branch__commit_count=0\n+\n+echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t\n+case \"$show_seconds\" in\n+\tt)\n+\t\tstart_timestamp=$(date +%s)\n+\t\tnext_sample_at=0\n+\t\t;;\n+\t'')\n+\t\tprogress=\"\"\n+\t\t;;\n+esac\n+\n while read commit parents; do\n \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n-\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n+\n+\tcase \"$show_seconds\" in\n+\tt)\n+\t\tif test $git_filter_branch__commit_count -gt $next_sample_at\n+\t\tthen\n+\t\t\tnow_timestamp=$(date +%s)\n+\t\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n+\t\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n+\t\t\tif test $elapsed_seconds -gt 0\n+\t\t\tthen\n+\t\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n+\t\t\telse\n+\t\t\t\tnext_sample_at=$(($next_sample_at + 1))\n+\t\t\tfi\n+\t\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n+\t\tfi\n+\t\t;;\n+\t'')\n+\t\tprogress=\"\"\n+\t\t;;\n+\tesac\n+\n+\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n \n \tcase \"$filter_subdir\" in\n \t\"\")\n-- \n2.5.1.408.g431338e\n"},{"id":"269438","messageId":"xmqqk2s6f2zj.fsf@gitster.mtv.corp.google.com","threadId":"40273","inReplyTo":"1441379798-15453-1-git-send-email-bernat@primeranks.net","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-04T18:34:08Z","receivedAt":"2015-09-04T18:34:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gábor Bernát writes:\n\n> @@ -277,9 +277,43 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n> # Rewrite the commits\n> \n> git_filter_branch__commit_count=0\n\nThis is not a new problem, but I wonder why we need such a\ncumbersomely long variable name. It is not like this is a part of\nsome shell script library that needs to be careful about namespace\npollution.\n\n> +echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t\n\nThat is very strange construct. I think you meant to say something\nlike\n\n\tif date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n\tthen\n\t\tshow_seconds=t\n\telse\n \tshow_seconds=\n\tfi\n\nA handful of points:\n\n * \"echo $(any-command)\" is suspect, unless you are trying to let\n the shell munge output from any-command, which is not the case.\n\n * \"grep\" without -E (or \"egrep\") takes BRE, which \"+\" (one or more)\n is not part of.\n\n * That semicolon is a syntax error. I think whoever suggested you\n to use it meant to squelch possible errors from \"date\" that does\n not understand the \"%s\" format.\n\n * I do not think you are clearing show_seconds to empty anywhere,\n so an environment variable the user may have when s/he starts\n filter-branch will seep through and confuse you.\n\n> +case \"$show_seconds\" in\n> +\tt)\n> +\t\tstart_timestamp=$(date +%s)\n> +\t\tnext_sample_at=0\n> +\t\t;;\n> +\t'')\n> +\t\tprogress=\"\"\n> +\t\t;;\n> +esac\n\nIn our codebase case labels and case/esac align, like you did in the\nlater part of the patch.\n> +\n> while read commit parents; do\n> \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n> -\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n> +\n> +\tcase \"$show_seconds\" in\n> +\tt)\n> +\t\tif test $git_filter_branch__commit_count -gt $next_sample_at\n> +\t\tthen\n> +\t\t\tnow_timestamp=$(date +%s)\n> +\t\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n> +\t\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n> +\t\t\tif test $elapsed_seconds -gt 0\n> +\t\t\tthen\n> +\t\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n> +\t\t\telse\n> +\t\t\t\tnext_sample_at=$(($next_sample_at + 1))\n> +\t\t\tfi\n> +\t\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n> +\t\tfi\n> +\t\t;;\n> +\t'')\n> +\t\tprogress=\"\"\n> +\t\t;;\n> +\tesac\n> +\n> +\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n\nIt would be easier to follow the logic of this loop whose _primary_\npoint is to rewrite one commit if you moved this part into a helper\nfunction. Then the loop would look more like:\n\n\twhile read commit parents\n do\n \t: $(( $git_filter_branch__commit_count++ ))\n\t\treport_progress\n\n case \"$filter_subdir\" in\n ...\n\n\t\t# all the work that is about rewriting this commit\n\t\t# comes here.\n\n\tdone\n"},{"id":"269442","messageId":"CAPig+cRh-7BDOoumLxyh6_tNspL3ANq_wCE5f_VoQt6UwUFckQ@mail.gmail.com","threadId":"40273","inReplyTo":"xmqqk2s6f2zj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-04T20:15:15Z","receivedAt":"2015-09-04T20:15:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 4, 2015 at 2:34 PM, Junio C Hamano wrote:\n> Gábor Bernát writes:\n>> +echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t\n>\n> That is very strange construct. I think you meant to say something\n> like\n>\n> if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n> then\n> show_seconds=t\n> else\n> show_seconds=\n> fi\n\nThe final format suggested[1] for this test was:\n\n { echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&\n show_eta=t\n\n> A handful of points:\n>\n> * \"echo $(any-command)\" is suspect, unless you are trying to let\n> the shell munge output from any-command, which is not the case.\n\nPrimarily my fault. I don't know what I was thinking when suggesting that.\n\n> * \"grep\" without -E (or \"egrep\") takes BRE, which \"+\" (one or more)\n> is not part of.\n\nThis seems to have mutated from the suggested form.\n\n> * That semicolon is a syntax error. I think whoever suggested you\n> to use it meant to squelch possible errors from \"date\" that does\n> not understand the \"%s\" format.\n\nThis also mutated. The suggested form wanted to suppress errors from\n'date' if it complained about \"%s\", and from 'grep'. In retrospect,\napplying it to 'grep' is questionable. I was recalling this warning\nfrom the Autoconf manual[2]:\n\n Some of the options required by Posix are not portable in\n practice. Don't use ‘grep -q’ to suppress output, because many\n grep implementations (e.g., Solaris) do not support -q. Don't use\n ‘grep -s’ to suppress output either, because Posix says -s does\n not suppress output, only some error messages; also, the -s\n option of traditional grep behaved like -q does in most modern\n implementations. Instead, redirect the standard output and\n standard error (in case the file doesn't exist) of grep to\n /dev/null. Check the exit status of grep to determine whether it\n found a match.\n\nhowever, Git tests use 'grep -q' heavily, so perhaps we don't worry about that.\n\n> * I do not think you are clearing show_seconds to empty anywhere,\n> so an environment variable the user may have when s/he starts\n> filter-branch will seep through and confuse you.\n\nThe empty assignment was implied in my example, but I should have been\nmore explicit and shown a more complete snippet:\n\n show_eta=\n ...\n { echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&\n show_eta=t\n\nThe suggested 'if' form has the attribute of being clearer.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/276531/focus=276837\n[2]: https://www.gnu.org/software/autoconf/manual/autoconf.html#grep\n"},{"id":"269528","messageId":"CANy2qHfHmydkn6BtoDFy0bOfrvRe03L+EO+ofjD5D3wDLdjW=A@mail.gmail.com","threadId":"40273","inReplyTo":"CAPig+cRh-7BDOoumLxyh6_tNspL3ANq_wCE5f_VoQt6UwUFckQ@mail.gmail.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gabor Bernat","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-06T09:49:38Z","receivedAt":"2015-09-06T09:49:38Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"On Fri, Sep 4, 2015 at 10:15 PM, Eric Sunshine wrote:\n> On Fri, Sep 4, 2015 at 2:34 PM, Junio C Hamano wrote:\n>> Gábor Bernát writes:\n>>> +echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t\n>>\n>> That is very strange construct. I think you meant to say something\n>> like\n>>\n>> if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n>> then\n>> show_seconds=t\n>> else\n>> show_seconds=\n>> fi\n>\n> The final format suggested[1] for this test was:\n>\n> { echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&\n> show_eta=t\n>\n>> A handful of points:\n>>\n>> * \"echo $(any-command)\" is suspect, unless you are trying to let\n>> the shell munge output from any-command, which is not the case.\n>\n> Primarily my fault. I don't know what I was thinking when suggesting that.\n\nYes, the initial construct was different, willing to make this change.\n\n>\n>> * \"grep\" without -E (or \"egrep\") takes BRE, which \"+\" (one or more)\n>> is not part of.\n>\n> This seems to have mutated from the suggested form.\n>\n>> * That semicolon is a syntax error. I think whoever suggested you\n>> to use it meant to squelch possible errors from \"date\" that does\n>> not understand the \"%s\" format.\n>\n> This also mutated. The suggested form wanted to suppress errors from\n> 'date' if it complained about \"%s\", and from 'grep'. In retrospect,\n> applying it to 'grep' is questionable. I was recalling this warning\n> from the Autoconf manual[2]:\n>\n> Some of the options required by Posix are not portable in\n> practice. Don't use ‘grep -q’ to suppress output, because many\n> grep implementations (e.g., Solaris) do not support -q. Don't use\n> ‘grep -s’ to suppress output either, because Posix says -s does\n> not suppress output, only some error messages; also, the -s\n> option of traditional grep behaved like -q does in most modern\n> implementations. Instead, redirect the standard output and\n> standard error (in case the file doesn't exist) of grep to\n> /dev/null. Check the exit status of grep to determine whether it\n> found a match.\n>\n> however, Git tests use 'grep -q' heavily, so perhaps we don't worry about that.\n\nSo we should keep it as it is.\n\n>\n>> * I do not think you are clearing show_seconds to empty anywhere,\n>> so an environment variable the user may have when s/he starts\n>> filter-branch will seep through and confuse you.\n>\n> The empty assignment was implied in my example, but I should have been\n> more explicit and shown a more complete snippet:\n>\n> show_eta=\n> ...\n> { echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&\n> show_eta=t\n>\n> The suggested 'if' form has the attribute of being clearer.\n\nMy bad, sorry for that. Will amend.\n\n>\n> [1]: http://thread.gmane.org/gmane.comp.version-control.git/276531/focus=276837\n> [2]: https://www.gnu.org/software/autoconf/manual/autoconf.html#grep\n\nAny other pain points, or this construction will satisfy everybody?\n"},{"id":"269530","messageId":"CAPig+cRifOpvz87j9xPP_sUpGtr_oz5SsQ-87ZmEEnQNZ3yXyA@mail.gmail.com","threadId":"40273","inReplyTo":"CANy2qHfHmydkn6BtoDFy0bOfrvRe03L+EO+ofjD5D3wDLdjW=A@mail.gmail.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-06T10:05:53Z","receivedAt":"2015-09-06T10:05:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Sep 6, 2015 at 5:49 AM, Gabor Bernat wrote:\n> On Fri, Sep 4, 2015 at 10:15 PM, Eric Sunshine wrote:\n>> On Fri, Sep 4, 2015 at 2:34 PM, Junio C Hamano wrote:\n>>> Gábor Bernát writes:\n>>>> +echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t\n>>>\n>>> That is very strange construct. I think you meant to say something\n>>> like\n>>>\n>>> if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n>>> then\n>>> show_seconds=t\n>>> else\n>>> show_seconds=\n>>> fi\n>>\n>> This also mutated. The suggested form wanted to suppress errors from\n>> 'date' if it complained about \"%s\", and from 'grep'. In retrospect,\n>> applying it to 'grep' is questionable. I was recalling this warning\n>> from the Autoconf manual[2]:\n>>\n>> Some of the options required by Posix are not portable in\n>> practice. Don't use ‘grep -q’ to suppress output, because many\n>> grep implementations (e.g., Solaris) do not support -q. Don't use\n>> ‘grep -s’ to suppress output either, because Posix says -s does\n>> not suppress output, only some error messages; also, the -s\n>> option of traditional grep behaved like -q does in most modern\n>> implementations. Instead, redirect the standard output and\n>> standard error (in case the file doesn't exist) of grep to\n>> /dev/null. Check the exit status of grep to determine whether it\n>> found a match.\n>>\n>> however, Git tests use 'grep -q' heavily, so perhaps we don't worry about that.\n>\n> So we should keep it as it is.\n\nUse of 'grep -q' seems to be fine, however, Junio's comment was about\nthe errant semicolon, which should not be kept.\n\n>>> * I do not think you are clearing show_seconds to empty anywhere,\n>>> so an environment variable the user may have when s/he starts\n>>> filter-branch will seep through and confuse you.\n>>\n>> The empty assignment was implied in my example, but I should have been\n>> more explicit and shown a more complete snippet:\n>>\n>> show_eta=\n>> ...\n>> { echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&\n>> show_eta=t\n>>\n>> The suggested 'if' form has the attribute of being clearer.\n>\n> My bad, sorry for that. Will amend.\n>\n> Any other pain points, or this construction will satisfy everybody?\n\nJunio's proposed if/then/else construct should be satisfactory.\n"},{"id":"269533","messageId":"1441545064-3126-1-git-send-email-bernat@primeranks.net","threadId":"40273","inReplyTo":"1441379798-15453-1-git-send-email-bernat@primeranks.net","subject":"[PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gábor Bernát","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-06T13:11:04Z","receivedAt":"2015-09-06T13:11:04Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"From: Gabor Bernat \n\nadds seconds progress and estimated seconds time if getting the current\ntimestamp is supported by the date %+s command\n\nSigned-off-by: Gabor Bernat \n---\n\nI've submitted this first to this list as a feature request, however\nin the meantime with the help of Jeff King , Junio C\nHamano , Eric Sunshine \nand Mikael Magnusson came up with solution, so now\nI submit it as a revised patch.\n\nThe current solution updates the progress for all commits until 1\nsecond time is elapsed. Afterwards updates it at most once a second.\n---\n git-filter-branch.sh | 42 +++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5b3f63d..924cf3d 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -277,9 +277,49 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n # Rewrite the commits\n \n git_filter_branch__commit_count=0\n+\n+if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n+then\n+\tshow_seconds=t\n+else\n+\tshow_seconds=\n+fi\n+\n+case \"$show_seconds\" in\n+t)\n+\tstart_timestamp=$(date +%s)\n+\tnext_sample_at=0\n+\t;;\n+'')\n+\tprogress=\"\"\n+\t;;\n+esac\n+\n while read commit parents; do\n \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n-\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n+\n+\tcase \"$show_seconds\" in\n+\tt)\n+\t\tif test $git_filter_branch__commit_count -gt $next_sample_at\n+\t\tthen\n+\t\t\tnow_timestamp=$(date +%s)\n+\t\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n+\t\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n+\t\t\tif test $elapsed_seconds -gt 0\n+\t\t\tthen\n+\t\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n+\t\t\telse\n+\t\t\t\tnext_sample_at=$(($next_sample_at + 1))\n+\t\t\tfi\n+\t\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n+\t\tfi\n+\t\t;;\n+\t'')\n+\t\tprogress=\"\"\n+\t\t;;\n+\tesac\n+\n+\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n \n \tcase \"$filter_subdir\" in\n \t\"\")\n-- \n2.6.0.rc0.3.gb3280a4\n"},{"id":"269541","messageId":"xmqqpp1va00p.fsf@gitster.mtv.corp.google.com","threadId":"40273","inReplyTo":"1441545064-3126-1-git-send-email-bernat@primeranks.net","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-06T18:13:58Z","receivedAt":"2015-09-06T18:13:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gábor Bernát writes:\n\n> +if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n> +then\n> +\tshow_seconds=t\n> +else\n> +\tshow_seconds=\n> +fi\n> +\n> +case \"$show_seconds\" in\n> +t)\n> +\tstart_timestamp=$(date +%s)\n> +\tnext_sample_at=0\n> +\t;;\n> +'')\n> +\tprogress=\"\"\n> +\t;;\n> +esac\n\nWhy the case statement here?\n\nFor that matter, you probably do not need $show_seconds and use the\nfact that start_timestamp and progress are only set to a non-empty\nstring when we measure progress, i.e. making all of the above\nsomething more like this:\n\n progress= start_timestamp=\n if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n then\n next_sample_at=0\n progress=\"dummy to ensure this is not empty\"\n start_timestamp=$(date '+%s')\n fi\n\nIf you are more efficiency-minded, I think you could even do with a\nsingle call to /bin/date, perhaps like so:\n\n next_sample_at= progress=\n start_timestamp=$(date '+%s' 2>/dev/null) \n case \"$start_timestamp\" in\n *[!0-9]* | \"\")\n # not a \"digit only\" output\n ;;\n ?*)\n progress=\"dummy to ensure this is not empty\"\n next_sample_at=0\n ;;\n esac\n\nbut I suspect that may be going a bit too far ;-).\n\n> while read commit parents; do\n> \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n> -\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n> +\n> +\tcase \"$show_seconds\" in\n> +\tt)\n> +\t\tif test $git_filter_branch__commit_count -gt $next_sample_at\n> +\t\tthen\n> +\t\t\tnow_timestamp=$(date +%s)\n> +\t\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n> +\t\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n> +\t\t\tif test $elapsed_seconds -gt 0\n> +\t\t\tthen\n> +\t\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n> +\t\t\telse\n> +\t\t\t\tnext_sample_at=$(($next_sample_at + 1))\n> +\t\t\tfi\n> +\t\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n> +\t\tfi\n> +\t\t;;\n> +\t'')\n> +\t\tprogress=\"\"\n> +\t\t;;\n> +\tesac\n> +\n> +\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n> \n> \tcase \"$filter_subdir\" in\n> \t\"\")\n\nIt would be easier to follow the logic of this loop whose _primary_\npoint is to rewrite one commit if you moved this part into a helper\nfunction. As written above, the deeply nested case statement whose\npurpose is only to show the progress is very distracting.\n\nThen the loop would read like:\n\n while read commit parents\n do\n : $(( $git_filter_branch__commit_count++ ))\n report_progress\n\n case \"$filter_subdir\" in\n ...\n\n # all the work that is about rewriting this commit\n # comes here.\n\n done\n\nwhich would be much less noisy and helpful to people who are\ninterested in learning what it is doing.\n\nAs I said earlier in this message, the report_progress helper\nfunction can use the fact that $progress or $start_timestamp are\nnon-empty if and only if you need to compute and tack $progress at\nthe end of the output, i.e.\n\n report_progress ()\n {\n if test -n \"$progress\"\n then\n ... do rate computation here ...\n progress=\" ($elapsed_seconds seconds passed,...\"\n fi\n printf \"\\rRewrite $commit (...)$progress\"\n }\n"},{"id":"269562","messageId":"1441629095-32004-1-git-send-email-bernat@primeranks.net","threadId":"40273","inReplyTo":"1441379798-15453-1-git-send-email-bernat@primeranks.net","subject":"[PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gábor Bernát","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-07T12:31:35Z","receivedAt":"2015-09-07T12:31:35Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"From: Gabor Bernat \n\nadds seconds progress and estimated seconds time if getting the current\ntimestamp is supported by the date %+s command\n\nSigned-off-by: Gabor Bernat \n---\n\nI've submitted this first to this list as a feature request, however\nin the meantime with the help of Jeff King , Junio C\nHamano , Eric Sunshine \nand Mikael Magnusson came up with solution, so now\nI submit it as a revised patch.\n\nThe current solution updates the progress for all commits until 1\nsecond time is elapsed. Afterwards updates it at most once a second.\n\nAmmended build up as agreed at [1].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/277314\n---\n git-filter-branch.sh | 32 +++++++++++++++++++++++++++++++-\n 1 file changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5b3f63d..565144a 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -275,11 +275,41 @@ commits=$(wc -l <../revs | tr -d \" \")\n test $commits -eq 0 && die \"Found nothing to rewrite\"\n \n # Rewrite the commits\n+report_progress ()\n+{\n+if test -n \"$progress\"\n+then\n+\tif test $git_filter_branch__commit_count -gt $next_sample_at\n+\tthen\n+\t\tnow_timestamp=$(date +%s)\n+\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n+\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n+\t\tif test $elapsed_seconds -gt 0\n+\t\tthen\n+\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n+\t\telse\n+\t\t\tnext_sample_at=$(($next_sample_at + 1))\n+\t\tfi\n+\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n+\tfi\n+fi\n+printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n+}\n \n git_filter_branch__commit_count=0\n+\n+progress= start_timestamp=\n+if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n+then\n+\t\tnext_sample_at=0\n+\t\tprogress=\"dummy to ensure this is not empty\"\n+\t\tstart_timestamp=$(date '+%s')\n+fi\n+\n while read commit parents; do\n \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n-\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n+\n+\treport_progress\n \n \tcase \"$filter_subdir\" in\n \t\"\")\n-- \n2.6.0.rc0.3.gb3280a4\n"},{"id":"269566","messageId":"55ED94CD.8020907@ramsayjones.plus.com","threadId":"40273","inReplyTo":"1441629095-32004-1-git-send-email-bernat@primeranks.net","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2015-09-07T13:44:45Z","receivedAt":"2015-09-07T13:44:45Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 07/09/15 13:31, Gábor Bernát wrote:\n> From: Gabor Bernat \n>\n> adds seconds progress and estimated seconds time if getting the current\n> timestamp is supported by the date %+s command\n\ns/%+s/+%s/\n\nATB,\nRamsay Jones\n"},{"id":"269567","messageId":"1441633928-18035-1-git-send-email-bernat@primeranks.net","threadId":"40273","inReplyTo":"1441379798-15453-1-git-send-email-bernat@primeranks.net","subject":"[PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gábor Bernát","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-07T13:52:08Z","receivedAt":"2015-09-07T13:52:08Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"From: Gabor Bernat \n\nadds seconds progress and estimated seconds time if getting the current\ntimestamp is supported by the date +%s command\n\nSigned-off-by: Gabor Bernat \n---\n\nI've submitted this first to this list as a feature request, however\nin the meantime with the help of Jeff King , Junio C\nHamano , Eric Sunshine \nand Mikael Magnusson came up with solution, so now\nI submit it as a revised patch.\n\nThe current solution updates the progress for all commits until 1\nsecond time is elapsed. Afterwards updates it at most once a second.\n\nAmmended build up as agreed at [1].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/277314\n---\n git-filter-branch.sh | 32 +++++++++++++++++++++++++++++++-\n 1 file changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5b3f63d..565144a 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -275,11 +275,41 @@ commits=$(wc -l <../revs | tr -d \" \")\n test $commits -eq 0 && die \"Found nothing to rewrite\"\n \n # Rewrite the commits\n+report_progress ()\n+{\n+if test -n \"$progress\"\n+then\n+\tif test $git_filter_branch__commit_count -gt $next_sample_at\n+\tthen\n+\t\tnow_timestamp=$(date +%s)\n+\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n+\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n+\t\tif test $elapsed_seconds -gt 0\n+\t\tthen\n+\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n+\t\telse\n+\t\t\tnext_sample_at=$(($next_sample_at + 1))\n+\t\tfi\n+\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n+\tfi\n+fi\n+printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n+}\n \n git_filter_branch__commit_count=0\n+\n+progress= start_timestamp=\n+if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n+then\n+\t\tnext_sample_at=0\n+\t\tprogress=\"dummy to ensure this is not empty\"\n+\t\tstart_timestamp=$(date '+%s')\n+fi\n+\n while read commit parents; do\n \tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n-\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n+\n+\treport_progress\n \n \tcase \"$filter_subdir\" in\n \t\"\")\n-- \n2.6.0.rc0.3.gb3280a4\n"},{"id":"269582","messageId":"CAPig+cRRMUhWwxAgVHKpMMne7XiOuYGTi_zgQMB=A+XNGUzLqQ@mail.gmail.com","threadId":"40273","inReplyTo":"1441633928-18035-1-git-send-email-bernat@primeranks.net","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-07T20:54:56Z","receivedAt":"2015-09-07T20:54:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát wrote:\n> From: Gabor Bernat \n>\n> adds seconds progress and estimated seconds time if getting the current\n> timestamp is supported by the date +%s command\n>\n> Signed-off-by: Gabor Bernat \n> ---\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 5b3f63d..565144a 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -275,11 +275,41 @@ commits=$(wc -l <../revs | tr -d \" \")\n> test $commits -eq 0 && die \"Found nothing to rewrite\"\n>\n> # Rewrite the commits\n> +report_progress ()\n> +{\n> +if test -n \"$progress\"\n> +then\n\nIndent code within the function...\n\n> + if test $git_filter_branch__commit_count -gt $next_sample_at\n> + then\n> + now_timestamp=$(date +%s)\n> + elapsed_seconds=$(($now_timestamp - $start_timestamp))\n> + remaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n> + if test $elapsed_seconds -gt 0\n> + then\n> + next_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n> + else\n> + next_sample_at=$(($next_sample_at + 1))\n> + fi\n> + progress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n> + fi\n> +fi\n> +printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n\nThe \"\\r\" causes this status line to be overwritten each time through,\nand since the processed commit count always increases, we know that\nthe original (without ETA) will never leave junk at the end of the\nline. However, with estimated seconds also being displayed, does this\nstill hold? While it's true that elapsed seconds will increase,\nestimated seconds may jump around, requiring different numbers of\ndigits to display. This may leave \"garbage\" digits at the end of line\nfrom previous iterations, can't it?\n\n> +}\n>\n> git_filter_branch__commit_count=0\n> +\n> +progress= start_timestamp=\n> +if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n> +then\n> + next_sample_at=0\n> + progress=\"dummy to ensure this is not empty\"\n> + start_timestamp=$(date '+%s')\n> +fi\n> +\n> while read commit parents; do\n> git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n> - printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)\"\n> +\n> + report_progress\n>\n> case \"$filter_subdir\" in\n> \"\")\n> --\n> 2.6.0.rc0.3.gb3280a4\n"},{"id":"269611","messageId":"xmqqsi6o95r7.fsf@gitster.mtv.corp.google.com","threadId":"40273","inReplyTo":"CAPig+cRRMUhWwxAgVHKpMMne7XiOuYGTi_zgQMB=A+XNGUzLqQ@mail.gmail.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-08T17:32:12Z","receivedAt":"2015-09-08T17:32:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát wrote:\n>...\n>> # Rewrite the commits\n>> +report_progress ()\n>> +{\n>> +if test -n \"$progress\"\n>> +then\n>\n> Indent code within the function...\n\nAlso git_filter_branch__commit_count is now used only inside this\nfunction, so it is easier to follow to increment it here.\n\nI suspect that the variable has this unwieldy name for historic\nreasons, perhaps an attempt to avoid name clashes with the end user\nscript, but it has many variables (e.g. $commits, $ref, etc.) that\nare way too generic and that I can see no attempt of name clash\navoidance, so renaming it to $total_commits or something _might_\nmake some sense.\n\n> ...\n>> +printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n>\n> The \"\\r\" causes this status line to be overwritten each time through,\n> and since the processed commit count always increases, we know that\n> the original (without ETA) will never leave junk at the end of the\n> line. However, with estimated seconds also being displayed, does this\n> still hold?\n\nGood point.\n\nPerhaps like this squashed in?\n\n git-filter-branch.sh | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 565144a..30ef513 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -277,27 +277,28 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n # Rewrite the commits\n report_progress ()\n {\n-if test -n \"$progress\"\n-then\n-\tif test $git_filter_branch__commit_count -gt $next_sample_at\n+\tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n+\n+\tif test -n \"$progress\"\n \tthen\n-\t\tnow_timestamp=$(date +%s)\n-\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n-\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n-\t\tif test $elapsed_seconds -gt 0\n+\t\tif test \"$git_filter_branch__commit_count\" -gt \"$next_sample_at\"\n \t\tthen\n-\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n-\t\telse\n-\t\t\tnext_sample_at=$(($next_sample_at + 1))\n+\t\t\tnow_timestamp=$(date \"+%s\")\n+\t\t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n+\t\t\t\tremaining_second=$(( ($commits - $git_filter_branch__commit_count) * $elapsed_seconds / $git_filter_branch__commit_count ))\n+\t\t\tif test $elapsed_seconds -gt 0\n+\t\t\tthen\n+\t\t\t\tnext_sample_at=$(( ($elapsed_seconds + 1) * $git_filter_branch__commit_count / $elapsed_seconds ))\n+\t\t\telse\n+\t\t\t\tnext_sample_at=$(($next_sample_at + 1))\n+\t\t\tfi\n+\t\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n \t\tfi\n-\t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n \tfi\n-fi\n-printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n+\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress \"\n }\n \n git_filter_branch__commit_count=0\n-\n progress= start_timestamp=\n if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'\n then\n@@ -306,9 +307,8 @@ then\n \t\tstart_timestamp=$(date '+%s')\n fi\n \n-while read commit parents; do\n-\tgit_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))\n-\n+while read commit parents\n+do\n \treport_progress\n \n \tcase \"$filter_subdir\" in\n"},{"id":"269612","messageId":"CAPig+cS7ObsWjqbLytCKp1PGF+224TYhC734dNa_HXYQ7p+GgQ@mail.gmail.com","threadId":"40273","inReplyTo":"xmqqsi6o95r7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-08T17:59:17Z","receivedAt":"2015-09-08T17:59:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Sep 8, 2015 at 1:32 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>> On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát wrote:\n>>...\n>>> # Rewrite the commits\n>>> +report_progress ()\n>>> +{\n>>> +if test -n \"$progress\"\n>>> +then\n>>\n>> Indent code within the function...\n>\n> Also git_filter_branch__commit_count is now used only inside this\n> function, so it is easier to follow to increment it here.\n\nMake sense.\n\n>>> +printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n>>\n>> The \"\\r\" causes this status line to be overwritten each time through,\n>> and since the processed commit count always increases, we know that\n>> the original (without ETA) will never leave junk at the end of the\n>> line. However, with estimated seconds also being displayed, does this\n>> still hold?\n>\n> Good point.\n> Perhaps like this squashed in?\n>\n> -printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n> + printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress \"\n\nYes, for an expedient \"fix\", this is what I had in mind, although I\nwould also have added an equal number of backspaces (\\b) following the\nspaces, as a minor aesthetic improvement.\n"},{"id":"269629","messageId":"20150908214437.GB24159@sigill.intra.peff.net","threadId":"40273","inReplyTo":"xmqqsi6o95r7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-09-08T21:44:38Z","receivedAt":"2015-09-08T21:44:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 08, 2015 at 10:32:12AM -0700, Junio C Hamano wrote:\n\n> Also git_filter_branch__commit_count is now used only inside this\n> function, so it is easier to follow to increment it here.\n> \n> I suspect that the variable has this unwieldy name for historic\n> reasons, perhaps an attempt to avoid name clashes with the end user\n> script, but it has many variables (e.g. $commits, $ref, etc.) that\n> are way too generic and that I can see no attempt of name clash\n> avoidance, so renaming it to $total_commits or something _might_\n> make some sense.\n\nI briefly wondered if it had the opposite reason; could it have a\nwell-defined name because it is meant to be a public value the\nuser-defined shell snippets can access?\n\nBut it is not documented, and I can imagine that \"the current count\" is\nnot really useful without \"total number of commits\", so in practice I\ndoubt anybody's filter branch script is relying on it.\n\nAnd looking through the history turns up d5b0c97 (git-filter-branch:\navoid collisions with variables in eval'ed commands, 2009-03-25), which\nseems fairly clear. :)\n\nThe original name was \"i\", which I think is probably too short. Calling\nit something meaningful but longer than one character is probably\nsufficient.\n\n-Peff\n"},{"id":"270421","messageId":"xmqq6133a6tf.fsf@gitster.mtv.corp.google.com","threadId":"40273","inReplyTo":"CAPig+cS7ObsWjqbLytCKp1PGF+224TYhC734dNa_HXYQ7p+GgQ@mail.gmail.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-21T19:52:28Z","receivedAt":"2015-09-21T19:52:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Tue, Sep 8, 2015 at 1:32 PM, Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>>> On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát wrote:\n>>>...\n>>>> # Rewrite the commits\n>>>> +report_progress ()\n>>>> +{\n>>>> +if test -n \"$progress\"\n>>>> +then\n>>>\n>>> Indent code within the function...\n>>\n>> Also git_filter_branch__commit_count is now used only inside this\n>> function, so it is easier to follow to increment it here.\n>\n> Make sense.\n>\n>>>> +printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n>>>\n>>> The \"\\r\" causes this status line to be overwritten each time through,\n>>> and since the processed commit count always increases, we know that\n>>> the original (without ETA) will never leave junk at the end of the\n>>> line. However, with estimated seconds also being displayed, does this\n>>> still hold?\n>>\n>> Good point.\n>> Perhaps like this squashed in?\n>>\n>> -printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n>> + printf \"\\rRewrite $commit\n>> ($git_filter_branch__commit_count/$commits)$progress \"\n>\n> Yes, for an expedient \"fix\", this is what I had in mind, although I\n> would also have added an equal number of backspaces (\\b) following the\n> spaces, as a minor aesthetic improvement.\n\nThis topic seems to have stalled. I do not want to discard topics\nbecause that means all the effort we spent to review and polish the\npatch so far gets wasted, but we cannot leave unfinished topics\nlinger for too long.\n\nFor now, I'll queue this SQUASH??? on top as a minimum fix (renaming\nof variables and other things noticed during the review may be worth\ndoing, but they are not as grave as the issues this fixes, which are\nshow stoppers).\n\nI do not think our in-core progress code does that (and we do not\nuse ESC[0K either), so I'll leave it out of the minimum fix.\n\n\n git-filter-branch.sh | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 565144a..71102d5 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -277,9 +277,8 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n # Rewrite the commits\n report_progress ()\n {\n-if test -n \"$progress\"\n-then\n-\tif test $git_filter_branch__commit_count -gt $next_sample_at\n+\tif test -n \"$progress\" &&\n+\t\ttest $git_filter_branch__commit_count -gt $next_sample_at\n \tthen\n \t\tnow_timestamp=$(date +%s)\n \t\telapsed_seconds=$(($now_timestamp - $start_timestamp))\n@@ -292,8 +291,7 @@ then\n \t\tfi\n \t\tprogress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n \tfi\n-fi\n-printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n+\tprintf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress \"\n }\n \n git_filter_branch__commit_count=0\n-- \n2.6.0-rc2-220-gd6fe230\n"},{"id":"270425","messageId":"CAPig+cRnVzRoyKOzPSJZd4JK_hB+_CBn0kjg4yYv=wWb-5vf7w@mail.gmail.com","threadId":"40273","inReplyTo":"xmqq6133a6tf.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-21T21:22:20Z","receivedAt":"2015-09-21T21:22:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Sep 21, 2015 at 3:52 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>> On Tue, Sep 8, 2015 at 1:32 PM, Junio C Hamano wrote:\n>>> Eric Sunshine writes:\n>>>> On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát wrote:\n>>>>...\n>>>>> # Rewrite the commits\n>>>>> +report_progress ()\n>>>>> +{\n>>>>> +if test -n \"$progress\"\n>>>>> +then\n>>>>\n>>>> Indent code within the function...\n>>>\n>>>>> +printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n>>>>\n>>>> The \"\\r\" causes this status line to be overwritten each time through,\n>>>> and since the processed commit count always increases, we know that\n>>>> the original (without ETA) will never leave junk at the end of the\n>>>> line. However, with estimated seconds also being displayed, does this\n>>>> still hold?\n>>>\n>>> Good point.\n>>> Perhaps like this squashed in?\n>>>\n>>> -printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n>>> + printf \"\\rRewrite $commit\n>>> ($git_filter_branch__commit_count/$commits)$progress \"\n>>\n>> Yes, for an expedient \"fix\", this is what I had in mind, although I\n>> would also have added an equal number of backspaces (\\b) following the\n>> spaces, as a minor aesthetic improvement.\n>\n> This topic seems to have stalled. I do not want to discard topics\n> because that means all the effort we spent to review and polish the\n> patch so far gets wasted, but we cannot leave unfinished topics\n> linger for too long.\n>\n> For now, I'll queue this SQUASH??? on top as a minimum fix (renaming\n> of variables and other things noticed during the review may be worth\n> doing, but they are not as grave as the issues this fixes, which are\n> show stoppers).\n\nLooks like a reasonable squash for moving this topic forward. Thanks.\n\n> I do not think our in-core progress code does that (and we do not\n> use ESC[0K either), so I'll leave it out of the minimum fix.\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 565144a..71102d5 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -277,9 +277,8 @@ test $commits -eq 0 && die \"Found nothing to rewrite\"\n> # Rewrite the commits\n> report_progress ()\n> {\n> -if test -n \"$progress\"\n> -then\n> - if test $git_filter_branch__commit_count -gt $next_sample_at\n> + if test -n \"$progress\" &&\n> + test $git_filter_branch__commit_count -gt $next_sample_at\n> then\n> now_timestamp=$(date +%s)\n> elapsed_seconds=$(($now_timestamp - $start_timestamp))\n> @@ -292,8 +291,7 @@ then\n> fi\n> progress=\" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)\"\n> fi\n> -fi\n> -printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress\"\n> + printf \"\\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress \"\n> }\n>\n> git_filter_branch__commit_count=0\n> --\n> 2.6.0-rc2-220-gd6fe230\n"},{"id":"270488","messageId":"CANy2qHcy=UD8xBeGVqGuEHVAgEvCSejt4LXk=vtpfQGSRkTg7g@mail.gmail.com","threadId":"40273","inReplyTo":"CALYJoz3xoiB2pVT+r0Nz+EYdE91WX6ypdmieMs1uubg=Vs4bog@mail.gmail.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gabor Bernat","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-22T15:53:05Z","receivedAt":"2015-09-22T15:53:05Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"On Mon, Sep 21, 2015 at 11:24 PM, Gábor Bernát\n wrote:\n> On Mon, Sep 21, 2015 at 11:22 PM, Eric Sunshine \n> wrote:\n>>\n>> On Mon, Sep 21, 2015 at 3:52 PM, Junio C Hamano wrote:\n>> > Eric Sunshine writes:\n>> >> On Tue, Sep 8, 2015 at 1:32 PM, Junio C Hamano \n>> >> wrote:\n>> >>> Eric Sunshine writes:\n>> >>>> On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát \n>> >>>> wrote:\n>> >>>>...\n>> >>>>> # Rewrite the commits\n>> >>>>> +report_progress ()\n>> >>>>> +{\n>> >>>>> +if test -n \"$progress\"\n>> >>>>> +then\n>> >>>>\n>> >>>> Indent code within the function...\n>> >>>\n>> >>>>> +printf \"\\rRewrite $commit\n>> >>>>> ($git_filter_branch__commit_count/$commits)$progress\"\n>> >>>>\n>> >>>> The \"\\r\" causes this status line to be overwritten each time through,\n>> >>>> and since the processed commit count always increases, we know that\n>> >>>> the original (without ETA) will never leave junk at the end of the\n>> >>>> line. However, with estimated seconds also being displayed, does this\n>> >>>> still hold?\n>> >>>\n>> >>> Good point.\n>> >>> Perhaps like this squashed in?\n>> >>>\n>> >>> -printf \"\\rRewrite $commit\n>> >>> ($git_filter_branch__commit_count/$commits)$progress\"\n>> >>> + printf \"\\rRewrite $commit\n>> >>> ($git_filter_branch__commit_count/$commits)$progress \"\n>> >>\n>> >> Yes, for an expedient \"fix\", this is what I had in mind, although I\n>> >> would also have added an equal number of backspaces (\\b) following the\n>> >> spaces, as a minor aesthetic improvement.\n>> >\n>> > This topic seems to have stalled. I do not want to discard topics\n>> > because that means all the effort we spent to review and polish the\n>> > patch so far gets wasted, but we cannot leave unfinished topics\n>> > linger for too long.\n>> >\n>> > For now, I'll queue this SQUASH??? on top as a minimum fix (renaming\n>> > of variables and other things noticed during the review may be worth\n>> > doing, but they are not as grave as the issues this fixes, which are\n>> > show stoppers).\n>>\n>> Looks like a reasonable squash for moving this topic forward. Thanks.\n>>\n>> > I do not think our in-core progress code does that (and we do not\n>> > use ESC[0K either), so I'll leave it out of the minimum fix.\n>> >\n>> > diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n>> > index 565144a..71102d5 100755\n>> > --- a/git-filter-branch.sh\n>> > +++ b/git-filter-branch.sh\n>> > @@ -277,9 +277,8 @@ test $commits -eq 0 && die \"Found nothing to\n>> > rewrite\"\n>> > # Rewrite the commits\n>> > report_progress ()\n>> > {\n>> > -if test -n \"$progress\"\n>> > -then\n>> > - if test $git_filter_branch__commit_count -gt $next_sample_at\n>> > + if test -n \"$progress\" &&\n>> > + test $git_filter_branch__commit_count -gt\n>> > $next_sample_at\n>> > then\n>> > now_timestamp=$(date +%s)\n>> > elapsed_seconds=$(($now_timestamp - $start_timestamp))\n>> > @@ -292,8 +291,7 @@ then\n>> > fi\n>> > progress=\" ($elapsed_seconds seconds passed, remaining\n>> > $remaining_second predicted)\"\n>> > fi\n>> > -fi\n>> > -printf \"\\rRewrite $commit\n>> > ($git_filter_branch__commit_count/$commits)$progress\"\n>> > + printf \"\\rRewrite $commit\n>> > ($git_filter_branch__commit_count/$commits)$progress \"\n>> > }\n>> >\n>> > git_filter_branch__commit_count=0\n>> > --\n>> > 2.6.0-rc2-220-gd6fe230\n>\n>\n> Agreed, :) did not abandoned this, just got caught up with many stuff.\n> Thanks for the help,\n>\n\nSo do I need to do anything else with this? :)\n"},{"id":"270494","messageId":"xmqqzj0e4eu6.fsf@gitster.mtv.corp.google.com","threadId":"40273","inReplyTo":"CANy2qHcy=UD8xBeGVqGuEHVAgEvCSejt4LXk=vtpfQGSRkTg7g@mail.gmail.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-22T16:08:01Z","receivedAt":"2015-09-22T16:08:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gabor Bernat writes:\n\n> On Mon, Sep 21, 2015 at 11:24 PM, Gábor Bernát\n> ...\n>> Agreed, :) did not abandoned this, just got caught up with many stuff.\n>> Thanks for the help,\n>\n> So do I need to do anything else with this? :)\n\nIf you can fetch from me to see if the output from\n\n git log -p origin/master..71400d97b12a\n\nlooks sensible, that would be good. There are two commits.\n\nThanks.\n"},{"id":"270606","messageId":"CANy2qHciYR_=QeEYi-RNG3ay6+ZQk04FUwX1cY+Lf5c-cSJRHQ@mail.gmail.com","threadId":"40273","inReplyTo":"xmqqzj0e4eu6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] filter-branch: add passed/remaining seconds on progress","fromName":"Gabor Bernat","fromEmail":"bernat@primeranks.net","sentAt":"2015-09-23T20:48:55Z","receivedAt":"2015-09-23T20:48:55Z","isPatch":true,"sender":{"key":"bernat@primeranks.net","avatar":"https://gravatar.com/avatar/d65839a755b3bd913bc793c89024c14017d830f72093b3e2c248011c8854e79e?d=mp&s=160"},"body":"On Tue, Sep 22, 2015 at 6:08 PM, Junio C Hamano wrote:\n> Gabor Bernat writes:\n>\n>> On Mon, Sep 21, 2015 at 11:24 PM, Gábor Bernát\n>> ...\n>>> Agreed, :) did not abandoned this, just got caught up with many stuff.\n>>> Thanks for the help,\n>>\n>> So do I need to do anything else with this? :)\n>\n> If you can fetch from me to see if the output from\n>\n> git log -p origin/master..71400d97b12a\n>\n> looks sensible, that would be good. There are two commits.\n>\n> Thanks.\nI can sign this off as good and sensible. Nice work, thanks :)\n"}]}