Re: [PATCH v2 10/20] builtin/bundle.c: let parse-options parse subcommands

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Aug 19, 2022, 17:50 UTC
Message-ID: <220819.867d346rzo.gmgdl@evledraar.gmail.com>
In-Reply-To: <20220819160411.1791200-11-szeder.dev@gmail.com>

On Fri, Aug 19 2022, SZEDER Gábor wrote:

Nit: I wouldn't mind keeping this variable:

Show 24 quoted lines

>  	};
> -	int result;
>  
>  	argc = parse_options(argc, argv, prefix, options, builtin_bundle_usage,
> -		PARSE_OPT_STOP_AT_NON_OPTION);
> +			     0);
>  
>  	packet_trace_identity("bundle");
>  
> -	if (argc < 2)
> -		usage_with_options(builtin_bundle_usage, options);
> -
> -	else if (!strcmp(argv[0], "create"))
> -		result = cmd_bundle_create(argc, argv, prefix);
> -	else if (!strcmp(argv[0], "verify"))
> -		result = cmd_bundle_verify(argc, argv, prefix);
> -	else if (!strcmp(argv[0], "list-heads"))
> -		result = cmd_bundle_list_heads(argc, argv, prefix);
> -	else if (!strcmp(argv[0], "unbundle"))
> -		result = cmd_bundle_unbundle(argc, argv, prefix);
> -	else {
> -		error(_("Unknown subcommand: %s"), argv[0]);
> -		usage_with_options(builtin_bundle_usage, options);
> -	}

Then just doing:

	result = fn(argc, argv, prefix);

Which would eliminate the need to change this:

> -	return result ? 1 : 0;
> +	return !!fn(argc, argv, prefix);
>  }

I wondered about why !! v.s. 0/1 for a second or so, but realized you were just golf-ing an existing pattern.

FWIW I *think* if we're changing this we could just make it "return fn()", as the functions themselves seem to return 0/1 or a !!'d variable.