Issue 2550871
Created on 2015-02-26 16:38 by antmail, last changed 2016-06-05 10:46 by antmail.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | Remove |
ext_translation.diff | antmail, 2015-02-26 16:38 | Fix many translation issues and localize date format input. | ||
i18n_wow.diff | antmail, 2016-06-03 17:40 |
Messages | |||
---|---|---|---|
msg5228 | Author: [hidden] (antmail) | Date: 2015-02-26 16:38 | |
Roundup tracker doesn't support date input in localized notation. You can't input date such as DD.MM.YYYY. Patches do the following: i18n.py - Add class I18NStaff. Introduce I18NStaff.date_input_re variable which hold locale specific regular expression for date parsing. Introduce I18NStaff.date_input_desc variable which hold locale specific desciption of date/time format. Pattern and description can be localized through language translation file/ (rebuild roundup's language file: cd locale && gmake template after patch). In apropriate language file find: "'''^ ((?P<y>\d\d\d\d)([/-](?P<m>\d\d?)([/-](?P<d>\d\d?))?)? # yyyy[-mm[-dd]] |(?P<a>\d\d?)[/-](?P<b>\d\d?))? # or mm-dd (?P<n>\.)? # . (((?P<H>\d?\d):(?P<M>\d\d))?(:(?P<S>\d\d?(\.\d+)?))?)? # hh:mm:ss (?P<o>[\d\smywd\-+]+)? # offset $'''" and replace it to desired expression which will parse your country specific date/time format. Regular expression must contain y,m,d group for full date, a,b - for m-d variant, H,M,S for time Also replace 'Proper date/time format is "yyyy-mm-dd", "yyyy-mm-dd.HH:MM:SS.SSS"' to something which describe date/time format. - combine I18NStaff with RoundupTranslation, RoundupNullTranslation; date.py - remove date regular expression variable (date_re) (it in i18n.py now) - set date __init__ default parameter from translator=i18n to translator=i18n.translation, which is more accurate. - remove date_re parameter from Date.set() - do use localized translator.date_input_re and translator.date_input_desc in Date.set() cgi/templating.py - in DateHtmlProperty when _value is a string not converted to date, plain() and now() throw exception because of calling _value.local(). Recently somebody fix it in plain() ( except AttributeError: ...). Repeat this fix. cgi/client.py Somebody already do the same thing as i want: - hide _error_message. - define function add_error_message. So, no need to change. By the way, there is a some issue. Jinja2 template bundled with roundup1.5 think that error_msg is a string. But it is a list. So, _error_msg in page has a bad format. When errors are localized they become unreadable. cgi/form_parser.py This is a transit point on the way to hyperdb. The goal is to pass translator object to rawToHyperDB method. So - introduce 'translator' variable, which is initialized from 'client' object; - define wrapper function rawToHyperDB which respect translator object; - replace direct call hyperdb.rawToHyperDB to wrapper. hyperdb.py - import translation from i18n for default translator value; - add parameter 'translator' to rawToHyperdb function and pass it to from_raw() method; - pass translator to Date object. |
|||
msg5249 | Author: [hidden] (jerrykan) | Date: 2015-03-03 13:16 | |
For reference it looks like some some initials discussion around this (or a very similar) patch can be found at issue2550825. I'm not very familiar with the i18n stuff, but I'll try to have a look at this patch in this patch in the next week or so to at least rovide some feedback. Feel free to bump the issue if I forget. |
|||
msg5333 | Author: [hidden] (rouilj) | Date: 2015-06-28 04:05 | |
John/jerrykahn bump. Have you had a chance to look at this? issue2550888 may need some feedback on this. |
|||
msg5346 | Author: [hidden] (jerrykan) | Date: 2015-07-23 04:06 | |
Hello John, I started to have a bit of a look at this patch but ended up spending the time trying to figure out how to make roundup display in another language (as a native English speaker I've not had to deal with i18n stuff much). By the time I got the i18n stuff working I ran out of time and haven't had a chance to get back to looking at it. One thing that may help in reviewing that patch is if Anthony could outline some steps to show what this patch does. ie. a set of steps to show what the current behaviour is, and another set of steps showing the difference in behaviour after the patch is applied. Then I may have a bit more of an idea about what needs to be checked. The changes hit a lot of different files, which raises concerns about if this is the best approach (I'm not saying it is a wrong approach, just that it needs looking at with an eye to future maintainability). At a quick glance there are also a number of unrelated changes (ie. random white-space fixes) in the patch which also raises flags that the patch may include unrelated changes and the patch will need to be cleaned up before I could be applied[1]. SeeYa, John. [1] I am of the opinion that white-space changes should only be made when code bordering that white-space is being modified. Otherwise it has a tendency to introduce bugs that are totally unrelated to intended changes being made and could have been avoided. On 28/06/15 14:05, John Rouillard wrote: > > John Rouillard added the comment: > > John/jerrykahn bump. Have you had a chance to look at this? > > issue2550888 > > may need some feedback on this. > > ---------- > nosy: +rouilj > > ________________________________________________ > Roundup tracker <issues@roundup-tracker.org> > <http://issues.roundup-tracker.org/issue2550871> > ________________________________________________ > |
|||
msg5552 | Author: [hidden] (rouilj) | Date: 2016-04-30 03:16 | |
Anthony, Can you address the concerns Jerry brought up and redo the patch removing unneeded whitespace changes and merge the patch from issue2550887 as well as provide some steps we can use to compare before and after behavior of the patch. -- rouilj |
|||
msg5573 | Author: [hidden] (antmail) | Date: 2016-06-03 17:25 | |
Yes, this patches hit many files but mostly to fix translation handling. There are single behavior change: user can set date field value in native format such as dd.mm.yyyy. From system administrator/developer/translator point of view also there is a single change: regular expression to parse date value may be localized in language-translation file. To get it you need: 1) apply patch; 2) regenerate roundup.pot file In 'locale' dir of roundup source: find ../roundup/ -path "../roundup/scripts/" -prune -o -iname "*.py" > rs.files xgettext -w 80 -o roundup.pot -f rs.files rm rs.files 3) merge new roundup.pot with target language .po file msgmerge -U --suffix=.bak target_language.po roundup.pot 4) In target language .po file find the msgid and add the translation: #: ../roundup/i18n.py:90 msgid "" "^\n" " ((?P<y>\\d\\d\\d\\d)([/-](?P<m>\\d\\d?)([/-](?P<d>\\d\\d?))?)? # " "yyyy[-mm[-dd]]\n" " |(?P<a>\\d\\d?)[/-](?P<b>\\d\\d?))? # or mm-dd\n" " (?P<n>\\.)? # .\n" " (((?P<H>\\d?\\d):(?P<M>\\d\\d))?(:(?P<S>\\d\\d?(\\.\\d+)?))?)? " "# hh:mm:ss\n" " (?P<o>[\\d\\smywd\\-+]+)? # offset\n" " $" msgstr "" "^\\s*\n" "(((?P<d>\\d\\d?)\\.)?((?P<m>\\d\\d?)\\.)(?P<y>\\d\\d\\d\\d) # [dd.]" "[mm.]yyyy \n" "|(?P<b>\\d\\d)\\.(?P<a>\\d\\d))? #or d.m\n" "(?P<n>\\.)? # may be . (now)\n" "(\\s+((?P<H>\\d\\d?):(?P<M>\\d\\d))?(:(?P<S>\\d\\d?(\\.\\d+)?))?)? #hh:mm:" "ss\n" "(?P<o>[\\d\\smywd\\-+]+)? # offset\n" "\\s*$" !!! You can skip 2),3) and just add to target language .po file lines from 4) !!! 5) Compile target language .po file: msgfmt --statistics -o target_language.mo target_language.po 6) Place target_language.mo file in roundup tracker subdir 'locale' 7) In config.ini of roundup tracker set 'language' to target_language such as: language = target_language.UTF8 8) restart roundup-tracker Now you can input date as dd.mm.yyyy, mm.yyyy etc. > John Rouillard added the comment: > Anthony, > Can you address the concerns Jerry brought up and > redo the patch removing unneeded whitespace changes > and merge the patch from issue2550887 > as well as provide some steps we can use to compare before and after > behavior of the patch. > -- rouilj > ________________________________________________ > Roundup tracker <issues@roundup-tracker.org> > <http://issues.roundup-tracker.org/issue2550871> > ________________________________________________ |
|||
msg5574 | Author: [hidden] (antmail) | Date: 2016-06-03 17:40 | |
The patch with no whitespace problems (i hope) attached here. Also, the code base of date.py has changed after this patch was written so i did some merging. Unfortunately I don't know what to get from Mercurial VCS to help other people to apply this patch. > John Rouillard added the comment: > Anthony, > Can you address the concerns Jerry brought up and > redo the patch removing unneeded whitespace changes > and merge the patch from issue2550887 > as well as provide some steps we can use to compare before and after > behavior of the patch. > -- rouilj > ________________________________________________ > Roundup tracker <issues@roundup-tracker.org> > <http://issues.roundup-tracker.org/issue2550871> > ________________________________________________ |
|||
msg5578 | Author: [hidden] (rouilj) | Date: 2016-06-05 05:28 | |
Hi Anthony: Anthony said: > The patch with no whitespace problems (i hope) attached here. It looks cleaner than the original. Did you merge the patch from issue2550887 into it? If so I will close issue2550887. > Also, the code base of date.py has changed after this patch > was written so i did some merging. Unfortunately I don't > know what to get from Mercurial VCS to help other people to > apply this patch. I don't understand what you mean here. Are you saying this patch will not apply to the current head of the default branch? John K. any ideas? I do have a couple of questions about the patch itself that I don't understand. On part of the patch has this: if allowdate and info['D'] is not None: - now = Date('.') + now = Date() why was this change needed? Also I see: - raise HyperdbValueError, _('property %s: %r is not a number')%( + raise HyperdbValueError, translator.gettext('property %s: %r is not a number')%( kw['propname'], value) in a number of places. What does it do? I thought _('....') is replaced by the localized version of .... and the items in the tuple are just substituted into the localized string. IIUC that is exactly the same thing that translator.gettext('...')%(tuple) will do. Translator.gettext won't translate any values in the tuple. So what does this change accomplish? Also will I need another patch for the Integer patch I just committed (reversing what I took out in issue2550886). The patch will add the translator: - def from_raw(self, value, **kw): + def from_raw(self, value, translator=translation, **kw): to the Integer::from_raw function and insert translator.gettext to replace _ . -- rouilj |
|||
msg5580 | Author: [hidden] (antmail) | Date: 2016-06-05 10:46 | |
> Hi Anthony: > Anthony said: >> The patch with no whitespace problems (i hope) attached here. > It looks cleaner than the original. Did you merge the patch from > issue2550887 into it? If so I will close issue2550887. Unfortunately, i don't know how to combine patch. May be the best way is to attach patch from issue2550887 to issue2550871 and then close issue2550887? >> Also, the code base of date.py has changed after this patch >> was written so i did some merging. Unfortunately I don't >> know what to get from Mercurial VCS to help other people to >> apply this patch. > I don't understand what you mean here. Are you saying this patch > will not apply to the current head of the default branch? Yes, when I pulled changes there is a need of manual merging in date.py. > I do have a couple of questions about the patch itself that I don't > understand. > On part of the patch has this: > if allowdate and info['D'] is not None: > - now = Date('.') > + now = Date() > why was this change needed? Sorry, I don't remember. I can only say that this is not crucial for patch functionality. Probably, i was touching something there but then not reverted it back fully. The choice of Date with period argument and Date alone depends of what seems more cleanly, of course. > Also I see: > - raise HyperdbValueError, _('property %s: %r is not a number')%( > + raise HyperdbValueError, translator.gettext('property %s: > %r is not a number')%( > kw['propname'], value) > in a number of places. What does it do? I thought _('....') is > replaced by the localized version of .... and the items in the tuple > are just substituted into the localized string. IIUC that is exactly > the same thing that > translator.gettext('...')%(tuple) > will do. Translator.gettext won't translate any values in the tuple. > So what does this change accomplish? A I remember this is the main part of the patch. The main problem is that localized translator object from context of request is lost as they move in depth of roundup code. All translation in base models (like date.py) are tied to _ variable which is defaulted to no translation. So the main difference of - raise HyperdbValueError, _('property %s: %r is not a number')%( + raise HyperdbValueError, translator.gettext('property %s: is to use translator.gettext() istead of _(). The patch is to carefully lower the translator object to the depth of code and then use it. > Also will I need another patch for the Integer patch I just > committed (reversing what I took out in issue2550886). > The patch will add the translator: > - def from_raw(self, value, **kw): > + def from_raw(self, value, translator=translation, **kw): > to the Integer::from_raw function and insert translator.gettext to > replace _ . There is no problem. Just the running tests will not output translated errors. > ________________________________________________ > Roundup tracker <issues@roundup-tracker.org> > <http://issues.roundup-tracker.org/issue2550871> > ________________________________________________ |
History | |||
---|---|---|---|
Date | User | Action | Args |
2016-06-05 10:46:06 | antmail | set | messages: + msg5580 |
2016-06-05 05:29:27 | rouilj | set | priority: normal dependencies: + Introduce an Integer class, Small fix to date format() function |
2016-06-05 05:28:03 | rouilj | set | messages: + msg5578 |
2016-06-03 17:40:31 | antmail | set | files:
+ i18n_wow.diff messages: + msg5574 |
2016-06-03 17:25:04 | antmail | set | messages: + msg5573 |
2016-04-30 03:16:15 | rouilj | set | messages: + msg5552 |
2015-07-23 04:06:02 | jerrykan | set | messages: + msg5346 |
2015-07-01 19:03:07 | jerrykan | link | issue2550888 dependencies |
2015-07-01 19:01:35 | jerrykan | link | issue2550887 dependencies |
2015-06-28 04:05:38 | rouilj | set | nosy:
+ rouilj messages: + msg5333 |
2015-03-03 13:16:42 | jerrykan | set | messages: + msg5249 |
2015-03-03 13:08:25 | jerrykan | set | nosy: + jerrykan |
2015-02-26 16:38:18 | antmail | create |