Re: [PATCH] filter-branch: add passed/remaining seconds on progress

Gábor Bernát [off-list ref] 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"
        }