More localization friendly source strings and context commenting

src/AppInstallerCLICore/ChannelStreams.h

@@ -11,36 +11,6 @@
 
 namespace AppInstaller::CLI::Execution
 {
-    namespace details
-    {
-        // List of approved types for output, others are potentially not localized.
-        template <typename T>
-        struct IsApprovedForOutput
-        {
-            static constexpr bool value = false;
-        };
-
-#define WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(_t_) \
-        template <> \
-        struct IsApprovedForOutput<_t_> \
-        { \
-            static constexpr bool value = true; \
-        }
-
-        // It is assumed that single char values need not be localized, as they are matched
-        // ordinally or they are punctuation / other.
-        WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(char);
-        // Localized strings (and from an Id for one for convenience).
-        WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(Resource::StringId);
-        WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(Resource::LocString);
-        // Strings explicitly declared as localization independent.
-        WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(Utility::LocIndView);
-        WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(Utility::LocIndString);
-        // Normalized strings come from user data and should therefore already by localized
-        // by how they are chosen (or there is no localized version).
-        WINGET_CREATE_ISAPPROVEDFOROUTPUT_SPECIALIZATION(Utility::NormalizedString);
-    }
-
     // The base stream for all channels.
     struct BaseStream
     {
@@ -102,7 +72,8 @@ namespace AppInstaller::CLI::Execution
             // * If your string came from outside of the source code, it is best to store it in a
             //      Utility::NormalizedString so that it has a normalized representation. This also
             //      informs the output that there is no localized version to use.
-            // TODO: Convert the rest of the code base and uncomment to enforce localization.
+            // TODO: This assertion is currently only applied to placeholders in localized strings.
+            //       Convert the rest of the code base and uncomment to enforce localization.
             //static_assert(details::IsApprovedForOutput<std::decay_t<T>>::value, "This type may not be localized, see comment for more information");
             if (m_enabled)
             {

src/AppInstallerCLICore/Command.cpp

@@ -11,18 +11,7 @@ using namespace AppInstaller::Settings;
 
 namespace AppInstaller::CLI
 {
-    constexpr std::string_view s_Command_ArgName_SilentAndInteractive = "silent|interactive"sv;
-
-    const Utility::LocIndString CommandException::Message() const
-    {
-        if (m_replace)
-        {
-            return Utility::LocIndString{ Utility::FindAndReplaceMessageToken(m_message, m_replace.value()) };
-        }
-
-        // Fall back to just using the message.
-        return Utility::LocIndString{ m_message.get() };
-    }
+    constexpr Utility::LocIndView s_Command_ArgName_SilentAndInteractive = "silent|interactive"_liv;
 
     Command::Command(
         std::string_view name,
@@ -49,9 +38,8 @@ namespace AppInstaller::CLI
 
     void Command::OutputIntroHeader(Execution::Reporter& reporter) const
     {
-        reporter.Info() <<
-            (Runtime::IsReleaseBuild() ? Resource::String::WindowsPackageManager : Resource::String::WindowsPackageManagerPreview) << " v"_liv << Runtime::GetClientVersion() << std::endl <<
-            Resource::String::MainCopyrightNotice << std::endl;
+        auto productName = Runtime::IsReleaseBuild() ? Resource::String::WindowsPackageManager : Resource::String::WindowsPackageManagerPreview;
+        reporter.Info() << productName(Runtime::GetClientVersion()) << std::endl << Resource::String::MainCopyrightNotice << std::endl;
     }
 
     void Command::OutputHelp(Execution::Reporter& reporter, const CommandException* exception) const
@@ -63,29 +51,7 @@ namespace AppInstaller::CLI
         // Error if given
         if (exception)
         {
-            auto error = reporter.Error();
-            error << exception->Message();
-
-            if (!exception->Params().empty())
-            {
-                error << " :"_liv;
-                bool first = true;
-                for (const auto& param : exception->Params())
-                {
-                    if (first)
-                    {
-                        first = false;
-                    }
-                    else
-                    {
-                        error << ',';
-                    }
-                    error << " '"_liv << param << '\'';
-                }
-            }
-
-            error << std::endl <<
-                std::endl;
+            reporter.Error() << exception->Message() << std::endl << std::endl;
         }
 
         // Description
@@ -115,7 +81,7 @@ namespace AppInstaller::CLI
         }
 
         // Output the command preamble and command chain
-        infoOut << Resource::String::Usage << ": winget"_liv << Utility::LocIndView{ commandChain };
+        infoOut << Resource::String::Usage("winget"_liv, Utility::LocIndView{ commandChain });
 
         auto commandAliases = Aliases();
         auto commands = GetVisibleCommands();
@@ -280,10 +246,10 @@ namespace AppInstaller::CLI
         }
 
         // Finally, the link to the documentation pages
-        std::string helpLink = HelpLink();
+        auto helpLink = Utility::LocIndString{ HelpLink() };
         if (!helpLink.empty())
         {
-            infoOut << std::endl << Resource::String::HelpLinkPreamble << ' ' << helpLink << std::endl;
+            infoOut << std::endl << Resource::String::HelpLinkPreamble(helpLink) << std::endl;
         }
     }
 
@@ -314,7 +280,7 @@ namespace AppInstaller::CLI
                 {
                     auto feature = ExperimentalFeature::GetFeature(command->Feature());
                     AICLI_LOG(CLI, Error, << "Trying to use command: " << *itr << " without enabling feature " << feature.JsonName());
-                    throw CommandException(Resource::String::FeatureDisabledMessage, feature.JsonName());
+                    throw CommandException(Resource::String::FeatureDisabledMessage(feature.JsonName()));
                 }
 
                 if (!Settings::GroupPolicies().IsEnabled(command->GroupPolicy()))
@@ -331,7 +297,7 @@ namespace AppInstaller::CLI
         }
 
         // TODO: If we get to a large number of commands, do a fuzzy search much like git
-        throw CommandException(Resource::String::UnrecognizedCommand, *itr);
+        throw CommandException(Resource::String::UnrecognizedCommand(Utility::LocIndView{ *itr }));
     }
 
     // The argument parsing state machine.
