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

More localization friendly source strings and context commenting #2454

Merged
merged 60 commits into from
Dec 16, 2022

Conversation

AmelBawa-msft
Copy link
Contributor

@AmelBawa-msft AmelBawa-msft commented Aug 16, 2022

What was done?

  • Added support for string localization with placeholders. For example:
<data name="CommandArgumentDescription" xml:space="preserve">
    <value>Filter results by command</value>
</data>
<data name="InvalidArgumentValueError" xml:space="preserve">
    <value>The value provided for the `{0}` argument is invalid; valid values are: {1}</value>
    <comment>Error message displayed when the user provides an invalid command line argument value. {0} is a placeholder replaced by the argument name. {1} is a placeholder replaced by a list of valid options.</comment>
</data>
info << String::CommandArgumentDescription;                           // => Filter results by command
info << String::InvalidArgumentValueError("First"_liv, "Second"_liv); // => The value provided for the `First` argument is invalid; valid values are: Second
  • Applied the localization placeholder change to most use cases in the code. Some remaining use cases (e.g. ShowFlow.cpp) will be handled separately as it contains localized strings with a special rendering style (e.g. text printed in bold, different color, etc...).
  • Updated existing unit tests
  • Documented strings with placeholders in the winget.resw file.

Test

  • Tested locally a randomly selected set of localized strings with placeholders.
  • Added unit tests for newly added functions
Microsoft Reviewers: Open in CodeFlow

Comment on lines 26 to 29
template<typename ...T>
Utility::LocIndString operator()(T ... args) const;
private:
std::string Resolve() const;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I don't think the comment "Resolve the string id" is saying much for the Resolve() function... I'd prefer the comment to explain what "resolve" means or what it resolves to. Something like "resolves the string ID to the localized string it represents, keeping replacement tokens"

src/AppInstallerCLICore/ChannelStreams.h Show resolved Hide resolved
src/AppInstallerCLICore/Command.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Command.cpp Show resolved Hide resolved
src/AppInstallerCLITests/WorkFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLITests/WorkFlow.cpp Show resolved Hide resolved
src/AppInstallerCLITests/Completion.cpp Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

Copy link
Contributor

@yao-msft yao-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

@AmelBawa-msft AmelBawa-msft merged commit 66b851f into microsoft:master Dec 16, 2022
@AmelBawa-msft AmelBawa-msft deleted the localization-format branch December 16, 2022 01:16
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.

5 participants