Re: [PATCH v3 13/16] ref-filter: allow porcelain to translate messages in the output

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Karthik Nayak <karthik.188@gmail.com> writes:

> cc'ing Matthieu since this patch was initially written by him.
>
> On Thu, Mar 31, 2016 at 3:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> +static struct ref_msg {
>>> +     const char *gone;
>>> +     const char *ahead;
>>> +     const char *behind;
>>> +     const char *ahead_behind;
>>> +} msgs = {
>>> +     "gone",
>>> +     "ahead %d",
>>> +     "behind %d",
>>> +     "ahead %d, behind %d"
>>> +};
>>> +
>>> +void setup_ref_filter_porcelain_msg(void)
>>> +{
>>> +     msgs.gone = _("gone");
>>> +     msgs.ahead = _("ahead %d");
>>> +     msgs.behind = _("behind %d");
>>> +     msgs.ahead_behind = _("ahead %d, behind %d");
>>> +}
>>
>> I do not think this patch is wrong, but I wonder if it would be
>> sufficient to have a single bit in file-scope static variable and
>> flip it in setup_ref_filter_porcelain_msg().  I.e.
>>
>>         static int use_porcelain_msg; /* false by default */
>>
>>         void setup_ref_filter_porcelain_msg(void)
>>         {
>>                 use_porcelain_msg = 1;
>>         }
>>
>>         static const char *P_(const char *msg)
>>         {
>>                 return use_porcelain_msg ? _(msg) : msg;
>>         }
>>
>> and then mark the message up perhaps like so:
>>
>>         -       *s = xstrdup("gone");
>>         +       *s = xstrdup(P_("gone"));
>>
>> which may make things much simpler.

... but less evolutive. The non-translatable strings also need to be
cast in stone, while the translatable ones may be subject to future
improvements/tweaks. If they are already duplicated in the code, then
updating one won't change the other, but factoring them means that the
porcelain message can't easily be changed without modifying the plumbing
one.

I'm not sure how important it is in this case, but it was in the case of
setup_unpack_trees_porcelain which I took inspiration from when we
discussed this (actually, in setup_unpack_trees_porcelain, there's isn't
any translation even in porcelain).

Note that this can be worked around later by adding another function like

        static const char *get_message(const char *porcelain, const char *plumbing)
        {
                return use_porcelain_msg ? porcelain : plumbing;
        }

to be called with get_message(_("this ref was gone"), "gone") or so.

>> We'd need to update Makefile to recognize X_() as another keyword;

(I guess you meant P_, not X_)

>> I'd think something like this should do:
>>
>>          XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
>>         -        --keyword=_ --keyword=N_ --keyword="Q_:1,2"
>>         +        --keyword=_ --keyword=N_ --keyword=P_ --keyword="Q_:1,2"

I'm a bit reluctant to modifying the Makefile for something not really
build-related.

> I'm not totally knowledgeable  about how porcelain works, although
> Matthieu did give me a
> brief explanation. I guess it'd better to hear his thoughts on this.

In summary: both would work. No strong opinion from me, but I slightly
prefer the version in the patch (i.e. the one I suggested IIRC) to
Junio's version.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/