Re: [PATCH 1/8] midx: expose 'write_midx_file_only()' publicly

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 11, 2021, 16:21 UTC
Message-ID: <YTzXmAOvGp2OiuYZ@nand.local>
In-Reply-To: <87a6kja0b2.fsf@evledraar.gmail.com>

On Sat, Sep 11, 2021 at 12:07:57PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 19 quoted lines

>
> On Fri, Sep 10 2021, Taylor Blau wrote:
>
> > -		if (ctx->m && midx_contains_pack(ctx->m, file_name))
> > -			return;
> > +		if (ctx->m) {
> > +			if (midx_contains_pack(ctx->m, file_name))
> > +				return;
> > +		} else if (ctx->to_include) {
> > +			if (!string_list_has_string(ctx->to_include, file_name))
> > +				return;
> > +		}
>
> I think this is equivalent to:
>
> 		if (ctx->m && midx_contains_pack(ctx->m, file_name))
> 			return;
> 		else if (!ctx->m && string_list_has_string(...))
> 			return;

They are equivalent, but it took me a minute to convince myself ;). They are only equivalent because *either* ctx->m or ctx->to_include is non-NULL.

So it wouldn't be the case that ctx->m being set and the subsequent midx_contains_pack() call returning zero would cause us to check the to_include list, because to_include will be NULL if ctx->m isn't.

Anyway, that makes them equivalent, so I'm happy to clean it up and decrease the nested-ness.

> > +int write_midx_file_only(const char *object_dir, struct string_list *packs_to_include, const char *preferred_pack_name, unsigned flags);
>
> Let's also line-wrap header changes like in the *.c file.

Sure.

Thanks, Taylor