{"thread":{"id":"42911","subject":"[PATCH v2] i18n: notes: mark comment for translation","startedAt":"2016-07-23T14:11:15Z","lastAt":"2016-07-28T16:09:09Z","messageCount":11,"participants":["Vasco Almeida","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"292038","messageId":"1469283027-23055-1-git-send-email-vascomalmeida@sapo.pt","threadId":"42911","inReplyTo":null,"subject":"[PATCH v2] i18n: notes: mark comment for translation","fromName":"Vasco Almeida","fromEmail":"vascomalmeida@sapo.pt","sentAt":"2016-07-23T14:10:27Z","receivedAt":"2016-07-23T14:11:15Z","isPatch":true,"sender":{"key":"vascomalmeida@sapo.pt","avatar":"https://avatars.githubusercontent.com/u/9001556?v=4"},"body":"Mark comment displayed when editing a note for translation.\n\nSigned-off-by: Vasco Almeida \n---\n\nIt seems that strbuf_add_commented_lines adds a trailing newline. So adding\nanother strbuf_addch(&buf, '\\n') would make 2 lines rather only one.\n\n builtin/notes.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 0572051..1d6a096 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {\n };\n \n static const char note_template[] =\n-\t\"\\nWrite/edit the notes for the following object:\\n\";\n+\tN_(\"Write/edit the notes for the following object:\");\n \n struct note_data {\n \tint given;\n@@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,\n \t\t\tcopy_obj_to_fd(fd, old_note);\n \n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_add_commented_lines(&buf, note_template, strlen(note_template));\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n \t\tstrbuf_addch(&buf, '\\n');\n \t\twrite_or_die(fd, buf.buf, buf.len);\n \n-- \n2.7.4\n\n"},{"id":"292113","messageId":"xmqqr3ah621l.fsf@gitster.mtv.corp.google.com","threadId":"42911","inReplyTo":"1469283027-23055-1-git-send-email-vascomalmeida@sapo.pt","subject":"Re: [PATCH v2] i18n: notes: mark comment for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-25T17:49:26Z","receivedAt":"2016-07-25T17:49:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vasco Almeida writes:\n\n> static const char note_template[] =\n> -\t\"\\nWrite/edit the notes for the following object:\\n\";\n> +\tN_(\"Write/edit the notes for the following object:\");\n> \n> struct note_data {\n> \tint given;\n> @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,\n> \t\t\tcopy_obj_to_fd(fd, old_note);\n> \n> \t\tstrbuf_addch(&buf, '\\n');\n> -\t\tstrbuf_add_commented_lines(&buf, note_template, strlen(note_template));\n> +\t\tstrbuf_addch(&buf, '\\n');\n> +\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n\nI do not quite understand why you want the blank lines surrounding\nthe message outside add_commented_lines() call. I think the intent\nis to produce\n\n #\n # Write/edit the notes for the following object:\n #\n\nwith the single call. If you pushed the newlines outside the\nmessage, wouldn't you end up having this instead (____ denoting an\nextra empty line each before and after the message)?\n\n ____\n # Write/edit the notes for the following object:\n ____\n\n"},{"id":"292192","messageId":"1469535363.1845.8.camel@sapo.pt","threadId":"42911","inReplyTo":"xmqqr3ah621l.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] i18n: notes: mark comment for translation","fromName":"Vasco Almeida","fromEmail":"vascomalmeida@sapo.pt","sentAt":"2016-07-26T12:16:03Z","receivedAt":"2016-07-26T12:16:20Z","isPatch":true,"sender":{"key":"vascomalmeida@sapo.pt","avatar":"https://avatars.githubusercontent.com/u/9001556?v=4"},"body":"A Seg, 25-07-2016 às 10:49 -0700, Junio C Hamano escreveu:\n> Vasco Almeida writes:\n> \n> > \n> >  static const char note_template[] =\n> > -\t\"\\nWrite/edit the notes for the following object:\\n\";\n> > +\tN_(\"Write/edit the notes for the following object:\");\n> >  \n> >  struct note_data {\n> >  \tint given;\n> > @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned\n> > char *object, struct note_data *d,\n> >  \t\t\tcopy_obj_to_fd(fd, old_note);\n> >  \n> >  \t\tstrbuf_addch(&buf, '\\n');\n> > -\t\tstrbuf_add_commented_lines(&buf, note_template,\n> > strlen(note_template));\n> > +\t\tstrbuf_addch(&buf, '\\n');\n> > +\t\tstrbuf_add_commented_lines(&buf, _(note_template),\n> > strlen(_(note_template)));\n> \n> I do not quite understand why you want the blank lines surrounding\n> the message outside add_commented_lines() call.  I think the intent\n> is to produce\n> \n>     #\n>     # Write/edit the notes for the following object:\n>     #\n\nIf this is what we want, I will send a re-roll accordingly.\n\n> with the single call.  If you pushed the newlines outside the\n> message, wouldn't you end up having this instead (____ denoting an\n> extra empty line each before and after the message)?\n> \n>     ____\n>     # Write/edit the notes for the following object:\n>     ____\n> \nYes, this was my intention. The original does:\n\n    #\n    # Write/edit the notes for the following object:\n    ____\n\n"},{"id":"292193","messageId":"1469535400-9242-1-git-send-email-vascomalmeida@sapo.pt","threadId":"42911","inReplyTo":"1469283027-23055-1-git-send-email-vascomalmeida@sapo.pt","subject":"[PATCH v3] i18n: notes: mark comment for translation","fromName":"Vasco Almeida","fromEmail":"vascomalmeida@sapo.pt","sentAt":"2016-07-26T12:16:40Z","receivedAt":"2016-07-26T12:17:30Z","isPatch":true,"sender":{"key":"vascomalmeida@sapo.pt","avatar":"https://avatars.githubusercontent.com/u/9001556?v=4"},"body":"Mark comment displayed when editing a note for translation.\n\nSigned-off-by: Vasco Almeida \n---\n builtin/notes.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 0572051..aec427b 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {\n };\n \n static const char note_template[] =\n-\t\"\\nWrite/edit the notes for the following object:\\n\";\n+\tN_(\"Write/edit the notes for the following object:\");\n \n struct note_data {\n \tint given;\n@@ -179,8 +179,9 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,\n \t\t\tcopy_obj_to_fd(fd, old_note);\n \n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_add_commented_lines(&buf, note_template, strlen(note_template));\n-\t\tstrbuf_addch(&buf, '\\n');\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n+\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n \t\twrite_or_die(fd, buf.buf, buf.len);\n \n \t\twrite_commented_object(fd, object);\n-- \n2.7.4\n\n"},{"id":"292236","messageId":"xmqqzip41gn5.fsf@gitster.mtv.corp.google.com","threadId":"42911","inReplyTo":"1469535400-9242-1-git-send-email-vascomalmeida@sapo.pt","subject":"Re: [PATCH v3] i18n: notes: mark comment for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T16:57:34Z","receivedAt":"2016-07-26T16:59:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vasco Almeida writes:\n\n> +\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n> +\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n> +\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n\nHmm, do we really need to make three separate calls?\n"},{"id":"292237","messageId":"xmqqvazs1g9o.fsf@gitster.mtv.corp.google.com","threadId":"42911","inReplyTo":"1469535363.1845.8.camel@sapo.pt","subject":"Re: [PATCH v2] i18n: notes: mark comment for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-26T17:05:39Z","receivedAt":"2016-07-26T17:07:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vasco Almeida writes:\n\n> A Seg, 25-07-2016 às 10:49 -0700, Junio C Hamano escreveu:\n>> Vasco Almeida writes:\n>> \n>> > \n>> >  static const char note_template[] =\n>> > -\t\"\\nWrite/edit the notes for the following object:\\n\";\n>> > +\tN_(\"Write/edit the notes for the following object:\");\n>> \n>> I do not quite understand why you want the blank lines surrounding\n>> the message outside add_commented_lines() call.  I think the intent\n>> is to produce\n>> \n>>     #\n>>     # Write/edit the notes for the following object:\n>>     #\n>\n> Yes, this was my intention. The original does:\n>\n>     #\n>     # Write/edit the notes for the following object:\n>     ____\n\nAh, of course, I misspoke. The original \"\\n\\n\" requests\nthat an empty line that is commented, followed by a line with the\nmessage that is also commented, is given at that point. The last\ncommented blank was my mistake.\n\nIn any case, I do not understand why you want to exclude the LFs\nfrom the message. If a translation for a particular language is\nvery long and would not fit on a single line, the translator is\nallowed to make the message much longer, i.e. the translated version\nof the part may contain one or more LFs (which is what the\nadd-commented-lines function was invented for). I'd think having\nLFs in the to-be-translated-original will serve as a good hint to\nsignal translators that is the case.\n\nThansk.\n\n"},{"id":"292307","messageId":"1469616036.1858.21.camel@sapo.pt","threadId":"42911","inReplyTo":"xmqqvazs1g9o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] i18n: notes: mark comment for translation","fromName":"Vasco Almeida","fromEmail":"vascomalmeida@sapo.pt","sentAt":"2016-07-27T10:40:36Z","receivedAt":"2016-07-27T10:40:51Z","isPatch":true,"sender":{"key":"vascomalmeida@sapo.pt","avatar":"https://avatars.githubusercontent.com/u/9001556?v=4"},"body":"A Ter, 26-07-2016 às 10:05 -0700, Junio C Hamano escreveu:\n> In any case, I do not understand why you want to exclude the LFs\n> from the message.\n\nAs Ævar Arnfjörð Bjarmason pointed out [1], it is to assure that a\ntranslator does not break the output by mistake, by removing a LF.\nI agree because I have seen a few mistakes about blanks in\ntranslations. I do them myself. I think it is easy to do them if you\nare translating a lot of strings, like if you were to start translating\nGit to a new language right now. Thus I think it is a good idea to\nassure that this kind of mistakes do not occur.\n\nAlthough msgfmt can catch some mistakes, like source string and\ntranslation must end both with \\n, it does not catch all possible\nmistakes. For example, I think it does not check a start \\n, which is\nrelevant here. For that translator must use other quality assurance\ntools. I know of translate toolkit [2] and msgcheck [3].\n\n> If a translation for a particular language is\n> very long and would not fit on a single line, the translator is\n> allowed to make the message much longer, i.e. the translated version\n> of the part may contain one or more LFs (which is what the\n> add-commented-lines function was invented for).  I'd think having\n> LFs in the to-be-translated-original will serve as a good hint to\n> signal translators that is the case.\n\nWhen I am translating I always assume that it is fine do add or remove\nsome lines as needed, unless I'm told otherwise (by a comment for\ntranslators, e.g.).\n\n[1] http://www.mail-archive.com/git@vger.kernel.org/msg98793.html\n[2] http://docs.translatehouse.org/projects/translate-toolkit/\n[3] https://github.com/flashcode/msgcheck\n"},{"id":"292308","messageId":"1469616819.1858.25.camel@sapo.pt","threadId":"42911","inReplyTo":"xmqqzip41gn5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] i18n: notes: mark comment for translation","fromName":"Vasco Almeida","fromEmail":"vascomalmeida@sapo.pt","sentAt":"2016-07-27T10:53:39Z","receivedAt":"2016-07-27T10:53:55Z","isPatch":true,"sender":{"key":"vascomalmeida@sapo.pt","avatar":"https://avatars.githubusercontent.com/u/9001556?v=4"},"body":"A Ter, 26-07-2016 às 09:57 -0700, Junio C Hamano escreveu:\n> Vasco Almeida writes:\n> \n> > \n> > +\t\tstrbuf_add_commented_lines(&buf, \"\\n\",\n> > strlen(\"\\n\"));\n> > +\t\tstrbuf_add_commented_lines(&buf, _(note_template),\n> > strlen(_(note_template)));\n> > +\t\tstrbuf_add_commented_lines(&buf, \"\\n\",\n> > strlen(\"\\n\"));\n> \n> Hmm, do we really need to make three separate calls?\n\nThis patch does (1)\n\n#\n# Write/edit the notes for the following object:\n#\n\nThe original source does (2)\n\n#\n# Write/edit the notes for the following object:\n\nHow do we want, (1) or (2) ?\n"},{"id":"292361","messageId":"xmqq7fc6x4dw.fsf@gitster.mtv.corp.google.com","threadId":"42911","inReplyTo":"1469616819.1858.25.camel@sapo.pt","subject":"Re: [PATCH v3] i18n: notes: mark comment for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-27T19:33:31Z","receivedAt":"2016-07-27T19:33:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vasco Almeida writes:\n\n> A Ter, 26-07-2016 às 09:57 -0700, Junio C Hamano escreveu:\n>> Vasco Almeida writes:\n>> \n>> > \n>> > +\t\tstrbuf_add_commented_lines(&buf, \"\\n\",\n>> > strlen(\"\\n\"));\n>> > +\t\tstrbuf_add_commented_lines(&buf, _(note_template),\n>> > strlen(_(note_template)));\n>> > +\t\tstrbuf_add_commented_lines(&buf, \"\\n\",\n>> > strlen(\"\\n\"));\n>> \n>> Hmm, do we really need to make three separate calls?\n>\n> This patch does (1)\n>\n> #\n> # Write/edit the notes for the following object:\n> #\n>\n> The original source does (2)\n>\n> #\n> # Write/edit the notes for the following object:\n>\n> How do we want, (1) or (2) ?\n\nAs I said earlier I was misreading the original one.\n\nThe input to strbuf_add_commented_lines() actually is a string that\nuses LF as a record terminator and asks the function to output each\nrecord on its own line prefixed with either \"#\" or \"# \", so I should\nhave considered the last LF as part of the second line.\n\nIn other words, the output should be as if you just did\n\n-\t\"\\nWrite/edit the notes for the following object:\\n\";\n+\tN_(\"\\nWrite/edit the notes for the following object:\\n\");\n\nin your patch, i.e. (2).\n\nThanks.\n"},{"id":"292396","messageId":"1469705175-7503-1-git-send-email-vascomalmeida@sapo.pt","threadId":"42911","inReplyTo":"1469283027-23055-1-git-send-email-vascomalmeida@sapo.pt","subject":"[PATCH v4] i18n: notes: mark comment for translation","fromName":"Vasco Almeida","fromEmail":"vascomalmeida@sapo.pt","sentAt":"2016-07-28T11:26:15Z","receivedAt":"2016-07-28T11:27:08Z","isPatch":true,"sender":{"key":"vascomalmeida@sapo.pt","avatar":"https://avatars.githubusercontent.com/u/9001556?v=4"},"body":"Mark comment displayed when editing a note for translation.\n\nSigned-off-by: Vasco Almeida \n---\n\nThis patch follows the original output and Ævar Arnfjörð Bjarmason\nsugestion to remove \\n from the source string in order to assure that the\nouput layout is not change by one translator forgetting to add \\n, for\ninstance.\n\n builtin/notes.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 0572051..f848b89 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {\n };\n \n static const char note_template[] =\n-\t\"\\nWrite/edit the notes for the following object:\\n\";\n+\tN_(\"Write/edit the notes for the following object:\");\n \n struct note_data {\n \tint given;\n@@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,\n \t\t\tcopy_obj_to_fd(fd, old_note);\n \n \t\tstrbuf_addch(&buf, '\\n');\n-\t\tstrbuf_add_commented_lines(&buf, note_template, strlen(note_template));\n+\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n+\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n \t\tstrbuf_addch(&buf, '\\n');\n \t\twrite_or_die(fd, buf.buf, buf.len);\n \n-- \n2.7.4\n\n"},{"id":"292420","messageId":"xmqq1t2du4mb.fsf@gitster.mtv.corp.google.com","threadId":"42911","inReplyTo":"1469705175-7503-1-git-send-email-vascomalmeida@sapo.pt","subject":"Re: [PATCH v4] i18n: notes: mark comment for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-28T16:09:00Z","receivedAt":"2016-07-28T16:09:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vasco Almeida writes:\n\n> Mark comment displayed when editing a note for translation.\n>\n> Signed-off-by: Vasco Almeida \n> ---\n>\n> This patch follows the original output and Ævar Arnfjörð Bjarmason\n> sugestion to remove \\n from the source string in order to assure that the\n> ouput layout is not change by one translator forgetting to add \\n, for\n> instance.\n\nWell, that cuts both ways. A translater adding an extra \\n would\nalso break the layout, so I am not convinced that is a very good\njustification.\n\nAs a parameter to strbuf_add_commented_lines(), an extra or a\nmissing \\n does not really matter, though, because the whole thing\nis a line-oriented comment ;-)\n\nAs to the patch text, it looks like it would produce more correct\noutput than what I queued tentatively on 'pu', so I'd replace it\nwith this one.\n\n> builtin/notes.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 0572051..f848b89 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -91,7 +91,7 @@ static const char * const git_notes_get_ref_usage[] = {\n> };\n> \n> static const char note_template[] =\n> -\t\"\\nWrite/edit the notes for the following object:\\n\";\n> +\tN_(\"Write/edit the notes for the following object:\");\n> \n> struct note_data {\n> \tint given;\n> @@ -179,7 +179,8 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,\n> \t\t\tcopy_obj_to_fd(fd, old_note);\n> \n> \t\tstrbuf_addch(&buf, '\\n');\n> -\t\tstrbuf_add_commented_lines(&buf, note_template, strlen(note_template));\n> +\t\tstrbuf_add_commented_lines(&buf, \"\\n\", strlen(\"\\n\"));\n> +\t\tstrbuf_add_commented_lines(&buf, _(note_template), strlen(_(note_template)));\n> \t\tstrbuf_addch(&buf, '\\n');\n> \t\twrite_or_die(fd, buf.buf, buf.len);\n"}]}