On Sat, Sep 11, 2021 at 12:07:57PM +0200, Ævar Arnfjörð Bjarmason wrote: > > 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