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

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Apr 11, 2021, 21:07 UTC
Message-ID: <YHNlFmpQa3Qlgh9g@camp.crustytoothpaste.net>
In-Reply-To: <87sg3xfbkd.fsf@evledraar.gmail.com>

On 2021-04-11 at 08:52:02, Ævar Arnfjörð Bjarmason wrote:

Show 14 quoted lines

> 
> On Sat, Apr 10 2021, brian m. carlson wrote:
> 
> > +	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.

Sure, I can do that.

Show 6 quoted lines

> > +			else
> > +				algo = &hash_algos[id];
> > +		}
> > +	}
> 
> Style nit: if .. {} else {} not if .. else {}.

Will fix.

Show 7 quoted lines

> > +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.

Okay, I can go for that.

-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US