Gábor Bernát <ber...@primeranks.net> writes:

> +if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
> +then
> +     show_seconds=t
> +else
> +     show_seconds=
> +fi
> +
> +case "$show_seconds" in
> +t)
> +     start_timestamp=$(date +%s)
> +     next_sample_at=0
> +     ;;
> +'')
> +     progress=""
> +     ;;
> +esac

Why the case statement here?

For that matter, you probably do not need $show_seconds and use the
fact that start_timestamp and progress are only set to a non-empty
string when we measure progress, i.e. making all of the above
something more like this:

        progress= start_timestamp=
        if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
        then
                next_sample_at=0
                progress="dummy to ensure this is not empty"
                start_timestamp=$(date '+%s')
        fi

If you are more efficiency-minded, I think you could even do with a
single call to /bin/date, perhaps like so:

        next_sample_at= progress=
        start_timestamp=$(date '+%s' 2>/dev/null) 
        case "$start_timestamp" in
        *[!0-9]* | "")
                # not a "digit only" output
                ;;
        ?*)
                progress="dummy to ensure this is not empty"
                next_sample_at=0
                ;;
        esac

but I suspect that may be going a bit too far ;-).

>  while read commit parents; do
>       git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
> -     printf "\rRewrite $commit ($git_filter_branch__commit_count/$commits)"
> +
> +     case "$show_seconds" in
> +     t)
> +             if test $git_filter_branch__commit_count -gt $next_sample_at
> +             then
> +                     now_timestamp=$(date +%s)
> +                     elapsed_seconds=$(($now_timestamp - $start_timestamp))
> +                     remaining_second=$(( ($commits - 
> $git_filter_branch__commit_count) * $elapsed_seconds / 
> $git_filter_branch__commit_count ))
> +                     if test $elapsed_seconds -gt 0
> +                     then
> +                             next_sample_at=$(( ($elapsed_seconds + 1) * 
> $git_filter_branch__commit_count / $elapsed_seconds ))
> +                     else
> +                             next_sample_at=$(($next_sample_at + 1))
> +                     fi
> +                     progress=" ($elapsed_seconds seconds passed, remaining 
> $remaining_second predicted)"
> +             fi
> +             ;;
> +     '')
> +             progress=""
> +             ;;
> +     esac
> +
> +     printf "\rRewrite $commit 
> ($git_filter_branch__commit_count/$commits)$progress"
>  
>       case "$filter_subdir" in
>       "")

It would be easier to follow the logic of this loop whose _primary_
point is to rewrite one commit if you moved this part into a helper
function.  As written above, the deeply nested case statement whose
purpose is only to show the progress is very distracting.

Then the loop would read like:

        while read commit parents
        do
                : $(( $git_filter_branch__commit_count++ ))
                report_progress

                case "$filter_subdir" in
                ...

                # all the work that is about rewriting this commit
                # comes here.

        done

which would be much less noisy and helpful to people who are
interested in learning what it is doing.

As I said earlier in this message, the report_progress helper
function can use the fact that $progress or $start_timestamp are
non-empty if and only if you need to compute and tack $progress at
the end of the output, i.e.

        report_progress ()
        {
                if test -n "$progress"
                then
                        ... do rate computation here ...
                        progress=" ($elapsed_seconds seconds passed,..."
                fi
                printf "\rRewrite $commit (...)$progress"
        }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to