Re: [PATCH 3/3] advice: translate all actions in error_resolve_conflict()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 24, 2023, 14:44 UTC
Message-ID: <xmqqcz4y5huz.fsf@gitster.g>
In-Reply-To: <20230323162234.995485-3-oswald.buddenhagen@gmx.de>

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

Show 6 quoted lines

> action_name() returns a N_()'d string (for good reasons), so we still
> need to _() it.
>
> In practice, this affects 'rebase'.
>
> Whether this is actually useful is debatable ...

Yes, it is debatable. It may be much better to add a new "else if" that covers the case we _know_ is not covered with the current code.

Having 'me' in _() would be consistent with its source marked with N_() as you found out, but I agree with you that it is debatable if this patch is moving things in the right direction. It would belong to the part that should never be exercised once we give action names proper covering by adding missing "else if".

We could even replace the fallback else with a "else BUG()" to ensure that the action names all callers pass have corresponding message that can be translated without sentence lego.

Thanks.

Show 18 quoted lines

> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---
>  advice.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/advice.c b/advice.c
> index d6232439c3..f75f3df582 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -192,7 +192,7 @@ int error_resolve_conflict(const char *me)
>  		error(_("Reverting is not possible because you have unmerged files."));
>  	else
>  		error(_("It is not possible to %s because you have unmerged files."),
> -			me);
> +			_(me));
>  
>  	if (advice_enabled(ADVICE_RESOLVE_CONFLICT))
>  		/*