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

Internationalize Core, CmdLine, ConsoleUI, and AutoUpdater #3482

Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 20, 2021

Background

In #2749 we created the ability to translate strings visible in GUI, so more users could use CKAN in their own languages. This has proven surprisingly successful, with six complete translations submitted so far covering a sizable chunk of the human population (listing them as an excuse to tag the authors for notifications):

Language Pull requests Translators
German (de-DE) #2749 @DasSkelett
Simplified Chinese (zh-CN) #3014, #3053 @050644zf, @Fierce-Cat
French (fr-FR) #3272 @vinix38
Brazilian Portuguese (pt-BR) #3340 @gsantos9489
Russian (ru-RU) #3383 @nt0g
Japanese (ja-JP) #3394 @utah239
Korean (ko-KR) #3606 @kingnoob1377

Motivation

GUI still displays many untranslated strings from the Core project, especially with regard to dependencies, conflicts, and installing, which can be important when there is a problem to report to the user. Now that #3471 has resolved a long-standing formatting issue related to translations, it should be safe to translate more strings. And if we translate Core, then it would be weird to view localized Core strings via an untranslated CmdLine or ConsoleUI front end. CmdLine probably has very few users, but most Mac users nowadays are probably on ConsoleUI.

Changes

  • GUI is moved into the CKAN.GUI namespace to prevent collisions with the other projects, which each already have their own namespaces
    • GUI's resources are moved from CKAN.Properties.Resources to CKAN.GUI.Properties.Resources so Core can rightfully claim the resources of the root namespace
  • User-visible strings in Core, CmdLine, ConsoleUI, and AutoUpdater are moved into each project's Properties/Resources.resx file (yes, I did this by hand, one string at a time), accessible via static properties on Properties.Resources, just like for GUI, so translations can now be created for these projects as well
    • The default English spellings now use en-GB, with occasional en-US strings (currently only deserialization and license) moved into that locale and replaced by their UK equivalents in the default locale
    • Each project has its own copy of SingleAssemblyResourceManager to make sure its resources can be found in the repacked EXE (maybe this should be shared, but I'm focused on getting it all done and working now)
  • build.cake now gets the resource DLLs from _build/out/CKAN-CmdLine/<configuration>/bin/net45 (the compiler puts them here thanks to CmdLine's project references to Core, CmdLine, and ConsoleUI)
  • The translations for GUI.MainChangesetNewInstall are moved to Core.RelationshipResolverUserReason and the weird GUI hack around that resource is removed
  • AutoUpdater is now built to CKAN-AutoUpdateHelper.exe and repacked to AutoUpdater.exe (which cannot be changed because the previous release will look for that name on GitHub) including all of its resource DLLs, using the ILRepack MSBuild Task
    • Its namespace is changed from AutoUpdater to CKAN.AutoUpdateHelper, because CKAN.AutoUpdate is a class in Core

I expect we may have some glitches as in #2899, but I do not use JetBrains Idea Rider, so hopefully we can work through those once they are identified.

The translation instructions are here and will need to be updated after merge to reflect the additional .resx files:

All past, present, and future translators are gratefully invited to start hacking away at the new .resx files! Recommended ways to collaborate:

Submitting your changes via git commits will ensure that your changes remain linked to your user name so everyone can see that you helped out! Thanks in advance for any contributions!

Tracking the translations Included in this PR:

Language Core? CmdLine? ConsoleUI? AutoUpdater?
German (de-DE)
Simplified Chinese (zh-CN) ✔️
French (fr-FR) ✔️ ✔️ ✔️ ✔️
Brazilian Portuguese (pt-BR)
Russian (ru-RU) ✔️ ✔️ ✔️ ✔️
Japanese (ja-JP)
Korean (ko-KR)

Exceptions and notes

A few things are not internationalized:

  • Help strings in CmdLine, which are set statically at compile time and can't be easily overridden (as far as I know, but I didn't dig too deeply into this), something to resolve in the future
  • ConsoleUI's YesNoDialog will show translations on the buttons but still responds to Y and N on the keyboard, because ConsoleUI's key bindings use full ConsoleKeyInfo objects to specify the key, which I do not know how to encode into a .resx file
  • The metadata about the mods (their names, descriptions, etc.) is not translated and probably never will be, because that would be nearly impossible to coordinate and manage

A few things need to be translated carefully:

  • For CmdLine, Properties.Resources.UserYesNoPromptSuffix, Properties.Resources.UserYesNoY, Properties.Resources.UserYesNoYes, Properties.Resources.UserYesNoN, Properties.Resources.UserYesNoNo, and Properties.Resources.UserYesNoInvalid must all match up! These strings are not just displayed but also compared to user input; we are involving the translator a bit in the UI design/function here so German users can say Ja oder Nein.
  • Also for CmdLine, Properties.Resources.UserSelectionPromptWithDefault and Properties.Resources.UserSelectionPromptWithoutDefault must match Properties.Resources.UserSelectionC and Properties.Resources.UserSelectionN

Side fixes

  • CmdLine and ConsoleUI now use the newer csproj format that doesn't require listing every source file
  • CmdLine's assembly name is changed from CmdLine to CKAN-CmdLine to match CKAN-GUI and CKAN-ConsoleUI
  • RelationshipDescriptor.RequiredVersion wasn't being used and is removed
  • Core/Types/ModuleResolution.cs wasn't being used and is removed
  • doc/building.md no longer mentions Travis, which has been out of use for quite a while
  • ckan help no longer prints useless help - and Usage: ckan help [options] lines at the start
  • A few more hard-coded "KSP" strings are updated to IGame.ShortName
  • The ConsoleUI/Properties.Settings.* files were not being used and are removed
  • The header for ckan verb --help for simple commands previously started with verb - description and is now ckan verb - description
  • A few instances of string.Format are removed where the result is passed to RaiseMessage or RaiseError, which do their own identical formatting (and I think I accidentally created an instance of this, and I look forward to it being spotted in the diffs)

@HebaruSan HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Pull request ConsoleUI Issues affecting the interactive console UI i18n Issues regarding the internationalization of CKAN and PRs that need translating labels Nov 20, 2021
@HebaruSan

This comment has been minimized.

@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch from b90ae70 to 35a5ea6 Compare November 21, 2021 00:10
@HebaruSan HebaruSan changed the title Internationalize Core, Cmdline, and ConsoleUI Internationalize Core, CmdLine, and ConsoleUI Nov 21, 2021
@HebaruSan

This comment has been minimized.

@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch 3 times, most recently from 6fcfc29 to b8dd543 Compare November 21, 2021 17:01
@HebaruSan
Copy link
Member Author

HebaruSan commented Nov 21, 2021

OK, the test updates are done and it should be safe to pull and rebase now.

The new tests are probably unnecessary, as Properties.Resources files are not applied in a way that uses the $this. syntax (i.e., there is no control that would have its .Language property set by ApplyResources(this, "$this");), but it still seems like something worth checking to me since it may still be possible for stray <data> elements to be created that would not be compiled correctly.

@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch from bef9363 to 96155bd Compare November 22, 2021 19:06
@HebaruSan HebaruSan changed the title Internationalize Core, CmdLine, and ConsoleUI Internationalize Core, CmdLine, ConsoleUI, and AutoUpdater Nov 22, 2021
@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch 2 times, most recently from f4fa019 to 4435467 Compare November 23, 2021 23:34
@HebaruSan

This comment was marked as resolved.

@vinix38
Copy link
Contributor

vinix38 commented Nov 24, 2021

The French translation is now done, but I will spend some more time proofreading and checking if everything fits as intended :)

Copy link
Contributor

@vinix38 vinix38 left a comment

Choose a reason for hiding this comment

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

French translation is good to serve 😁

@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch from 8d3a0d5 to 9df9ac8 Compare January 19, 2022 23:51
@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch from 9df9ac8 to 65223eb Compare April 12, 2022 04:40
@HebaruSan

This comment was marked as outdated.

@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch 3 times, most recently from 20832f6 to 016a224 Compare July 19, 2022 14:47
@HebaruSan HebaruSan force-pushed the feature/i18n-core-cmdline-consoleui branch from 016a224 to 1b1d181 Compare August 8, 2022 19:34
@HebaruSan
Copy link
Member Author

Rebased to resolve conflicts with #3515.

Copy link
Member

@techman83 techman83 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 given a cursory read over, all looks ok with the following extra assumptions:

  • Build changes included to support Internationalisation work?
  • Renaming of ksp -> instance part of an ongoing generalisation of the codebase for multigame

<Compile Include="Main.cs" />
<Compile Include="Options.cs" />
<Compile Include="ProgressReporter.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
Copy link
Member

Choose a reason for hiding this comment

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

Assume the optimisations make manually defining these not necessary?

Copy link
Member Author

@HebaruSan HebaruSan Aug 9, 2022

Choose a reason for hiding this comment

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

