From a12f1879296a1105dbe37bae02e12778ba0409d6 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 29 Feb 2024 20:16:27 -0600 Subject: [PATCH 01/13] Make Ignore Warnings suppress the output level entirely --- src/AppInstallerCLICore/Argument.cpp | 1 + .../Commands/ValidateCommand.cpp | 1 - src/AppInstallerCLICore/Core.cpp | 6 ++++++ src/AppInstallerCLICore/ExecutionReporter.cpp | 5 +++++ src/AppInstallerCLICore/ExecutionReporter.h | 20 +++++++++++++++++++ 5 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 7497468840..976ff3fe3f 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -393,6 +393,7 @@ namespace AppInstaller::CLI args.push_back(ForType(Args::Type::RainbowStyle)); args.push_back(ForType(Args::Type::RetroStyle)); args.push_back(ForType(Args::Type::VerboseLogs)); + args.push_back(ForType(Args::Type::IgnoreWarnings)); args.emplace_back(Args::Type::DisableInteractivity, Resource::String::DisableInteractivityArgumentDescription, ArgumentType::Flag, false); } diff --git a/src/AppInstallerCLICore/Commands/ValidateCommand.cpp b/src/AppInstallerCLICore/Commands/ValidateCommand.cpp index afef0c25ee..b569fb9239 100644 --- a/src/AppInstallerCLICore/Commands/ValidateCommand.cpp +++ b/src/AppInstallerCLICore/Commands/ValidateCommand.cpp @@ -16,7 +16,6 @@ namespace AppInstaller::CLI { return { Argument::ForType(Execution::Args::Type::ValidateManifest), - Argument::ForType(Execution::Args::Type::IgnoreWarnings), }; } diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 89d5c892eb..90bac8cb18 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -126,6 +126,12 @@ namespace AppInstaller::CLI Logging::Log().SetLevel(Logging::Level::Verbose); } + // Disable warnings if requested + if (context.Args.Contains(Execution::Args::Type::IgnoreWarnings)) + { + context.Reporter.SetLevelEnabled(Execution::Reporter::Level::Warning, false); + } + context.UpdateForArgs(); context.SetExecutingCommand(command.get()); command->ValidateArguments(context.Args); diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 5350a87300..e186aac98d 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -54,6 +54,11 @@ namespace AppInstaller::CLI::Execution OutputStream Reporter::GetOutputStream(Level level) { + if (!levelEnabled(level)) + { + return OutputStream(*m_out, false, false); + } + OutputStream result = GetBasicOutputStream(); switch (level) diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index a7909d29c9..d959896b6f 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -165,13 +165,33 @@ namespace AppInstaller::CLI::Execution m_progressSink = sink; } + void SetLevelEnabled(Level reporterLevel, bool setEnabled = true) { + const bool isEnabled = levelEnabled(reporterLevel); + // If the reporter is already in the state we want, do nothing + if (isEnabled == setEnabled) + { + return; + } + if (setEnabled) { + m_enabledLevels.push_back(reporterLevel); + } + else { + m_enabledLevels.erase(std::remove(m_enabledLevels.begin(), m_enabledLevels.end(), reporterLevel), m_enabledLevels.end()); + } + } + private: Reporter(std::shared_ptr outStream, std::istream& inStream); + bool levelEnabled(Level reporterLevel) { + return std::find(m_enabledLevels.begin(), m_enabledLevels.end(), reporterLevel) != m_enabledLevels.end(); + } + // Gets a stream for output for internal use. OutputStream GetBasicOutputStream(); Channel m_channel = Channel::Output; + std::vector m_enabledLevels = { Level::Verbose, Level::Info, Level::Warning, Level::Error }; std::shared_ptr m_out; std::istream& m_in; std::optional m_style; From 5cd5bc9f74c505cc6773b3c60c9b91c25ff57e56 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 29 Feb 2024 20:21:01 -0600 Subject: [PATCH 02/13] Clean up --- src/AppInstallerCLICore/ExecutionReporter.cpp | 1 + src/AppInstallerCLICore/ExecutionReporter.h | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index e186aac98d..6cec904b18 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -54,6 +54,7 @@ namespace AppInstaller::CLI::Execution OutputStream Reporter::GetOutputStream(Level level) { + // If the level is not enabled, return a default stream which is disabled if (!levelEnabled(level)) { return OutputStream(*m_out, false, false); diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index d959896b6f..4b023492e3 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -173,9 +173,11 @@ namespace AppInstaller::CLI::Execution return; } if (setEnabled) { + // Add the level to the list of enabled levels m_enabledLevels.push_back(reporterLevel); } else { + // Remove the level from the list of enabled levels m_enabledLevels.erase(std::remove(m_enabledLevels.begin(), m_enabledLevels.end(), reporterLevel), m_enabledLevels.end()); } } @@ -191,7 +193,6 @@ namespace AppInstaller::CLI::Execution OutputStream GetBasicOutputStream(); Channel m_channel = Channel::Output; - std::vector m_enabledLevels = { Level::Verbose, Level::Info, Level::Warning, Level::Error }; std::shared_ptr m_out; std::istream& m_in; std::optional m_style; @@ -200,6 +201,9 @@ namespace AppInstaller::CLI::Execution wil::srwlock m_progressCallbackLock; std::atomic m_progressCallback; std::atomic m_progressSink; + + // Enable all levels by default + std::vector m_enabledLevels = { Level::Verbose, Level::Info, Level::Warning, Level::Error }; }; // Indirection to enable change without tracking down every place From ff162deea4cf447da146ffcfd4e40f947a91fbca Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Fri, 1 Mar 2024 08:15:13 -0600 Subject: [PATCH 03/13] Move to using flags instead --- src/AppInstallerCLICore/Core.cpp | 2 +- src/AppInstallerCLICore/ExecutionReporter.cpp | 18 ++--- src/AppInstallerCLICore/ExecutionReporter.h | 70 ++++++++++--------- .../Workflows/ConfigurationFlow.cpp | 2 +- .../Workflows/PromptFlow.cpp | 6 +- .../Workflows/WorkflowBase.cpp | 2 +- .../Workflows/WorkflowBase.h | 6 +- 7 files changed, 54 insertions(+), 52 deletions(-) diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 90bac8cb18..1fb5bb1403 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -129,7 +129,7 @@ namespace AppInstaller::CLI // Disable warnings if requested if (context.Args.Contains(Execution::Args::Type::IgnoreWarnings)) { - context.Reporter.SetLevelEnabled(Execution::Reporter::Level::Warning, false); + context.Reporter.SetLevelEnabled(Execution::ReporterLevel::Warning, false); } context.UpdateForArgs(); diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 6cec904b18..3b66b39651 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -52,10 +52,10 @@ namespace AppInstaller::CLI::Execution } } - OutputStream Reporter::GetOutputStream(Level level) + OutputStream Reporter::GetOutputStream(ReporterLevel level) { // If the level is not enabled, return a default stream which is disabled - if (!levelEnabled(level)) + if (WI_AreAllFlagsClear(m_enabledLevels, level)) { return OutputStream(*m_out, false, false); } @@ -64,16 +64,16 @@ namespace AppInstaller::CLI::Execution switch (level) { - case Level::Verbose: + case ReporterLevel::Verbose: result.AddFormat(TextFormat::Default); break; - case Level::Info: + case ReporterLevel::Info: result.AddFormat(TextFormat::Default); break; - case Level::Warning: + case ReporterLevel::Warning: result.AddFormat(TextFormat::Foreground::BrightYellow); break; - case Level::Error: + case ReporterLevel::Error: result.AddFormat(TextFormat::Foreground::BrightRed); break; default: @@ -117,7 +117,7 @@ namespace AppInstaller::CLI::Execution } } - bool Reporter::PromptForBoolResponse(Resource::LocString message, Level level) + bool Reporter::PromptForBoolResponse(Resource::LocString message, ReporterLevel level) { const std::vector options { @@ -167,14 +167,14 @@ namespace AppInstaller::CLI::Execution } } - void Reporter::PromptForEnter(Level level) + void Reporter::PromptForEnter(ReporterLevel level) { auto out = GetOutputStream(level); out << std::endl << Resource::String::PressEnterToContinue << std::endl; m_in.get(); } - std::filesystem::path Reporter::PromptForPath(Resource::LocString message, Level level) + std::filesystem::path Reporter::PromptForPath(Resource::LocString message, ReporterLevel level) { auto out = GetOutputStream(level); diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index 4b023492e3..891cdaf702 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -23,6 +23,18 @@ namespace AppInstaller::CLI::Execution { #define WINGET_OSTREAM_FORMAT_HRESULT(hr) "0x" << Logging::SetHRFormat << hr + // The level for the Output channel. + enum class ReporterLevel : uint32_t + { + None = 0x0, + Verbose = 0x1, + Info = 0x2, + Warning = 0x4, + Error = 0x8, + All = Verbose | Info | Warning | Error, + }; + DEFINE_ENUM_FLAG_OPERATORS(ReporterLevel); + // One of the options available to the users when prompting for something. struct BoolPromptOption { @@ -48,14 +60,14 @@ namespace AppInstaller::CLI::Execution Disabled, }; - // The level for the Output channel. - enum class Level - { - Verbose, - Info, - Warning, - Error, - }; + //// The level for the Output channel. + //enum class Level + //{ + // Verbose, + // Info, + // Warning, + // Error, + //}; Reporter(std::ostream& outStream, std::istream& inStream); Reporter(const Reporter&) = delete; @@ -71,22 +83,22 @@ namespace AppInstaller::CLI::Execution ~Reporter(); // Get a stream for verbose output. - OutputStream Verbose() { return GetOutputStream(Level::Verbose); } + OutputStream Verbose() { return GetOutputStream(ReporterLevel::Verbose); } // Get a stream for informational output. - OutputStream Info() { return GetOutputStream(Level::Info); } + OutputStream Info() { return GetOutputStream(ReporterLevel::Info); } // Get a stream for warning output. - OutputStream Warn() { return GetOutputStream(Level::Warning); } + OutputStream Warn() { return GetOutputStream(ReporterLevel::Warning); } // Get a stream for error output. - OutputStream Error() { return GetOutputStream(Level::Error); } + OutputStream Error() { return GetOutputStream(ReporterLevel::Error); } // Get a stream for outputting completion words. OutputStream Completion() { return OutputStream(*m_out, m_channel == Channel::Completion, false); } // Gets a stream for output of the given level. - OutputStream GetOutputStream(Level level); + OutputStream GetOutputStream(ReporterLevel level); void EmptyLine() { GetBasicOutputStream() << std::endl; } @@ -98,13 +110,13 @@ namespace AppInstaller::CLI::Execution void SetStyle(AppInstaller::Settings::VisualStyle style); // Prompts the user, return true if they consented. - bool PromptForBoolResponse(Resource::LocString message, Level level = Level::Info); + bool PromptForBoolResponse(Resource::LocString message, ReporterLevel level = ReporterLevel::Info); // Prompts the user, continues when Enter is pressed - void PromptForEnter(Level level = Level::Info); + void PromptForEnter(ReporterLevel level = ReporterLevel::Info); // Prompts the user for a path. - std::filesystem::path PromptForPath(Resource::LocString message, Level level = Level::Info); + std::filesystem::path PromptForPath(Resource::LocString message, ReporterLevel level = ReporterLevel::Info); // Used to show indefinite progress. Currently an indefinite spinner is the form of // showing indefinite progress. @@ -165,30 +177,20 @@ namespace AppInstaller::CLI::Execution m_progressSink = sink; } - void SetLevelEnabled(Level reporterLevel, bool setEnabled = true) { - const bool isEnabled = levelEnabled(reporterLevel); - // If the reporter is already in the state we want, do nothing - if (isEnabled == setEnabled) + void SetLevelEnabled(ReporterLevel reporterLevel, bool setEnabled = true) + { + if (setEnabled) { - return; + WI_SetAllFlags(m_enabledLevels, reporterLevel); } - if (setEnabled) { - // Add the level to the list of enabled levels - m_enabledLevels.push_back(reporterLevel); - } - else { - // Remove the level from the list of enabled levels - m_enabledLevels.erase(std::remove(m_enabledLevels.begin(), m_enabledLevels.end(), reporterLevel), m_enabledLevels.end()); + else + { + WI_ClearAllFlags(m_enabledLevels, reporterLevel); } } private: Reporter(std::shared_ptr outStream, std::istream& inStream); - - bool levelEnabled(Level reporterLevel) { - return std::find(m_enabledLevels.begin(), m_enabledLevels.end(), reporterLevel) != m_enabledLevels.end(); - } - // Gets a stream for output for internal use. OutputStream GetBasicOutputStream(); @@ -203,7 +205,7 @@ namespace AppInstaller::CLI::Execution std::atomic m_progressSink; // Enable all levels by default - std::vector m_enabledLevels = { Level::Verbose, Level::Info, Level::Warning, Level::Error }; + ReporterLevel m_enabledLevels = ReporterLevel::All; }; // Indirection to enable change without tracking down every place diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index 77c1fbd9d1..efaf79ee12 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1064,7 +1064,7 @@ namespace AppInstaller::CLI::Workflow } auto promptString = m_isApply ? Resource::String::ConfigurationWarningPromptApply : Resource::String::ConfigurationWarningPromptTest; - if (!context.Reporter.PromptForBoolResponse(promptString, Reporter::Level::Warning)) + if (!context.Reporter.PromptForBoolResponse(promptString, ReporterLevel::Warning)) { AICLI_TERMINATE_CONTEXT(WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED); } diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp index 45a8542116..f1d6145801 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp @@ -267,7 +267,7 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Info() << Resource::String::InstallersRequireInstallLocation << std::endl; for (auto packageContext : packagesToPrompt) { - *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::Reporter::Level::Warning); + *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::ReporterLevel::Warning); if (packageContext->IsTerminated()) { AICLI_TERMINATE_CONTEXT(packageContext->GetTerminationHR()); @@ -335,7 +335,7 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Warn() << Resource::String::InstallersAbortTerminal << std::endl; for (auto packageContext : packagesToPrompt) { - *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::Reporter::Level::Warning); + *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::ReporterLevel::Warning); if (packageContext->IsTerminated()) { AICLI_TERMINATE_CONTEXT(packageContext->GetTerminationHR()); @@ -354,7 +354,7 @@ namespace AppInstaller::CLI::Workflow return; } - bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PromptToProceed, Execution::Reporter::Level::Warning); + bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PromptToProceed, Execution::ReporterLevel::Warning); if (accepted) { AICLI_LOG(CLI, Info, << "Proceeding with installation"); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index b0045126af..394c33431b 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -46,7 +46,7 @@ namespace AppInstaller::CLI::Workflow std::string_view name, std::string_view id, std::string_view version = {}, - Execution::Reporter::Level level = Execution::Reporter::Level::Info) + Execution::ReporterLevel level = Execution::ReporterLevel::Info) { auto out = context.Reporter.GetOutputStream(level); out << prefix; diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 6595256426..31a2aafc9e 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -365,9 +365,9 @@ namespace AppInstaller::CLI::Workflow // Outputs: None struct ReportManifestIdentityWithVersion : public WorkflowTask { - ReportManifestIdentityWithVersion(Utility::LocIndView prefix, Execution::Reporter::Level level = Execution::Reporter::Level::Info) : + ReportManifestIdentityWithVersion(Utility::LocIndView prefix, Execution::ReporterLevel level = Execution::ReporterLevel::Info) : WorkflowTask("ReportManifestIdentityWithVersion"), m_prefix(prefix), m_level(level) {} - ReportManifestIdentityWithVersion(Resource::StringId label = Resource::String::ReportIdentityFound, Execution::Reporter::Level level = Execution::Reporter::Level::Info) : + ReportManifestIdentityWithVersion(Resource::StringId label = Resource::String::ReportIdentityFound, Execution::ReporterLevel level = Execution::ReporterLevel::Info) : WorkflowTask("ReportManifestIdentityWithVersion"), m_label(label), m_level(level) {} void operator()(Execution::Context& context) const override; @@ -375,7 +375,7 @@ namespace AppInstaller::CLI::Workflow private: Utility::LocIndView m_prefix; std::optional m_label; - Execution::Reporter::Level m_level; + Execution::ReporterLevel m_level; }; // Selects the installer from the manifest, if one is applicable. From 2def2577e7465c046d6c80ee7a6401faf664635b Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Fri, 1 Mar 2024 08:24:58 -0600 Subject: [PATCH 04/13] Fix punctuation --- src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index eba36a6e48..545c185353 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -262,7 +262,7 @@ They can be configured through the settings file 'winget settings'. Filter results by id - Suppresses warning outputs. + Suppresses warning outputs This application is licensed to you by its owner. From cf99419ed68cda74b1dd861dfaa799bd50088731 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Fri, 1 Mar 2024 08:29:04 -0600 Subject: [PATCH 05/13] Cleanup --- src/AppInstallerCLICore/ExecutionReporter.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index 891cdaf702..3b43f305bd 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -60,15 +60,6 @@ namespace AppInstaller::CLI::Execution Disabled, }; - //// The level for the Output channel. - //enum class Level - //{ - // Verbose, - // Info, - // Warning, - // Error, - //}; - Reporter(std::ostream& outStream, std::istream& inStream); Reporter(const Reporter&) = delete; Reporter& operator=(const Reporter&) = delete; From e0e46972cddee243995e96eb57a2dfa7598a2aca Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Sat, 2 Mar 2024 16:00:44 -0600 Subject: [PATCH 06/13] Move enum back to struct + check level for interactivity --- src/AppInstallerCLICore/ChannelStreams.h | 4 ++ src/AppInstallerCLICore/Core.cpp | 2 +- src/AppInstallerCLICore/ExecutionReporter.cpp | 28 ++++++--- src/AppInstallerCLICore/ExecutionReporter.h | 57 +++++++++---------- .../Workflows/ConfigurationFlow.cpp | 2 +- .../Workflows/PromptFlow.cpp | 36 ++++++++---- .../Workflows/WorkflowBase.cpp | 2 +- .../Workflows/WorkflowBase.h | 6 +- 8 files changed, 82 insertions(+), 55 deletions(-) diff --git a/src/AppInstallerCLICore/ChannelStreams.h b/src/AppInstallerCLICore/ChannelStreams.h index 5fa7d77651..9c14550699 100644 --- a/src/AppInstallerCLICore/ChannelStreams.h +++ b/src/AppInstallerCLICore/ChannelStreams.h @@ -94,6 +94,10 @@ namespace AppInstaller::CLI::Execution OutputStream& operator<<(const VirtualTerminal::ConstructedSequence& sequence); OutputStream& operator<<(const std::filesystem::path& path); + inline bool IsEnabled() { + return m_enabled; + } + private: // Applies the format for the stream. void ApplyFormat(); diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index 1fb5bb1403..b4ba3693bc 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -129,7 +129,7 @@ namespace AppInstaller::CLI // Disable warnings if requested if (context.Args.Contains(Execution::Args::Type::IgnoreWarnings)) { - context.Reporter.SetLevelEnabled(Execution::ReporterLevel::Warning, false); + context.Reporter.SetLevelMask(Execution::Reporter::Level::Warning, false); } context.UpdateForArgs(); diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 3b66b39651..1cf5184ffe 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -52,7 +52,7 @@ namespace AppInstaller::CLI::Execution } } - OutputStream Reporter::GetOutputStream(ReporterLevel level) + OutputStream Reporter::GetOutputStream(Level level) { // If the level is not enabled, return a default stream which is disabled if (WI_AreAllFlagsClear(m_enabledLevels, level)) @@ -64,16 +64,16 @@ namespace AppInstaller::CLI::Execution switch (level) { - case ReporterLevel::Verbose: + case Level::Verbose: result.AddFormat(TextFormat::Default); break; - case ReporterLevel::Info: + case Level::Info: result.AddFormat(TextFormat::Default); break; - case ReporterLevel::Warning: + case Level::Warning: result.AddFormat(TextFormat::Foreground::BrightYellow); break; - case ReporterLevel::Error: + case Level::Error: result.AddFormat(TextFormat::Foreground::BrightRed); break; default: @@ -117,7 +117,7 @@ namespace AppInstaller::CLI::Execution } } - bool Reporter::PromptForBoolResponse(Resource::LocString message, ReporterLevel level) + bool Reporter::PromptForBoolResponse(Resource::LocString message, Level level) { const std::vector options { @@ -167,14 +167,14 @@ namespace AppInstaller::CLI::Execution } } - void Reporter::PromptForEnter(ReporterLevel level) + void Reporter::PromptForEnter(Level level) { auto out = GetOutputStream(level); out << std::endl << Resource::String::PressEnterToContinue << std::endl; m_in.get(); } - std::filesystem::path Reporter::PromptForPath(Resource::LocString message, ReporterLevel level) + std::filesystem::path Reporter::PromptForPath(Resource::LocString message, Level level) { auto out = GetOutputStream(level); @@ -325,4 +325,16 @@ namespace AppInstaller::CLI::Execution } m_out->RestoreDefault(); } + + void Reporter::SetLevelMask(Level reporterLevel, bool setEnabled) { + + if (setEnabled) + { + WI_SetAllFlags(m_enabledLevels, reporterLevel); + } + else + { + WI_ClearAllFlags(m_enabledLevels, reporterLevel); + } + } } \ No newline at end of file diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index 3b43f305bd..2e1e1ebdd8 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -23,18 +23,6 @@ namespace AppInstaller::CLI::Execution { #define WINGET_OSTREAM_FORMAT_HRESULT(hr) "0x" << Logging::SetHRFormat << hr - // The level for the Output channel. - enum class ReporterLevel : uint32_t - { - None = 0x0, - Verbose = 0x1, - Info = 0x2, - Warning = 0x4, - Error = 0x8, - All = Verbose | Info | Warning | Error, - }; - DEFINE_ENUM_FLAG_OPERATORS(ReporterLevel); - // One of the options available to the users when prompting for something. struct BoolPromptOption { @@ -60,6 +48,17 @@ namespace AppInstaller::CLI::Execution Disabled, }; + // The level for the Output channel. + enum class Level : uint32_t + { + None = 0x0, + Verbose = 0x1, + Info = 0x2, + Warning = 0x4, + Error = 0x8, + All = Verbose | Info | Warning | Error, + }; + Reporter(std::ostream& outStream, std::istream& inStream); Reporter(const Reporter&) = delete; Reporter& operator=(const Reporter&) = delete; @@ -74,22 +73,22 @@ namespace AppInstaller::CLI::Execution ~Reporter(); // Get a stream for verbose output. - OutputStream Verbose() { return GetOutputStream(ReporterLevel::Verbose); } + OutputStream Verbose() { return GetOutputStream(Level::Verbose); } // Get a stream for informational output. - OutputStream Info() { return GetOutputStream(ReporterLevel::Info); } + OutputStream Info() { return GetOutputStream(Level::Info); } // Get a stream for warning output. - OutputStream Warn() { return GetOutputStream(ReporterLevel::Warning); } + OutputStream Warn() { return GetOutputStream(Level::Warning); } // Get a stream for error output. - OutputStream Error() { return GetOutputStream(ReporterLevel::Error); } + OutputStream Error() { return GetOutputStream(Level::Error); } // Get a stream for outputting completion words. OutputStream Completion() { return OutputStream(*m_out, m_channel == Channel::Completion, false); } // Gets a stream for output of the given level. - OutputStream GetOutputStream(ReporterLevel level); + OutputStream GetOutputStream(Level level); void EmptyLine() { GetBasicOutputStream() << std::endl; } @@ -101,13 +100,13 @@ namespace AppInstaller::CLI::Execution void SetStyle(AppInstaller::Settings::VisualStyle style); // Prompts the user, return true if they consented. - bool PromptForBoolResponse(Resource::LocString message, ReporterLevel level = ReporterLevel::Info); + bool PromptForBoolResponse(Resource::LocString message, Level level); // Prompts the user, continues when Enter is pressed - void PromptForEnter(ReporterLevel level = ReporterLevel::Info); + void PromptForEnter(Level level = Level::Info); // Prompts the user for a path. - std::filesystem::path PromptForPath(Resource::LocString message, ReporterLevel level = ReporterLevel::Info); + std::filesystem::path PromptForPath(Resource::LocString message, Level level); // Used to show indefinite progress. Currently an indefinite spinner is the form of // showing indefinite progress. @@ -168,16 +167,10 @@ namespace AppInstaller::CLI::Execution m_progressSink = sink; } - void SetLevelEnabled(ReporterLevel reporterLevel, bool setEnabled = true) - { - if (setEnabled) - { - WI_SetAllFlags(m_enabledLevels, reporterLevel); - } - else - { - WI_ClearAllFlags(m_enabledLevels, reporterLevel); - } + void SetLevelMask(Level reporterLevel, bool setEnabled = true); + + Level GetLevelMask() { + return m_enabledLevels; } private: @@ -196,9 +189,11 @@ namespace AppInstaller::CLI::Execution std::atomic m_progressSink; // Enable all levels by default - ReporterLevel m_enabledLevels = ReporterLevel::All; + Level m_enabledLevels = Level::All; }; + DEFINE_ENUM_FLAG_OPERATORS(Reporter::Level); + // Indirection to enable change without tracking down every place extern const VirtualTerminal::Sequence& HelpCommandEmphasis; extern const VirtualTerminal::Sequence& HelpArgumentEmphasis; diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index efaf79ee12..77c1fbd9d1 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1064,7 +1064,7 @@ namespace AppInstaller::CLI::Workflow } auto promptString = m_isApply ? Resource::String::ConfigurationWarningPromptApply : Resource::String::ConfigurationWarningPromptTest; - if (!context.Reporter.PromptForBoolResponse(promptString, ReporterLevel::Warning)) + if (!context.Reporter.PromptForBoolResponse(promptString, Reporter::Level::Warning)) { AICLI_TERMINATE_CONTEXT(WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED); } diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp index f1d6145801..a431fa8c72 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp @@ -5,6 +5,7 @@ #include "ShowFlow.h" #include +using namespace AppInstaller::CLI::Execution; using namespace AppInstaller::Settings; using namespace AppInstaller::Utility::literals; @@ -12,6 +13,21 @@ namespace AppInstaller::CLI::Workflow { namespace { + bool IsInteractivityAllowed(Execution::Context& context, Reporter::Level reporterLevel) { + // Interactivity can be disabled for several reasons: + // * We are running in a non-interactive context (e.g., COM call) + // * It is disabled in the settings + // * It was disabled from the command line + // * The output level required has been disabled + if (WI_AreAllFlagsClear(context.Reporter.GetLevelMask(), reporterLevel)) + { + AICLI_LOG(CLI, Verbose, << "Skipping prompt. Interactivity is disabled due to reporter level being suppressed"); + return false; + } + + return IsInteractivityAllowed(context); + } + bool IsInteractivityAllowed(Execution::Context& context) { // Interactivity can be disabled for several reasons: @@ -88,9 +104,9 @@ namespace AppInstaller::CLI::Workflow bool accepted = context.Args.Contains(Execution::Args::Type::AcceptSourceAgreements); - if (!accepted && IsInteractivityAllowed(context)) + if (!accepted && IsInteractivityAllowed(context, Reporter::Level::Info)) { - accepted = context.Reporter.PromptForBoolResponse(Resource::String::SourceAgreementsPrompt); + accepted = context.Reporter.PromptForBoolResponse(Resource::String::SourceAgreementsPrompt, Reporter::Level::Info); } if (accepted) @@ -198,9 +214,9 @@ namespace AppInstaller::CLI::Workflow if (showPrompt) { AICLI_LOG(CLI, Verbose, << "Prompting to accept package agreements"); - if (IsInteractivityAllowed(context)) + if (IsInteractivityAllowed(context, Reporter::Level::Info)) { - bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PackageAgreementsPrompt); + bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PackageAgreementsPrompt, Reporter::Level::Info); if (accepted) { AICLI_LOG(CLI, Info, << "Package agreements accepted in prompt"); @@ -267,7 +283,7 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Info() << Resource::String::InstallersRequireInstallLocation << std::endl; for (auto packageContext : packagesToPrompt) { - *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::ReporterLevel::Warning); + *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::Reporter::Level::Warning); if (packageContext->IsTerminated()) { AICLI_TERMINATE_CONTEXT(packageContext->GetTerminationHR()); @@ -286,7 +302,7 @@ namespace AppInstaller::CLI::Workflow private: void PromptForInstallRoot(Execution::Context& context) { - if (!IsInteractivityAllowed(context)) + if (!IsInteractivityAllowed(context, Reporter::Level::Info)) { AICLI_LOG(CLI, Error, << "Install location is required but was not provided."); context.Reporter.Error() << Resource::String::InstallLocationNotProvided << std::endl; @@ -294,7 +310,7 @@ namespace AppInstaller::CLI::Workflow } AICLI_LOG(CLI, Info, << "Prompting for install root."); - m_installLocation = context.Reporter.PromptForPath(Resource::String::PromptForInstallRoot); + m_installLocation = context.Reporter.PromptForPath(Resource::String::PromptForInstallRoot, Reporter::Level::Info); AICLI_LOG(CLI, Info, << "Proceeding with installation using install root: " << m_installLocation); } @@ -335,7 +351,7 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Warn() << Resource::String::InstallersAbortTerminal << std::endl; for (auto packageContext : packagesToPrompt) { - *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::ReporterLevel::Warning); + *packageContext << ReportManifestIdentityWithVersion(" - "_liv, Execution::Reporter::Level::Warning); if (packageContext->IsTerminated()) { AICLI_TERMINATE_CONTEXT(packageContext->GetTerminationHR()); @@ -349,12 +365,12 @@ namespace AppInstaller::CLI::Workflow void PromptToProceed(Execution::Context& context) { AICLI_LOG(CLI, Info, << "Prompting before proceeding with installer that aborts terminal."); - if (!IsInteractivityAllowed(context)) + if (!IsInteractivityAllowed(context, Reporter::Level::Warning)) { return; } - bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PromptToProceed, Execution::ReporterLevel::Warning); + bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PromptToProceed, Reporter::Level::Warning); if (accepted) { AICLI_LOG(CLI, Info, << "Proceeding with installation"); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 394c33431b..b0045126af 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -46,7 +46,7 @@ namespace AppInstaller::CLI::Workflow std::string_view name, std::string_view id, std::string_view version = {}, - Execution::ReporterLevel level = Execution::ReporterLevel::Info) + Execution::Reporter::Level level = Execution::Reporter::Level::Info) { auto out = context.Reporter.GetOutputStream(level); out << prefix; diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 31a2aafc9e..6595256426 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -365,9 +365,9 @@ namespace AppInstaller::CLI::Workflow // Outputs: None struct ReportManifestIdentityWithVersion : public WorkflowTask { - ReportManifestIdentityWithVersion(Utility::LocIndView prefix, Execution::ReporterLevel level = Execution::ReporterLevel::Info) : + ReportManifestIdentityWithVersion(Utility::LocIndView prefix, Execution::Reporter::Level level = Execution::Reporter::Level::Info) : WorkflowTask("ReportManifestIdentityWithVersion"), m_prefix(prefix), m_level(level) {} - ReportManifestIdentityWithVersion(Resource::StringId label = Resource::String::ReportIdentityFound, Execution::ReporterLevel level = Execution::ReporterLevel::Info) : + ReportManifestIdentityWithVersion(Resource::StringId label = Resource::String::ReportIdentityFound, Execution::Reporter::Level level = Execution::Reporter::Level::Info) : WorkflowTask("ReportManifestIdentityWithVersion"), m_label(label), m_level(level) {} void operator()(Execution::Context& context) const override; @@ -375,7 +375,7 @@ namespace AppInstaller::CLI::Workflow private: Utility::LocIndView m_prefix; std::optional m_label; - Execution::ReporterLevel m_level; + Execution::Reporter::Level m_level; }; // Selects the installer from the manifest, if one is applicable. From ef1589f3e5c73c0ed1b2c77be9a04b4307330f34 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Sat, 2 Mar 2024 16:16:14 -0600 Subject: [PATCH 07/13] Fix PromptForEnter --- src/AppInstallerCLICore/ExecutionReporter.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 1cf5184ffe..4f23179a64 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -170,6 +170,11 @@ namespace AppInstaller::CLI::Execution void Reporter::PromptForEnter(Level level) { auto out = GetOutputStream(level); + if (!out.IsEnabled()) + { + return; + } + out << std::endl << Resource::String::PressEnterToContinue << std::endl; m_in.get(); } From 8bbf337aea82b01a0c7f961b479a41515cc644a2 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 14 Mar 2024 13:50:54 -0500 Subject: [PATCH 08/13] Move checks to PromptFor --- src/AppInstallerCLICore/ExecutionReporter.cpp | 11 +++++-- src/AppInstallerCLICore/ExecutionReporter.h | 13 +++++---- .../Workflows/ConfigurationFlow.cpp | 2 +- .../Workflows/PromptFlow.cpp | 29 +++++-------------- 4 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 4f23179a64..ec12d8d3ec 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -117,8 +117,15 @@ namespace AppInstaller::CLI::Execution } } - bool Reporter::PromptForBoolResponse(Resource::LocString message, Level level) + bool Reporter::PromptForBoolResponse(Resource::LocString message, Level level, bool resultIfDisabled) { + auto out = GetOutputStream(level); + + if (!out.IsEnabled()) + { + return resultIfDisabled; + } + const std::vector options { BoolPromptOption{ Resource::String::PromptOptionYes, 'Y', true }, @@ -179,7 +186,7 @@ namespace AppInstaller::CLI::Execution m_in.get(); } - std::filesystem::path Reporter::PromptForPath(Resource::LocString message, Level level) + std::filesystem::path Reporter::PromptForPath(Resource::LocString message, Level level, std::filesystem::path resultIfDisabled) { auto out = GetOutputStream(level); diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index 2e1e1ebdd8..b60e62fd59 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -100,13 +100,13 @@ namespace AppInstaller::CLI::Execution void SetStyle(AppInstaller::Settings::VisualStyle style); // Prompts the user, return true if they consented. - bool PromptForBoolResponse(Resource::LocString message, Level level); + bool PromptForBoolResponse(Resource::LocString message, Level level = Level::Info, bool resultIfDisabled = false); // Prompts the user, continues when Enter is pressed void PromptForEnter(Level level = Level::Info); // Prompts the user for a path. - std::filesystem::path PromptForPath(Resource::LocString message, Level level); + std::filesystem::path PromptForPath(Resource::LocString message, Level level = Level::Info); // Used to show indefinite progress. Currently an indefinite spinner is the form of // showing indefinite progress. @@ -167,12 +167,13 @@ namespace AppInstaller::CLI::Execution m_progressSink = sink; } - void SetLevelMask(Level reporterLevel, bool setEnabled = true); - - Level GetLevelMask() { - return m_enabledLevels; + bool IsLevelEnabled(Level reporterLevel) + { + return WI_AreAllFlagsSet(m_enabledLevels, reporterLevel); } + void SetLevelMask(Level reporterLevel, bool setEnabled = true); + private: Reporter(std::shared_ptr outStream, std::istream& inStream); // Gets a stream for output for internal use. diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index 77c1fbd9d1..8de9ae64a2 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1064,7 +1064,7 @@ namespace AppInstaller::CLI::Workflow } auto promptString = m_isApply ? Resource::String::ConfigurationWarningPromptApply : Resource::String::ConfigurationWarningPromptTest; - if (!context.Reporter.PromptForBoolResponse(promptString, Reporter::Level::Warning)) + if (!context.Reporter.PromptForBoolResponse(promptString, Reporter::Level::Warning, true)) { AICLI_TERMINATE_CONTEXT(WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED); } diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp index a431fa8c72..2371cf1c43 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp @@ -13,21 +13,6 @@ namespace AppInstaller::CLI::Workflow { namespace { - bool IsInteractivityAllowed(Execution::Context& context, Reporter::Level reporterLevel) { - // Interactivity can be disabled for several reasons: - // * We are running in a non-interactive context (e.g., COM call) - // * It is disabled in the settings - // * It was disabled from the command line - // * The output level required has been disabled - if (WI_AreAllFlagsClear(context.Reporter.GetLevelMask(), reporterLevel)) - { - AICLI_LOG(CLI, Verbose, << "Skipping prompt. Interactivity is disabled due to reporter level being suppressed"); - return false; - } - - return IsInteractivityAllowed(context); - } - bool IsInteractivityAllowed(Execution::Context& context) { // Interactivity can be disabled for several reasons: @@ -104,9 +89,9 @@ namespace AppInstaller::CLI::Workflow bool accepted = context.Args.Contains(Execution::Args::Type::AcceptSourceAgreements); - if (!accepted && IsInteractivityAllowed(context, Reporter::Level::Info)) + if (!accepted && IsInteractivityAllowed(context)) { - accepted = context.Reporter.PromptForBoolResponse(Resource::String::SourceAgreementsPrompt, Reporter::Level::Info); + accepted = context.Reporter.PromptForBoolResponse(Resource::String::SourceAgreementsPrompt, Reporter::Level::Info, false); } if (accepted) @@ -214,9 +199,9 @@ namespace AppInstaller::CLI::Workflow if (showPrompt) { AICLI_LOG(CLI, Verbose, << "Prompting to accept package agreements"); - if (IsInteractivityAllowed(context, Reporter::Level::Info)) + if (IsInteractivityAllowed(context)) { - bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PackageAgreementsPrompt, Reporter::Level::Info); + bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PackageAgreementsPrompt); if (accepted) { AICLI_LOG(CLI, Info, << "Package agreements accepted in prompt"); @@ -302,7 +287,7 @@ namespace AppInstaller::CLI::Workflow private: void PromptForInstallRoot(Execution::Context& context) { - if (!IsInteractivityAllowed(context, Reporter::Level::Info)) + if (!IsInteractivityAllowed(context)) { AICLI_LOG(CLI, Error, << "Install location is required but was not provided."); context.Reporter.Error() << Resource::String::InstallLocationNotProvided << std::endl; @@ -310,7 +295,7 @@ namespace AppInstaller::CLI::Workflow } AICLI_LOG(CLI, Info, << "Prompting for install root."); - m_installLocation = context.Reporter.PromptForPath(Resource::String::PromptForInstallRoot, Reporter::Level::Info); + m_installLocation = context.Reporter.PromptForPath(Resource::String::PromptForInstallRoot); AICLI_LOG(CLI, Info, << "Proceeding with installation using install root: " << m_installLocation); } @@ -365,7 +350,7 @@ namespace AppInstaller::CLI::Workflow void PromptToProceed(Execution::Context& context) { AICLI_LOG(CLI, Info, << "Prompting before proceeding with installer that aborts terminal."); - if (!IsInteractivityAllowed(context, Reporter::Level::Warning)) + if (!IsInteractivityAllowed(context)) { return; } From 996e747c4a22632c7cdd1a32df0bd6b0800326db Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 14 Mar 2024 13:53:16 -0500 Subject: [PATCH 09/13] Return default if disabled on PromptForPath --- src/AppInstallerCLICore/ExecutionReporter.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index ec12d8d3ec..91ae6e1576 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -190,6 +190,11 @@ namespace AppInstaller::CLI::Execution { auto out = GetOutputStream(level); + if (!out.IsEnabled()) + { + return resultIfDisabled; + } + // Try prompting until we get a valid answer for (;;) { From 7290e81164c834d2b7d413e3d68563a2b3e90bc0 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 14 Mar 2024 13:55:45 -0500 Subject: [PATCH 10/13] Forgot to save this file --- src/AppInstallerCLICore/Workflows/PromptFlow.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp index 2371cf1c43..e6def38f91 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp @@ -91,7 +91,7 @@ namespace AppInstaller::CLI::Workflow if (!accepted && IsInteractivityAllowed(context)) { - accepted = context.Reporter.PromptForBoolResponse(Resource::String::SourceAgreementsPrompt, Reporter::Level::Info, false); + accepted = context.Reporter.PromptForBoolResponse(Resource::String::SourceAgreementsPrompt); } if (accepted) @@ -296,6 +296,10 @@ namespace AppInstaller::CLI::Workflow AICLI_LOG(CLI, Info, << "Prompting for install root."); m_installLocation = context.Reporter.PromptForPath(Resource::String::PromptForInstallRoot); + if (m_installLocation.empty()) + { + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALL_LOCATION_REQUIRED); + } AICLI_LOG(CLI, Info, << "Proceeding with installation using install root: " << m_installLocation); } @@ -355,7 +359,7 @@ namespace AppInstaller::CLI::Workflow return; } - bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PromptToProceed, Reporter::Level::Warning); + bool accepted = context.Reporter.PromptForBoolResponse(Resource::String::PromptToProceed, Reporter::Level::Warning, true); if (accepted) { AICLI_LOG(CLI, Info, << "Proceeding with installation"); From 5dba5e526327186ab302f91f8f2acaa5e136b966 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 14 Mar 2024 14:07:32 -0500 Subject: [PATCH 11/13] Fix redefinition --- src/AppInstallerCLICore/ExecutionReporter.cpp | 1 - src/AppInstallerCLICore/ExecutionReporter.h | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 91ae6e1576..85de8e314b 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -132,7 +132,6 @@ namespace AppInstaller::CLI::Execution BoolPromptOption{ Resource::String::PromptOptionNo, 'N', false }, }; - auto out = GetOutputStream(level); out << message << std::endl; // Try prompting until we get a recognized option diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index b60e62fd59..84e7aa6528 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -106,7 +106,7 @@ namespace AppInstaller::CLI::Execution void PromptForEnter(Level level = Level::Info); // Prompts the user for a path. - std::filesystem::path PromptForPath(Resource::LocString message, Level level = Level::Info); + std::filesystem::path PromptForPath(Resource::LocString message, Level level = Level::Info, std::filesystem::path resultIfDisabled = std::filesystem::path::path()); // Used to show indefinite progress. Currently an indefinite spinner is the form of // showing indefinite progress. From 92e36f6aa53b7aa1180a01c66a734bc6211a3d13 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 14 Mar 2024 16:29:24 -0500 Subject: [PATCH 12/13] Default false and add logging messages --- src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp | 2 +- src/AppInstallerCLICore/Workflows/PromptFlow.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index 8de9ae64a2..279be1662a 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -1064,7 +1064,7 @@ namespace AppInstaller::CLI::Workflow } auto promptString = m_isApply ? Resource::String::ConfigurationWarningPromptApply : Resource::String::ConfigurationWarningPromptTest; - if (!context.Reporter.PromptForBoolResponse(promptString, Reporter::Level::Warning, true)) + if (!context.Reporter.PromptForBoolResponse(promptString, Reporter::Level::Warning, false)) { AICLI_TERMINATE_CONTEXT(WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED); } diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp index e6def38f91..8f7c21d362 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp @@ -298,6 +298,8 @@ namespace AppInstaller::CLI::Workflow m_installLocation = context.Reporter.PromptForPath(Resource::String::PromptForInstallRoot); if (m_installLocation.empty()) { + AICLI_LOG(CLI, Error, << "Install location is required but the provided path was empty."); + context.Reporter.Error() << Resource::String::InstallLocationNotProvided << std::endl; AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALL_LOCATION_REQUIRED); } AICLI_LOG(CLI, Info, << "Proceeding with installation using install root: " << m_installLocation); From 0de356d22bb27ed7f6105acb4b2f1a99ce29d812 Mon Sep 17 00:00:00 2001 From: Kaleb Luedtke Date: Thu, 14 Mar 2024 18:45:36 -0500 Subject: [PATCH 13/13] Cleanup whitespace changes --- src/AppInstallerCLICore/Core.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index e8ea475c07..fe19bb0aec 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" #include "Public/AppInstallerCLICore.h" @@ -119,7 +119,6 @@ namespace AppInstaller::CLI Logging::Telemetry().LogCommand(command->FullName()); command->ParseArguments(invocation, context.Args); - context.UpdateForArgs(); context.SetExecutingCommand(command.get()); command->ValidateArguments(context.Args);