Re: [PATCH] C style: use standard style for "TRANSLATORS" comments

[Junio C Hamano]

Jiang Xin  writes:

> 2017-05-12 5:20 GMT+08:00 Ævar Arnfjörð Bjarmason :
>> Change all the "TRANSLATORS: [...]" comments in the C code to use the
>> regular Git coding style, and amend the style guide so that the
>> example there uses that style.
>>
>> This custom style was necessary back in 2010 when the gettext support
>> was initially added, and was subsequently documented in commit
>> cbcfd4e3ea ("i18n: mention "TRANSLATORS:" marker in
>> Documentation/CodingGuidelines", 2014-04-18).
>>
>> GNU xgettext hasn't had the parsing limitation that necessitated this
>> exception for almost 3 years. Since its 0.19 release on 2014-06-02
>> it's been able to recognize TRANSLATOR comments in the standard Git
>> comment syntax[1].
>
> My gettext version is 0.19.8.1.  I applied this patch and checked the
> new generated `git.pot` file, all "TRANSLATORS:" directions are well
> kept as usual.

Ævar, sorry that this patch fell through cracks about 20 days ago.
I'll queue with Acked-by by Jiang.

Thanks, both.

Re: [PATCH] C style: use standard style for "TRANSLATORS" comments

[Jiang Xin]

2017-05-12 5:20 GMT+08:00 Ævar Arnfjörð Bjarmason :
> Change all the "TRANSLATORS: [...]" comments in the C code to use the
> regular Git coding style, and amend the style guide so that the
> example there uses that style.
>
> This custom style was necessary back in 2010 when the gettext support
> was initially added, and was subsequently documented in commit
> cbcfd4e3ea ("i18n: mention "TRANSLATORS:" marker in
> Documentation/CodingGuidelines", 2014-04-18).
>
> GNU xgettext hasn't had the parsing limitation that necessitated this
> exception for almost 3 years. Since its 0.19 release on 2014-06-02
> it's been able to recognize TRANSLATOR comments in the standard Git
> comment syntax[1].

My gettext version is 0.19.8.1.  I applied this patch and checked the
new generated `git.pot` file, all "TRANSLATORS:" directions are well
kept as usual.

This patch is nice.

-- 
Jiang Xin

Re: [PATCH] C style: use standard style for "TRANSLATORS" comments

[Johannes Sixt]

Am 11.05.2017 um 23:20 schrieb Ævar Arnfjörð Bjarmason:

diff --git a/builtin/notes.c b/builtin/notes.c
index 7b891471c4..fb856e53b6 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -340,8 +340,10 @@ static struct notes_tree *init_notes_check(const char 
*subcommand,
  
  	ref = (flags & NOTES_INIT_WRITABLE) ? t->update_ref : t->ref;

if (!starts_with(ref, "refs/notes/"))
-   /* TRANSLATORS: the first %s will be replaced by a
-  git notes command: 'add', 'merge', 'remove', etc.*/
+   /*
+* TRANSLATORS: the first %s will be replaced by a git
+* notes command: 'add', 'merge', 'remove', etc.
+*/


Rewrapping lines is generally frowned upon because it makes it difficult 
to see whether something was changed. Keeping the line wrapping will 
also reduce the noise in the next .pot commit, I think (not sure if that 
is a worthwhile goal, though).



I hate it when J. Random Developer insists in a particular line length 
and when they have their editor do the wrapping, logical entities are 
suddenly split into two lines: it is "git notes", one logical thing; not 
two words "git" "notes" that happen to occur in sequence.



-- Hannes

Re: [PATCH] C style: use standard style for "TRANSLATORS" comments

[Brandon Williams]

On 05/11, Jonathan Nieder wrote:
> Hi,
> 
> Ævar Arnfjörð Bjarmason wrote:
> 
> > Change all the "TRANSLATORS: [...]" comments in the C code to use the
> > regular Git coding style, and amend the style guide so that the
> > example there uses that style.
> 
> Hooray!
> 
> [...]
> > --- a/Documentation/CodingGuidelines
> > +++ b/Documentation/CodingGuidelines
> > @@ -256,12 +256,12 @@ For C programs:
> >  
> > Note however that a comment that explains a translatable string to
> 
> The "Note however" isn't needed since it's not contradicting
> the previous point any more.  This can be an entirely separate item:
> 
>  - A comment that explains a translatable string to translators
>uses a convention of starting with a magic token "TRANSLATORS: "
>[etc]
> 
> It might even make sense to remove the explanation of TRANSLATORS
> comments from this file altogether, since they're intuitive to use.
> A more common place to want to learn about them is po/README, which
> already explains them.
> 
> [...]
> > --- a/bisect.c
> > +++ b/bisect.c
> > @@ -995,8 +995,10 @@ int bisect_next_all(const char *prefix, int 
> > no_checkout)
> >  
> > steps_msg = xstrfmt(Q_("(roughly %d step)", "(roughly %d steps)",
> >   steps), steps);
> > -   /* TRANSLATORS: the last %s will be replaced with
> > -  "(roughly %d steps)" translation */
> > +   /*
> > +* TRANSLATORS: the last %s will be replaced with "(roughly %d
> > +* steps)" translation.
> > +*/
> 
> Nice.
> 
> [...]
> > +++ b/ref-filter.c
> > @@ -1251,13 +1251,17 @@ char *get_head_description(void)
> > state.branch);
> > else if (state.detached_from) {
> > if (state.detached_at)
> > -   /* TRANSLATORS: make sure this matches
> > -  "HEAD detached at " in wt-status.c */
> > +   /*
> > +* TRANSLATORS: make sure this matches "HEAD
> > +* detached at " in wt-status.c
> > +*/
> 
> optional: could treat "HEAD detached at " as an unbreakable phrase
> for the sake of line-breaking, for easier grepping.
> 
> But what's here is also perfectly fine.
> 
> [...]
> > -   /* TRANSLATORS: make sure this matches
> > -  "HEAD detached from " in wt-status.c */
> > +   /*
> > +* TRANSLATORS: make sure this matches "HEAD
> > +* detached from " in wt-status.c
> > +*/
> 
> Likewise.
> 
> The rest also look good. This is great.

I agree with Jonathan.  I like having everything more uniform :)

-- 
Brandon Williams

Re: [PATCH] C style: use standard style for "TRANSLATORS" comments

[Jonathan Nieder]

Hi,

Ævar Arnfjörð Bjarmason wrote:

> Change all the "TRANSLATORS: [...]" comments in the C code to use the
> regular Git coding style, and amend the style guide so that the
> example there uses that style.

Hooray!

[...]
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -256,12 +256,12 @@ For C programs:
>  
> Note however that a comment that explains a translatable string to

The "Note however" isn't needed since it's not contradicting
the previous point any more.  This can be an entirely separate item:

 - A comment that explains a translatable string to translators
   uses a convention of starting with a magic token "TRANSLATORS: "
   [etc]

It might even make sense to remove the explanation of TRANSLATORS
comments from this file altogether, since they're intuitive to use.
A more common place to want to learn about them is po/README, which
already explains them.

[...]
> --- a/bisect.c
> +++ b/bisect.c
> @@ -995,8 +995,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
>  
>   steps_msg = xstrfmt(Q_("(roughly %d step)", "(roughly %d steps)",
> steps), steps);
> - /* TRANSLATORS: the last %s will be replaced with
> -"(roughly %d steps)" translation */
> + /*
> +  * TRANSLATORS: the last %s will be replaced with "(roughly %d
> +  * steps)" translation.
> +  */

Nice.

[...]
> +++ b/ref-filter.c
> @@ -1251,13 +1251,17 @@ char *get_head_description(void)
>   state.branch);
>   else if (state.detached_from) {
>   if (state.detached_at)
> - /* TRANSLATORS: make sure this matches
> -"HEAD detached at " in wt-status.c */
> + /*
> +  * TRANSLATORS: make sure this matches "HEAD
> +  * detached at " in wt-status.c
> +  */

optional: could treat "HEAD detached at " as an unbreakable phrase
for the sake of line-breaking, for easier grepping.

But what's here is also perfectly fine.

[...]
> - /* TRANSLATORS: make sure this matches
> -"HEAD detached from " in wt-status.c */
> + /*
> +  * TRANSLATORS: make sure this matches "HEAD
> +  * detached from " in wt-status.c
> +  */

Likewise.

The rest also look good. This is great.

Thanks,
Jonathan

[PATCH] C style: use standard style for "TRANSLATORS" comments

[Ævar Arnfjörð Bjarmason]

Change all the "TRANSLATORS: [...]" comments in the C code to use the
regular Git coding style, and amend the style guide so that the
example there uses that style.

This custom style was necessary back in 2010 when the gettext support
was initially added, and was subsequently documented in commit
cbcfd4e3ea ("i18n: mention "TRANSLATORS:" marker in
Documentation/CodingGuidelines", 2014-04-18).

GNU xgettext hasn't had the parsing limitation that necessitated this
exception for almost 3 years. Since its 0.19 release on 2014-06-02
it's been able to recognize TRANSLATOR comments in the standard Git
comment syntax[1].

Usually we'd like to keep compatibility with software that's that
young, but in this case literally the only person who needs to be
using a gettext newer than 3 years old is Jiang Xin (the only person
who runs & commits "make pot" results), so I think in this case we can
make an exception.

This xgettext parsing feature was added after a thread on the Git
mailing list[2] which continued on the bug-gettext[3] list, but we
never subsequently changed our style & styleguide, do so.

There are already longstanding changes in git that use the standard
comment style & have their TRANSLATORS comments extracted properly
without getting the literal "*"'s mixed up in the text, as would
happen before xgettext 0.19.

Commit 7ff2683253 ("builtin-am: implement -i/--interactive",
2015-08-04) added one such comment, which in commit df0617bfa7 ("l10n:
git.pot: v2.6.0 round 1 (123 new, 41 removed)", 2015-09-05) got picked
up in the po/git.pot file with the right format, showing that Jiang
already runs a modern xgettext.

The xgettext parser does not handle the sort of non-standard comment
style that I'm amending here in sequencer.c, but that isn't standard
Git comment syntax anyway. With this change to sequencer.c & "make
pot" the comment in the pot file is now correct:

 #. TRANSLATORS: %s will be "revert", "cherry-pick" or
-#. * "rebase -i".
+#. "rebase -i".

1. http://git.savannah.gnu.org/cgit/gettext.git/commit/?id=10af7fe6bd
2. 
<2ce9ec406501d112e032c8208417f8100bed04c6.1397712142.git.worldhello@gmail.com>
   
(https://public-inbox.org/git/2ce9ec406501d112e032c8208417f8100bed04c6.1397712142.git.worldhello@gmail.com/)
3. https://lists.gnu.org/archive/html/bug-gettext/2014-04/msg00016.html

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

On Thu, May 11, 2017 at 10:43 PM, Brandon Williams  wrote:
> On 05/11, Ævar Arnfjörð Bjarmason wrote:
>> On Thu, May 11, 2017 at 10:21 PM, Brandon Williams  wrote:
>> > On 05/11, Ævar Arnfjörð Bjarmason wrote:
>> >> [...]
>> >> +#ifdef NO_PTHREADS
>> >> + else if (num_threads && num_threads != 1) {
>> >> + /* TRANSLATORS: %s is the configuration
>> >> +variable for tweaking threads, currently
>> >> +grep.threads */
>> >
>> > nit: this comment isn't formatted properly:
>> >   /*
>> >* ... comment ...
>> >*/
>>
>> Comments for translators use a different style, see cbcfd4e3ea ("i18n:
>> mention "TRANSLATORS:" marker in Documentation/CodingGuidelines",
>> 2014-04-18). Otherwise the "*" gets interpolated into the string the
>> translators see in their UI.
>>
>
> Ah got it, I wasn't aware of that.

As it turns out this is just something we've been cargo-culting for
years for no reason. Will fix this comment in my v2, but first let's
do this.

 Documentation/CodingGuidelines | 10 +-
 bisect.c   |  6 --
 builtin/blame.c| 15 +--
 builtin/notes.c|  6 --
 builtin/remote.c   |  7 +--
 notes-utils.c  |  7 +--
 parse-options.c|  6 --
 ref-filter.c   | 12 
 sequencer.c|  3 ++-
 9 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index a4191aa388..9fd7383819 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -256,12 +256,12 @@ For C programs:
 
Note however that a comment that explains a translatable string to
translators uses a convention of starting with a magic token
-   "TRANSLATORS: " immediately after the opening delimiter, even when
-   it spans multiple lines.  We do not add an asterisk at the beginning
-   of each line, either.  E.g.
+   "TRANSLATORS: ", e.g.
 
-   /* TRANSLATORS: here is a comment that explains the string
-  to be translated, that follows immediately after it */
+   /*
+* TRANSLATORS: here is a comment that explains the string to
+* be translated, that follows immediately after it.
+*/
_("Here is a translatable string explained by the above.");
 
  - Double negation is often harder to understand than no negation
diff --git a/bisect.c b/bisect.c
index