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

Localization #649

Merged
merged 68 commits into from
Feb 9, 2024
Merged

Localization #649

merged 68 commits into from
Feb 9, 2024

Conversation

rpatters1
Copy link
Collaborator

This PR introduces a mechanism for localization of scripts. It consists of the following:

  • library.localization provides the run-time localization service that can be used everywhere except the plugindef function.
  • library.localization_developer provides an API for auto-translating strings using OpenAI APIs.
  • rgp-lua.md describes a new feature where the user's OS-configured locale will now be passed to the plugindef function starting in RGP Lua 0.71. This allows plugindef to localize strings while remaining dependency-free. The change is fully backwards compatible with all previous versions of RGP Lua and with JW Lua, provided you check locale for nil.
  • auto_layout is a demonstration script of auto-layout features coming in RGP Lua 0.71.
  • auto_layout_localizing_script shows how to use library.localization_developer to translate strings.
  • All of the transposition scripts have been localized in Spanish and German. While they require RGP Lua 0.71 to be fully functional, they still work in JW Lua and earlier versions of RGP Lua. One way this is accomplished is by using the new mixin _FallbackCall method to call any method that is new in 0.71.

In developing this approach, I have come to realize that LLM translation is amazing, but it is not reliable enough to put in front of a user without human inspection and revision. I speak enough Spanish and German to be comfortable that the translations for transposition are usable. I do not feel the same way about the French, Chinese, Arabic, and Farsi that I put into the auto_layout sample. (Although a Farsi-speaking friend of mine said they are pretty good.) I mainly included Farsi and Arabic in the sample to confirm that right-to-left text works reasonably well.

Unfortunately, auto_layout requires 0.71, but it is mainly a test of auto-layout features rather than localization per se.

I welcome any and all suggestions. I have specifically added @ThistleSifter and @asherber as reviewers, but I welcome comments from anyone. I haven't marked it as a draft, but if there is enough conversation about the best way to make it work, I will so mark it. Localization adds complexity but it makes for a much better UX. That said, I would like to make the DX as smooth as possible.

@asherber
Copy link
Member

That's a big PR! It'll take me some time to sift through it.

I agree with your take that machine translation needs human review -- especially when we're talking about isolated words and phrases that may mean something particular in a musical/Finale context that the AI is unaware of.

One question I have is about OS locale vs Finale language. According to this page, the only non-English languages that Finale comes in are French, German, Italian, Japanese, Dutch, Polish, and Swedish. I suspect that there are many users with a non-English locale who are running a version of Finale that doesn't match their locale. Will users want to see plugin dialogs in a language that doesn't match the rest of Finale? Is there any way to determine the language of the currently running Finale? (Probably not.)

@rpatters1
Copy link
Collaborator Author

rpatters1 commented Jan 22, 2024

What a nice can of worms @asherber opened. I'll try to share what I know about it in a manner that is halfway coherent.

  • The localizations for the six languages (other than English) mentioned on the website are each maintained by separate international affiliates who have partnership deals with MakeMusic.
  • MakeMusic itself maintains the localizations for English and Spanish. (Spanish was introduced in 2020. I don't remember the exact version number, perhaps the first F27 version?)
  • The six affiliate localizations appear originally to have created by directly modifying hard-coded strings in Finale source code. I was tangentially involved in the multiyear effort to modernize the UI code so that this was no longer necessary: a major development effort behind the scenes from ~Finale 2012 until the Spanish release in 2020.
  • Each Finale version appears to contain only one localization, but all the plugins (including the MakeMusic-supplied plugins) contain all the localizations. You can explore them yourself. Use ResEdit on Windows or just open the plugin bundle on Mac.
  • What you will discover is that the plugins are only partially translated, with some languages being more complete than others. The last time I looked, only Spanish was 100% translated, and I'm guessing it's because MM controls that version. That's one reason why I took the trouble to translate the Patterson Plugins to Spanish, and I'm planning to do the same for the RGP Lua dialogs (finally) this cycle.

As part of testing my plugins originally and now testing Lua, I took the following steps.

  • Set up (in addition to US English) US Spanish, German, and Japanese on both macOS and Windows.
  • Switching between them requires a reboot on macOS and a logout/login on Windows.
  • Downloaded the Spanish Finale demos (both Mac and Win) from MakeMusic and the German Finale demos from Klemm.

As for how it all works, it matters which version of Finale you are running and which operating system.

  • Windows appears to be the simplest. For the plugins you get the language you specified in the Settings app, irrespective of which Finale version you run. (I haven't tested pre-Win10, but this is how it works in WIn10+.)
  • macOS is not so simple. The German version of Finale appears to override the OS locale setting and force the plugins to load the German localization. The Spanish version of Finale, on the other hand, defers to the OS locale setting. So I can run the Spanish version with my language set to English, and I'll get the English version of the plugins. The only way to see the Spanish plugin versions is to reboot into the Spanish version of the OS.
  • On both OSes, the finenv.UI():GetUserLocaleName() value matches the version that the rest of the plugins are getting. Specifically, on the macOS German version, it returns "de" as the language code, even if you are running the English (or Spanish) OS.

There are other differences. The Spanish version uses the built-in OS folder-name translation. That is, all the default folder paths are the same between English Finale and Spanish Finale. The merely appear with different translations in the Finder. The German version, otoh, has hard-coded German folder names. I attribute this to the German version predating modern localization techniques. It's possible that the other Finale languages use yet different approaches. I haven't tried to test them.

So now we get down to Aaron's key question:

Will users want to see plugin dialogs in a language that doesn't match the rest of Finale?

There probably is no single answer to what users want. But I can say with certainty that what they actually see now is a hodge-podge mix of languages. I have always taken it is as given that if you have set a particular locale as your primary default locale, you should present that locale to them if you have it. But ymmv.

@rpatters1
Copy link
Collaborator Author

rpatters1 commented Jan 22, 2024

Something else I'd like us to consider is, should we build in a standard way to extend the number of localizations? For example, a user might prefer to see the dialogs in Vietnamese, but there is no Vietnamese built into the script. We could conceivably modify the localize function to attempt to load it, so that the user (or external developer) could provide it without modifying the script. It seems like a lot of overhead for a rare situation, though, so there would have to be a way to try only once per execution of the plugin (Lua state).

@ThistleSifter
Copy link
Member

One thing that stands out to me is the use of the entire English phrase as the key for language strings in the examples, rather than a regular identifier. Although it might seem nice and readable, it's not good practice and it will introduce more difficulties in the long run, especially when the need arises to change the strings. Best to encourage good habits to start with.

Some changes to the mixin library could be made to make the code less verbose but I'll think on it some more before I make some suggestions.

@ThistleSifter
Copy link
Member

For dealing with localised strings:

FCMControl.lua

function methods:SetLocalizedText(key)
    mixin_helper.assert_argument_type(2, key, "string", "FCString")

    self:SetText(localization.localize(key))
end

FCMUI.lua

function methods:AlertLocalizedError(message_key, title_key)
    mixin_helper.assert_argument_type(2, message_key, "string", "FCString")
    mixin_helper.assert_argument_type(3, title_key, "string", "FCString")

    self:AlertError(localization.localize(message_key)), localization.localize(title_key))
end

etc, etc

@rpatters1
Copy link
Collaborator Author

rpatters1 commented Jan 22, 2024

@ThistleSifter I thought about building this into mixin and I think it's a sound idea in principal. The main downside I see is that now every script that uses mixin will copy in the localization library into the dist version whether they use it or not. But maybe that's not a big deal given everything else that's copied in.

Taking that as given:

  • Is there any reason to have separate Localized versions of the methods? (I'm not saying there isn't; I just want to think it through.) The current methods could instead localize automatically.
  • Your reply made me realize I should be using CreateChildUI when it is available, e.g.:
global_dialog:_FallbackCall("CreateChildUI", finenv.UI()):AlertError( --...
  • I'm guessing that CreateChildUI called on an FCXCustomLuaWindow instance automatically returns FCMUI. But what about finenv.UI()?

EDIT: CreateChildUI is probably a candidate for implementing directly in FCMCustomLuaWindow.lua, thus avoiding the need for calling _FallbackCall. I suppose it could handle the conversion of finenv.UI to FCMUI if needed.

@ThistleSifter
Copy link
Member

The main downside I see is that now every script that uses mixin will copy in the localization library into the dist version whether they use it or not.

Yes, the bundled scripts are big enough as it is. I guess it comes down to whether or not you want to enforce localisation as part of a coding style expectation in this repo. If all the scripts that use mixins also use the localization library, then there is no functional difference I suppose. But that's another can of worms.

  • Is there any reason to have separate Localized versions of the methods? (I'm not saying there isn't; I just want to think it through.) The current methods could instead localize automatically.

Yes, because then the mixin method would go beyond being an enhancement and break existing functionality. (Ie if I wanted to set plain text, for whatever reason, I can't do that any more except by calling SetText__ with a FCString object). It would also break other mixin methods (eg the measurement setting methods) which use SetText internally.

  • I'm guessing that CreateChildUI called on an FCXCustomLuaWindow instance automatically returns FCMUI. But what about finenv.UI()?

mixin.UI() returns FCMUI.

@rpatters1
Copy link
Collaborator Author

If anyone is tracking this branch, I'm going to rebase it on upstream/master when I get the chance.

@rpatters1 rpatters1 marked this pull request as draft January 31, 2024 19:38
@rpatters1
Copy link
Collaborator Author

Okay, this is ready for more review and suggestions. Per comments from @asherber and @ThistleSifter I have made the following revisions:

  • Base.lua is now recommended as a fallback localization. This allows constant keys no matter what we want the messages to say. I believe that all comments, plugindef functions, samples, and utilities have been updated to recommend and use Base localization. If you find one I missed, let me know.
  • The new DoAutoSizeWidth method in RGP Lua 0.71 will accept an optional minimum size argument. All scripts, samples, and utilities have been updated to use this feature as appropriate. (I am still using SetWidth on the transposition scripts because I want to leave them JW Lua compatible.)
  • The strings with trailing spaces in auto_layout.lua have been modified to have format parameters. I remembered that the way I have always handled localized strings with format parameters is that the caller constructs the final string from the localized string resource using (in the case of c or c++) sprintf or one of its equivalents. For Lua I am using string.format, which is the Lua version of sprintf. If one of you has a better idea of how to do this, I am interested. But I don't want to muck up the mixin API too much with a feature that is not that often used.

Once I have signoff, I will merge it. I'm thinking I'll use squash and merge, because the long commit history is rather annoying.

@ThistleSifter
Copy link
Member

  • Base.lua is now recommended as a fallback localization.

I'm not sure that this is a suitable option for our purposes. Take /src/localization/transpose_by_step/Base.lua for example. What locale is this? Sure I can take a guess, but guessing shouldn't even be an option. Someone should be able to look at the names of the files in the directory and know exactly which languages and territories are provided.

Do we actually need a general fallback other than en or if that doesn't exist, then the first one alphabetically? Are we trying to address an issue (that of incomplete or partial translations) that should be addressed somewhere else? I'm not talking about fallbacks within the same language (eg en as fallback for en_GB or en_AU), that's a different scenario.

 

  • If one of you has a better idea of how to do this, I am interested.

What about changing the signature of localize to function localization.localize(input_string, ...) where ... is a series of parameters to insert? Mixin *Localized methods could accept tables if substitutions are required. For example:

finenv.UI():AlertErrorLocalized(["input_error", "Some", "inserted text"], "error_title")

@rpatters1
Copy link
Collaborator Author

rpatters1 commented Feb 8, 2024

Passing a table to *Localized seems like it falls right back into the same problem that passing a table to AddStrings does: you can't properly enforce the types. If string.format is going to be called, shouldn't the person using it be calling it?

If Base localization is good enough for Apple, it's good enough for me. There has to be a fallback, and forcing English to be the fallback is not appropriate for an international project.

@rpatters1
Copy link
Collaborator Author

Lol, fwiw, Apple is definitely still recommending full text as keys. Check out the docs on string catalogs. I'm not saying we should fall back, though. Apple has way more support for sting catalogs than I am contemplating here.

@asherber
Copy link
Member

asherber commented Feb 8, 2024

I don't have a strong feeling about fallback languages, and I find the discussion very interesting. I note that Finale itself is developed in English and then localized, and that keywords in Lua (along with almost every programming language) are in English; it doesn't seem unreasonable in the context of software for English to be the general fallback language, which I think is part of what @ThistleSifter is suggesting.

What if name of the fallback file specified its locale (e.g., Base_en.lua)?

Feel free to ignore if this isn't helpful; I admit that I haven't dealt with localization issues before.

@rpatters1
Copy link
Collaborator Author

rpatters1 commented Feb 8, 2024

The language a language is developed in is irrelevant to how it should be localized. Finale is released with only a single localization and that localization is the language it's in. I have the Spanish and German versions on my computer, for example, and both of those (like the English version) have the one localization in Base.lproj.

But if @ThistleSifter is comfortable with Base_en.lua, that is an easy change to make.

EDIT: I said it was easy, but I'm not sure how to automatically require it.

@asherber
Copy link
Member

asherber commented Feb 8, 2024

Re Base_en.lua what I was thinking is that instead of Base.lua being a recommended fallback (per @rpatters1's earlier post), the recommendation would be Base_<locale>.lua. The code to find the fallback file would pick the first one that matches this pattern (which I think was part of @ThistleSifter's suggestions).

@rpatters1
Copy link
Collaborator Author

I suppose there may be a way to pattern match in require by mucking with the the path, but I'm not sure it could be made to work with the preloads in dist. Anyway, here's a simpler way:

  • get rid of the Base.lua concept (and rename the current Base.lua files to en.lua.
  • add an api to the localization library set/get/fallback_locale.
  • this value will default to en.

That allows a script writer to specify a different fallback language if need be.

@asherber
Copy link
Member

asherber commented Feb 8, 2024

Ah yes, I missed your reference to require. What you propose sounds reasonable to me, but I'll butt out because it's not really my area.

@ThistleSifter
Copy link
Member

Lol, fwiw, Apple is definitely still recommending full text as keys.

Out in the open source world, where people have to work together, snake case (sometimes other, but snake is the most popular esp. in the web languages) keys have been used for referencing translatable strings for well over 15 years. Organisations that can dictate terms because they have a functional monopoly can have some interesting policies, to put it gently. Not always bad, but it's often worth looking a little wider to see what everyone else is doing.

 

* add an api to the `localization` library `set`/`get`/`fallback_locale`.

Yes, I like this idea.

@ThistleSifter
Copy link
Member

Passing a table to *Localized seems like it falls right back into the same problem that passing a table to AddStrings does: you can't properly enforce the types

Yes, good catch. I have been thinking about this for the past couple of days because it would be nice to have and I think I have a solution for the error reporting but I have to work out how to do it. It might need to wait for the mixin rewrite because that has completely rewritten error handling with greater ease and flexibility for hooking in to it.

@ThistleSifter
Copy link
Member

ThistleSifter commented Feb 8, 2024

Hold on, I've got it. I'll write it up and paste it here tomorrow.

Edit: or maybe now.
Does not handle nested tables.
It should be able to report an error for any type of key.

local function to_key_string(value)
    if type(value) == "string" then
        value = "\"" .. value .. "\""
    end

    return "[" .. tostring(value) .. "]"
end

function mixin_helper.process_string_arguments(self, method_func, ...)
    for i = 1, select("#", ...) do
        local v = select(i, ...)
        mixin_helper.assert_argument_type(i + 1, v, "string", "number", "FCString", "FCStrings", "table")

        if type(v) == "userdata" and v:ClassName() == "FCStrings" then
            for str in each(v) do
                method_func(self, str)
            end
        elseif type(v) == "table" then
            for k2, v2 in pairsbykeys(v) do
                mixin_helper.assert_argument_type(tostring(i + 1) .. to_key_string(k2), v2, "string", "number", "FCString")
                method_func(self, v2)
            end
        else
            method_func(self, v)
        end
    end
end

* master:
  chore: autopublish 2024-02-07T20:57:40Z
  Update ties_remove_dangling.lua
  Update ties_remove_dangling.lua
  Update ties_remove_dangling.lua
  Update ties_remove_dangling.lua
  Update ties_remove_dangling.lua
  Update ties_remove_dangling.lua
@rpatters1
Copy link
Collaborator Author

rpatters1 commented Feb 8, 2024

I'll hold off checking in until seeing your brainstorm.

How do you feel about adding to FCMCustomWindow CreateOkButtonLocalized, CreateCancelButtonLocalized, and CreateCloseButtonLocalized. They could either take a string argument as a key (which would break conformity with the other "Create..." apis) or the keys could just be hardcoded to "ok", "cancel", and "close". It's annoying having constantly to add a SetTextLocalized to these methods.

* master:
  chore: autopublish 2024-02-08T23:53:39Z
  Update FCMCtrlStatic.lua
@ThistleSifter
Copy link
Member

I ended up editing in my brainstorm to the comment above last night. Here's a quick demo anyway:

local t_num = {"this", "is", false, "a", "test"}
local t_str = {}
t_str.a = "this"
t_str.b = "is"
t_str.c = false
t_str.d = "a"
t_str.e = "test"

...

popup:AddStrings(t_num) -- bad argument #1[3] to 'AddStrings' ... etc

popup:AddStrings("foo", t_str) -- bad argument #2["c"] to 'AddStrings' ... etc

 

How do you feel about adding to FCMCustomWindow CreateOkButtonLocalized, CreateCancelButtonLocalized, and CreateCloseButtonLocalized.

What about CreateOkButtonAutoLocalized and not having a key argument? Could also bundle in some default translations in another library file so that every script author doesn't have to retranslate "OK" unless they wish to.

@rpatters1
Copy link
Collaborator Author

rpatters1 commented Feb 9, 2024

I am really puzzled by this statement as to how it is supposed to work:

    mixin_helper.assert_argument_type(tostring(i + 1) .. to_key_string(k2), v2, "string", "number", "FCString")

I just tested it, and it passes a string to the first argument mixin_helper.assert_argument_type that that is expecting a number. And I don't understand it: 2[1] (for the first item in my array table). This appears to be wrong (though it produces seemingly correct results), but there could be wrinkle of Lua I don't understand here.

EDIT: Never mind. I guess the first argument is just used to identify the bad argument in the error message. (I read your followup message.)

@ThistleSifter
Copy link
Member

Technically it doesn't matter what you pass for the value of argument_number, just that it probably should be a number or something that starts with a number, since it should fit in the same form as the standard error message. It's probably worth updating the docs to reflect that, maybe number | string, but only use string if you have a good reason to. Something like that.

@rpatters1
Copy link
Collaborator Author

Okay, here it goes. I'm committing it.

@rpatters1 rpatters1 merged commit 726b266 into finale-lua:master Feb 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants