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] Move rc files to resx #6057

Merged
merged 37 commits into from
Aug 24, 2020

Conversation

arjunbalgovind
Copy link
Contributor

@arjunbalgovind arjunbalgovind commented Aug 19, 2020

Summary of the Pull Request

What is this about?
This PR adds in the following two scripts for resource conversion:

  • convert-resx-to-rc.ps1: To convert resx files in a folder to a single rc file with language specific conditioning. This will be run pre-build for each C++ project with rc file.
  • convert-stringtable-to-resx.ps1: To convert a file containing the rows of a string table to a resx file. Since this script is to be used only once while creating the resx file the script is not very robust, i.e. the input is required in a very specific format.

In addition to the scripts, resx files have been added and rc files replaced for the following projects:

  • ColorPicker (C++)
  • ImageResizerExt
  • Microsoft.Launcher (C++)
  • Keyboard Manager
  • powerpreview

For the above projects, LocProject.json files were added to enable CDPX localization

PR Checklist

Info on Pull Request

What does this include?

  • The conversion script uses resgen which is a VS tool to extract the resources from a resx to a plaintext file and vice versa.
  • The resx to rc file script reads all the resx files in the argument directory and adds in string tables to a base rc file, and adds the language specific rc file syntax based on the resx file name. The language identifiers have been hardcoded in, since localization will be done on a limited set of languages and PowerShell doesn't have appropriate APIs to auto-generate this information.
  • The files are added to a Generated Files folder so that they are skipped by .gitignore
  • A base resource.h and base rc file is required in order to add any basic syntax for the file as well as non-localizable resources.
  • The script generates new .h and .rc files by adding the string tables to these base files in the generated files folder.
  • The script has try-catch blocks in case a file read/write fails because of the file being in use by another process (such as VS)
  • The script checks if the content to be written to the file is different from the existing file to avoid rewriting. This avoids unnecessary building of files which use resources when there is no file change on incremental builds.

Pending work

  • The same work will be done for FZ, PowerRename and SG in separate PRs to avoid conflicts with other PRs.

Validation Steps Performed

How does someone test & validate?

  • Validated that the solution builds, and the installer builds.
  • Validated that existing French resources in ImageResizerExt are loaded on switching OS language
  • Validated with a duplicate resx file for French with a few strings modified that it works for Keyboard Manager as well.
  • Validated that english strings are loaded as a fallback when using French OS language if the resources dont exist.
  • Validated on CDPX pipeline that all the localized resx files are created, and the MSI works as expected

@arjunbalgovind arjunbalgovind added the Area-Localization issues regarding to Localization the application label Aug 19, 2020
@arjunbalgovind arjunbalgovind changed the base branch from master to user/arbalgov/cdpxLoc August 19, 2020 23:51
@arjunbalgovind arjunbalgovind changed the base branch from user/arbalgov/cdpxLoc to master August 19, 2020 23:51
@enricogior
Copy link
Contributor

I would suggest to split this PR since we have work in progress with FZ and this needs to be coordinated to minimize conflicts.

@arjunbalgovind
Copy link
Contributor Author

I would suggest to split this PR since we have work in progress with FZ and this needs to be coordinated to minimize conflicts.

I will limit the changes in this PR to all the modules except FZ, PowerRename and Shortcut Guide. Once this is complete I'll make separate PRs for them.

@arjunbalgovind arjunbalgovind requested review from enricogior and a team August 21, 2020 01:45
@arjunbalgovind arjunbalgovind marked this pull request as ready for review August 21, 2020 01:45
@enricogior
Copy link
Contributor

@arjunbalgovind
is the PR ready for review?

@arjunbalgovind
Copy link
Contributor Author

@arjunbalgovind
is the PR ready for review?

@enricogior yes it can be reviewed now

Copy link
Contributor

@ryanbodrug-microsoft ryanbodrug-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM!

@arjunbalgovind arjunbalgovind merged commit f2cfd90 into master Aug 24, 2020
@arjunbalgovind arjunbalgovind deleted the user/arbalgov/resxRCConversion branch August 25, 2020 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Localization issues regarding to Localization the application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants