Re: [PATCH v2] t1016: clean up style

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

I can confirm that everything looks correct now.  Like in my reply
to v1.

https://lore.kernel.org/git/20241103135111.13508-1-algonell@gmail.com/T/#m0d6626e6495b978b7116bf1561effe10f6792d24

On Sun, Nov 3, 2024, at 14:50, Andrew Kreimer wrote:
> Adhere to Documentation/CodingGuidelines:
>   - Whitespace and redirect operator.
>   - Case arms indentation.
>   - Tabs for indentation.
>
> Signed-off-by: Andrew Kreimer <algonell@gmail.com>
> ---
> v2:
>   - Base on: "The seventh batch".
>   - Remove the extra line in between functions.
>   - Elaborate on the changes (thank you, Kristoffer).
>
>  t/t1016-compatObjectFormat.sh | 262 +++++++++++++++++-----------------
>  1 file changed, 130 insertions(+), 132 deletions(-)
>
> diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh
> index 8341a2fe83..32e0af392e 100755
> --- a/t/t1016-compatObjectFormat.sh
> +++ b/t/t1016-compatObjectFormat.sh
> @@ -24,84 +24,83 @@ TEST_PASSES_SANITIZE_LEAK=true
>  # the commit is identical to the commit in the other repository.
>
>  compat_hash () {
> -    case "$1" in
> -    "sha1")
> -	echo "sha256"
> -	;;
> -    "sha256")
> -	echo "sha1"
> -	;;
> -    esac
> +	case "$1" in
> +	"sha1")
> +		echo "sha256"
> +		;;
> +	"sha256")
> +		echo "sha1"
> +		;;
> +	esac
>  }
>
>  hello_oid () {
> -    case "$1" in
> -    "sha1")
> -	echo "$hello_sha1_oid"
> -	;;
> -    "sha256")
> -	echo "$hello_sha256_oid"
> -	;;
> -    esac
> +	case "$1" in
> +	"sha1")
> +		echo "$hello_sha1_oid"
> +		;;
> +	"sha256")
> +		echo "$hello_sha256_oid"
> +		;;
> +	esac
>  }
>
>  tree_oid () {
> -    case "$1" in
> -    "sha1")
> -	echo "$tree_sha1_oid"
> -	;;
> -    "sha256")
> -	echo "$tree_sha256_oid"
> -	;;
> -    esac
> +	case "$1" in
> +	"sha1")
> +		echo "$tree_sha1_oid"
> +		;;
> +	"sha256")
> +		echo "$tree_sha256_oid"
> +		;;
> +	esac
>  }
>
>  commit_oid () {
> -    case "$1" in
> -    "sha1")
> -	echo "$commit_sha1_oid"
> -	;;
> -    "sha256")
> -	echo "$commit_sha256_oid"
> -	;;
> -    esac
> +	case "$1" in
> +	"sha1")
> +		echo "$commit_sha1_oid"
> +		;;
> +	"sha256")
> +		echo "$commit_sha256_oid"
> +		;;
> +	esac
>  }
>
>  commit2_oid () {
> -    case "$1" in
> -    "sha1")
> -	echo "$commit2_sha1_oid"
> -	;;
> -    "sha256")
> -	echo "$commit2_sha256_oid"
> -	;;
> -    esac
> +	case "$1" in
> +	"sha1")
> +		echo "$commit2_sha1_oid"
> +		;;
> +	"sha256")
> +		echo "$commit2_sha256_oid"
> +		;;
> +	esac
>  }
>
>  del_sigcommit () {
> -    local delete="$1"
> -
> -    if test "$delete" = "sha256" ; then
> -	local pattern="gpgsig-sha256"
> -    else
> -	local pattern="gpgsig"
> -    fi
> -    test-tool delete-gpgsig "$pattern"
> +	local delete="$1"
> +
> +	if test "$delete" = "sha256" ; then
> +		local pattern="gpgsig-sha256"
> +	else
> +		local pattern="gpgsig"
> +	fi
> +	test-tool delete-gpgsig "$pattern"
>  }
>
> -
>  del_sigtag () {
> -    local storage="$1"
> -    local delete="$2"
> -
> -    if test "$storage" = "$delete" ; then
> -	local pattern="trailer"
> -    elif test "$storage" = "sha256" ; then
> -	local pattern="gpgsig"
> -    else
> -	local pattern="gpgsig-sha256"
> -    fi
> -    test-tool delete-gpgsig "$pattern"
> +	local storage="$1"
> +	local delete="$2"
> +
> +	if test "$storage" = "$delete" ; then
> +		local pattern="trailer"
> +	elif test "$storage" = "sha256" ; then
> +		local pattern="gpgsig"
> +	else
> +		local pattern="gpgsig-sha256"
> +	fi
> +	test-tool delete-gpgsig "$pattern"
>  }
>
>  base=$(pwd)
> @@ -146,9 +145,9 @@ do
>  	'
>  	test_expect_success "create a $hash branch" '
>  		git checkout -b branch $(commit_oid $hash) &&
> -		echo "More more more give me more!" > more &&
> +		echo "More more more give me more!" >more &&
>  		eval more_${hash}_oid=$(git hash-object more) &&
> -		echo "Another and another and another" > another &&
> +		echo "Another and another and another" >another &&
>  		eval another_${hash}_oid=$(git hash-object another) &&
>  		git update-index --add more another &&
>  		git commit -m "Add more files!" &&
> @@ -165,15 +164,15 @@ do
>  	'
>  	test_expect_success GPG2 "create additional $hash signed commits" '
>  		git commit --gpg-sign --allow-empty -m "This is an additional signed
> commit" &&
> -		git cat-file commit HEAD | del_sigcommit sha256 >
> "../${hash}_signedcommit3" &&
> -		git cat-file commit HEAD | del_sigcommit sha1 >
> "../${hash}_signedcommit4" &&
> +		git cat-file commit HEAD | del_sigcommit sha256
> >"../${hash}_signedcommit3" &&
> +		git cat-file commit HEAD | del_sigcommit sha1
> >"../${hash}_signedcommit4" &&
>  		eval signedcommit3_${hash}_oid=$(git hash-object -t commit -w
> ../${hash}_signedcommit3) &&
>  		eval signedcommit4_${hash}_oid=$(git hash-object -t commit -w
> ../${hash}_signedcommit4)
>  	'
>  	test_expect_success GPG2 "create additional $hash signed tags" '
>  		git tag -s -m "This is an additional signed tag" signedtag34 HEAD &&
> -		git cat-file tag signedtag34 | del_sigtag "${hash}" sha256 >
> ../${hash}_signedtag3 &&
> -		git cat-file tag signedtag34 | del_sigtag "${hash}" sha1 >
> ../${hash}_signedtag4 &&
> +		git cat-file tag signedtag34 | del_sigtag "${hash}" sha256
> >../${hash}_signedtag3 &&
> +		git cat-file tag signedtag34 | del_sigtag "${hash}" sha1
> >../${hash}_signedtag4 &&
>  		eval signedtag3_${hash}_oid=$(git hash-object -t tag -w
> ../${hash}_signedtag3) &&
>  		eval signedtag4_${hash}_oid=$(git hash-object -t tag -w
> ../${hash}_signedtag4)
>  	'
> @@ -181,81 +180,80 @@ done
>  cd "$base"

All changes up to here are straightforward.

After this my pager[1] gets confused and `--color-words` becomes useful.

🔗 1: https://github.com/dandavison/delta

>
>  compare_oids () {
> -    test "$#" = 5 && { local PREREQ="$1"; shift; } || PREREQ=
> -    local type="$1"
> -    local name="$2"
> -    local sha1_oid="$3"
> -    local sha256_oid="$4"
> -
> -    echo ${sha1_oid} > ${name}_sha1_expected
> -    echo ${sha256_oid} > ${name}_sha256_expected
> -    echo ${type} > ${name}_type_expected
> -
> -    git --git-dir=repo-sha1/.git rev-parse
> --output-object-format=sha256 ${sha1_oid} > ${name}_sha1_sha256_found
> -    git --git-dir=repo-sha256/.git rev-parse
> --output-object-format=sha1 ${sha256_oid} > ${name}_sha256_sha1_found
> -    local sha1_sha256_oid="$(cat ${name}_sha1_sha256_found)"
> -    local sha256_sha1_oid="$(cat ${name}_sha256_sha1_found)"
> -
> -    test_expect_success $PREREQ "Verify ${type} ${name}'s sha1 oid" '
> -	git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1
> ${sha256_oid} > ${name}_sha1 &&
> -	test_cmp ${name}_sha1 ${name}_sha1_expected
> -'
> -
> -    test_expect_success $PREREQ "Verify ${type} ${name}'s sha256 oid" '

At this point the diff gets weird.  There seems to be too many
whitespace changes to get a good diff.  But that can be handled (see
next part).

> -	git --git-dir=repo-sha1/.git rev-parse --output-object-format=sha256
> ${sha1_oid} > ${name}_sha256 &&
> -	test_cmp ${name}_sha256 ${name}_sha256_expected
> -'
> +	test "$#" = 5 && { local PREREQ="$1"; shift; } || PREREQ=
> +	local type="$1"
> +	local name="$2"
> +	local sha1_oid="$3"
> +	local sha256_oid="$4"
> +
> +	echo ${sha1_oid} >${name}_sha1_expected
> +	echo ${sha256_oid} >${name}_sha256_expected
> +	echo ${type} >${name}_type_expected
> +
> +	git --git-dir=repo-sha1/.git rev-parse --output-object-format=sha256
> ${sha1_oid} >${name}_sha1_sha256_found
> +	git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1
> ${sha256_oid} >${name}_sha256_sha1_found
> +	local sha1_sha256_oid="$(cat ${name}_sha1_sha256_found)"
> +	local sha256_sha1_oid="$(cat ${name}_sha256_sha1_found)"
> +

Indeed this looks like only indentation and redirection whitespace fixes.

> +	test_expect_success $PREREQ "Verify ${type} ${name}'s sha1 oid" '
> +		git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1
> ${sha256_oid} >${name}_sha1 &&
> +		test_cmp ${name}_sha1 ${name}_sha1_expected
> +	'

At this point it _looks like_ some `test_expect_success` are moved
around.  But my diff deceives me: all of the eight `test_` are provided
in the same order in the preimage and postimage.

Here I removed the fixes for the trailing `'`, i.e. moved them back to
the beginning of the line:

```
'
```

And that gave me a nice clean diff which again highlighted the
indentation fixes as well as the redirection operator fixes.

Indeed it all looks correct.

> […]