placeholder_delimiter config is ignored
- Dominant language
- Python
- Stars
- 246
- Forks
- 46
- PR merge metrics
- No merged PRs in 30d
Description
Hi @danhper and @ZRunner,
Thank you both (and other contributors) for this great piece of software.
I am facing an issue when attempting to change the placeholder delimiter (because I have data containing the default placeholder delimiter `%`)
```python
Python 3.7.3 (default, Jan 22 2021, 20:04:44)
[GCC 8.3.0] on linux
>>> import i18n
>>> i18n.set('error_on_missing_placeholder', True)
>>> i18n.set('placeholder_delimiter', '#')
>>> i18n.add_translation('hash', 'Hash: 10 # !')
>>> i18n.t('hash') # should raise the exception because it is using an invalid placeholder definition
'Hash: 10 # !'
>>> i18n.add_translation('perct', 'Percent: 10 % !')
>>> i18n.t('perct') # this one should not since I have changed the default delimiter
'Percent: 10 % !'
>>> i18n.t('perct')
Traceback (most recent call last):
File "", line 1, in
File "/opt/collectd-reporting/lib/python3.7/site-packages/i18n/translator.py", line 24, in t
return translate(key, locale=locale, **kwargs)
File "/opt/collectd-reporting/lib/python3.7/site-packages/i18n/translator.py", line 44, in translate
return TranslationFormatter(translation).format(**kwargs)
File "/opt/collectd-reporting/lib/python3.7/site-packages/i18n/translator.py", line 16, in format
return self.substitute(**kwargs)
File "/usr/lib/python3.7/string.py", line 132, in substitute
return self.pattern.sub(convert, self.template)
File "/usr/lib/python3.7/string.py", line 129, in convert
self._invalid(mo)
File "/usr/lib/python3.7/string.py", line 105, in _invalid
(lineno, colno))
ValueError: Invalid placeholder in string: line 1, col 13
```
First I looked at the units tests, but none where testing that config feature :sweat_smile:
So I looked at the [source code](https://github.com/danhper/python-i18n/blob/master/i18n/translator.py#L8), and see that it uses the Template feature of the string module.
And the documentation state :
> Note further that you cannot change the delimiter after class creation (i.e. a different delimiter must be set in the subclass’s class namespace).
Which is confirmed by the use of `__init_subclass__(cls)` in the Template [source code](https://github.com/python/cpython/blob/3.10/Lib/string.py#L69)
To my comprehension, because your module defines the class itself, despite it contains the following code, it will always get the value of the [default delimiter](https://github.com/danhper/python-i18n/blob/master/i18n/config.py#L20) as the moment of its definition (never get the chance to call `i18n.set()` before definition) :
```python
class TranslationFormatter(Template):
delimiter = config.get('placeholder_delimiter')
```
So I said, OK, I am just going to extend it with another class setting up my own delimiter (like [this Template example](https://stackoverflow.com/a/24363424)), but it is impossible since you use that class hardcoded.
In @danhper code for _i18n/translator.py_:
```python
def translate(key, **kwargs):
locale = kwargs.pop('locale', config.get('locale'))
translation = translations.get(key, locale=locale)
if 'count' in kwargs:
translation = pluralize(key, translation, kwargs['count'])
return TranslationFormatter(translation).format(**kwargs)
```
In @ZRunner code for _i18n/translator.py_:
```python
def translate(key: str, **kwargs) -> Union[List[Union[str, Any]], str]:
"""Actually translate something and apply plurals/kwargs if needed
If the translation is a list of strings, each string will be formatted accordingly and the whole list will be returned"""
locale = kwargs.pop('locale', config.get('locale'))
translation = translations.get(key, locale=locale)
if isinstance(translation, list):
# if we can apply plural/formats to the items, let's try
if all(isinstance(data, (str, dict)) for data in translation):
# if needed, we should format every item in the list
if 'count' in kwargs:
translation = [pluralize(key, data, kwargs['count']) for data in translation]
# items may be non-plural dictionnaries, which we cannot format
return [TranslationFormatter(data).format(**kwargs) if isinstance(data, str) else data for data in translation]
return translation
if 'count' in kwargs:
translation = pluralize(key, translation, kwargs['count'])
return TranslationFormatter(translation).format(**kwargs)
```
So I recommend, as a solution, to allow in the config to specify a class, to be used by the translate() function.
In _i18n/config.py_
```python
settings = {
...
'class': TranslationFormatter,
...
```
In @danhper code for _i18n/translator.py_:
```python
def translate(key, **kwargs):
locale = kwargs.pop('locale', config.get('locale'))
translation = translations.get(key, locale=locale)
class_ref = config.get('class')
if 'count' in kwargs:
translation = pluralize(key, translation, kwargs['count'])
return class_ref(translation).format(**kwargs)
```
And in In @ZRunner code for _i18n/translator.py_:
```python
def translate(key: str, **kwargs) -> Union[List[Union[str, Any]], str]:
"""Actually translate something and apply plurals/kwargs if needed
If the translation is a list of strings, each string will be formatted accordingly and the whole list will be returned"""
locale = kwargs.pop('locale', config.get('locale'))
translation = translations.get(key, locale=locale)
class_ref = config.get('class')
if isinstance(translation, list):
# if we can apply plural/formats to the items, let's try
if all(isinstance(data, (str, dict)) for data in translation):
# if needed, we should format every item in the list
if 'count' in kwargs:
translation = [pluralize(key, data, kwargs['count']) for data in translation]
# items may be non-plural dictionnaries, which we cannot format
return [class_ref(data).format(**kwargs) if isinstance(data, str) else data for data in translation]
return translation
if 'count' in kwargs:
translation = pluralize(key, translation, kwargs['count'])
return class_ref(translation).format(**kwargs)
```
this way I would be able to use it like this :
```python
>>> import i18n
>>> class MyDelimiter(i18n.translator.TranslationFormatter):
>>> delimiter = '#'
>>> i18n.set('error_on_missing_placeholder', True)
>>> i18n.set('class', MyDelimiter)
>>> i18n.add_translation('hash', 'Hash: 10 #{value} !')
>>> i18n.t('hash', value='€')
'Hash: 10 € !'
```
Maybe that would be a little less efficient in terms of perfomance, but that would be way more flexible.
What do you think ?
Best regards.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.