There are two main styles of .csproj file; the older one requires specifying all the files, but the newer one scans whatever files are in the directory instead. We're switching from the old style to new style, as was previously done for Core, since it's much more convenient for maintenance.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think one of these lines pulls in the standard rules for finding all the files:

<Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />

<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />

@HebaruSan
Copy link
Member Author

  • Build changes included to support Internationalisation work?

Yes, build.cake is updated to repack all the new resource DLLs into the final ckan.exe.

  • Renaming of ksp -> instance part of an ongoing generalisation of the codebase for multigame

Yup, we did a pass of those changes already but there are still a few left. I try to catch them when I'm changing something nearby.

@HebaruSan HebaruSan merged commit 4296d75 into KSP-CKAN:master Aug 9, 2022
@HebaruSan HebaruSan deleted the feature/i18n-core-cmdline-consoleui branch August 9, 2022 04:39
@gsantos9489
Copy link
Contributor

Hello Guys, it's good to see you again.
I had a period of time out of the digital world.
I'd like to participate again contributing more with the brazilian portuguese translation.

Glad to see that the translation project moved to Crowdin, I'll made my contributions there.

Best regards.

@HebaruSan
Copy link
Member Author

Welcome back, @gsantos9489, and it's great to hear from you as well!

The Crowdin thing is fantastic, but FYI we have not yet fully integrated it into our work flows. I need to get on #3653 to figure out how to do that. It's probably time to give up hope of finding a standard XML diffing solution and fall back to making our own custom utility.

@gsantos9489
Copy link
Contributor

Well, I'll keep myself updated about the translation work flow and read about Crowdin <-> GitHub integration.
If the Crowding solution integration doesn't work at the end... We do like the old days, manual work will do the job (lol).

@HebaruSan
Copy link
Member Author

If the Crowding solution integration doesn't work at the end...

Oh don't worry, it will work; it's just a matter of figuring out how to check what is in that first set of changes so they can be merged safely.

@HebaruSan
Copy link
Member Author

✔️ We now have a script to filter out non-significant changes from diffs, so keeping up with Crowdin should be a lot easier:

#!/bin/bash
# Find differences in string resources between differently formatted XML files
# DEPENDENCIES:
# sudo apt install xmlstarlet html-xml-utils libxml2-utils
FROM=${1:-master}
TO=${2:-HEAD}
function xnorm()
{
# Strip comments
# Remove boilerplate header elements
# Standardize indentation
cat | xmlstarlet canonic --without-comments - \
| hxremove 'schema' , resheader \
| xmllint --noblanks --format --encode UTF-8 -
}
for F in $(git diff --name-only $FROM $TO | grep resx)
do
if ! OUT=$(diff -U 0 --label $FROM \
--label $TO \
<(git show $FROM:$F | xnorm) \
<(git show $TO:$F | xnorm))
then
echo "${F}"
echo "${OUT}"
echo
fi
done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmdline Issues affecting the command line ConsoleUI Issues affecting the interactive console UI Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI i18n Issues regarding the internationalization of CKAN and PRs that need translating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants