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

Add infrastructure to be able translate REPL This is based on suggestions from #185 #186

Merged

Conversation

kant2002
Copy link
Contributor

This is based on suggestions from #185

I have translation for Ukrainian and Russian, but want to gather feedback on the approach.
I think that's it, and no additional strings needed, or that amount would be very small.

I have to add dependency on Fable.Browser.Navigator since tha allow account for user preferences.

Comment on lines +20 to +23
nuget Fable.Browser.Dom == 2.4.4
nuget Fable.Browser.Event == 1.4.5
nuget Fable.Browser.MediaQueryList
nuget Fable.Browser.Navigator == 2.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there conflict between dependencies for you to need to lock the versions ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It pulls to latest version and something broke. It some versions mismatch, and i feel that REPL is a bit picky. Also it produce larger diff in lock file. If you don't mind updating deps, I'm fine :) also I can work separately on that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, keep it like that I will have a look when testing your PR.

Comment on lines 103 to 127
module Translations =
let enTranslations = dict["msg_iframe_loaded", "Iframe loaded";
"win_header_console", "Console";
"msg_initializing", "Initializing";
"msg_repl_name", "Fable REPL";
"msg_desktop_experience", "For best experience we recommend running the REPL on a desktop";
"btn_continue", "Continue";
"msg_gist_description", "Created with Fable REPL";
"msg_compilation_failed", "Failed to compile";
"msg_assemblies_load_failed", "Assemblies couldn't be loaded. Some firewalls prevent download of binary files, please check.";
"msg_gist_token_invalid", "An error occured when creating the gist. Is the token valid?";
"msg_gist_token_missing", "You need to register your GitHub API token before sharing to Gist";
"msg_compilation_successful", "Compiled successfuly";
"msg_shareable_url_ready_text", "Copy it from the address bar";
"msg_shareable_url_ready_title", "Shareable link is ready";
"msg_load_gist_error", "An error occured when loading the gist";
"msg_update_url_failed", "An error occured when updating the URL";
"msg_fatal_error", "Should not happen";
"msg_live_sample_text", "Live sample";
"msg_code_text", "Code";
"msg_problems", "Problems";
"msg_problems_info", "Problems: ";
"msg_invalid_iframe_error", "`%A` is not a known value for an iframe message";
"msg_collapse_sidebar", "Collapse sidebar";
"msg_widget_general", "General";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would have used a Record to force us to have translation covering all the keys. This is true, that you then can't have a partial translation implemented but I feel like this is a plus.

As this will also avoid typos in the key name which would be properties instead of strings.

This also avoid the duplication work you did below and have intellisense for free.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add 2 languages *UK and RU, maybe Kazakh and Turkish, but that's it. I plan at least find somebody who knows Spanish and other languages. Communication can be hard in this case. over long period of time. Please consider this.
I understand why do you want it that way. I actually misinterpret part with records and go with dict.

I ambivalent on this, and if you insist that this is better for you/maintenance, I will rework this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the record option yes please.

If we don't know how to translate something we can always referenced to the english record. This is because the REPL don't have a lot of active development done because it serves its purpose well so I want to limit the friction or things to know when contributing.

I will be able to do French on my side.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you take a look at records approach ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MangelMaxime I know it's Christmas, and you maybe busy, but after that, can you take a look? Also, Merry Chrismas (even if a bit late)

kant2002 and others added 7 commits January 1, 2024 16:24
This is based on suggestions from fable-compiler#185

I have translation for Ukrainian and Russian, but want to gather feedback on the approach.
I think that's it, and no additional strings needed, or that amount would be very small.

I have to add dependency on Fable.Browser.Navigator since tha allow account for user preferences.
@MangelMaxime
Copy link
Member

Hello @kant2002.

Sorry for the long wait.

I have been conflicted about this PR because this is always intimidating to add translation to a project. And also, I had some plan to rewrite the REPL which didn't happen yet as I probably want to rework some stuff upstream first.

With all that said, I made a few changes to move the translations to their own file and also allows the user change the language setting from the application itself.

This is because sometimes even if your browser is in a certain local you can prefer to see the application in english for example.

@MangelMaxime MangelMaxime merged commit 5040b54 into fable-compiler:main Jan 1, 2024
@kant2002 kant2002 deleted the kant/translation-preparation branch January 1, 2024 17:15
@kant2002
Copy link
Contributor Author

kant2002 commented Jan 1, 2024

No big deal, If I was really need it, I either work with fork, or bombard you with notifications. I think I still have small things which I can improve unrelated to translation.

Most interesting for me is phone support for REPL which is not ideal. So you maybe hear from me soon )))

@kant2002
Copy link
Contributor Author

kant2002 commented Jan 1, 2024

I'm really glad that I may improve submit important for me changes upstream and worry less about live with them in personal fork.

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.

2 participants