[PATCH v2] i18n: notes: mark comment for translation

[PATCH v2] i18n: notes: mark comment for translation

[Vasco Almeida]

Mark comment displayed when editing a note for translation.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---

It seems that strbuf_add_commented_lines adds a trailing newline. So adding
another strbuf_addch(&buf, '\n') would make 2 lines rather only one.

 builtin/notes.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 0572051..1d6a096 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {
 };
 
 static const char note_template[] =
-	"\nWrite/edit the notes for the following object:\n";
+	N_("Write/edit the notes for the following object:");
 
 struct note_data {
 	int given;
@@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,
 			copy_obj_to_fd(fd, old_note);
 
 		strbuf_addch(&buf, '\n');
-		strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
+		strbuf_addch(&buf, '\n');
+		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));
 		strbuf_addch(&buf, '\n');
 		write_or_die(fd, buf.buf, buf.len);
 
-- 
2.7.4

Re: [PATCH v2] i18n: notes: mark comment for translation

[Junio C Hamano]

Vasco Almeida <vascomalmeida@sapo.pt> writes:

>  static const char note_template[] =
> -	"\nWrite/edit the notes for the following object:\n";
> +	N_("Write/edit the notes for the following object:");
>  
>  struct note_data {
>  	int given;
> @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,
>  			copy_obj_to_fd(fd, old_note);
>  
>  		strbuf_addch(&buf, '\n');
> -		strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
> +		strbuf_addch(&buf, '\n');
> +		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));

I do not quite understand why you want the blank lines surrounding
the message outside add_commented_lines() call.  I think the intent
is to produce

    #
    # Write/edit the notes for the following object:
    #

with the single call.  If you pushed the newlines outside the
message, wouldn't you end up having this instead (____ denoting an
extra empty line each before and after the message)?

    ____
    # Write/edit the notes for the following object:
    ____

Re: [PATCH v2] i18n: notes: mark comment for translation

[Vasco Almeida]

A Seg, 25-07-2016 às 10:49 -0700, Junio C Hamano escreveu:
> Vasco Almeida <vascomalmeida@sapo.pt> writes:
> 
> > 
> >  static const char note_template[] =
> > -	"\nWrite/edit the notes for the following object:\n";
> > +	N_("Write/edit the notes for the following object:");
> >  
> >  struct note_data {
> >  	int given;
> > @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned
> > char *object, struct note_data *d,
> >  			copy_obj_to_fd(fd, old_note);
> >  
> >  		strbuf_addch(&buf, '\n');
> > -		strbuf_add_commented_lines(&buf, note_template,
> > strlen(note_template));
> > +		strbuf_addch(&buf, '\n');
> > +		strbuf_add_commented_lines(&buf, _(note_template),
> > strlen(_(note_template)));
> 
> I do not quite understand why you want the blank lines surrounding
> the message outside add_commented_lines() call.  I think the intent
> is to produce
> 
>     #
>     # Write/edit the notes for the following object:
>     #

If this is what we want, I will send a re-roll accordingly.

> with the single call.  If you pushed the newlines outside the
> message, wouldn't you end up having this instead (____ denoting an
> extra empty line each before and after the message)?
> 
>     ____
>     # Write/edit the notes for the following object:
>     ____
> 
Yes, this was my intention. The original does:

    #
    # Write/edit the notes for the following object:
    ____

[PATCH v3] i18n: notes: mark comment for translation

[Vasco Almeida]

Mark comment displayed when editing a note for translation.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 builtin/notes.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 0572051..aec427b 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {
 };
 
 static const char note_template[] =
-	"\nWrite/edit the notes for the following object:\n";
+	N_("Write/edit the notes for the following object:");
 
 struct note_data {
 	int given;
@@ -179,8 +179,9 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,
 			copy_obj_to_fd(fd, old_note);
 
 		strbuf_addch(&buf, '\n');
-		strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
-		strbuf_addch(&buf, '\n');
+		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
+		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));
+		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
 		write_or_die(fd, buf.buf, buf.len);
 
 		write_commented_object(fd, object);
-- 
2.7.4

Re: [PATCH v3] i18n: notes: mark comment for translation

[Junio C Hamano]

Vasco Almeida <vascomalmeida@sapo.pt> writes:

> +		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
> +		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));
> +		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));

Hmm, do we really need to make three separate calls?

Re: [PATCH v2] i18n: notes: mark comment for translation

[Junio C Hamano]

Vasco Almeida <vascomalmeida@sapo.pt> writes:

> A Seg, 25-07-2016 às 10:49 -0700, Junio C Hamano escreveu:
>> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>> 
>> > 
>> >  static const char note_template[] =
>> > -	"\nWrite/edit the notes for the following object:\n";
>> > +	N_("Write/edit the notes for the following object:");
>> 
>> I do not quite understand why you want the blank lines surrounding
>> the message outside add_commented_lines() call.  I think the intent
>> is to produce
>> 
>>     #
>>     # Write/edit the notes for the following object:
>>     #
>
> Yes, this was my intention. The original does:
>
>     #
>     # Write/edit the notes for the following object:
>     ____

Ah, of course, I misspoke.  The original "\n<message>\n" requests
that an empty line that is commented, followed by a line with the
message that is also commented, is given at that point.  The last
commented blank was my mistake.

In any case, I do not understand why you want to exclude the LFs
from the message.  If a translation for a particular language is
very long and would not fit on a single line, the translator is
allowed to make the message much longer, i.e. the translated version
of the <message> part may contain one or more LFs (which is what the
add-commented-lines function was invented for).  I'd think having
LFs in the to-be-translated-original will serve as a good hint to
signal translators that is the case.

Thansk.

Re: [PATCH v2] i18n: notes: mark comment for translation

[Vasco Almeida]

A Ter, 26-07-2016 às 10:05 -0700, Junio C Hamano escreveu:
> In any case, I do not understand why you want to exclude the LFs
> from the message.

As Ævar Arnfjörð Bjarmason pointed out [1], it is to assure that a
translator does not break the output by mistake, by removing a LF.
I agree because I have seen a few mistakes about blanks in
translations. I do them myself. I think it is easy to do them if you
are translating a lot of strings, like if you were to start translating
Git to a new language right now. Thus I think it is a good idea to
assure that this kind of mistakes do not occur.

Although msgfmt can catch some mistakes, like source string and
translation must end both with \n, it does not catch all possible
mistakes. For example, I think it does not check a start \n, which is
relevant here. For that translator must use other quality assurance
tools. I know of translate toolkit [2] and msgcheck [3].

> If a translation for a particular language is
> very long and would not fit on a single line, the translator is
> allowed to make the message much longer, i.e. the translated version
> of the <message> part may contain one or more LFs (which is what the
> add-commented-lines function was invented for).  I'd think having
> LFs in the to-be-translated-original will serve as a good hint to
> signal translators that is the case.

When I am translating I always assume that it is fine do add or remove
some lines as needed, unless I'm told otherwise (by a comment for
translators, e.g.).

[1] http://www.mail-archive.com/git@vger.kernel.org/msg98793.html
[2] http://docs.translatehouse.org/projects/translate-toolkit/
[3] https://github.com/flashcode/msgcheck

Re: [PATCH v3] i18n: notes: mark comment for translation

[Vasco Almeida]

A Ter, 26-07-2016 às 09:57 -0700, Junio C Hamano escreveu:
> Vasco Almeida <vascomalmeida@sapo.pt> writes:
> 
> > 
> > +		strbuf_add_commented_lines(&buf, "\n",
> > strlen("\n"));
> > +		strbuf_add_commented_lines(&buf, _(note_template),
> > strlen(_(note_template)));
> > +		strbuf_add_commented_lines(&buf, "\n",
> > strlen("\n"));
> 
> Hmm, do we really need to make three separate calls?

This patch does (1)

#
# Write/edit the notes for the following object:
#

The original source does (2)

#
# Write/edit the notes for the following object:

How do we want, (1) or (2) ?

Re: [PATCH v3] i18n: notes: mark comment for translation

[Junio C Hamano]

Vasco Almeida <vascomalmeida@sapo.pt> writes:

> A Ter, 26-07-2016 às 09:57 -0700, Junio C Hamano escreveu:
>> Vasco Almeida <vascomalmeida@sapo.pt> writes:
>> 
>> > 
>> > +		strbuf_add_commented_lines(&buf, "\n",
>> > strlen("\n"));
>> > +		strbuf_add_commented_lines(&buf, _(note_template),
>> > strlen(_(note_template)));
>> > +		strbuf_add_commented_lines(&buf, "\n",
>> > strlen("\n"));
>> 
>> Hmm, do we really need to make three separate calls?
>
> This patch does (1)
>
> #
> # Write/edit the notes for the following object:
> #
>
> The original source does (2)
>
> #
> # Write/edit the notes for the following object:
>
> How do we want, (1) or (2) ?

As I said earlier I was misreading the original one.

The input to strbuf_add_commented_lines() actually is a string that
uses LF as a record terminator and asks the function to output each
record on its own line prefixed with either "#" or "# ", so I should
have considered the last LF as part of the second line.

In other words, the output should be as if you just did

-	"\nWrite/edit the notes for the following object:\n";
+	N_("\nWrite/edit the notes for the following object:\n");

in your patch, i.e. (2).

Thanks.

[PATCH v4] i18n: notes: mark comment for translation

[Vasco Almeida]

Mark comment displayed when editing a note for translation.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---

This patch follows the original output and Ævar Arnfjörð Bjarmason
sugestion to remove \n from the source string in order to assure that the
ouput layout is not change by one translator forgetting to add \n, for
instance.

 builtin/notes.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 0572051..f848b89 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {
 };
 
 static const char note_template[] =
-	"\nWrite/edit the notes for the following object:\n";
+	N_("Write/edit the notes for the following object:");
 
 struct note_data {
 	int given;
@@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,
 			copy_obj_to_fd(fd, old_note);
 
 		strbuf_addch(&buf, '\n');
-		strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
+		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
+		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));
 		strbuf_addch(&buf, '\n');
 		write_or_die(fd, buf.buf, buf.len);
 
-- 
2.7.4

Re: [PATCH v4] i18n: notes: mark comment for translation

[Junio C Hamano]

Vasco Almeida <vascomalmeida@sapo.pt> writes:

> Mark comment displayed when editing a note for translation.
>
> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> ---
>
> This patch follows the original output and Ævar Arnfjörð Bjarmason
> sugestion to remove \n from the source string in order to assure that the
> ouput layout is not change by one translator forgetting to add \n, for
> instance.

Well, that cuts both ways.  A translater adding an extra \n would
also break the layout, so I am not convinced that is a very good
justification.

As a parameter to strbuf_add_commented_lines(), an extra or a
missing \n does not really matter, though, because the whole thing
is a line-oriented comment ;-)

As to the patch text, it looks like it would produce more correct
output than what I queued tentatively on 'pu', so I'd replace it
with this one.

>  builtin/notes.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 0572051..f848b89 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {
>  };
>  
>  static const char note_template[] =
> -	"\nWrite/edit the notes for the following object:\n";
> +	N_("Write/edit the notes for the following object:");
>  
>  struct note_data {
>  	int given;
> @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,
>  			copy_obj_to_fd(fd, old_note);
>  
>  		strbuf_addch(&buf, '\n');
> -		strbuf_add_commented_lines(&buf, note_template, strlen(note_template));
> +		strbuf_add_commented_lines(&buf, "\n", strlen("\n"));
> +		strbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));
>  		strbuf_addch(&buf, '\n');
>  		write_or_die(fd, buf.buf, buf.len);