Message ID | 20231206115215.94467-3-l.s.r@web.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | standardize incompatibility messages | expand |
On Wed, Dec 06, 2023 at 12:51:56PM +0100, René Scharfe wrote: > diff --git a/builtin/repack.c b/builtin/repack.c > index edaee4dbec..c54777bbe5 100644 > --- a/builtin/repack.c > +++ b/builtin/repack.c > @@ -1203,19 +1203,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix) > if (delete_redundant && repository_format_precious_objects) > die(_("cannot delete packs in a precious-objects repo")); > > - if (keep_unreachable && > - (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE))) > - die(_("options '%s' and '%s' cannot be used together"), "--keep-unreachable", "-A"); > + die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A", > + keep_unreachable, "-k/--keep-unreachable", > + pack_everything & PACK_CRUFT, "--cruft"); Oof, thanks for cleaning this one up. I had to read this hunk a couple of times to convince myself that it was doing the right thing, but I believe it is. > - if (pack_everything & PACK_CRUFT) { > + if (pack_everything & PACK_CRUFT) > pack_everything |= ALL_INTO_ONE; And adding the ALL_INTO_ONE bit here does not effect either of the below two checks, so moving them up is fine, too. > - if (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE)) > - die(_("options '%s' and '%s' cannot be used together"), "--cruft", "-A"); > - if (keep_unreachable) > - die(_("options '%s' and '%s' cannot be used together"), "--cruft", "-k"); > - } Thanks, Taylor
diff --git a/builtin/repack.c b/builtin/repack.c index edaee4dbec..c54777bbe5 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -1203,19 +1203,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (delete_redundant && repository_format_precious_objects) die(_("cannot delete packs in a precious-objects repo")); - if (keep_unreachable && - (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE))) - die(_("options '%s' and '%s' cannot be used together"), "--keep-unreachable", "-A"); + die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A", + keep_unreachable, "-k/--keep-unreachable", + pack_everything & PACK_CRUFT, "--cruft"); - if (pack_everything & PACK_CRUFT) { + if (pack_everything & PACK_CRUFT) pack_everything |= ALL_INTO_ONE; - if (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE)) - die(_("options '%s' and '%s' cannot be used together"), "--cruft", "-A"); - if (keep_unreachable) - die(_("options '%s' and '%s' cannot be used together"), "--cruft", "-k"); - } - if (write_bitmaps < 0) { if (!write_midx && (!(pack_everything & ALL_INTO_ONE) || !is_bare_repository()))
The repack option --keep-unreachable is incompatible with -A, --cruft is incompatible with -A and -k, and -k is short for --keep-unreachable. So they are all incompatible with each other. Use the function for checking three mutually incompatible options, die_for_incompatible_opt3(), to perform this check in one place and without repetition. This is shorter and clearer. Signed-off-by: René Scharfe <l.s.r@web.de> --- builtin/repack.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) -- 2.43.0