commit cf597eca613528b7ae625beedb79294f2e38fcae
parent 7cea3d9582f9bd68a4406c73f07309b1df79f120
Author: Kaleb Luedtke <jluedtk@jci.com>
Date: Thu, 14 Mar 2024 21:21:01 -0700
Make Ignore Warnings suppress the output level entirely (#4221)
<!-- To check a checkbox place an "x" between the brackets. e.g: [x] -->
- [x] I have signed the [Contributor License
Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs).
- [x] This pull request is related to an issue.
* Resolves #4213
* Closes #4214
Instead of handling each case individually, this PR makes the
`--ignore-warnings` a common argument and suppresses all output from
`context.Reporter.Warn()`. Support is added for enabling or disabling
each reporter level individually.
cc @yao-msft
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/4221)
Diffstat:
9 files changed, 79 insertions(+), 15 deletions(-)
diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp
@@ -406,6 +406,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);
args.push_back(ForType(Args::Type::Proxy));
args.push_back(ForType(Args::Type::NoProxy));
diff --git 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/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/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp
@@ -356,6 +356,12 @@ namespace AppInstaller::CLI::Execution
Logging::Log().SetLevel(Logging::Level::Verbose);
}
+ // Disable warnings if requested
+ if (Args.Contains(Args::Type::IgnoreWarnings))
+ {
+ Reporter.SetLevelMask(Reporter::Level::Warning, false);
+ }
+
// Set proxy
if (Args.Contains(Args::Type::Proxy))
{
diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp
@@ -54,6 +54,12 @@ namespace AppInstaller::CLI::Execution
OutputStream Reporter::GetOutputStream(Level level)
{
+ // If the level is not enabled, return a default stream which is disabled
+ if (WI_AreAllFlagsClear(m_enabledLevels, level))
+ {
+ return OutputStream(*m_out, false, false);
+ }
+
OutputStream result = GetBasicOutputStream();
switch (level)
@@ -111,15 +117,21 @@ 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<BoolPromptOption> options
{
BoolPromptOption{ Resource::String::PromptOptionYes, 'Y', true },
BoolPromptOption{ Resource::String::PromptOptionNo, 'N', false },
};
- auto out = GetOutputStream(level);
out << message << std::endl;
// Try prompting until we get a recognized option
@@ -164,14 +176,24 @@ 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();
}
- 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);
+ if (!out.IsEnabled())
+ {
+ return resultIfDisabled;
+ }
+
// Try prompting until we get a valid answer
for (;;)
{
@@ -319,4 +341,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
@@ -49,12 +49,14 @@ namespace AppInstaller::CLI::Execution
};
// The level for the Output channel.
- enum class Level
+ enum class Level : uint32_t
{
- Verbose,
- Info,
- Warning,
- Error,
+ None = 0x0,
+ Verbose = 0x1,
+ Info = 0x2,
+ Warning = 0x4,
+ Error = 0x8,
+ All = Verbose | Info | Warning | Error,
};
Reporter(std::ostream& outStream, std::istream& inStream);
@@ -98,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 = Level::Info);
+ 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 = 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.
@@ -165,9 +167,15 @@ namespace AppInstaller::CLI::Execution
m_progressSink = sink;
}
+ bool IsLevelEnabled(Level reporterLevel)
+ {
+ return WI_AreAllFlagsSet(m_enabledLevels, reporterLevel);
+ }
+
+ void SetLevelMask(Level reporterLevel, bool setEnabled = true);
+
private:
Reporter(std::shared_ptr<BaseStream> outStream, std::istream& inStream);
-
// Gets a stream for output for internal use.
OutputStream GetBasicOutputStream();
@@ -180,8 +188,13 @@ namespace AppInstaller::CLI::Execution
wil::srwlock m_progressCallbackLock;
std::atomic<ProgressCallback*> m_progressCallback;
std::atomic<IProgressSink*> m_progressSink;
+
+ // Enable all levels by default
+ 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
@@ -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, false))
{
AICLI_TERMINATE_CONTEXT(WINGET_CONFIG_ERROR_WARNING_NOT_ACCEPTED);
}
diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp
@@ -5,6 +5,7 @@
#include "ShowFlow.h"
#include <winget/UserSettings.h>
+using namespace AppInstaller::CLI::Execution;
using namespace AppInstaller::Settings;
using namespace AppInstaller::Utility::literals;
@@ -295,6 +296,12 @@ 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_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);
}
@@ -354,7 +361,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, Reporter::Level::Warning, true);
if (accepted)
{
AICLI_LOG(CLI, Info, << "Proceeding with installation");
diff --git 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'.</value>
<value>Filter results by id</value>
</data>
<data name="IgnoreWarningsArgumentDescription" xml:space="preserve">
- <value>Suppresses warning outputs.</value>
+ <value>Suppresses warning outputs</value>
</data>
<data name="InstallationDisclaimer1" xml:space="preserve">
<value>This application is licensed to you by its owner.</value>
