Subject: Re: [PATCH v6 20/30] t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 07 Mar 2022 11:53:27 +0100

On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:

> From: Jeff Hostetler <[email-masked]>
> [...]
> +/*
> + * test-fsmonitor-client.c: client code to send commands/requests to
> + * a `git fsmonitor--daemon` daemon.
> + */
> +
> +#include "test-tool.h"
> +#include "cache.h"
> +#include "parse-options.h"
> +#include "fsmonitor-ipc.h"
> +
> +#ifndef HAVE_FSMONITOR_DAEMON_BACKEND
> +int cmd__fsmonitor_client(int argc, const char **argv)
> +{
> +	die("fsmonitor--daemon not available on this platform");

UX strings added in the series should be marked for translation with
_(), but in this case this is correct as this is a test helper, so only
git developers will see it. We shouldn't waste translator time on it.

> +		die("unable to read index file");

...ditto

> +		die(_("could not query fsmonitor--daemon"));

...but this one is marked for translation, but shouldn't be.

There are other such cases in this series, found by e.g. diffing
origin/master.. and grepping for '"', not noting all of those in review,
I trust you can eyeball them & see what's missing ... :)

> +	if (argc == 2 && !strcmp(argv[1], "-h"))
> +		usage_with_options(fsmonitor_client_usage, options);

I don't think you need this special handling for -h...

> +
> +	subcmd = argv[1];
> +	argv--;
> +	argc++;
> +
> +	argc = parse_options(argc, argv, NULL, options, fsmonitor_client_usage, 0);

...because this will do it for you.