Re: [PATCH v7 14/17] ref-filter: allow porcelain to translate messages in the output

[Karthik Nayak <karthik.188@gmail.com>]

On Mon, Nov 21, 2016 at 2:11 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> 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: */
>

Will update the commit message and add the comment. Thanks :)

-- 
Regards,
Karthik Nayak