Re: [PATCH v3 13/16] ref-filter: allow porcelain to translate messages in the output
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Apr 4, 2016, 08:05 UTC
- Message-ID
- <vpqa8l9ztmd.fsf@anie.imag.fr>
- In-Reply-To
- <CAOLa=ZQjxmVHuXc0fZioe9OLi9vjNwgHQq-8vf1opWxbuedM9Q@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 47 quoted lines
> 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_)
Show 5 quoted lines
>> 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/