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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 7, 2022, 10:53 UTC
Message-ID: <220307.868rtm2fme.gmgdl@evledraar.gmail.com>
In-Reply-To: <2b563c240eb1b592e95e89400aa5d57df1bf29b8.1646160212.git.gitgitgadget@gmail.com>

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

Show 16 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
> [...]
> +/*
> + * 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...

Show 6 quoted lines

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

...because this will do it for you.