Re: [PATCH v7 14/17] ref-filter: allow porcelain to translate messages in the output
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Nov 21, 2016, 08:41 UTC
- Message-ID
- <vpq1sy58bsl.fsf@anie.imag.fr>
- In-Reply-To
- <CAOLa=ZRU--FyFWNvv7RGH2mho36hvUu2ye=JOapXZ6QoEy=nRw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 51 quoted lines
> cc'in Matthieu since he wrote the patch.
>
> On Sat, Nov 19, 2016 at 4:16 AM, Jakub Narębski <jnareb@gmail.com> wrote:
>> W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
>>> From: Karthik Nayak <karthik.188@gmail.com>
>>>
>>> Introduce setup_ref_filter_porcelain_msg() so that the messages used in
>>> the atom %(upstream:track) can be translated if needed. This is needed
>>> as we port branch.c to use ref-filter's printing API's.
>>>
>>> Written-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>>> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
>>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>>> ---
>>> ref-filter.c | 28 ++++++++++++++++++++++++----
>>> ref-filter.h | 2 ++
>>> 2 files changed, 26 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ref-filter.c b/ref-filter.c
>>> index b47b900..944671a 100644
>>> --- a/ref-filter.c
>>> +++ b/ref-filter.c
>>> @@ -15,6 +15,26 @@
>>> #include "version.h"
>>> #include "wt-status.h"
>>>
>>> +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");
>>> +}
>>
>> Do I understand it correctly that this mechanism is here to avoid
>> repeated calls into gettext, as those messages would get repeated
>> over and over; otherwise one would use foo = N_("...") and _(foo),
>> isn't it?That's not the primary goal. The primary goal is to keep untranslated, and immutable messages in plumbing commands. We may decide one day that _("gone") is not the best message for the end user and replace it with, say, _("vanished"), but the "gone" has to remain the same forever and regardless of the user's config for scripts using it.
We could have written
in_porcelain ? _("gone") : "gone"here and there in the code, but having a single place where we set all the messages seems simpler. Call setup_ref_filter_porcelain_msg() and get the porcelain messages, don't call it and keep the plumbing messages.
Note that it's not the first place in the code where we do this, see setup_unpack_trees_porcelain in unpack-trees.c for another instance.
Karthik: the commit message could be improved, for example:
Introduce setup_ref_filter_porcelain_msg() so that the messages used in the atom %(upstream:track) can be translated if needed. By default, keep the messages untranslated, which is the right behavior for plumbing commands. This is needed as we port branch.c to use ref-filter's printing API's.
Why not a comment right below "} msgs = {" saying e.g.:/* Untranslated plumbing messages: */
>> I wonder if there is some way to avoid duplication here, but I don't >> see anything easy and safe (e.g. against running setup_*() twice).
I think we do need duplication for the reason above.
-- Matthieu Moy http://www-verimag.imag.fr/~moy/