@@ -367,14 +333,14 @@ namespace AppInstaller::CLI
             const std::optional<Execution::Args::Type>& Type() const { return m_type; }
 
             // The actual argument string associated with Type.
-            const std::string& Arg() const { return m_arg; }
+            const Utility::LocIndString& Arg() const { return m_arg; }
 
             // If set, indicates that the last argument produced an error.
             const std::optional<CommandException>& Exception() const { return m_exception; }
 
         private:
             std::optional<Execution::Args::Type> m_type;
-            std::string m_arg;
+            Utility::LocIndString m_arg;
             std::optional<CommandException> m_exception;
         };
 
@@ -432,7 +398,7 @@ namespace AppInstaller::CLI
         // If the next argument was to be a value, but none was provided, convert it to an exception.
         else if (m_state.Type() && m_invocationItr == m_invocation.end())
         {
-            throw CommandException(Resource::String::MissingArgumentError, m_state.Arg());
+            throw CommandException(Resource::String::MissingArgumentError(m_state.Arg()));
         }
     }
 
@@ -472,7 +438,7 @@ namespace AppInstaller::CLI
     //  4. If the argument is only a double --, all further arguments are only considered as positional.
     ParseArgumentsStateMachine::State ParseArgumentsStateMachine::StepInternal()
     {
-        std::string_view currArg = *m_invocationItr;
+        auto currArg = Utility::LocIndView{ *m_invocationItr };
         ++m_invocationItr;
 
         // If the previous step indicated a value was needed, set it and forget it.
@@ -488,15 +454,15 @@ namespace AppInstaller::CLI
             const CLI::Argument* nextPositional = NextPositional();
             if (!nextPositional)
             {
-                return CommandException(Resource::String::ExtraPositionalError, currArg);
+                return CommandException(Resource::String::ExtraPositionalError(currArg));
             }
 
             m_executionArgs.AddArg(nextPositional->ExecArgType(), currArg);
         }
         // The currentArg must not be empty, and starts with a -
         else if (currArg.length() == 1)
         {
-            return CommandException(Resource::String::InvalidArgumentSpecifierError, currArg);
+            return CommandException(Resource::String::InvalidArgumentSpecifierError(currArg));
         }
         // Now it must be at least 2 chars
         else if (currArg[1] != APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR)
@@ -507,7 +473,7 @@ namespace AppInstaller::CLI
             auto itr = std::find_if(m_arguments.begin(), m_arguments.end(), [&](const Argument& arg) { return (currChar == arg.Alias()); });
             if (itr == m_arguments.end())
             {
-                return CommandException(Resource::String::InvalidAliasError, currArg);
+                return CommandException(Resource::String::InvalidAliasError(currArg));
             }
 
             if (itr->Type() == ArgumentType::Flag)
@@ -521,11 +487,11 @@ namespace AppInstaller::CLI
                     auto itr2 = std::find_if(m_arguments.begin(), m_arguments.end(), [&](const Argument& arg) { return (currChar == arg.Alias()); });
                     if (itr2 == m_arguments.end())
                     {
-                        return CommandException(Resource::String::AdjoinedNotFoundError, currArg);
+                        return CommandException(Resource::String::AdjoinedNotFoundError(currArg));
                     }
                     else if (itr2->Type() != ArgumentType::Flag)
                     {
-                        return CommandException(Resource::String::AdjoinedNotFlagError, currArg);
+                        return CommandException(Resource::String::AdjoinedNotFlagError(currArg));
                     }
                     else
                     {
@@ -541,7 +507,7 @@ namespace AppInstaller::CLI
                 }
                 else
                 {
-                    return CommandException(Resource::String::SingleCharAfterDashError, currArg);
+                    return CommandException(Resource::String::SingleCharAfterDashError(currArg));
                 }
             }
             else
@@ -584,7 +550,7 @@ namespace AppInstaller::CLI
                     {
                         if (hasValue)
                         {
-                            return CommandException(Resource::String::FlagContainAdjoinedError, currArg);
+                            return CommandException(Resource::String::FlagContainAdjoinedError(currArg));
                         }
 
                         m_executionArgs.AddArg(arg.ExecArgType());
@@ -604,7 +570,7 @@ namespace AppInstaller::CLI
 
             if (!argFound)
             {
-                return CommandException(Resource::String::InvalidNameError, currArg);
+                return CommandException(Resource::String::InvalidNameError(currArg));
             }
         }
 
@@ -661,36 +627,36 @@ namespace AppInstaller::CLI
             {
                 auto setting = Settings::AdminSettingToString(arg.AdminSetting());
                 AICLI_LOG(CLI, Error, << "Trying to use argument: " << arg.Name() << " disabled by admin setting " << setting);
-                throw CommandException(Resource::String::FeatureDisabledByAdminSettingMessage, Utility::LocIndView{ setting }, {});
+                throw CommandException(Resource::String::FeatureDisabledByAdminSettingMessage(setting));
             }
 
             if (!ExperimentalFeature::IsEnabled(arg.Feature()) && execArgs.Contains(arg.ExecArgType()))
             {
                 auto feature = ExperimentalFeature::GetFeature(arg.Feature());
                 AICLI_LOG(CLI, Error, << "Trying to use argument: " << arg.Name() << " without enabling feature " << feature.JsonName());
-                throw CommandException(Resource::String::FeatureDisabledMessage, feature.JsonName());
+                throw CommandException(Resource::String::FeatureDisabledMessage(feature.JsonName()));
             }
 
             if (arg.Required() && !execArgs.Contains(arg.ExecArgType()))
             {
-                throw CommandException(Resource::String::RequiredArgError, arg.Name());
+                throw CommandException(Resource::String::RequiredArgError(arg.Name()));
             }
 
             if (arg.Limit() < execArgs.GetCount(arg.ExecArgType()))
             {
-                throw CommandException(Resource::String::TooManyArgError, arg.Name());
+                throw CommandException(Resource::String::TooManyArgError(arg.Name()));
             }
         }
 
         if (execArgs.Contains(Execution::Args::Type::Silent) && execArgs.Contains(Execution::Args::Type::Interactive))
         {
-            throw CommandException(Resource::String::TooManyBehaviorsError, s_Command_ArgName_SilentAndInteractive);
+            throw CommandException(Resource::String::TooManyBehaviorsError(s_Command_ArgName_SilentAndInteractive));
         }
 
         if (execArgs.Contains(Execution::Args::Type::CustomHeader) && !execArgs.Contains(Execution::Args::Type::Source) &&
            !execArgs.Contains(Execution::Args::Type::SourceName))
         {
-            throw CommandException(Resource::String::HeaderArgumentNotApplicableWithoutSource, Argument::ForType(Execution::Args::Type::CustomHeader).Name());
+            throw CommandException(Resource::String::HeaderArgumentNotApplicableWithoutSource(Argument::ForType(Execution::Args::Type::CustomHeader).Name()));
         }
 
         if (execArgs.Contains(Execution::Args::Type::Count))
@@ -719,15 +685,17 @@ namespace AppInstaller::CLI
                 {
                     applicableArchitectures.emplace_back(Utility::ToString(i));
                 }
-                throw CommandException(Resource::String::InvalidArgumentValueError, Argument::ForType(Execution::Args::Type::InstallArchitecture).Name(), std::forward<std::vector<Utility::LocIndString>>((applicableArchitectures)));
+
+                auto validOptions = Utility::Join(", "_liv, applicableArchitectures);
+                throw CommandException(Resource::String::InvalidArgumentValueError(Argument::ForType(Execution::Args::Type::InstallArchitecture).Name(), validOptions));
             }
         }
 
         if (execArgs.Contains(Execution::Args::Type::Locale))
         {
             if (!Locale::IsWellFormedBcp47Tag(execArgs.GetArg(Execution::Args::Type::Locale)))
             {
-                throw CommandException(Resource::String::InvalidArgumentValueErrorWithoutValidValues, Argument::ForType(Execution::Args::Type::Locale).Name(), {});
+                throw CommandException(Resource::String::InvalidArgumentValueErrorWithoutValidValues(Argument::ForType(Execution::Args::Type::Locale).Name()));
             }
         }
 

