Re: [PATCH 2/2] test-lib: teach test_seq the -f option

[Todd Zullinger <tmz@pobox.com>]

Jeff King wrote:
> The "seq" tool has a "-f" option to produce printf-style formatted
> lines. Let's teach our test_seq helper the same trick. This lets us get
> rid of some shell loops in test snippets (which are particularly verbose
> in our test suite because we have to "|| return 1" to keep the &&-chain
> going).

This is a nice improvement.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index bee4a2ca34..8c176f4efc 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1454,6 +1454,13 @@ test_cmp_fspath () {
>  # from 1.
>  
>  test_seq () {
> +	local fmt="%d"
> +	case "$1" in
> +	-f)
> +		fmt="$2"
> +		shift 2
> +		;;
> +	esac
>  	case $# in
>  	1)	set 1 "$@" ;;
>  	2)	;;
> @@ -1462,7 +1469,7 @@ test_seq () {
>  	test_seq_counter__=$1
>  	while test "$test_seq_counter__" -le "$2"
>  	do
> -		echo "$test_seq_counter__"
> +		printf "$fmt\n" "$test_seq_counter__"
>  		test_seq_counter__=$(( $test_seq_counter__ + 1 ))
>  	done
>  }

Is it a sharp edge worth caring about that someone might
write `test_seq -f 1 5` where we'd pass 1 as the format
string?

If so, perhaps a check like this might be sufficient to
catch it early?

	diff --git i/t/test-lib-functions.sh w/t/test-lib-functions.sh
	index 8c176f4efc..87b59d5895 100644
	--- i/t/test-lib-functions.sh
	+++ w/t/test-lib-functions.sh
	@@ -1458,6 +1458,10 @@ test_seq () {
		case "$1" in
		-f)
			fmt="$2"
	+		case "$fmt" in
	+			*%*)	: ;;
	+			*)	BUG "no % in -f argument" ;;
	+		esac
			shift 2
			;;
		esac

I don't know whether it's worth the extra code or not.  I
just wondered about how it would fail in the face of a minor
typo.  It certainly should cause any test to fail if it were
to output 1 instead of the intended format string, so it's
arguably fine as-is.

Adding -f to the usage note above, as Justin suggested might
help folks avoid making the mistake of cuddling the format
string against -f, e.g.: -f%d.  That is caught by the
parameter count check (though perhaps not everyone would
notice why, thinking they did pass an argument to -f).

-- 
Todd