Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3190 closed defect (fixed)

bad string concatenation for translators

Reported by: dsd Owned by: humitos
Priority: Unspecified by Maintainer Milestone: Unspecified
Component: Finance Version: Unspecified
Severity: Unspecified Keywords: easy-hack, patch
Cc: cjl, humitos, dsd Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

finance.py:update_toolbar() creates the "This month" label for a toolbar button based on concatenation of two translated strings.

This won't work for languages that put the words in a different order, and also it's already broken which has "Esto Mes" (this month) and "Esto Semana" (this week) - both are wrong.

"Esto" only means "this" in a neutral context where there is no noun (which has a gender). When there is a noun, as there is here, the translation of "this" varies. In this case the correct translations would be "Esta semana" and "Este mes". It isn't possible to produce these correct translations with the current scheme.

Attachments (2)

0001-Translate-period-string-properly-SL-3190.patch (2.1 KB) - added by humitos 8 years ago.
new version - v2
0002-Ability-to-translate-header-period-SL-3190.patch (1.9 KB) - added by humitos 8 years ago.
new version - v2

Download all attachments as: .zip

Change History (11)

comment:1 Changed 8 years ago by cjl

  • Cc cjl added

Closing #3211 dupe

"Next Year" must be "Año siguiente", not "Siguiente Año". The same for "Previous Year", should be "Año anterior", not "Anterior Año". And also "This Year" should be "Este año", not "Esto Año".

This is an internationalization error in the activity. It assumes an English-like syntax of adjective (previous/this/next) + self.period (day, week, year, forever)

Lines 454-457 of finance.py

# Update the label self.period to reflect the period.
self.prevperiodbtn.set_tooltip(_('Previous') + ' ' + self.period)
self.thisperiodbtn.set_tooltip(_('This') + ' ' + self.period)
self.nextperiodbtn.set_tooltip(_('Next') + ' ' + self.period)

Lines 388-402 of finance.py

def update_header(self):

if self.period == _('Day'):

text = self.period_start.strftime("%B %d, %Y")

elif self.period == _('Week'):

text = _('Week of') + self.period_start.strftime(" %B %d, %Y")

elif self.period == _('Month'):

text = self.period_start.strftime("%B, %Y")

elif self.period == _('Year'):

text = self.period_start.strftime("%Y")

elif self.period == _('Forever'):

text = _('Forever')

There seems to be a similar erroneous compositing/concatenation of strings with fixed syntax at Lines 227-234 of finance.py

Proper internationalization practice would not use concatenated strings. It is preferred to spell out all of the permutations to allow the localizers to correctly represent the output string in their native language's syntactic structure, even if this means more strings to work on.

comment:2 Changed 8 years ago by godiard

  • Keywords easy-hack added

comment:3 Changed 8 years ago by humitos

  • Cc humitos added
  • Keywords patch added

comment:4 Changed 8 years ago by godiard

  • Owner changed from godiard to humitos
  • Status changed from new to assigned

humitos, can you test this with the translation before we push it?

comment:5 Changed 8 years ago by humitos

This patch won't work because I'm comparing self.period == 'month' and self.period is a translated string, so it could be "mes" for example or in another language. I should think about another way to compare this.

comment:6 Changed 8 years ago by humitos

I've just sent a new patch to the mailing list that fixes this problem.

Changed 8 years ago by humitos

new version - v2

Changed 8 years ago by humitos

new version - v2

comment:7 Changed 8 years ago by godiard

  • Resolution set to fixed
  • Status changed from assigned to closed

Pushed as efe760da6d87c421ce0f958449463f46317305ff and 5a58b3436ae65c197c540f96ceed735539664b8a

comment:8 Changed 8 years ago by godiard

  • Cc dsd added

Version 8 have this ticket resolved, please add to 12.1.0

comment:9 Changed 8 years ago by dsd

This patch changes strings and it looks like Spanish and English are the only languages that have been updated. Other languages will have regressed.

If you still want this update, please put out a request on the localisation list so that we can give people a chance to adapt to this. After a week or so you can produce a new release with the changes and that way we'll have given fair notice to everyone.

Note: See TracTickets for help on using tickets.