src/AppInstallerCLICore/Command.h

@@ -23,25 +23,10 @@ namespace AppInstaller::CLI
     struct CommandException
     {
         CommandException(Resource::LocString message) : m_message(std::move(message)) {}
-
-        // The message should be a localized string.
-        // The parameters can be either localized or not.
-        // We 'convert' the param to a localization independent view here if needed.
-        CommandException(Resource::LocString message, Resource::LocString param) : m_message(std::move(message)), m_params({ param }) {}
-        CommandException(Resource::LocString message, std::string_view param) : m_message(std::move(message)), m_params({ Utility::LocIndString{ param } }) {}
-
-        // The message should be a localized string, but the replacement and parameters are not.
-        // This supports replacing %1 in the message with the replace value.
-        CommandException(Resource::LocString message, Utility::LocIndView replace, std::vector<Utility::LocIndString>&& params) :
-            m_message(std::move(message)), m_replace(replace), m_params(std::move(params)) {}
-
-        const Utility::LocIndString Message() const;
-        const std::vector<Utility::LocIndString>& Params() const { return m_params; }
+        const Utility::LocIndString Message() const { return m_message; }
 
     private:
         Resource::LocString m_message;
-        std::optional<Utility::LocIndString> m_replace;
-        std::vector<Utility::LocIndString> m_params;
     };
 
     // Flags to control the behavior of the command output.

src/AppInstallerCLICore/Commands/FeaturesCommand.cpp

@@ -53,7 +53,7 @@ namespace AppInstaller::CLI
             {
                 table.OutputLine({
                     std::string{ feature.Name() },
-                    Resource::Loader::Instance().ResolveString(ExperimentalFeature::IsEnabled(feature.GetFeature()) ? Resource::String::FeaturesEnabled : Resource::String::FeaturesDisabled),
+                    Resource::LocString{ ExperimentalFeature::IsEnabled(feature.GetFeature()) ? Resource::String::FeaturesEnabled : Resource::String::FeaturesDisabled},
                     std::string { feature.JsonName() },
                     std::string{ feature.Link() } });
             }

src/AppInstallerCLICore/Commands/InstallCommand.cpp

@@ -117,7 +117,8 @@ namespace AppInstaller::CLI
         {
             if (ConvertToScopeEnum(execArgs.GetArg(Args::Type::InstallScope)) == Manifest::ScopeEnum::Unknown)
             {
-                throw CommandException(Resource::String::InvalidArgumentValueError, s_ArgumentName_Scope, { "user"_lis, "machine"_lis });
+                auto validOptions = Utility::Join(", "_liv, std::vector<Utility::LocIndString>{ "user"_lis, "machine"_lis});
+                throw CommandException(Resource::String::InvalidArgumentValueError(s_ArgumentName_Scope, validOptions));
             }
         }
     }

src/AppInstallerCLICore/Commands/RootCommand.cpp

@@ -187,15 +187,15 @@ namespace AppInstaller::CLI
             info << std::endl <<
                 "Windows: "_liv << Runtime::GetOSVersion() << std::endl;
 
-            info << Resource::String::SystemArchitecture << ": "_liv << Utility::ToString(Utility::GetSystemArchitecture()) << std::endl;
+            info << Resource::String::SystemArchitecture(Utility::ToString(Utility::GetSystemArchitecture())) << std::endl;
 
             if (Runtime::IsRunningInPackagedContext())
             {
-                info << Resource::String::Package << ": "_liv << Runtime::GetPackageVersion() << std::endl;
+                info << Resource::String::Package(Runtime::GetPackageVersion()) << std::endl;
             };
 
-            info << std::endl << Resource::String::Logs << ": "_liv << Runtime::GetPathTo(Runtime::PathName::DefaultLogLocationForDisplay).u8string() << std::endl;
-            info << std::endl << Resource::String::UserSettings << ": "_liv << UserSettings::SettingsFilePath(true).u8string() << std::endl;
+            info << std::endl << Resource::String::Logs(Utility::LocIndView{ Runtime::GetPathTo(Runtime::PathName::DefaultLogLocationForDisplay).u8string() }) << std::endl;
+            info << std::endl << Resource::String::UserSettings(Utility::LocIndView{ UserSettings::SettingsFilePath(true).u8string() }) << std::endl;
 
             info << std::endl;