Re: [PATCH (optional)] t0021: use arithmetic expansion to trim whitespace from wc -c output

[Lars Schneider <larsxschneider@gmail.com>]

> On 06 Nov 2016, at 20:31, Johannes Sixt <j6t@kdbg.org> wrote:
> 
> Am 06.11.2016 um 16:45 schrieb Lars Schneider:
>> 
>>> On 03 Nov 2016, at 21:22, Johannes Sixt <j6t@kdbg.org> wrote:
>>> This is a pure optimization that reduces the number of forks, which
>>> helps a bit on Windows.
>>> 
>>> There would be a solution with perl that does not require trimming
>>> of whitespace, but perl startup times are unbearable on Windows.
>>> wc -c is better.
> 
> I was wrong here. I had looked at the perl invocations due to
> git-sendemail, and they are awfully slow. A do-almost-nothing perl
> invocation is in the same ballpark as wc. Therefore I changed my mind
> and suggest the patch below instead.
> 
>> Since the file size function became very simple with your patch,
>> shouldn't we get rid of it? If you agree, then we could squash the 
>> patch below into your patch.
> 
> In the new patch, the function is not that trivial (it uses perl), and
> the call sites can remain as they are (simple shell variables and
> substitutions).
> 
> ---- 8< ----
> [PATCH] t0021: compute file size with a single process instead of a pipeline
> 
> Avoid unwanted coding patterns (prodigal use of pipelines), and in
> particular a useless use of cat.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> t/t0021-conversion.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index db71acacb3..cb72fa49de 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -22,7 +22,7 @@ generate_random_characters () {
> }
> 
> file_size () {
> -	cat "$1" | wc -c | sed "s/^[ ]*//"
> +	perl -e 'print -s $ARGV[0]' "$1"
> }
> 
> filter_git () {
> -- 
> 2.11.0.rc0.55.gd967357
> 

I like this better, too. Looks good to me and works on macOS.

Thanks,
Lars