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

From: Junio C Hamano <[email-masked]>
Date: Fri, 24 Mar 2023 07:44:36 -0700

Oswald Buddenhagen <[email-masked]> writes:

> 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.

> Signed-off-by: Oswald Buddenhagen <[email-masked]>
> ---
>  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))
>  		/*