Re: [PATCH 04/16] t/helper/fsmonitor-client: cleanup call to parse_options()

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Jeff Hostetler <jeffhost@microsoft.com>
Subject: Re: [PATCH 04/16] t/helper/fsmonitor-client: cleanup call to parse_options()
Date: Mon, 14 Mar 2022 08:58:53 +0100	[thread overview]
Message-ID: <220314.86fsnlezgv.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <578c3682e2fb35af86087e6a0c277aa8766919d7.1647033303.git.gitgitgadget@gmail.com>

On Fri, Mar 11 2022, Jeff Hostetler via GitGitGadget wrote:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> fixup! t/helper/fsmonitor-client: create IPC client to talk to \
> FSMonitor Daemon
>
> Elminate unnecessary code in cmd__fsmonitor_client() WRT
> parsing of options.
>
> Fix name of test-tool in usage.
>
> Don't localize die() message.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>  t/helper/test-fsmonitor-client.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/t/helper/test-fsmonitor-client.c b/t/helper/test-fsmonitor-client.c
> index f7a5b3a32fa..d59a640f1f9 100644
> --- a/t/helper/test-fsmonitor-client.c
> +++ b/t/helper/test-fsmonitor-client.c
> @@ -49,7 +49,7 @@ static int do_send_query(const char *token)
>  
>  	ret = fsmonitor_ipc__send_query(token, &answer);
>  	if (ret < 0)
> -		die(_("could not query fsmonitor--daemon"));
> +		die("could not query fsmonitor--daemon");

Good, since this is just a test helper.

>  	write_in_full(1, answer.buf, answer.len);
>  	strbuf_release(&answer);
> @@ -85,8 +85,8 @@ int cmd__fsmonitor_client(int argc, const char **argv)
>  	const char *token = NULL;
>  
>  	const char * const fsmonitor_client_usage[] = {
> -		N_("test-helper fsmonitor-client query [<token>]"),
> -		N_("test-helper fsmonitor-client flush"),
> +		N_("test-tool fsmonitor-client query [<token>]"),
> +		N_("test-tool fsmonitor-client flush"),

These are still marked for N_() translation.

Even if tehse were built-ins only the one containing <token> should have
N_(), as the other one is a literal command whose translation won't
change.

>  		NULL,
>  	};
>  
> @@ -96,17 +96,12 @@ int cmd__fsmonitor_client(int argc, const char **argv)
>  		OPT_END()
>  	};
>  
> -	if (argc < 2)
> -		usage_with_options(fsmonitor_client_usage, options);
> +	argc = parse_options(argc, argv, NULL, options, fsmonitor_client_usage, 0);
>  
> -	if (argc == 2 && !strcmp(argv[1], "-h"))
> +	if (argc != 1)
>  		usage_with_options(fsmonitor_client_usage, options);
>  
> -	subcmd = argv[1];
> -	argv--;
> -	argc++;
> -
> -	argc = parse_options(argc, argv, NULL, options, fsmonitor_client_usage, 0);
> +	subcmd = argv[0];
>  
>  	setup_git_directory();

Looks good.