Re: [PATCH] filter-branch: add passed/remaining seconds on progress
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 21, 2015, 19:52 UTC
- Message-ID
- <xmqq6133a6tf.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <CAPig+cS7ObsWjqbLytCKp1PGF+224TYhC734dNa_HXYQ7p+GgQ@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 35 quoted lines
> On Tue, Sep 8, 2015 at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>>> On Mon, Sep 7, 2015 at 9:52 AM, Gábor Bernát <bernat@primeranks.net> wrote:
>>>...
>>>> # Rewrite the commits
>>>> +report_progress ()
>>>> +{
>>>> +if test -n "$progress"
>>>> +then
>>>
>>> Indent code within the function...
>>
>> Also git_filter_branch__commit_count is now used only inside this
>> function, so it is easier to follow to increment it here.
>
> Make sense.
>
>>>> +printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress"
>>>
>>> The "\r" causes this status line to be overwritten each time through,
>>> and since the processed commit count always increases, we know that
>>> the original (without ETA) will never leave junk at the end of the
>>> line. However, with estimated seconds also being displayed, does this
>>> still hold?
>>
>> Good point.
>> Perhaps like this squashed in?
>>
>> -printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress"
>> + printf "\rRewrite $commit
>> ($git_filter_branch__commit_count/$commits)$progress "
>
> Yes, for an expedient "fix", this is what I had in mind, although I
> would also have added an equal number of backspaces (\b) following the
> spaces, as a minor aesthetic improvement.This topic seems to have stalled. I do not want to discard topics because that means all the effort we spent to review and polish the patch so far gets wasted, but we cannot leave unfinished topics linger for too long.
For now, I'll queue this SQUASH??? on top as a minimum fix (renaming of variables and other things noticed during the review may be worth doing, but they are not as grave as the issues this fixes, which are show stoppers).
I do not think our in-core progress code does that (and we do not use ESC[0K either), so I'll leave it out of the minimum fix.
git-filter-branch.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 565144a..71102d5 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -277,9 +277,8 @@ test $commits -eq 0 && die "Found nothing to rewrite" # Rewrite the commits report_progress () { -if test -n "$progress" -then - if test $git_filter_branch__commit_count -gt $next_sample_at + if test -n "$progress" && + test $git_filter_branch__commit_count -gt $next_sample_at then now_timestamp=$(date +%s) elapsed_seconds=$(($now_timestamp - $start_timestamp)) @@ -292,8 +291,7 @@ then fi progress=" ($elapsed_seconds seconds passed, remaining $remaining_second predicted)" fi -fi -printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress" + printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)$progress " } git_filter_branch__commit_count=0
-- 2.6.0-rc2-220-gd6fe230