Don't translate semi-automatic error report description to t.e.o

[Remy Blank]

The description for semi-automatic error reports is currently translated in the user's locale. This is probably ok for reports to a local Trac instance, but is a disadvantage when reporting to t.e.o. See for example #9333, #9316.

Ideally, the description should be translated when reporting locally and in English when reporting to t.e.o.

I'd like to get this done before the string freeze tonight.

[Christian Boos]

Have a look at the deactivate/reactivate pair (though there's probably no need to reactivate afterwards).

[Mikael Relbe]

May I suggest that error reports are generated on both English and selected locale?

[Daniel Musketa <daniel@…>]

How about having the locale used for (semi) automatic error reports defined in the local trac.ini? It would be independant from the actual user's setting.

[Remy Blank]

Replying to cboos:

Have a look at the deactivate/reactivate pair (though there's probably no need to reactivate afterwards).

I actually didn't want to change the locale, just use either _() or N_() on the description text, and pass the result to the template. But with the new requests, this is not doable anymore.

I'm not sure it makes sense to report in both English and the selected locale. Keep in mind that we're only talking about the description of the newly-created ticket.

About selecting the locale for error reports, on the Trac site (and trac-hacks), it should always be English. For reports to a local Trac, it would make sense to allow setting a fixed locale, probably the language used by the administrator.

[Remy Blank]

Don't translate error reports to Trac or plugins.

[Remy Blank]

The patch above makes semi-automatic error reports in the current locale to the admin Trac (if defined), and always in English to t.e.o or the trackers defined for plugins. It also fixes the insertion of the user agent and jQuery version into the error report when it is translated, which is currently broken.

As the change is quite substantial, I'd be grateful if I could get a review of the patch before applying it (and an exception to tonight's string freeze). The patch includes an updated message catalog and a complete French translation.

Allowing to define an "error report locale" would make the patch even more complicated, so I'd rather not make that change now.

[Remy Blank]

Actually, I shouldn't have used N_(), but rather a direct call to safefmt():

trac/web/main.py

@@ -48 +48 @@
 from trac.util.concurrency import threading
 from trac.util.datefmt import format_datetime, http_date, localtz, timezone
 from trac.util.text import exception_to_unicode, shorten_line, to_unicode
-from trac.util.translation import N_, tag_, _
+from trac.util.translation import safefmt, tag_, _
 from trac.web.api import *
 from trac.web.chrome import Chrome
 from trac.web.clearsilver import HDFWrapper
@@ -632 +632 @@
             enabled_plugins=enabled_plugins, traceback=to_unicode(traceback))
 
     # Generate the description once in English, once in the current locale
-    description_en = get_description(N_)
+    description_en = get_description(lambda s, **kw: safefmt(s, kw))
     try:
         description = get_description(_)
     except Exception:

Come to think of it, I believe that the current implementation of N_() is actually wrong, as it shouldn't allow keyword arguments, only a single string literal, and return it unchanged. The keyword arguments should be passed to the subsequent call to gettext(). Something like:

trac/util/translation.py

@@ -39 +39 @@
 def dgettext_noop(domain, string, **kwargs):
     return gettext_noop(string, **kwargs)
 
-N_ = gettext_noop
+N_ = _noop = lambda string: string
 
 def ngettext_noop(singular, plural, num, **kwargs):
     string = (plural, singular)[num == 1]
@@ -79 +79 @@
     _functions = {
       'gettext': gettext_noop,
       '_': gettext_noop,
-      'N_': gettext_noop,
+      'N_': _noop,
       'ngettext': ngettext_noop,
       'tgettext': tgettext_noop,
       'tag_': tgettext_noop,
@@ -277 +277 @@
           }
         def wrapdomain(symbol):
             if symbol == 'N_':
-                return gettext_noop
+                return _noop
             return lambda *args, **kw: _functions[symbol](domain, *args, **kw)
         return [wrapdomain(s) for s in symbols]
 

[Remy Blank]

Made N_() a real no-op in [9705].

[Remy Blank]

Oops, sorry…

[Christian Boos]

Tested the patch with the comment:6 changes. It works well, please commit.

Extra tweak to avoid wikification of things like AppleWebKit in the user agent string:

trac/web/main.py

@@ -621 @@
-User agent: #USER_AGENT#
+User agent: `#USER_AGENT#`

[Remy Blank]

Thanks for the review. Patch applied in [9708].

I have completed the French translation. Is this the last string change, or are there any others (from #7687, maybe)? Should I wait with the final message extraction?

[Christian Boos]

Yes, I still have a few changes.

[Remy Blank]

Current French translation against [9708].

[Remy Blank]

As I'll be out on a business trip until Wednesday evening, I have attached my current French translation. Feel free to apply it if you find it useful.

[Christian Boos]

attachment:messages-fr-r9708.patch​ applied in r9734.