PocketByte / LocoLaser

Localization tool to import localized strings from external source to your project.
Apache License 2.0
31 stars 1 forks source link

String substitution from variables #11

Closed avently closed 3 years ago

avently commented 4 years ago

Hello. I found this library very useful and two improvements may be made for better usability. I hope we can find a solutiion for them:

Let's focus here on a first issue from the list. As an example of what I mean by string substitution take a look at the following example: my src/commonMain/resources/com.me.appname/server.properties file that I use in a common project (other locales have another name like server_ru.properties):

strategy.price.dropped={symbol} Price was dropped from {from.price} to {to.price}
strategy.order.place.success=The order is placed: {order} | {instrument}

Here we can see:

In code I read the file line by line (actually a value of any property can have multiple lines ended with \ just like terminals may do in shell scripts) but I don't think it's important for Locolaser since all properties can be written in one line. This is how I use the properties in code:

"strategy.price.dropped".l(instrument.symbol, droppedFrom, instrument.lastPrice)

fun String.l(vararg args: Any = arrayOf<String>()): String {
// finds `this` inside map for active language
// on every argument the app calls toString()
// replaces variables with args
}

The String extension l() gets a property (as this inside the method), replaces all placeholders aka variables with the values provided in the args array, and returns the result.

So, what's next. I think LocoLaser can support named arguments inside generated kotlin files, so it can work like this:

This way I can be sure that I provided all arguments for a translation and a correct order of arguments without looking into translation files.

What do you think about this proposal?

KamiSempai commented 4 years ago

I like the idea, but I also see one problem. Not all platforms support strings substitution, and no one from current list of supported platforms. It can lead to mixed results depends on set of used resources.

For example, I can implement this feature for Google Sheets, so names of parameters will be provided by Google Sheets. But then, if you would to generate Kotlin classes offline, you can face with different result if your local files doesn't support parameter names.

Only one solution that I see is to use parameter names like: string1, string2, etc. Real parameter names can be placed in comments.

KamiSempai commented 4 years ago

Another one solution is to generate extension functions for platforms that supports this functionality. But this functions can't be used in common code. Which is make them a little bit useless in multi platform projects.

avently commented 4 years ago

There is no need of platfrom support. There is no need to do something wih sheets variables. The only thing we need is to replace something that looks like variable with provided arguments. Another example which shows the idea.

Let's say you have a string in sheets:

strategy.price.dropped={symbol} Price was dropped from {from.price} to {to.price}

then you have generated a function:

fun strategyPriceDropped(symbol: String, fromPrice: String, toPrice: String): String =
    "{symbol} Price was dropped from {from.price} to {to.price}".localize(symbol, fromPrice, toPrice)

Function which do the job by replacing all variables (=placeholders for parameters) with actual data of parameters:

fun String.localize(vararg args: Any = arrayOf<String>()): String {
    val str = this
    var result = ""
    var skipping = false
    var index = -1
    for (char in str) {
        if (char == '{') {
            index++
            if (index >= args.size) throw IllegalArgumentException("Not enough arguments to format a string: $str")
            skipping = true
            result += args[index].toString()
        }
        if (!skipping) result += char

        if (char == '}') skipping = false
    }
    if (index != args.size - 1) throw IllegalArgumentException("Too many arguments to format a string: $str")
    if (skipping) throw IllegalArgumentException("Expected '}' while parsing string: $str")
    return result
}

Is this demo code described the idea better?

avently commented 4 years ago

Of course an actual implementation on the library's side should be more complex. Added a way to specify a custom locale:

    fun justFun() {
        val resultWithDefaultLocale = strategyPriceDropped("XBTUSD", "10000", "9999")

        val resultWithCustomLocale = strategyPriceDropped("XBTUSD", "10000", "9999", "ru")
    }

    fun strategyPriceDropped(symbol: String, fromPrice: String, toPrice: String, locale: String = ""): String =
            localize("strategy.price.dropped", locale, symbol, fromPrice, toPrice)

    private fun localize(key: String, locale: String, vararg args: Any = arrayOf<String>()): String {
        val str = localizedStringByKey(key, locale)
        var result = ""
        var skipping = false
        var index = -1
        for (char in str) {
            if (char == '{') {
                index++
                if (index >= args.size) throw IllegalArgumentException("Not enough arguments to format a string: $str")
                skipping = true
                result += args[index].toString()
            }
            if (!skipping) result += char

            if (char == '}') skipping = false
        }
        if (index != args.size - 1) throw IllegalArgumentException("Too many arguments to format a string: $str")
        if (skipping) throw IllegalArgumentException("Expected '}' while parsing string: $str")
        return result
    }

    fun localizedStringByKey(key: String, locale: String): String =
            if (locale.isEmpty()) {
                // Finds value by key using device's locale (or locale that was previously setup by user) inside static map
            } else {
                // Finds value by key inside static map with provided locale
            }
avently commented 4 years ago

@KamiSempai hello. Could you please clarify the status of the issue? I mean will you make the feature possible someday when you'll have a time? Because it's reeeealy useful from my point of view.

KamiSempai commented 4 years ago

For now I working on migration to Kotlin Poet to make code generation simplest. Then I will start implementing this feature.

KamiSempai commented 4 years ago

Probably this should help. The version of library 1.5.0 https://github.com/PocketByte/LocoLaser/blob/master/docs/kotlin_mpp.md#custom-repository

But I'm not yet have implement parsing of string resources that you provide. @avently What is the library do you use for localization?

avently commented 4 years ago

What is the library do you use for localization?

I have an app with client-server architecture. Client part (Android + common code) uses LocoLaser, server part uses my code for localizing property files (server.properties, server_ru.properties, etc) like I already wrote in this issue. Properties files contain strings with placeholders and I read these files, parse them, replacing placeholders. Some parts of actual code I wrote in this issue too.

Based on mpp example (development branch) and instruction from your link I made some changes to my code: added inside config

{
      "type": "kotlin-abs-key-value",
      "res_dir": "./build/generated/locolaser/keyvalue/",
      "formatting_type" : "web"
    },

added inside docs file key | en | ru testing_field | Something that makes me {{what}} | То, что делает меня {{каким}}

And I expected that {{what}}:

Also I expected that old strings that have Something with old placeholder %s will not be considered as a string with placeholder because I specified "formatting_type" : "web".

Instead this is how it looks like in values/string.xml:

<string name="testing_field">Something that makes me {{what}}</string>

And in AbsKeyValueStringRepository:

override val testing_field: String
        get() = this.stringProvider.getString("testing_field")

And old strings with "%s" were look like:

override fun toast_something(s1: String): String = this.stringProvider.getString("toast_something", Pair("s1", s1))

Am I doing something wrong? Or

But I'm not yet have implement parsing of string resources that you provide

it means that the web is still not working yet?

AndroidStringRepository has a stringProvider but it's private and I can't use it for passing to AbsKeyValueStringRepository. Why? I should get actual values for keys somewhere and AndroidStringRepository should give such possibility.

Also, in previous version as well as in the new version there is a problem with UPPERCASE and lowercase strings. For example, if you have a strings with key "Something" and "something" in common StringRepository there will be only one key with lowercase letters but in values/strings.xml there will be both keys. So something wrong with kotlin code generation.

P.s. implementation looks really cool right now

KamiSempai commented 4 years ago

added inside docs file key | en | ru testing_field | Something that makes me {{what}} | То, что делает меня {{каким}}

Google sheets use Java formatting. So you could use strings like "Something that makes me %s". Of Course it will erase parameter name, but for now Google Sheets doesn't allow to provide parameters names (I need to think how to bring it into Google Sheets, without breaking old functionality). In the next version I Will provide opportunity to specify Formatting Type for any source or platform, it will solve the problem with naming, but it also will bring a new one problem, cause web formatting yet is not well implemented and can recognize only strings. So you will lost formatting for any other type, for example Float or Double.

One more note about your example. Don't translate formatting keys. In your case for ru language it's better to use string "То, что делает меня {{what}}"

AndroidStringRepository has a stringProvider but it's private and I can't use it for passing to AbsKeyValueStringRepository. Why? I should get actual values for keys somewhere and AndroidStringRepository should give such possibility.

Android implementation of StringProvider works only with Android resources. For backend you should implement your own implementation of StringProvider. Also I would notice that you don't need to access stringProvider property cause it not a part of common interface. To make code multiplatform you should get strings only thru the common interface.

Also, in previous version as well as in the new version there is a problem with UPPERCASE and lowercase strings. For example, if you have a strings with key "Something" and "something" in common StringRepository there will be only one key with lowercase letters but in values/strings.xml there will be both keys. So something wrong with kotlin code generation.

There is some restriction for the keys. The key should be in underscore case format and lowercase. If it does, the tool automatically convert it in the suitable format. I strongly recommend you don't use same strings with different character case. Key should look like a key. Also it's better to add some context into a key. For example, the key screen_main_btn_something also tells that it will be used on Main screen to display a button text.

avently commented 4 years ago

but for now Google Sheets doesn't allow to provide parameters names

What does it mean actually? I just wrote a string with double curly brackets (Something that makes me {{what}}) and I don't need any support from Google Docs in this case. Also it will be backward compatible because you chose a great way to type a parameter (I mean double curly brackets). I don't think somebody used this before with your lib. Also, even if somebody used curly brackets it doesn't matter since without specifying web as a type of parameter nothing will be changed in the resulting code.

I strongly recommend you don't use same strings with different character case

It was only one place in the whole codebase where I needed to specify one minute as something_1m and one month as a something_1M. I changed this because I got the wrong result after localization.

For backend you should implement your own implementation of StringProvider

But where I should get real values for keys from StringRepository interface? AndroidStringRepository uses values from values/strings.xml but AbsKeyValueStringRepository lacks this opportunity. Let's say I have a Google Docs sheet and in result I will get only interface with a keys, right? No values like in AndroidStringRepository.

What if you will make a in-built static files (like .properties as in my example) with keys and values for different languages? One file per language. It will remove the need to specify a custom StringProvider and will work out of the box. The ability to provide a language key (en, ru, etc. It will be a file name) should be present as well (and ability to change it in runtime). If you want to leave this task for us, not a problem but string files should be present anyway after importing Google Docs, do you agree?

KamiSempai commented 3 years ago

@avently Please check if Properties Resources is suits for you. Also, please take in acount Migration instruction. Cause there is some changes in library that requre some changes in config and dependencies.

avently commented 3 years ago

Hello, Denis. I'm unable to check everything you did because I don't have a PC near me. Maybe after a month or so.

Does the new implementation allow to use google docs for kotlin multiplatform development?

KamiSempai commented 3 years ago

@avently Is the Issue still relevant? Can I close it?

avently commented 3 years ago

@KamiSempai as far as I understand you added properties as a source of data. But how can I use google sheets with variables as a source? There are no examples of usage of properties module so can't be sure that I really understand what can be achieved with it.

After I switched to Locolaser I'm so happy to use Google Sheets.

How properties module can help with variables? Do they have the ability to use variables at all? If yes, will Locolaser generate functions with parameters named as a variable?

I mean something like this:

In google sheet I have something like:

key | base | ru

some.key | Some key with ${variable} | Какой-то ключ с ${переменной}

I expect to get the following function generated:

fun some_key(variable: String): String {
return repository["some.key"].replaceVarsWith(variable)
}

Will I get this result? If yes, the issue can be closed. If not, let's see what we can receive in the output from this example?

P.s. I still use Locolaser from early 2020 just because it does the right job. I apply it to client part of the app because I want to rewrite the whole server part into Nim. And in there I will use Locolaser two (I need Google Sheets:) ).

KamiSempai commented 3 years ago

Since version 2.1.0 LocoLaser allow to set formatting type for Google Sheets. See here https://github.com/PocketByte/LocoLaser/blob/master/resource-googlesheet/README.md#config

If you do set formatting_type to "web" you will be able to use "web" formatted strings like "Some key with {{variable}}", instead of Java formatted strings like "Some key with %s". But, please, take in account that one Worksheet can hold only strings with one format, "web" or "java", not both.

So if you will use resources from such Google Sheet, LocoLaser will able to generate Repository containing methods like fun some_key(variable: String): String.

You only need to implement StringProvider for AbsKeyValueStringRepository in the following way

class StringProviderImpl(private val repository: HashMap<String, String>) : StringProvider {
    override fun getString(key: String, vararg args: Pair<String, Any>): String {
        return args.fold(repository[key] ?: return key) { acc, pair -> 
            return@fold acc.replace("{{${pair.first}}}", pair.second.toString())
        }
    }
}
avently commented 3 years ago

@KamiSempai ideal! This is what I need, thank you, Denis! Issue should be definitely closed now.

avently commented 3 years ago

@KamiSempai looks like I found something unexpected. I'm trying to make a json localized files in order to use them as a source for StringProvider for custom platform. The problem is that I can't specify JSON Object in config instead of "json". Take a look at example:

This one doesn't work with error: ERROR: Invalid Config! Source type is "json", but expected "googlesheet".

{
  "platform": [
  {
      "type" : "json",
      "res_dir": "./build/generated/locolaser/json/"
    }
  ],
"source" : {
"type": "googlesheet",
....
}
}

This one works ok, but files are placed in top-level directory which I would like to avoid:

{
  "platform": [
  "json"
  ],
"source" : {
"type": "googlesheet",
....
}
}

Did I do something wrong? It's not a big deal I can move files in gradle task but other option can be useful for someone too

KamiSempai commented 3 years ago

@avently This is an issue and it's already fixed in development branch. But it's not ready yet to be released.

avently commented 3 years ago

I see debug info in a console. If it intentional then everything is fine. I see json files generated in separate directories per lang. I don't use other options except files location so can't say about them, Thank you, fast as always!