threads / patch / 42911

v2i18n: notes: mark comment for translation

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

## tl;dr

11 messages between Jul 23, 2016 and Jul 28, 2016. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Vasco Almeida· Jul 23, 2016, 14:10 UTC · lore
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(-)
Show changes to builtin/notes.c +3 −2
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
Junio C Hamano· Jul 25, 2016, 17:49 UTC · re: Vasco Almeida · lore

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

Vasco Almeida <vascomalmeida@sapo.pt> writes:
Show 13 quoted lines
>  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:
    ____
Vasco Almeida· Jul 26, 2016, 12:16 UTC · re: Junio C Hamano · lore

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

A Seg, 25-07-2016 às 10:49 -0700, Junio C Hamano escreveu:
Show 27 quoted lines
> 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.
Show 8 quoted lines
> 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:
    ____
Junio C Hamano· Jul 26, 2016, 17:05 UTC · re: Vasco Almeida · lore

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

Vasco Almeida <vascomalmeida@sapo.pt> writes:
Show 21 quoted lines
> 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.
Vasco Almeida· Jul 27, 2016, 10:40 UTC · re: Junio C Hamano · lore

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

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

Show 7 quoted lines
> 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

Vasco Almeida· Jul 26, 2016, 12:16 UTC · re: Vasco Almeida · lore

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

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(-)
Show changes to builtin/notes.c +4 −3
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
Junio C Hamano· Jul 26, 2016, 16:57 UTC · re: Vasco Almeida · lore

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

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?
Vasco Almeida· Jul 27, 2016, 10:53 UTC · re: Junio C Hamano · lore

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

A Ter, 26-07-2016 às 09:57 -0700, Junio C Hamano escreveu:
Show 11 quoted lines
> 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) ?
Junio C Hamano· Jul 27, 2016, 19:33 UTC · re: Vasco Almeida · lore

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

Vasco Almeida <vascomalmeida@sapo.pt> writes:
Show 25 quoted lines
> 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.
Vasco Almeida· Jul 28, 2016, 11:26 UTC · re: Vasco Almeida · lore

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

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(-)
Show changes to builtin/notes.c +3 −2
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
Junio C Hamano· Jul 28, 2016, 16:09 UTC · re: Vasco Almeida · lore

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

Vasco Almeida <vascomalmeida@sapo.pt> writes:
Show 9 quoted lines
> 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.

Show 25 quoted lines
>  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);

← back to recent threads