Support for other TranslationStrings instances in tokens

[KirillMysnik]

Also removed LangStrings.get_strings method that modified instances in-place and found its use literally nowhere.

Example

../resource/source-python/translations/new_translations_test.ini

[magic_string]
en="{full_name} is the new president"

[sub 1]
en="{first_name} {last_name}"

[sub 2]
en="Hillary"

[sub 3]
en="Clinton"

../addons/source-python/plugins/new_translations_test.py

from translations.strings import LangStrings


strings = LangStrings("new_translations_test")


tokenized_string = strings['magic_string'].tokenized(
    full_name=strings['sub 1'].tokenized(
        first_name=strings['sub 2'],
        last_name=strings['sub 3']
    )
)

print(tokenized_string.get_string(first_name="Donald", last_name="Trump"))

Result:
Donald Trump is the new president

For reference

http://forums.sourcepython.com/viewtopic.php?f=8&t=1043
http://forums.sourcepython.com/viewtopic.php?f=38&t=1053

[KirillMysnik]

@Ayuto @invincibleqc @satoon101 What do you guys think? I suspect you might have not been notified of this one.

[Ayuto]

Looks good, but please take a look at my comments before merging.

[Ayuto]

This could just be result.update(self). But we might even want to create a deep copy.

[KirillMysnik]

Yes, you're right. This must be a leftover from something bigger, but now it can indeed be written as result.update(self).

[Ayuto]

I think we should create a helper method for this duplicate. Maybe something like this:

        # Code...

        # Expose all TranslationStrings instances in self.tokens
        exposed_tokens = {}

        self._update_exposed_tokens(exposed_tokens, language, self.tokens, **tokens)
        self._update_exposed_tokens(exposed_tokens, language, tokens)

        return self[language].format(**exposed_tokens)

    @staticmethod
    def _update_exposed_tokens(exposed_tokens, language, tokens, **kwargs):
        for token_name, token in tokens.items():
            if isinstance(token, TranslationStrings):
                # Pass additional tokens - these will be used to format
                # the string
                token = token.get_string(language, **kwargs)

            exposed_tokens[token_name] = token

[KirillMysnik]

I'm not sure about this one... Is there a real need to do so? It's impossible to put everything this method does into its name, so anybody reading the code will need to look into _update_exposed_tokens anyways.

[Ayuto]

There is no need to do that, but it removes duplicated code. And in my opinion it's easier to read the code, because it's easier to see the difference between both calls.

[Ayuto]

This method was added because of PR #43.

[KirillMysnik]

Okay, now I see where it comes from. But in my opinion, we shouldn't tokenize the original TranslationStrings instances that are stored in LangStrings. How about this:

def get_strings(self, key, **tokens):
    """Return a new TranslationStrings object with updated tokens."""
    strings = TranslationStrings()
    strings.update(self[key])
    strings.tokens.update(tokens)
    return strings

[Ayuto]

Well, if you want to prevent in-place updates, you would change the __getitem__ method. But actually, that doesn't really matter, because you always pass all tokens, right?

[KirillMysnik]

Yes, everyone should probably pass the complete dict of tokens, but with current implementation of get_strings those tokens will get stuck in LangStrings for absolutely no reason.
Regarding to __getitem__, if somebody gets the empty TranslationStrings instance that is stored in LangStrings and does something filthy with its tokens, it's their problem. Creating a new TranslationStrings instance in __getitem__ and then another one while tokenizing it? That's too much of instantiating, I say. Would be nice to hear opinions from other contributors, too. @Mahi, I believe this method originates from your PR?

I also don't see any reason for keeping LangStrings.get_strings now when we have TranslationStrings.tokenized method.