Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

#9341 closed defect (fixed)

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

Reported by: Remy Blank Owned by: Remy Blank
Priority: high Milestone: 0.12
Component: general Version: 0.12dev
Severity: major Keywords:
Cc: felix.schwarz@…, mikael@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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.

Attachments (2)

9341-error-reports-r9697.patch (33.6 KB ) - added by Remy Blank 12 years ago.
Don't translate error reports to Trac or plugins.
messages-fr-r9708.patch (18.4 KB ) - added by Remy Blank 12 years ago.
Current French translation against [9708].

Download all attachments as: .zip

Change History (17)

comment:1 by Christian Boos, 12 years ago

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

comment:2 by Mikael Relbe, 12 years ago

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

Last edited 12 years ago by Mikael Relbe (previous) (diff)

comment:3 by Daniel Musketa <daniel@…>, 12 years ago

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.

in reply to:  1 comment:4 by Remy Blank, 12 years ago

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.

by Remy Blank, 12 years ago

Don't translate error reports to Trac or plugins.

comment:5 by Remy Blank, 12 years ago

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.

comment:6 by Remy Blank, 12 years ago

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

  • trac/web/main.py

    diff --git a/trac/web/main.py b/trac/web/main.py
    a b  
    4848from trac.util.concurrency import threading
    4949from trac.util.datefmt import format_datetime, http_date, localtz, timezone
    5050from trac.util.text import exception_to_unicode, shorten_line, to_unicode
    51 from trac.util.translation import N_, tag_, _
     51from trac.util.translation import safefmt, tag_, _
    5252from trac.web.api import *
    5353from trac.web.chrome import Chrome
    5454from trac.web.clearsilver import HDFWrapper
     
    632632            enabled_plugins=enabled_plugins, traceback=to_unicode(traceback))
    633633
    634634    # Generate the description once in English, once in the current locale
    635     description_en = get_description(N_)
     635    description_en = get_description(lambda s, **kw: safefmt(s, kw))
    636636    try:
    637637        description = get_description(_)
    638638    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

    diff --git a/trac/util/translation.py b/trac/util/translation.py
    a b  
    3939def dgettext_noop(domain, string, **kwargs):
    4040    return gettext_noop(string, **kwargs)
    4141
    42 N_ = gettext_noop
     42N_ = _noop = lambda string: string
    4343
    4444def ngettext_noop(singular, plural, num, **kwargs):
    4545    string = (plural, singular)[num == 1]
     
    7979    _functions = {
    8080      'gettext': gettext_noop,
    8181      '_': gettext_noop,
    82       'N_': gettext_noop,
     82      'N_': _noop,
    8383      'ngettext': ngettext_noop,
    8484      'tgettext': tgettext_noop,
    8585      'tag_': tgettext_noop,
     
    277277          }
    278278        def wrapdomain(symbol):
    279279            if symbol == 'N_':
    280                 return gettext_noop
     280                return _noop
    281281            return lambda *args, **kw: _functions[symbol](domain, *args, **kw)
    282282        return [wrapdomain(s) for s in symbols]
    283283

comment:7 by Felix Schwarz, 12 years ago

Cc: felix.schwarz@… added

comment:8 by Remy Blank, 12 years ago

Cc: felix.schwarz@… removed

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

comment:9 by Remy Blank, 12 years ago

Cc: felix.schwarz@… added

Oops, sorry…

comment:10 by Christian Boos, 12 years ago

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

    diff --git a/trac/web/main.py b/trac/web/main.py
    a b Request parameters:  
    621 User agent: #USER_AGENT#
     621User agent: `#USER_AGENT#`

comment:11 by Remy Blank, 12 years ago

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?

comment:12 by Christian Boos, 12 years ago

Yes, I still have a few changes.

by Remy Blank, 12 years ago

Attachment: messages-fr-r9708.patch added

Current French translation against [9708].

comment:13 by Remy Blank, 12 years ago

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.

comment:14 by Mikael Relbe, 12 years ago

Cc: mikael@… added

comment:15 by Christian Boos, 12 years ago

Resolution: fixed
Status: newclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.