Skip to content

placeholder_delimiter config is ignored #29

@mbideau

Description

@mbideau

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 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 "<stdin>", line 1, in <module>
  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 😅

So I looked at the source code, 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

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 as the moment of its definition (never get the chance to call i18n.set() before definition) :

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), but it is impossible since you use that class hardcoded.

In @danhper code for i18n/translator.py:

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:

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

settings = {
  ...
  'class': TranslationFormatter,
  ...

In @danhper code for i18n/translator.py:

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:

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 :

>>> 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions