diff mbox series

[2/7] repack: use die_for_incompatible_opt3() for -A/-k/--cruft

Message ID 20231206115215.94467-3-l.s.r@web.de (mailing list archive)
State New, archived
Headers show
Series standardize incompatibility messages | expand

Commit Message

René Scharfe Dec. 6, 2023, 11:51 a.m. UTC
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

Comments

Taylor Blau Dec. 6, 2023, 7:18 p.m. UTC | #1
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 mbox series

Patch

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()))