Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix formatting and translation for other languages in rule_to_text #186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix formatting and translation for other languages in rule_to_text #186

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions recurrence/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1188,10 +1188,8 @@ def get_positional_weekdays(rule):

if rule.interval > 1:
parts.append(
_('every %(number)s %(freq)s') % {
'number': rule.interval,
'freq': timeintervals[rule.freq]
})
_(f'every {rule.interval} {timeintervals[rule.freq]}')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the order between "translation" and "interpolation".

For instance, if we look at the spanish translations: The old code first looks up the translation of 'every %(number)s %(freq)s', which is 'cada %(number)s %(freq)s', and then fills in the value of number and freq in the string.

The new code will first fill in the values, and then try to look up the string in the translation catalog. For instance, if number is 5 and freq is "years" (which will be translated to "años"), it will look in the translation catalog for "every 5 años", which will not exist.

These same remarks apply to all the other changes in this pull request.

While the change could be made from %-based formatting to .format()-based formatting, f-strings are not capable of dealing with the case of translations.

Also, I can not see what problem this pull request is trying to solve. Maybe that could be better described?

)
else:
parts.append(frequencies[rule.freq])

Expand All @@ -1202,31 +1200,31 @@ def get_positional_weekdays(rule):
items = _(', ').join(
[months_display[month] for month in
[month_index - 1 for month_index in rule.bymonth]])
parts.append(_('each %(items)s') % {'items': items})
parts.append(_(f'each {items}'))
if rule.byday or rule.bysetpos:
parts.append(
_('on the %(items)s') % {
'items': get_positional_weekdays(rule)})
_(f'on the {get_positional_weekdays(rule)}')
)

if rule.freq == MONTHLY:
if rule.bymonthday:
items = _(', ').join([
dateformat.format(datetime.datetime(1, 1, day), 'jS') if day > 0
else last_of_month_display.get(day, day)
for day in rule.bymonthday])
parts.append(_('on the %(items)s') % {'items': items})
parts.append(_(f'on the {items}'))
elif rule.byday:
if rule.byday or rule.bysetpos:
parts.append(
_('on the %(items)s') % {
'items': get_positional_weekdays(rule)})
_(f'on the {get_positional_weekdays(rule)}')
)

if rule.freq == WEEKLY:
if rule.byday:
items = _(', ').join([
weekdays_display[to_weekday(day).number]
for day in rule.byday])
parts.append(_('each %(items)s') % {'items': items})
parts.append(_(f'each {items}'))

# daily freqencies has no additional formatting,
# hour/minute/second formatting not supported
Expand All @@ -1235,11 +1233,9 @@ def get_positional_weekdays(rule):
if rule.count == 1:
parts.append(_('occuring once'))
else:
parts.append(_('occuring %(number)s times') % {
'number': rule.count})
parts.append(_(f'occuring {rule.count} times'))
elif rule.until:
parts.append(_('until %(date)s') % {
'date': dateformat.format(rule.until, 'Y-m-d')})
parts.append(_(f"until {dateformat.format(rule.until, 'Y-m-d')}"))

return _(', ').join(parts)

Expand Down