Subject: Re: [PATCH v8 05/37] hook: teach hook.runHookDir

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 12 Mar 2021 09:33:46 +0100

On Thu, Mar 11 2021, Emily Shaffer wrote:

> +	switch (should_run_hookdir) {
> +		case HOOKDIR_NO:

Style: case shouldn't be indented

> +			strbuf_addstr(&hookdir_annotation, _(" (will not run)"));
> +			break;
> +		case HOOKDIR_ERROR:
> +			strbuf_addstr(&hookdir_annotation, _(" (will error and not run)"));
> +			break;
> +		case HOOKDIR_INTERACTIVE:
> +			strbuf_addstr(&hookdir_annotation, _(" (will prompt)"));
> +			break;
> +		case HOOKDIR_WARN:
> +			strbuf_addstr(&hookdir_annotation, _(" (will warn but run)"));
> +			break;
> +		case HOOKDIR_YES:
> +		/*
> +		 * The default behavior should agree with
> +		 * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just
> +		 * do the default behavior.
> +		 */
> +		case HOOKDIR_UNKNOWN:
> +		default:
> +			break;

We should avoid this sort of translation lego.

> +	}
> +
>  	list_for_each(pos, head) {
>  		struct hook *item = list_entry(pos, struct hook, list);
>  		item = list_entry(pos, struct hook, list);
>  		if (item) {
>  			/* Don't translate 'hookdir' - it matches the config */
> -			printf("%s: %s\n",
> +			printf("%s: %s%s\n",

native speakers in some languages to read the sentance backwards.
Because if you concatenate strings like this you force.

(We don't currently have a RTL language in po/, still, but let's not
create churn for if/when we do if we can help it)>

I have a patch on top to fix this, will send it as some general reply of
proposed fixup.s

>  			       (item->from_hookdir
> +	git hook list pre-commit >actual &&
> +	# the hookdir annotation is translated
> +	test_i18ncmp expected actual

This (and the rest of test_i18ncmp in this series) can and should just
be "test_cmp" or "test_i18ncmp", the poison mode is dead. See my recent
patches to search/replace test_i18ncmp.

The reason the function isn't gone entirely was to help a series like
yours in "seen", but if we're re-rolling...