Re: [PATCH v2 3/3] repo: add -z as an alias for --format=nul to git-repo-structure

[Junio C Hamano <gitster@pobox.com>]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh
> index 36a71a144e..df7d4ea524 100755
> --- a/t/t1901-repo-structure.sh
> +++ b/t/t1901-repo-structure.sh
> @@ -101,6 +101,13 @@ test_expect_success 'keyvalue and nul format' '
>  		tr "\n=" "\0\n" <expect >expect_nul &&
>  		git repo structure --format=nul >out 2>err &&
> +		test_cmp expect_nul out &&
> +		test_line_count = 0 err &&

Not limited to this step, but I have a couple of comments.

 * Instead of munging the expected file so that it contains a NUL,
   and compare the actual output with it, munge the NUL terminated
   outout to make it text and compare with the expected file in text
   format.  This matters when tests start to fail as test_cmp will
   show the "diff" output when it fails, and comparing NUL
   terminated files, which are "binary" in the eyes of the "diff"
   utility.

 * I see your -z output is "<key> LF <value> NUL", but was there a
   particular reason why "<key> NUL <value> NUL" was not chosen?
   Unless there is a compelling reason not to, it would be a lot
   more future-proof to use NUL for both, primarily because it would
   allow future developers to include arbitrary non-NUL bytes in the
   <key> part in the future (and we wouldn't know what end-user
   controlled substring they may want to add).

> +
> +		# "-z", as a synonym to "--format=nul", participates in the
> +		# usual "last one wins" rule.
> +		git repo structure --format=table -z >out 2>err &&
> +
>  		test_cmp expect_nul out &&
>  		test_line_count = 0 err
>  	)