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

Enhanced Internationalization Support (i18n) #1320

Merged
merged 36 commits into from
Jul 2, 2024
Merged

Conversation

mdaneri
Copy link
Contributor

@mdaneri mdaneri commented May 30, 2024

Description of the Change

Enhanced Internationalization Support (i18n)

Related Issue

#1285

@Badgerati
Copy link
Owner

I'll go through this one when I've got a good chunk of time free, possibly this weekend. I have had some additional thoughts on how we might make localisation a base feature in Pode as well, which other people can re-use - such as Pode.Web.

For example, having strings available as $locale:<key>. When I go through this, I'll see if we can merge the two localisations ideas, or if they're distinctly separate features.

fix build process
add language test
@mdaneri
Copy link
Contributor Author

mdaneri commented May 30, 2024

Personally, I'll keep it separate. Pode.Web requirements are a bit different. You want the user to be able to i18n their application. In the context of Pode, it's more of a system thing. Sure, we can implement something that allows the user to i18n everything inside a Route, but it should differ from the exception message generated by Pode.

I load the language in Pode.psm1 as first thing and assign the content to $global:msgTable. But we can add an additional variable for the user to use

@mdaneri
Copy link
Contributor Author

mdaneri commented May 30, 2024

I've no idea why it's failing on Ubuntu. I tried on Centos and works fine

@mdaneri
Copy link
Contributor Author

mdaneri commented May 31, 2024

Looks like the global message variable is not injected inside the runspace

Add Korean
additional messages migrated
added a new UICulture property to the module `Import-Module -Name "Pode" -ArgumentList @{UICulture ='ko-KR'}`
failback to English if culture doesn't exist
Copy link
Owner

@Badgerati Badgerati left a comment

Choose a reason for hiding this comment

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

I've done a very quick scan, haven't had chance to validate the message/key remapping and translation messages yet

pode.build.ps1 Show resolved Hide resolved
src/Locales/ar/Pode.psd1 Outdated Show resolved Hide resolved
src/Pode.psm1 Outdated Show resolved Hide resolved
src/Pode.psm1 Outdated Show resolved Hide resolved
tests/unit/Localization.Tests.ps1 Fixed Show fixed Hide fixed
tests/unit/Localization.Tests.ps1 Fixed Show fixed Hide fixed
@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 4, 2024

I tried to convert the Pode.psd1 to a hashtable

@{
  noCertificateFoundExceptionMessage = "No certificate could be found in {0}{1} for '{2}'"
  incompatiblePodeDllExceptionMessage = "An existing incompatible Pode.DLL version {0} is loaded. Version {1} is required. Open a new Powershell/pwsh session and retry."
  noOpenApiUrlSuppliedExceptionMessage = "No OpenAPI URL supplied for {0}."
  noPathSuppliedForRouteExceptionMessage = "No Path supplied for the Route."
  invalidJsonJwtExceptionMessage = "Invalid JSON value found in JWT"
....

This is the error

Import-LocalizedData: The following error occurred while PowerShell was loading the 'C:\Users\m_dan\Documents\GitHub\Pode\src\Locales\en\Pode.psd1' script data file: At line:48 char:104 + … SE connection Name is required either from -Name or $WebEvent.Sse.Nam … +                                                    
   ~~~~~~~~~ A
variable that cannot be referenced in restricted language mode or a Data section is being referenced. Variables that can be referenced include the following: $PSCulture, $PSUICulture, $true, $false, $null.  At line:135 char:63 + … ForUsingVariableNotFoundExceptionMessage = "Value for '$using ={0}' c … +           
~~~~~~ A variable that cannot be referenced in restricted language mode or a Data section is being referenced. Variables that can be referenced include the following: $PSCulture, $PSUICulture, $true, $false, $null..

Import-LocalizedData wantsConvertFrom-StringData -StringData

@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 4, 2024

I added some documentation to the Tutorial/Basic part. But I think to write also a getting started paragraph

@Badgerati
Copy link
Owner

I tried to convert the Pode.psd1 to a hashtable

@{
  noCertificateFoundExceptionMessage = "No certificate could be found in {0}{1} for '{2}'"
  incompatiblePodeDllExceptionMessage = "An existing incompatible Pode.DLL version {0} is loaded. Version {1} is required. Open a new Powershell/pwsh session and retry."
  noOpenApiUrlSuppliedExceptionMessage = "No OpenAPI URL supplied for {0}."
  noPathSuppliedForRouteExceptionMessage = "No Path supplied for the Route."
  invalidJsonJwtExceptionMessage = "Invalid JSON value found in JWT"
....

This is the error

Import-LocalizedData: The following error occurred while PowerShell was loading the 'C:\Users\m_dan\Documents\GitHub\Pode\src\Locales\en\Pode.psd1' script data file: At line:48 char:104 + … SE connection Name is required either from -Name or $WebEvent.Sse.Nam … +                                                    
   ~~~~~~~~~ A
variable that cannot be referenced in restricted language mode or a Data section is being referenced. Variables that can be referenced include the following: $PSCulture, $PSUICulture, $true, $false, $null.  At line:135 char:63 + … ForUsingVariableNotFoundExceptionMessage = "Value for '$using ={0}' c … +           
~~~~~~ A variable that cannot be referenced in restricted language mode or a Data section is being referenced. Variables that can be referenced include the following: $PSCulture, $PSUICulture, $true, $false, $null..

Import-LocalizedData wantsConvertFrom-StringData -StringData

I've just tested this myself, and reproduced the error. It's because the $WebEvent variable in the following wasn't being escaped:

sseConnectionNameRequiredExceptionMessage = An SSE connection Name is required, either from -Name or $WebEvent.Sse.Name

In mine hashtable conversion:

sseConnectionNameRequiredExceptionMessage = "An SSE connection Name is required, either from -Name or `$WebEvent.Sse.Name"

Escaping the variables then allows the module to load and work without error

@Badgerati Badgerati added this to the 2.11.0 milestone Jun 5, 2024
@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 5, 2024

I missed It.
I’ll try again.
I’ll convert everything after I add all the messages

Convert-HashtableToPsd1.ps1 Fixed Show fixed Hide fixed
@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 8, 2024

Question. Do you think the listener requires localization? I was looking the code, but I’m not seeing anything interesting that required to be localized

src/Locales/en/Pode.psd1 Outdated Show resolved Hide resolved
docs/Tutorials/Basics.md Outdated Show resolved Hide resolved
docs/Tutorials/Basics.md Outdated Show resolved Hide resolved
examples/FileBrowser/public/string_performance_test.html Outdated Show resolved Hide resolved
src/Misc/default-file-browsing.html.pode Show resolved Hide resolved
src/Public/Logging.ps1 Outdated Show resolved Hide resolved
src/Public/Logging.ps1 Outdated Show resolved Hide resolved
src/Locales/en-gb/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/en-gb/Pode.psd1 Outdated Show resolved Hide resolved
src/Public/Utilities.ps1 Outdated Show resolved Hide resolved
Copy link
Owner

@Badgerati Badgerati left a comment

Choose a reason for hiding this comment

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

Been through and double checked all the transactions, just a few issues with some of the translations where some might be better explicitly have the word "Schedule" instead of translating the word - as it's in direct reference to a Pode object

src/Locales/en-gb/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/en/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/es/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/fr/Pode.psd1 Show resolved Hide resolved
src/Locales/fr/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/it/Pode.psd1 Show resolved Hide resolved
src/Locales/it/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/pl/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/pt/Pode.psd1 Outdated Show resolved Hide resolved
src/Locales/zn/Pode.psd1 Outdated Show resolved Hide resolved
@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 15, 2024

Semaphore is translated correctly
It's the English language that uses a strange 2 words (traffic light) for a semaphore :)
Semaphore definition in English https://www.merriam-webster.com/dictionary/semaphore

Scheduler instead has some issue

I'm working to fix the Italian translation because I speak Italian, but for the other language, we have to ask for help from the community on Discord

@Badgerati
Copy link
Owner

Semaphore is translated correctly
It's the English language that uses a strange 2 words (traffic light) for a semaphore :)
Semaphore definition in English https://www.merriam-webster.com/dictionary/semaphore

Fair enough!

I was not sure to use en-gb as international english or create a new one
anyway I demoted en-us because Pode is born in UK not US :)

fixed the Italian language content
@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 21, 2024

I made en-gb the international English language.
I thought to create a 3rd English resource file just for international, but there are no words that we use that differ from the GB ones

Italian psd1 is good to go

@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 21, 2024

let me know what you want to do we the other languages

@Badgerati Badgerati linked an issue Jun 24, 2024 that may be closed by this pull request
@Badgerati
Copy link
Owner

All looks good to me :)

For the other languages outside of EN/IT: I have run them all through Google Translate, and they did all come back seemingly valid. If we want that extra level of assurance, we can ask on Discord if anyone is able to review the translations they know.

Otherwise, I happy to approve/merge if there's nothing else to commit

@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 24, 2024

I don't have anything to add here.
The only thing that remain is to ask for help with the translation.

Copy link
Owner

@Badgerati Badgerati left a comment

Choose a reason for hiding this comment

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

All work outside of the translations is good. We've a relatively high degree of confidence in the translation themselves, and we can fix after if anything is found.

@Badgerati Badgerati merged commit edfff96 into Badgerati:develop Jul 2, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhanced Internationalization Support (i18n)
2 participants