Re: [PATCH 02/15] builtin/hash-object: allow literally hashing with a given algorithm

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 11, 2021, 08:52 UTC
Message-ID: <87sg3xfbkd.fsf@evledraar.gmail.com>
In-Reply-To: <20210410152140.3525040-3-sandals@crustytoothpaste.net>

On Sat, Apr 10 2021, brian m. carlson wrote:

Show 8 quoted lines

> +	algo = the_hash_algo;
> +	if (object_format) {
> +		if (flags & HASH_WRITE_OBJECT)
> +			errstr = "Can't use -w with --object-format";
> +		else {
> +			int id = hash_algo_by_name(object_format);
> +			if (id == GIT_HASH_UNKNOWN)
> +				errstr = "Unknown object format";

An established pattern, but shouldn't these be N_()'d while we're at it? At least for new strings.

> +			else
> +				algo = &hash_algos[id];
> +		}
> +	}

Style nit: if .. {} else {} not if .. else {}.

> +test_expect_success '--literally with --object-format' '
> +	test $(test_oid --hash=sha1 hello) = $(git hash-object -t blob --literally --object-format=sha1 hello) &&
> +	test $(test_oid --hash=sha256 hello) = $(git hash-object -t blob --literally --object-format=sha256 hello)
> +'

This would be more readable and easier to debug with 4x tempfiles and 2x test_cmp.