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.

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

Conversation

Ibrohimbek
Copy link

'number': rule.interval,
'freq': timeintervals[rule.freq]
})
_(f'every {rule.interval} {timeintervals[rule.freq]}')

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants