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

[Karthik Nayak]

On Mon, Nov 21, 2016 at 2:11 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> cc'in Matthieu since he wrote the patch.
>>
>> On Sat, Nov 19, 2016 at 4:16 AM, Jakub Narębski  wrote:
>>> W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
 From: Karthik Nayak 

 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 
 Mentored-by: Christian Couder 
 Mentored-by: Matthieu Moy 
 Signed-off-by: Karthik Nayak 
 ---
  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

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

[Matthieu Moy]

Karthik Nayak  writes:

> cc'in Matthieu since he wrote the patch.
>
> On Sat, Nov 19, 2016 at 4:16 AM, Jakub Narębski  wrote:
>> W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
>>> From: Karthik Nayak 
>>>
>>> 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 
>>> Mentored-by: Christian Couder 
>>> Mentored-by: Matthieu Moy 
>>> Signed-off-by: Karthik Nayak 
>>> ---
>>>  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/

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

[Karthik Nayak]

cc'in Matthieu since he wrote the patch.

On Sat, Nov 19, 2016 at 4:16 AM, Jakub Narębski  wrote:
> W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
>> From: Karthik Nayak 
>>
>> 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 
>> Mentored-by: Christian Couder 
>> Mentored-by: Matthieu Moy 
>> Signed-off-by: Karthik Nayak 
>> ---
>>  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?
>
> 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).
>

That is the intention.

-- 
Regards,
Karthik Nayak

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

[Jakub Narębski]

W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
> From: Karthik Nayak 
> 
> 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 
> Mentored-by: Christian Couder 
> Mentored-by: Matthieu Moy 
> Signed-off-by: Karthik Nayak 
> ---
>  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?

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

Best,
-- 
Jakub Narębski

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

[Jacob Keller]

On Tue, Nov 8, 2016 at 12:12 PM, Karthik Nayak  wrote:
> From: Karthik Nayak 
>
> 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.
>

So any user that wants these translated calls
setup_ref_filter_porcelain_msg but this will impact all callers from
that point on. Ok, I think that's ok? Otherwise they get default
without translation.

> Written-by: Matthieu Moy 
> Mentored-by: Christian Couder 
> Mentored-by: Matthieu Moy 
> Signed-off-by: Karthik Nayak 
> ---
>  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");
> +}
> +
>  typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>
>  struct align {
> @@ -1130,15 +1150,15 @@ static void fill_remote_ref_details(struct used_atom 
> *atom, const char *refname,
> else if (atom->u.remote_ref.option == RR_TRACK) {
> if (stat_tracking_info(branch, _ours,
>_theirs, NULL)) {
> -   *s = xstrdup("gone");
> +   *s = xstrdup(msgs.gone);
> } else if (!num_ours && !num_theirs)
> *s = "";
> else if (!num_ours)
> -   *s = xstrfmt("behind %d", num_theirs);
> +   *s = xstrfmt(msgs.behind, num_theirs);
> else if (!num_theirs)
> -   *s = xstrfmt("ahead %d", num_ours);
> +   *s = xstrfmt(msgs.ahead, num_ours);
> else
> -   *s = xstrfmt("ahead %d, behind %d",
> +   *s = xstrfmt(msgs.ahead_behind,
>  num_ours, num_theirs);
> if (!atom->u.remote_ref.nobracket && *s[0]) {
> const char *to_free = *s;
> diff --git a/ref-filter.h b/ref-filter.h
> index 0014b92..da17145 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
>  int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
> unset);
>  /*  Get the current HEAD's description */
>  char *get_head_description(void);
> +/*  Set up translated strings in the output. */
> +void setup_ref_filter_porcelain_msg(void);
>
>  #endif /*  REF_FILTER_H  */
> --
> 2.10.2
>

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

[Karthik Nayak]

From: Karthik Nayak 

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 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 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");
+}
+
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
 struct align {
@@ -1130,15 +1150,15 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
   _theirs, NULL)) {
-   *s = xstrdup("gone");
+   *s = xstrdup(msgs.gone);
} else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
-   *s = xstrfmt("behind %d", num_theirs);
+   *s = xstrfmt(msgs.behind, num_theirs);
else if (!num_theirs)
-   *s = xstrfmt("ahead %d", num_ours);
+   *s = xstrfmt(msgs.ahead, num_ours);
else
-   *s = xstrfmt("ahead %d, behind %d",
+   *s = xstrfmt(msgs.ahead_behind,
 num_ours, num_theirs);
if (!atom->u.remote_ref.nobracket && *s[0]) {
const char *to_free = *s;
diff --git a/ref-filter.h b/ref-filter.h
index 0014b92..da17145 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 /*  Get the current HEAD's description */
 char *get_head_description(void);
+/*  Set up translated strings in the output. */
+void setup_ref_filter_porcelain_msg(void);
 
 #endif /*  REF_FILTER_H  */
-- 
2.10.2