Gábor Bernát <[email protected]> writes:
> @@ -277,9 +277,43 @@ test $commits -eq 0 && die "Found nothing to rewrite"
> # Rewrite the commits
>
> git_filter_branch__commit_count=0
This is not a new problem, but I wonder why we need such a
cumbersomely long variable name. It is not like this is a part of
some shell script library that needs to be careful about namespace
pollution.
> +echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t
That is very strange construct. I think you meant to say something
like
if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
then
show_seconds=t
else
show_seconds=
fi
A handful of points:
* "echo $(any-command)" is suspect, unless you are trying to let
the shell munge output from any-command, which is not the case.
* "grep" without -E (or "egrep") takes BRE, which "+" (one or more)
is not part of.
* That semicolon is a syntax error. I think whoever suggested you
to use it meant to squelch possible errors from "date" that does
not understand the "%s" format.
* I do not think you are clearing show_seconds to empty anywhere,
so an environment variable the user may have when s/he starts
filter-branch will seep through and confuse you.
> +case "$show_seconds" in
> + t)
> + start_timestamp=$(date +%s)
> + next_sample_at=0
> + ;;
> + '')
> + progress=""
> + ;;
> +esac
In our codebase case labels and case/esac align, like you did in the
later part of the patch.
> +
> 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"
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. Then the loop would look more 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
