diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 240e0d747c..3f229f99a5 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -22,6 +22,7 @@ aspirational Authenticode AUTOLISTEN azureedge +Bawa binlog binver bstr diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index 41c77e1db0..c5d973159d 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -2,6 +2,12 @@ ## New Features +### Interactive package selection (experimental) + +Set `experimentalFeatures.interactivePackageSelection` to `true` in settings to enable numbered choices when multiple packages match a single-package `install`, `show`, or `download` command in an interactive terminal. Enter a package number to continue or `0` to cancel. + +This feature is disabled by default. Redirected and noninteractive callers retain the existing ambiguity error. Use `--id --exact --source ` to select a package explicitly, or `--disable-interactivity` to prevent prompts. + ### Source priority Source priority is now available without enabling an experimental feature. Use `winget source add --priority ` or `winget source edit --name --priority ` to configure it. Higher values take precedence; sources with equal priority still require disambiguation when multiple matches remain. diff --git a/doc/Settings.md b/doc/Settings.md index ba7c8af041..749a3b5cd8 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -450,3 +450,15 @@ This feature enables support for fonts via `winget settings`. The `winget font l "fonts": true }, ``` + +### interactivePackageSelection + +Enables numbered choices for ambiguous single-package `install`, `show`, and `download` commands, including `show --versions`. Disabled by default. + +```json + "experimentalFeatures": { + "interactivePackageSelection": true + }, +``` + +`--disable-interactivity` and redirected input or output still prevent prompting. diff --git a/doc/specs/#5345 - Interactive package selection.md b/doc/specs/#5345 - Interactive package selection.md new file mode 100644 index 0000000000..0f48497320 --- /dev/null +++ b/doc/specs/#5345 - Interactive package selection.md @@ -0,0 +1,127 @@ +--- +author: AmelBawa-msft, GitHub Copilot +created on: 2026-09-28 +last updated: 2026-10-01 +issue id: 5345 +--- + +# Interactive package selection + +For [#5345](https://github.com/microsoft/winget-cli/issues/5345) + +## Abstract + +Let users resolve ambiguous package matches without restarting their command. An experimental setting enables numbered choices for single-package `install`, `show`, and `download` when interactive input and output are available. + +## Inspiration + +The same query can match multiple packages, including packages from different sources. Users should be able to choose deliberately without copying an ID into another invocation. + +## Solution Design + +This feature is disabled by default. Enable it in settings: + +```json +{ + "experimentalFeatures": { + "interactivePackageSelection": true + } +} +``` + +Apply existing search matching and source-priority rules first. If multiple candidates remain, eligible CLI call sites opt into selection. Shared workflows remain noninteractive by default. + +Display candidates in their existing order with stable, one-indexed numbers. A valid number selects the existing package object without searching again. Preserve command options and continue normal version selection, applicability checks, and agreement handling. + +| Situation | Behavior | +| --- | --- | +| Experimental feature disabled (default) | No selection prompts; retain ambiguity errors with refinement guidance. | +| No match | Existing no-match error. | +| One match after existing policy | Continue without prompting. | +| Multiple matches for single-package `install`, `show`, or `download` | Prompt if eligible, including `show --versions`. | +| Truncated results | Retain ambiguity error and request refinement. | +| Invalid or empty input | Explain the valid range and prompt again; no default. | +| `0` | Cancel without acting on a package. | +| Ctrl+C | Cancel immediately, including while waiting for input. | +| EOF or input failure | Report the existing prompt input error. | +| `--disable-interactivity`, interactivity disabled in settings or context | Retain ambiguity error without reading input. | +| `--silent` | Controls installer UI, not selection prompts; normal interactivity rules apply. | +| Redirected input or output, or disabled informational output | Do not prompt. | +| `--no-vt` | Use the same text and numeric input without terminal escape sequences. | +| Multi-package operations, including individual package contexts | No disambiguation prompts, either per package or up front; retain existing ambiguity errors. | +| Upgrade, uninstall, repair, pin, search, list, or completion | No selection prompts. | +| COM API, PowerShell cmdlets, or configuration/DSC | No new prompts or API changes. | + +Apart from the experimental setting, the prompt adds no command-line flags, group policies, manifest fields, or schema versions. Existing interactivity controls and the experimental-features group policy apply. Package validation pipelines and manifest authoring tools are unchanged; manifest examples and schema snippets are not applicable. + +## UI/UX Design + +Reuse the existing ambiguity table with a leading selection number. Show Name, Id, and Source, including Source when all candidates use the same source. For example: + +```text +Multiple packages match. Choose one. + +# Name Id Source +------------------------------------------ +1 Contoso Editor Contoso.Editor winget +2 Contoso Editor Contoso.Editor.Pro winget + +Enter a number (1-2), or 0 to cancel: 1 +Selected: Contoso Editor [Contoso.Editor] +``` + +For distinct candidates from different sources: + +```text +# Name Id Source +--------------------------------------- +1 Contoso Editor Contoso.Editor winget +2 Contoso Editor Contoso.Editor private +``` + +Each candidate occupies one row, using the existing ambiguity report's package identity and source. Sources grouped within a candidate do not add rows. Use the same introductory text for every command. Do not add another confirmation after selection. Existing consent prompts still apply. + +Ambiguity errors include the candidate list and refinement guidance, even when interactive selection is disabled: + +```text +Specify a package with --id --exact --source . +``` + +For `configure export`, use `--package-id --source ` instead. + +The original version option remains authoritative. + +## Capabilities + +### Accessibility + +Numeric, line-oriented input works without color, cursor navigation, or arrow keys. All identifying information is text and uses localized labels. Invalid input includes recovery instructions. + +### Security + +There is no default selection or inferred equivalence. Choosing a package does not accept agreements or bypass existing source, trust, or installer checks. + +### Reliability + +Selection uses the displayed candidate object rather than re-running a potentially different search. EOF fails explicitly; cancellation never starts installation or download. + +### Compatibility + +The feature is disabled by default. After opt-in, scripts can preserve ambiguity errors with `--disable-interactivity`, or avoid ambiguity with exact ID and source selectors. + +### Performance, Power, and Efficiency + +Rendering uses available package metadata, without downloading manifests for display. No terminal redraw loop is required. + +## Potential Issues + +Long candidate lists require scrolling. Narrow terminals truncate table cells using the existing formatter; users can widen the terminal or cancel and refine their query if candidates are indistinguishable. The selection message includes the full name and ID. Source-defined result truncation must not be presented as a complete selectable list. Matching names and IDs do not prove that packages from different sources are equivalent. + +## Future Considerations + +Cross-source equivalence heuristics, arrow-key navigation, and selection for installed-package operations are separate changes. + +## Resources + +- [Package matching background](%23292%20-%20winget%20should%20install%20an%20app%20if%20there%20is%20an%20exact%20match.md) +- [Settings reference](../Settings.md) diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index de8dc24389..e162aaac12 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -339,6 +339,11 @@ "type": "boolean", "default": false }, + "interactivePackageSelection": { + "description": "Enable interactive selection for ambiguous package matches", + "type": "boolean", + "default": false + }, "resume": { "description": "Enable support for some commands to resume", "type": "boolean", diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj index 8dd3db7bd9..f0698daaa3 100644 --- a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj +++ b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -432,6 +432,7 @@ + Create diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters index 753d2451d8..7f8606c35a 100644 --- a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters +++ b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters @@ -346,6 +346,9 @@ Source Files + + Source Files + Commands diff --git a/src/AppInstallerCLICore/Commands/DownloadCommand.cpp b/src/AppInstallerCLICore/Commands/DownloadCommand.cpp index 21854d985c..7ee7a857af 100644 --- a/src/AppInstallerCLICore/Commands/DownloadCommand.cpp +++ b/src/AppInstallerCLICore/Commands/DownloadCommand.cpp @@ -126,7 +126,7 @@ namespace AppInstaller::CLI Workflow::OpenSource() << Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << - Workflow::EnsureOneMatchFromSearchResult(OperationType::Download) << + Workflow::EnsureOneMatchFromSearchResult(OperationType::Download, PackageSelectionBehavior::Prompt) << Workflow::GetManifestFromPackage(false); } diff --git a/src/AppInstallerCLICore/Commands/DscPackageResource.cpp b/src/AppInstallerCLICore/Commands/DscPackageResource.cpp index d43aac207d..c45dfb2e4f 100644 --- a/src/AppInstallerCLICore/Commands/DscPackageResource.cpp +++ b/src/AppInstallerCLICore/Commands/DscPackageResource.cpp @@ -237,7 +237,7 @@ namespace AppInstaller::CLI } *SubContext << - Workflow::SelectSinglePackageVersionForInstallOrUpgrade(Workflow::OperationType::Install, allowDowngrade) << + Workflow::SelectSinglePackageVersionForInstallOrUpgrade(Workflow::OperationType::Install, Workflow::PackageSelectionBehavior::Disabled, allowDowngrade) << Workflow::InstallSinglePackage; if (SubContext->IsTerminated()) diff --git a/src/AppInstallerCLICore/Commands/InstallCommand.cpp b/src/AppInstallerCLICore/Commands/InstallCommand.cpp index ab52f9076c..9d61c1235c 100644 --- a/src/AppInstallerCLICore/Commands/InstallCommand.cpp +++ b/src/AppInstallerCLICore/Commands/InstallCommand.cpp @@ -167,7 +167,7 @@ namespace AppInstaller::CLI { context << Checkpoint("PreInstallCheckpoint", {}) << // TODO: Capture context data - InstallOrUpgradeSinglePackage(OperationType::Install); + InstallOrUpgradeSinglePackage(OperationType::Install, PackageSelectionBehavior::Prompt); } } } diff --git a/src/AppInstallerCLICore/Commands/ShowCommand.cpp b/src/AppInstallerCLICore/Commands/ShowCommand.cpp index 796711bf19..4813195f8e 100644 --- a/src/AppInstallerCLICore/Commands/ShowCommand.cpp +++ b/src/AppInstallerCLICore/Commands/ShowCommand.cpp @@ -93,7 +93,7 @@ namespace AppInstaller::CLI Workflow::OpenSource() << Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << - Workflow::EnsureOneMatchFromSearchResult(OperationType::Show) << + Workflow::EnsureOneMatchFromSearchResult(OperationType::Show, PackageSelectionBehavior::Prompt) << Workflow::ReportPackageIdentity << Workflow::ShowAppVersions; } @@ -101,7 +101,7 @@ namespace AppInstaller::CLI else { context << - GetManifest( /* considerPins */ false) << + GetManifest( /* considerPins */ false, PackageSelectionBehavior::Prompt) << Workflow::ReportManifestIdentity << Workflow::SelectInstaller << Workflow::ShowManifestInfo; diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h index d4cfd96fe7..eae42fbdee 100644 --- a/src/AppInstallerCLICore/ExecutionContext.h +++ b/src/AppInstallerCLICore/ExecutionContext.h @@ -202,7 +202,7 @@ namespace AppInstaller::CLI::Execution private: DestructionToken m_disableSignalTerminationHandlerOnExit = false; - bool m_isTerminated = false; + std::atomic m_isTerminated = false; HRESULT m_terminationHR = S_OK; size_t m_CtrlSignalCount = 0; ContextFlag m_flags = ContextFlag::None; diff --git a/src/AppInstallerCLICore/ExecutionContextData.h b/src/AppInstallerCLICore/ExecutionContextData.h index 4609893dc2..40cf11537d 100644 --- a/src/AppInstallerCLICore/ExecutionContextData.h +++ b/src/AppInstallerCLICore/ExecutionContextData.h @@ -69,6 +69,7 @@ namespace AppInstaller::CLI::Execution RepairString, MsixDigests, InstallerDownloadAuthenticators, + SelectedIndex, Max }; @@ -100,6 +101,12 @@ namespace AppInstaller::CLI::Execution using value_t = Repository::SearchResult; }; + template <> + struct DataMapping + { + using value_t = std::optional; + }; + template <> struct DataMapping { diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index 6bf720e6e9..9286f22998 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -3,6 +3,8 @@ #include "pch.h" #include "ExecutionReporter.h" #include +#include +#include namespace AppInstaller::CLI::Execution @@ -10,6 +12,16 @@ namespace AppInstaller::CLI::Execution using namespace Settings; using namespace VirtualTerminal; +#ifndef AICLI_DISABLE_TEST_HOOKS + using ReadConsoleFunction = std::function; + static ReadConsoleFunction* s_readConsoleOverride = nullptr; + + void TestHook_SetReadConsole_Override(ReadConsoleFunction* value) + { + s_readConsoleOverride = value; + } +#endif + const Sequence& HelpCommandEmphasis = TextFormat::Foreground::Bright; const Sequence& HelpArgumentEmphasis = TextFormat::Foreground::Bright; const Sequence& ManifestInfoEmphasis = TextFormat::Foreground::Bright; @@ -25,6 +37,17 @@ namespace AppInstaller::CLI::Execution namespace { + BOOL ReadConsoleChunk(wchar_t* buffer, DWORD size, DWORD* charactersRead) + { +#ifndef AICLI_DISABLE_TEST_HOOKS + if (s_readConsoleOverride) + { + return (*s_readConsoleOverride)(buffer, size, charactersRead); + } +#endif + return ReadConsoleW(GetStdHandle(STD_INPUT_HANDLE), buffer, size, charactersRead, nullptr); + } + DWORD GetStdHandleType(DWORD stdHandle) { DWORD result = FILE_TYPE_UNKNOWN; @@ -44,6 +67,9 @@ namespace AppInstaller::CLI::Execution { m_outStreamFileType = GetStdHandleType(STD_OUTPUT_HANDLE); m_inStreamFileType = GetStdHandleType(STD_INPUT_HANDLE); + DWORD mode = 0; + m_consoleStreams = GetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), &mode) && + GetConsoleMode(GetStdHandle(STD_OUTPUT_HANDLE), &mode); } Reporter::Reporter(std::ostream& outStream, std::istream& inStream) : @@ -79,6 +105,7 @@ namespace AppInstaller::CLI::Execution { m_outStreamFileType = other.m_outStreamFileType; m_inStreamFileType = other.m_inStreamFileType; + m_consoleStreams = other.m_consoleStreams; SetChannel(other.m_channel); @@ -176,6 +203,146 @@ namespace AppInstaller::CLI::Execution return m_inStreamFileType == FILE_TYPE_CHAR; } + bool Reporter::CanPrompt(Level level) + { + return m_consoleStreams && GetOutputStream(level).IsEnabled(); + } + + std::optional Reporter::ReadLine(std::function isCancelled) + { + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_consoleStreams); + + if (isCancelled && isCancelled()) + { + return std::nullopt; + } + + wil::unique_handle inputThread; + THROW_IF_WIN32_BOOL_FALSE(DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), + inputThread.put(), THREAD_TERMINATE, FALSE, 0)); + + std::string response; + bool readSucceeded = false; + DWORD readError = ERROR_SUCCESS; + ProgressCallback progress; + wil::unique_event readCompleted{ wil::EventOptions::ManualReset }; + wil::unique_event cancellationCompleted{ wil::EventOptions::ManualReset }; + auto cancellation = progress.SetCancellationFunction([&]() + { + // Retry until the read ends to cover cancellation immediately before it starts. + while (!readCompleted.wait(10)) + { + if (!CancelSynchronousIo(inputThread.get())) + { + DWORD error = GetLastError(); + if (error != ERROR_NOT_FOUND) + { + LOG_WIN32(error); + } + } + } + cancellationCompleted.SetEvent(); + }); + SetProgressCallback(&progress); + { + auto unregister = wil::scope_exit([&]() + { + readCompleted.SetEvent(); + SetProgressCallback(nullptr); + }); + if (!isCancelled || !isCancelled()) + { + if (m_inStreamFileType == FILE_TYPE_CHAR) + { + std::wstring consoleResponse; + do + { + wchar_t buffer[256]; + DWORD charactersRead = 0; + SetLastError(ERROR_SUCCESS); + // The CRT loses ERROR_OPERATION_ABORTED when Ctrl+C ends a console read. + bool succeeded = ReadConsoleChunk(buffer, ARRAYSIZE(buffer), &charactersRead); + readError = GetLastError(); + readSucceeded = succeeded && charactersRead != 0; + if (!readSucceeded || readError == ERROR_OPERATION_ABORTED) + { + break; + } + consoleResponse.append(buffer, charactersRead); + } while (consoleResponse.back() != L'\n'); + if (readError == ERROR_OPERATION_ABORTED) + { + readCompleted.SetEvent(); + // The console read can end before the Ctrl+C handler records cancellation. + if (!isCancelled || !isCancelled()) + { + cancellationCompleted.wait(); + } + } + response = Utility::ConvertToUTF8(consoleResponse); + readSucceeded = readSucceeded && response.find('\x1a') == std::string::npos; + } + else + { + SetLastError(ERROR_SUCCESS); + readSucceeded = static_cast(std::getline(m_in, response)); + readError = GetLastError(); + } + } + } + if (progress.IsCancelledBy(CancelReason::Any) || (isCancelled && isCancelled()) || readError == ERROR_OPERATION_ABORTED) + { + return std::nullopt; + } + THROW_HR_IF(APPINSTALLER_CLI_ERROR_PROMPT_INPUT_ERROR, !readSucceeded); + return response; + } + + std::optional Reporter::PromptForIntegerResponse(Resource::LocString message, Level level, + Resource::LocString invalid, std::function isCancelled) + { + return PromptForIntegerResponseWithinRange(std::move(message), 0, std::numeric_limits::max(), + level, std::move(invalid), std::move(isCancelled)); + } + + std::optional Reporter::PromptForIntegerResponseWithinRange(Resource::LocString message, uint64_t minimum, uint64_t maximum, + Level level, Resource::LocString invalid, std::function isCancelled) + { + THROW_HR_IF(E_INVALIDARG, minimum > maximum); + + if (!CanPrompt(level)) + { + AICLI_LOG(CLI, Verbose, << "Skipping integer prompt. Console streams or output are unavailable."); + return std::nullopt; + } + + auto out = GetOutputStream(level); + for (;;) + { + if (isCancelled && isCancelled()) + { + return std::nullopt; + } + + out << message << ' ' << std::flush; + auto response = ReadLine(isCancelled); + if (!response || (isCancelled && isCancelled())) + { + return std::nullopt; + } + + Utility::Trim(*response); + uint64_t value = 0; + auto result = std::from_chars(response->data(), response->data() + response->size(), value); + if (result.ec == std::errc{} && result.ptr == response->data() + response->size() && value >= minimum && value <= maximum) + { + return value; + } + + out << invalid << std::endl; + } + } + bool Reporter::PromptForBoolResponse(Resource::LocString message, Level level, bool resultIfDisabled) { auto out = GetOutputStream(level); diff --git a/src/AppInstallerCLICore/ExecutionReporter.h b/src/AppInstallerCLICore/ExecutionReporter.h index d4598b8d67..52f03253f1 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.h +++ b/src/AppInstallerCLICore/ExecutionReporter.h @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -113,6 +114,24 @@ namespace AppInstaller::CLI::Execution // Check if the input stream is interactive or not. bool InputStreamIsInteractive() const; + bool CanPrompt(Level level = Level::Info); + + // Reads one line without output; returns nullopt on cancellation. + std::optional ReadLine(std::function isCancelled = {}); + +#ifndef AICLI_DISABLE_TEST_HOOKS + void SetConsoleStreamsForTest(bool value) { m_consoleStreams = value; } + void SetInputStreamFileTypeForTest(DWORD value) { m_inStreamFileType = value; } +#endif + + // Prompts for a non-negative integer; returns nullopt if unavailable or cancelled. + std::optional PromptForIntegerResponse(Resource::LocString message, Level level = Level::Info, + Resource::LocString invalid = Resource::String::NumberedSelectionInvalid, std::function isCancelled = {}); + + // Prompts for an integer in [minimum, maximum]; returns nullopt if unavailable or cancelled. + std::optional PromptForIntegerResponseWithinRange(Resource::LocString message, uint64_t minimum, uint64_t maximum, + Level level = Level::Info, Resource::LocString invalid = Resource::String::NumberedSelectionInvalid, std::function isCancelled = {}); + // Prompts the user, return true if they consented. bool PromptForBoolResponse(Resource::LocString message, Level level = Level::Info, bool resultIfDisabled = false); @@ -210,6 +229,7 @@ namespace AppInstaller::CLI::Execution std::atomic m_progressSink; DWORD m_outStreamFileType = FILE_TYPE_UNKNOWN; DWORD m_inStreamFileType = FILE_TYPE_UNKNOWN; + bool m_consoleStreams = false; // Enable all levels by default Level m_enabledLevels = Level::All; diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 68e194d5f1..1d3ea9b14e 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -515,6 +515,8 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(NoUninstallInfoFound); WINGET_DEFINE_RESOURCE_STRINGID(NoUpgradeArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(NoVTArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(NumberedSelectionInvalid); + WINGET_DEFINE_RESOURCE_STRINGID(NumberedSelectionPrompt); WINGET_DEFINE_RESOURCE_STRINGID(OpenLogsArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(OpenSourceFailedNoMatch); WINGET_DEFINE_RESOURCE_STRINGID(OpenSourceFailedNoMatchHelp); @@ -532,6 +534,10 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PackageDependencies); WINGET_DEFINE_RESOURCE_STRINGID(PackageIsPinned); WINGET_DEFINE_RESOURCE_STRINGID(PackageRequiresDependencies); + WINGET_DEFINE_RESOURCE_STRINGID(PackageSelectionRefine); + WINGET_DEFINE_RESOURCE_STRINGID(PackageSelectionRefineForExport); + WINGET_DEFINE_RESOURCE_STRINGID(PackageSelectionSelected); + WINGET_DEFINE_RESOURCE_STRINGID(PackageSelectionTitle); WINGET_DEFINE_RESOURCE_STRINGID(PendingWorkError); WINGET_DEFINE_RESOURCE_STRINGID(PinAddBlockingArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinAddCommandLongDescription); diff --git a/src/AppInstallerCLICore/TableOutput.cpp b/src/AppInstallerCLICore/TableOutput.cpp new file mode 100644 index 0000000000..5af774dde1 --- /dev/null +++ b/src/AppInstallerCLICore/TableOutput.cpp @@ -0,0 +1,143 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TableOutput.h" +#include + +namespace AppInstaller::CLI::Execution +{ + TableOutputBase::TableOutputBase(Reporter& reporter, std::vector header) : + m_reporter(reporter) + { + THROW_HR_IF(E_INVALIDARG, header.empty()); + for (auto& name : header) + { + auto width = Utility::UTF8ColumnWidth(name.get()); + m_columns.push_back({ std::move(name), width }); + } + } + + void TableOutputBase::OutputLine(std::vector line) + { + THROW_HR_IF(E_INVALIDARG, line.size() != m_columns.size()); + m_buffer.emplace_back(std::move(line)); + } + + void TableOutputBase::Complete(bool showLineNumbers) + { + if (!IsEmpty() && !m_bufferEvaluated) + { + EvaluateAndFlushBuffer(showLineNumbers); + } + } + + void TableOutputBase::EvaluateAndFlushBuffer(bool showLineNumbers) + { + for (const auto& row : m_buffer) + { + for (size_t i = 0; i < m_columns.size(); ++i) + { + m_columns[i].MaxLength = std::max(m_columns[i].MaxLength, Utility::UTF8ColumnWidth(row[i])); + } + } + + for (auto& column : m_columns) + { + if (column.MaxLength) + { + column.MaxLength = std::max(column.MaxLength, column.MinLength); + } + } + + m_columns.back().SpaceAfter = false; + for (size_t i = m_columns.size() - 1; i > 0; --i) + { + if (m_columns[i].MaxLength) + { + break; + } + m_columns[i - 1].SpaceAfter = false; + } + + m_lineNumberWidth = showLineNumbers ? std::to_string(m_buffer.size()).size() : 0; + size_t totalRequired = m_lineNumberWidth ? m_lineNumberWidth + 1 : 0; + for (const auto& column : m_columns) + { + totalRequired += column.MaxLength + (column.SpaceAfter ? 1 : 0); + } + + auto consoleWidth = GetConsoleWidth(); + if (consoleWidth && totalRequired >= *consoleWidth) + { + size_t extra = (totalRequired - *consoleWidth) + 1; + while (extra) + { + auto widest = std::max_element(m_columns.begin(), m_columns.end(), + [](const auto& left, const auto& right) { return left.MaxLength < right.MaxLength; }); + if (!widest->MaxLength) + { + break; + } + --widest->MaxLength; + --totalRequired; + --extra; + } + } + + std::vector header; + for (const auto& column : m_columns) + { + header.emplace_back(column.Name.get()); + } + OutputLineToStream(header); + m_reporter.Info() << std::string(totalRequired, '-') << std::endl; + size_t lineNumber = 0; + for (const auto& row : m_buffer) + { + OutputLineToStream(row, ++lineNumber); + } + m_bufferEvaluated = true; + } + + void TableOutputBase::OutputLineToStream(const std::vector& line, size_t lineNumber) + { + auto out = m_reporter.Info(); + if (m_lineNumberWidth) + { + const std::string number = lineNumber ? std::to_string(lineNumber) : "#"; + out << number << std::string(m_lineNumberWidth - number.size() + 1, ' '); + } + for (size_t i = 0; i < m_columns.size(); ++i) + { + const auto& column = m_columns[i]; + if (column.MaxLength) + { + std::string_view value = line[i]; + size_t valueLength = Utility::UTF8ColumnWidth(value); + if (valueLength > column.MaxLength) + { + size_t actualWidth; + out << Utility::UTF8TrimRightToColumnWidth(value, column.MaxLength - 1, actualWidth) << "\xE2\x80\xA6"; + // Wide characters can leave one column unused before the ellipsis. + if (actualWidth != column.MaxLength - 1) + { + out << ' '; + } + if (column.SpaceAfter) + { + out << ' '; + } + } + else + { + out << value; + if (column.SpaceAfter) + { + out << std::string(column.MaxLength - valueLength + 1, ' '); + } + } + } + } + out << std::endl; + } +} diff --git a/src/AppInstallerCLICore/TableOutput.h b/src/AppInstallerCLICore/TableOutput.h index 6bfb59848b..c6439f0d42 100644 --- a/src/AppInstallerCLICore/TableOutput.h +++ b/src/AppInstallerCLICore/TableOutput.h @@ -5,56 +5,22 @@ #include "Resources.h" #include -#include +#include #include #include namespace AppInstaller::CLI::Execution { - // Enables output data in a table format. - // TODO: Improve for use with sparse data. - template - struct TableOutput + struct TableOutputBase { - using header_t = std::array; - using line_t = std::array; - - TableOutput(Reporter& reporter, header_t&& header) : - m_reporter(reporter), - m_hasConsole(GetConsoleWidth().has_value()) - { - for (size_t i = 0; i < FieldCount; ++i) - { - m_columns[i].Name = std::move(header[i]); - m_columns[i].MinLength = Utility::UTF8ColumnWidth(m_columns[i].Name.get()); - m_columns[i].MaxLength = 0; - } - } - - void OutputLine(line_t&& line) - { - m_empty = false; - - // Always buffer every row so that column widths are computed from the full dataset - // before any output is written. This guarantees that the widest value in any column - // is always fully visible and columns are perfectly aligned, whether output goes to - // a console or is redirected. Complete() triggers the actual output. - m_buffer.emplace_back(std::move(line)); - } + TableOutputBase(Reporter& reporter, std::vector header); - void Complete() - { - if (!m_empty) - { - EvaluateAndFlushBuffer(); - } - } - - bool IsEmpty() - { - return m_empty; - } + // Buffers rows until Complete() computes column widths and renders the table. + void OutputLine(std::vector line); + void Complete(bool showLineNumbers = false); + bool IsEmpty() const { return m_buffer.empty(); } + size_t GetRowCount() const { return m_buffer.size(); } private: // A column in the table. @@ -67,152 +33,28 @@ namespace AppInstaller::CLI::Execution }; Reporter& m_reporter; - std::array m_columns; - std::vector m_buffer; + std::vector m_columns; + std::vector> m_buffer; + size_t m_lineNumberWidth = 0; bool m_bufferEvaluated = false; - bool m_empty = true; - bool m_hasConsole = false; - - void EvaluateAndFlushBuffer() - { - if (m_bufferEvaluated) - { - return; - } - - // Determine the maximum length for all columns - for (const auto& line : m_buffer) - { - for (size_t i = 0; i < FieldCount; ++i) - { - m_columns[i].MaxLength = std::max(m_columns[i].MaxLength, Utility::UTF8ColumnWidth(line[i])); - } - } - - // If there are actually columns with data, then also bring in the minimum size - for (size_t i = 0; i < FieldCount; ++i) - { - if (m_columns[i].MaxLength) - { - m_columns[i].MaxLength = std::max(m_columns[i].MaxLength, m_columns[i].MinLength); - } - } - - // Only output the extra space if: - // 1. Not the last field - m_columns[FieldCount - 1].SpaceAfter = false; - - // 2. Not empty (taken care of by not doing anything if empty) - // 3. There are non-empty fields after - for (size_t i = FieldCount - 1; i > 0; --i) - { - if (m_columns[i].MaxLength) - { - break; - } - else - { - m_columns[i - 1].SpaceAfter = false; - } - } - - // Determine the total width required to not truncate any columns - size_t totalRequired = 0; - for (size_t i = 0; i < FieldCount; ++i) - { - totalRequired += m_columns[i].MaxLength + (m_columns[i].SpaceAfter ? 1 : 0); - } - - auto consoleWidthOpt = GetConsoleWidth(); - - // If there is a console and the total space would be too big, shrink columns. - // We don't want to use the last column, lest we auto-wrap. - // When there is no console (e.g. output redirected to a file), skip truncation entirely. - if (consoleWidthOpt && totalRequired >= *consoleWidthOpt) - { - size_t extra = (totalRequired - *consoleWidthOpt) + 1; - - while (extra) - { - size_t targetIndex = 0; - size_t targetVal = m_columns[0].MaxLength; - for (size_t j = 1; j < FieldCount; ++j) - { - if (m_columns[j].MaxLength > targetVal) - { - targetIndex = j; - targetVal = m_columns[j].MaxLength; - } - } - m_columns[targetIndex].MaxLength -= 1; - extra -= 1; - } - - totalRequired = *consoleWidthOpt - 1; - } - - // Header line - line_t headerLine; - - for (size_t i = 0; i < FieldCount; ++i) - { - headerLine[i] = m_columns[i].Name.get(); - } - - OutputLineToStream(headerLine); - - m_reporter.Info() << std::string(totalRequired, '-') << std::endl; + void EvaluateAndFlushBuffer(bool showLineNumbers); + void OutputLineToStream(const std::vector& line, size_t lineNumber = 0); + }; - for (const auto& line : m_buffer) - { - OutputLineToStream(line); - } + // Retains fixed-size headers and rows for existing table callers. + template + struct TableOutput : public TableOutputBase + { + using header_t = std::array; + using line_t = std::array; - m_bufferEvaluated = true; - } + TableOutput(Reporter& reporter, header_t&& header) : + TableOutputBase(reporter, { std::make_move_iterator(header.begin()), std::make_move_iterator(header.end()) }) {} - void OutputLineToStream(const line_t& line) + void OutputLine(line_t&& line) { - auto out = m_reporter.Info(); - - for (size_t i = 0; i < FieldCount; ++i) - { - const auto& col = m_columns[i]; - - if (col.MaxLength) - { - size_t valueLength = Utility::UTF8ColumnWidth(line[i]); - - if (valueLength > col.MaxLength) - { - size_t actualWidth; - out << Utility::UTF8TrimRightToColumnWidth(line[i], col.MaxLength - 1, actualWidth) << "\xE2\x80\xA6"; // UTF8 encoding of ellipsis (…) character - - // Some characters take 2 unit space, the trimmed string length might be 1 less than the expected length. - if (actualWidth != col.MaxLength - 1) - { - out << ' '; - } - - if (col.SpaceAfter) - { - out << ' '; - } - } - else - { - out << line[i]; - - if (col.SpaceAfter) - { - out << std::string(col.MaxLength - valueLength + 1, ' '); - } - } - } - } - - out << std::endl; + TableOutputBase::OutputLine({ std::make_move_iterator(line.begin()), std::make_move_iterator(line.end()) }); } }; } diff --git a/src/AppInstallerCLICore/Workflows/MultiQueryFlow.cpp b/src/AppInstallerCLICore/Workflows/MultiQueryFlow.cpp index c673b13f3d..bb26671cc6 100644 --- a/src/AppInstallerCLICore/Workflows/MultiQueryFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/MultiQueryFlow.cpp @@ -71,7 +71,7 @@ namespace AppInstaller::CLI::Workflow { case OperationType::Install: case OperationType::Upgrade: - searchContext << Workflow::SelectSinglePackageVersionForInstallOrUpgrade(m_operationType); + searchContext << Workflow::SelectSinglePackageVersionForInstallOrUpgrade(m_operationType, PackageSelectionBehavior::Disabled); break; case OperationType::Uninstall: searchContext << diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp index 5e298f5c92..bf4712be47 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.cpp @@ -395,6 +395,42 @@ namespace AppInstaller::CLI::Workflow } } + void PromptForSelection::operator()(Execution::Context& context) const + { + context.Add(std::optional{}); + AICLI_RETURN_IF_TERMINATED(context); + const size_t count = m_table.GetRowCount(); + THROW_HR_IF(E_INVALIDARG, !count); + + if (!IsInteractivityAllowed(context)) + { + return; + } + if (!context.Reporter.CanPrompt()) + { + AICLI_LOG(CLI, Verbose, << "Skipping selection prompt. Console streams or output are unavailable."); + return; + } + + auto out = context.Reporter.Info(); + out << m_title << std::endl << std::endl; + m_table.Complete(true); + out << std::endl; + + const auto prompt = Resource::String::NumberedSelectionPrompt(count); + AICLI_RETURN_IF_TERMINATED(context); + auto response = context.Reporter.PromptForIntegerResponseWithinRange(prompt, 0, count, Reporter::Level::Info, + Resource::String::NumberedSelectionInvalid, [&]() { return context.IsTerminated(); }); + AICLI_RETURN_IF_TERMINATED(context); + if (!response || *response == 0) + { + out << Resource::String::Cancelled << std::endl; + AICLI_TERMINATE_CONTEXT(E_ABORT); + } + + context.Add(std::optional{ static_cast(*response - 1) }); + } + void HandleSourceAgreements::operator()(Execution::Context& context) const { bool allAccepted = true; diff --git a/src/AppInstallerCLICore/Workflows/PromptFlow.h b/src/AppInstallerCLICore/Workflows/PromptFlow.h index 7219cb1fff..97652fd54d 100644 --- a/src/AppInstallerCLICore/Workflows/PromptFlow.h +++ b/src/AppInstallerCLICore/Workflows/PromptFlow.h @@ -2,9 +2,26 @@ // Licensed under the MIT License. #pragma once #include "ExecutionContext.h" +#include "TableOutput.h" namespace AppInstaller::CLI::Workflow { + // Prompts for a numbered choice among the table's rows. + // Required Args: None + // Inputs: None + // Outputs: SelectedIndex (zero-based index, or nullopt if prompting is unavailable) + struct PromptForSelection : public WorkflowTask + { + PromptForSelection(Execution::TableOutputBase& table, Resource::LocString title) : + WorkflowTask("PromptForSelection"), m_table(table), m_title(std::move(title)) {} + + void operator()(Execution::Context& context) const override; + + private: + Execution::TableOutputBase& m_table; + Resource::LocString m_title; + }; + // Handles all opened source(s) agreements if needed. // Required Args: The source to be checked for agreements // Inputs: None diff --git a/src/AppInstallerCLICore/Workflows/ShowFlow.cpp b/src/AppInstallerCLICore/Workflows/ShowFlow.cpp index eb872b2110..c465225a2f 100644 --- a/src/AppInstallerCLICore/Workflows/ShowFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ShowFlow.cpp @@ -226,7 +226,7 @@ namespace AppInstaller::CLI::Workflow OpenSource() << SearchSourceForSingle << HandleSearchResultFailures << - EnsureOneMatchFromSearchResult(OperationType::Show) << + EnsureOneMatchFromSearchResult(OperationType::Show, m_selectionBehavior) << GetManifestFromPackage(m_considerPins); } } diff --git a/src/AppInstallerCLICore/Workflows/ShowFlow.h b/src/AppInstallerCLICore/Workflows/ShowFlow.h index 618a1795cb..6992adf387 100644 --- a/src/AppInstallerCLICore/Workflows/ShowFlow.h +++ b/src/AppInstallerCLICore/Workflows/ShowFlow.h @@ -44,12 +44,14 @@ namespace AppInstaller::CLI::Workflow // Outputs: Manifest struct GetManifest : public WorkflowTask { - GetManifest(bool considerPins) : WorkflowTask("GetManifest"), m_considerPins(considerPins) {} + GetManifest(bool considerPins, PackageSelectionBehavior selectionBehavior = PackageSelectionBehavior::Disabled) : + WorkflowTask("GetManifest"), m_considerPins(considerPins), m_selectionBehavior(selectionBehavior) {} void operator()(Execution::Context& context) const override; private: bool m_considerPins; + PackageSelectionBehavior m_selectionBehavior; }; // Reusable helpers for `show` style line output diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index 2e8f35100d..a2c243673e 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -322,7 +322,7 @@ namespace AppInstaller::CLI::Workflow context << HandleSearchResultFailures << - EnsureOneMatchFromSearchResult(m_operationType) << + EnsureOneMatchFromSearchResult(m_operationType, m_selectionBehavior) << GetInstalledPackageVersion; if ( m_operationType != OperationType::Upgrade && @@ -372,7 +372,7 @@ namespace AppInstaller::CLI::Workflow context << SearchSourceForSingle << - SelectSinglePackageVersionForInstallOrUpgrade(m_operationType) << + SelectSinglePackageVersionForInstallOrUpgrade(m_operationType, m_selectionBehavior) << InstallSinglePackage; } } diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.h b/src/AppInstallerCLICore/Workflows/UpdateFlow.h index cb84b3452d..0d686f6483 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.h +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.h @@ -39,14 +39,16 @@ namespace AppInstaller::CLI::Workflow // Outputs: None struct SelectSinglePackageVersionForInstallOrUpgrade : public WorkflowTask { - SelectSinglePackageVersionForInstallOrUpgrade(OperationType operation, bool allowDowngrade = false) : - WorkflowTask("SelectSinglePackageVersionForInstallOrUpgrade"), m_operationType(operation), m_allowDowngrade(allowDowngrade) {} + SelectSinglePackageVersionForInstallOrUpgrade(OperationType operation, PackageSelectionBehavior selectionBehavior, bool allowDowngrade = false) : + WorkflowTask("SelectSinglePackageVersionForInstallOrUpgrade"), m_operationType(operation), m_allowDowngrade(allowDowngrade), + m_selectionBehavior(selectionBehavior) {} void operator()(Execution::Context& context) const override; private: mutable OperationType m_operationType; bool m_allowDowngrade; + PackageSelectionBehavior m_selectionBehavior; }; // Install or upgrade a single package @@ -55,12 +57,13 @@ namespace AppInstaller::CLI::Workflow // Outputs: None struct InstallOrUpgradeSinglePackage : public WorkflowTask { - InstallOrUpgradeSinglePackage(OperationType operation) : - WorkflowTask("InstallOrUpgradeSinglePackage"), m_operationType(operation) {} + InstallOrUpgradeSinglePackage(OperationType operation, PackageSelectionBehavior selectionBehavior = PackageSelectionBehavior::Disabled) : + WorkflowTask("InstallOrUpgradeSinglePackage"), m_operationType(operation), m_selectionBehavior(selectionBehavior) {} void operator()(Execution::Context& context) const override; private: mutable OperationType m_operationType; + PackageSelectionBehavior m_selectionBehavior; }; } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 3a238f57fc..d4994cf59d 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -48,6 +48,65 @@ namespace AppInstaller::CLI::Workflow } } + Execution::TableOutputBase GetMultiplePackageFoundResultTable(Execution::Context& context) + { + auto& searchResult = context.Get(); + + Execution::TableOutput<2> table(context.Reporter, + { + Resource::String::SearchName, + Resource::String::SearchId + }); + + for (size_t i = 0; i < searchResult.Matches.size(); ++i) + { + auto package = searchResult.Matches[i].Package; + + table.OutputLine({ + package->GetProperty(PackageProperty::Name), + package->GetProperty(PackageProperty::Id) + }); + } + + return table; + } + + Execution::TableOutputBase GetMultiplePackageFoundResultTableWithSource(Execution::Context& context) + { + auto& searchResult = context.Get(); + + Execution::TableOutput<3> table(context.Reporter, + { + Resource::String::SearchName, + Resource::String::SearchId, + Resource::String::SearchSource + }); + + for (size_t i = 0; i < searchResult.Matches.size(); ++i) + { + auto package = searchResult.Matches[i].Package; + + std::string sourceName; + auto available = package->GetAvailable(); + if (!available.empty()) + { + auto source = available[0]->GetSource(); + if (source) + { + sourceName = source.GetDetails().Name; + } + } + + table.OutputLine({ + package->GetProperty(PackageProperty::Name), + package->GetProperty(PackageProperty::Id), + std::move(sourceName) + }); + } + + return table; + } + void ReportIdentity( Execution::Context& context, Utility::LocIndView prefix, @@ -1043,7 +1102,12 @@ namespace AppInstaller::CLI::Workflow else { context.Reporter.Info() << std::endl << Resource::String::SearchFailureErrorListMatches << std::endl; - context << ReportMultiplePackageFoundResultWithSource; + auto table = GetMultiplePackageFoundResultTableWithSource(context); + table.Complete(); + if (searchResult.Truncated) + { + context.Reporter.Info() << '<' << Resource::String::SearchTruncated << '>' << std::endl; + } } } @@ -1052,75 +1116,6 @@ namespace AppInstaller::CLI::Workflow } } - void ReportMultiplePackageFoundResult(Execution::Context& context) - { - auto& searchResult = context.Get(); - - Execution::TableOutput<2> table(context.Reporter, - { - Resource::String::SearchName, - Resource::String::SearchId - }); - - for (size_t i = 0; i < searchResult.Matches.size(); ++i) - { - auto package = searchResult.Matches[i].Package; - - table.OutputLine({ - package->GetProperty(PackageProperty::Name), - package->GetProperty(PackageProperty::Id) - }); - } - - table.Complete(); - - if (searchResult.Truncated) - { - context.Reporter.Info() << '<' << Resource::String::SearchTruncated << '>' << std::endl; - } - } - - void ReportMultiplePackageFoundResultWithSource(Execution::Context& context) - { - auto& searchResult = context.Get(); - - Execution::TableOutput<3> table(context.Reporter, - { - Resource::String::SearchName, - Resource::String::SearchId, - Resource::String::SearchSource - }); - - for (size_t i = 0; i < searchResult.Matches.size(); ++i) - { - auto package = searchResult.Matches[i].Package; - - std::string sourceName; - auto available = package->GetAvailable(); - if (!available.empty()) - { - auto source = available[0]->GetSource(); - if (source) - { - sourceName = source.GetDetails().Name; - } - } - - table.OutputLine({ - package->GetProperty(PackageProperty::Name), - package->GetProperty(PackageProperty::Id), - std::move(sourceName) - }); - } - - table.Complete(); - - if (searchResult.Truncated) - { - context.Reporter.Info() << '<' << Resource::String::SearchTruncated << '>' << std::endl; - } - } - void ReportListResult::operator()(Execution::Context& context) const { auto& searchResult = context.Get(); @@ -1396,25 +1391,46 @@ namespace AppInstaller::CLI::Workflow } } + size_t selectedIndex = 0; if (searchResult.Matches.size() > 1) { Logging::Telemetry().LogMultiAppMatch(); - if (operationTargetsInstalled) + auto table = operationTargetsInstalled ? GetMultiplePackageFoundResultTable(context) : + GetMultiplePackageFoundResultTableWithSource(context); + bool selectionSupported = m_selectionBehavior == PackageSelectionBehavior::Prompt && + Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::InteractivePackageSelection); + std::optional selection; + if (selectionSupported && !searchResult.Truncated) { - context.Reporter.Warn() << Resource::String::MultipleInstalledPackagesFound << std::endl; - context << ReportMultiplePackageFoundResult; + context << PromptForSelection(table, Resource::String::PackageSelectionTitle); + AICLI_RETURN_IF_TERMINATED(context); + selection = context.Get(); + } + + if (selection) + { + selectedIndex = *selection; + auto package = searchResult.Matches[selectedIndex].Package; + context.Reporter.Info() << Resource::String::PackageSelectionSelected(package->GetProperty(PackageProperty::Name), + package->GetProperty(PackageProperty::Id)) << std::endl; } else { - context.Reporter.Warn() << Resource::String::MultiplePackagesFound << std::endl; - context << ReportMultiplePackageFoundResultWithSource; + context.Reporter.Warn() << (operationTargetsInstalled ? Resource::String::MultipleInstalledPackagesFound : + Resource::String::MultiplePackagesFound) << std::endl; + table.Complete(); + if (searchResult.Truncated) + { + context.Reporter.Info() << '<' << Resource::String::SearchTruncated << '>' << std::endl; + } + context.Reporter.Info() << (m_operationType == OperationType::Export ? + Resource::String::PackageSelectionRefineForExport : Resource::String::PackageSelectionRefine) << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND); } - - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND); } - std::shared_ptr package = searchResult.Matches.at(0).Package; + std::shared_ptr package = searchResult.Matches.at(selectedIndex).Package; Logging::Telemetry().LogAppFound(package->GetProperty(PackageProperty::Name), package->GetProperty(PackageProperty::Id)); context.Add(std::move(package)); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 7d9cba0ae3..3c026e0310 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -46,6 +46,12 @@ namespace AppInstaller::CLI::Workflow Repair, }; + enum class PackageSelectionBehavior + { + Disabled, + Prompt, + }; + // A task in the workflow. struct WorkflowTask { @@ -230,18 +236,6 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void HandleSearchResultFailures(Execution::Context& context); - // Outputs the search results when multiple packages found but only one expected. - // Required Args: None - // Inputs: SearchResult - // Outputs: None - void ReportMultiplePackageFoundResult(Execution::Context& context); - - // Outputs the search results when multiple packages found but only one expected. - // Required Args: None - // Inputs: SearchResult - // Outputs: None - void ReportMultiplePackageFoundResultWithSource(Execution::Context& context); - // Ensures that there is at least one result in the search. // Required Args: bool indicating if the search result is from installed source // Inputs: SearchResult @@ -263,13 +257,14 @@ namespace AppInstaller::CLI::Workflow // Outputs: Package struct EnsureOneMatchFromSearchResult : public WorkflowTask { - EnsureOneMatchFromSearchResult(OperationType operation) : - WorkflowTask("EnsureOneMatchFromSearchResult"), m_operationType(operation) {} + EnsureOneMatchFromSearchResult(OperationType operation, PackageSelectionBehavior selectionBehavior = PackageSelectionBehavior::Disabled) : + WorkflowTask("EnsureOneMatchFromSearchResult"), m_operationType(operation), m_selectionBehavior(selectionBehavior) {} void operator()(Execution::Context& context) const override; private: OperationType m_operationType; + PackageSelectionBehavior m_selectionBehavior; }; // Gets the manifest from package. diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 31d153dffa..75b2d64342 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -407,6 +407,30 @@ They can be configured through the settings file 'winget settings'. Multiple packages found matching input criteria. Please refine the input. + + Multiple packages match. Choose one. + Introduces a numbered list of packages the user can choose from. + + + Enter a number (1-{0}), or 0 to cancel: + {Locked="{0}"} {0} is the number of choices in the list. The input is a numeric choice; 0 cancels the command. + + + Invalid selection. + Shown after invalid or empty input to a numbered selection prompt. + + + Selected: {0} [{1}] + {Locked="{0}","{1}"} {0} is the package name and {1} is its identifier. Confirms the user's choice before continuing the original command. + + + Specify a package with --id <ID> --exact --source <SOURCE>. + {Locked="--id","--exact","--source","ID","SOURCE"} Guidance when multiple packages match. ID and SOURCE are placeholders. + + + Specify a package with --package-id <ID> --source <SOURCE>. + {Locked="--package-id","--source","ID","SOURCE"} Guidance for configure export when multiple packages match. ID and SOURCE are placeholders. + Filter results by name diff --git a/src/AppInstallerCLITests/ExperimentalFeature.cpp b/src/AppInstallerCLITests/ExperimentalFeature.cpp index d14bbbf5ea..6c75f6040b 100644 --- a/src/AppInstallerCLITests/ExperimentalFeature.cpp +++ b/src/AppInstallerCLITests/ExperimentalFeature.cpp @@ -7,6 +7,8 @@ #include #include +#include +#include using namespace AppInstaller::Settings; using namespace TestCommon; @@ -72,4 +74,34 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]") REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest)); } +} + +TEST_CASE("ExperimentalFeature InteractivePackageSelection", "[experimentalFeature]") +{ + auto again = DeleteUserSettingsFiles(); + auto [json, enabled] = GENERATE( + std::make_pair(std::string_view{ "{}" }, false), + std::make_pair(std::string_view{ R"({ "experimentalFeatures": { "interactivePackageSelection": true } })" }, true), + std::make_pair(std::string_view{ R"({ "experimentalFeatures": { "interactivePackageSelection": false } })" }, false), + std::make_pair(std::string_view{ R"({ "experimentalFeatures": { "interactivePackageSelection": "string" } })" }, false)); + bool policyEnabled = GENERATE(false, true); + auto policiesKey = RegCreateVolatileTestRoot(); + SetRegistryValue(policiesKey.get(), ExperimentalFeaturesPolicyValueName, policyEnabled); + GroupPolicyTestOverride policies{ policiesKey.get() }; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + CAPTURE(json, policyEnabled); + REQUIRE(userSettingTest.Get() == enabled); + REQUIRE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::InteractivePackageSelection, userSettingTest) == + (enabled && policyEnabled)); + + auto feature = ExperimentalFeature::GetFeature(ExperimentalFeature::Feature::InteractivePackageSelection); + std::string_view jsonName = feature.JsonName(); + REQUIRE(jsonName == "interactivePackageSelection"); + auto features = ExperimentalFeature::GetAllFeatures(); + REQUIRE(std::any_of(features.begin(), features.end(), [](const auto& item) + { + return item.GetFeature() == ExperimentalFeature::Feature::InteractivePackageSelection; + })); } \ No newline at end of file diff --git a/src/AppInstallerCLITests/PromptFlow.cpp b/src/AppInstallerCLITests/PromptFlow.cpp index f07e22c45d..d1e2ce5d7a 100644 --- a/src/AppInstallerCLITests/PromptFlow.cpp +++ b/src/AppInstallerCLITests/PromptFlow.cpp @@ -2,13 +2,1212 @@ // Licensed under the MIT License. #include "pch.h" #include "WorkflowCommon.h" +#include "TestHooks.h" #include +#include +#include #include +#include +#include +#include +#include +#include using namespace TestCommon; using namespace AppInstaller::CLI; +using namespace AppInstaller::CLI::Workflow; +using namespace AppInstaller::Repository; using namespace AppInstaller::Settings; +TEST_CASE("PackageSelection_Prompt", "[PackageSelection][PromptFlow]") +{ + TestUserSettings settings; + auto response = GENERATE("1", "2", "10", " 2 \t", "0"); + std::istringstream input{ response }; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + Execution::TableOutput<1> table(context.Reporter, { Resource::String::SearchName }); + for (size_t i = 1; i <= 10; ++i) + { + table.OutputLine({ std::to_string(i) }); + } + + context << PromptForSelection(table, Resource::String::PackageSelectionTitle); + auto selection = context.Get(); + if (std::string_view{ response } == "0") + { + REQUIRE_FALSE(selection); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + } + else + { + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(selection == static_cast(std::stoul(response) - 1)); + } + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(10).get()) != std::string::npos); +} + +TEST_CASE("PackageSelection_InvalidInput", "[PackageSelection][PromptFlow]") +{ + TestUserSettings settings; + auto response = GENERATE("", " ", "-1", "-0", "+1", "3", "1x", "1.0", "1 2", + "4294967297", "18446744073709551615", "99999999999999999999999999"); + std::istringstream input{ std::string{ response } + "\n2\n" }; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + Execution::TableOutput<2> table(context.Reporter, { Resource::String::SearchName, Resource::String::SearchSource }); + table.OutputLine({ GENERATE("First", ""), "FirstSource" }); + table.OutputLine({ "Second", "FirstSource" }); + + context << PromptForSelection(table, Resource::String::PackageSelectionTitle); + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Get() == size_t{1}); + const std::string prompt = Resource::String::NumberedSelectionPrompt(2).get(); + const std::string invalid = Resource::LocString{ Resource::String::NumberedSelectionInvalid }.get(); + REQUIRE(output.str().find(prompt + " " + invalid + '\n' + prompt + " ") != std::string::npos); +} + +TEST_CASE("PromptFlow_Selection_CustomTitle", "[PromptFlow]") +{ + TestUserSettings settings; + std::istringstream input{ "wrong\n2\n" }; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + auto text = [](std::string value) + { + return Resource::LocString{ AppInstaller::Utility::LocIndString{ std::move(value) } }; + }; + Execution::TableOutput<1> table(context.Reporter, { text("Choice") }); + table.OutputLine({ "First" }); + table.OutputLine({ "Second" }); + context << PromptForSelection(table, text("Choose a value")); + + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Get() == size_t{1}); + const std::string prompt = Resource::String::NumberedSelectionPrompt(2).get(); + const std::string invalid = Resource::LocString{ Resource::String::NumberedSelectionInvalid }.get(); + REQUIRE_FALSE(prompt.empty()); + REQUIRE_FALSE(invalid.empty()); + REQUIRE(output.str() == "Choose a value\n\n# Choice\n--------\n1 First\n2 Second\n\n" + prompt + " " + invalid + '\n' + prompt + " "); +} + +TEST_CASE("PromptFlow_Selection_Unavailable", "[PromptFlow]") +{ + TestUserSettings settings; + std::istringstream input{ "1\n" }; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + context.Add(std::optional{0}); + Execution::TableOutput<1> table(context.Reporter, { Resource::String::SearchName }); + table.OutputLine({ "First" }); + + SECTION("Context disabled") + { + context.SetFlags(Execution::ContextFlag::DisableInteractivity); + } + SECTION("Argument disabled") + { + context.Args.AddArg(Execution::Args::Type::DisableInteractivity); + } + SECTION("Setting disabled") + { + settings.Set(true); + } + SECTION("Redirected streams") + { + context.Reporter.SetConsoleStreamsForTest(false); + } + SECTION("Hidden output") + { + context.Reporter.SetLevelMask(Execution::Reporter::Level::Info, false); + } + + context << PromptForSelection(table, Resource::String::PackageSelectionTitle); + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE_FALSE(context.Get()); + REQUIRE(output.str().empty()); + REQUIRE(input.peek() == '1'); +} + +TEST_CASE("PromptFlow_Selection_InputFailure", "[PromptFlow]") +{ + TestUserSettings settings; + std::istringstream input; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + Execution::TableOutput<1> table(context.Reporter, { Resource::String::SearchName }); + auto count = GENERATE(size_t{0}, size_t{1}, size_t{2}); + for (size_t i = 0; i < count; ++i) + { + table.OutputLine({ "First" }); + } + PromptForSelection prompt(table, Resource::String::PackageSelectionTitle); + + REQUIRE_THROWS_HR(prompt(context), count ? APPINSTALLER_CLI_ERROR_PROMPT_INPUT_ERROR : E_INVALIDARG); + REQUIRE_FALSE(context.Get()); +} + +TEST_CASE("ReporterPromptForIntegerResponse", "[PromptFlow]") +{ + auto response = GENERATE("0", "1", "42", " 42 \t", "18446744073709551615"); + auto level = GENERATE(Execution::Reporter::Level::Info, Execution::Reporter::Level::Warning, Execution::Reporter::Level::Error); + std::istringstream input{ std::string{ response } + '\n' + "next\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetStyle(VisualStyle::NoVT); + reporter.SetLevelMask(Execution::Reporter::Level::All, false); + reporter.SetLevelMask(level); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + + auto result = reporter.PromptForIntegerResponse(message, level); + REQUIRE(result.has_value()); + REQUIRE(*result == std::stoull(response)); + REQUIRE(input.peek() == 'n'); + REQUIRE(output.str() == "Number: "); +} + +TEST_CASE("ReporterPromptForIntegerResponse_InvalidInput", "[PromptFlow]") +{ + auto response = GENERATE("", " ", "-1", "-0", "+1", "1x", "1.0", "1 2", "18446744073709551616"); + std::istringstream input{ std::string{ response } + '\n' + "2\n" + "next\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetStyle(VisualStyle::NoVT); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + + REQUIRE(reporter.PromptForIntegerResponse(message) == uint64_t{2}); + REQUIRE(input.peek() == 'n'); + const std::string invalid = Resource::LocString{ Resource::String::NumberedSelectionInvalid }.get(); + REQUIRE_FALSE(invalid.empty()); + REQUIRE(output.str() == "Number: " + invalid + '\n' + "Number: "); +} + +TEST_CASE("ReporterPromptForIntegerResponse_InputFailure", "[PromptFlow]") +{ + std::istringstream input; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetStyle(VisualStyle::NoVT); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + std::string expectedOutput = "Number: "; + + SECTION("EOF") + { + REQUIRE_THROWS_HR(reporter.PromptForIntegerResponse(message), APPINSTALLER_CLI_ERROR_PROMPT_INPUT_ERROR); + } + SECTION("Cancelled before prompting") + { + input.str("1\n"); + REQUIRE_FALSE(reporter.PromptForIntegerResponse(message, Execution::Reporter::Level::Info, + Resource::String::NumberedSelectionInvalid, []() { return true; })); + REQUIRE(input.peek() == '1'); + expectedOutput.clear(); + } + REQUIRE(output.str() == expectedOutput); +} + +TEST_CASE("ReporterPromptForIntegerResponseWithinRange", "[PromptFlow]") +{ + const uint64_t minimum = GENERATE(uint64_t{0}, uint64_t{2}, std::numeric_limits::max() - 2); + const uint64_t maximum = minimum + GENERATE(uint64_t{0}, uint64_t{2}); + const uint64_t response = GENERATE_COPY(minimum, maximum, minimum + (maximum - minimum) / 2); + auto level = GENERATE(Execution::Reporter::Level::Info, Execution::Reporter::Level::Warning, Execution::Reporter::Level::Error); + std::istringstream input{ std::to_string(response) + "\n" "next\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetStyle(VisualStyle::NoVT); + reporter.SetLevelMask(Execution::Reporter::Level::All, false); + reporter.SetLevelMask(level); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + + auto result = reporter.PromptForIntegerResponseWithinRange(message, minimum, maximum, level); + REQUIRE(result.has_value()); + REQUIRE(*result == response); + REQUIRE(input.peek() == 'n'); + REQUIRE(output.str() == "Number: "); +} + +TEST_CASE("ReporterPromptForIntegerResponseWithinRange_InvalidInput", "[PromptFlow]") +{ + auto response = GENERATE("wrong", "0", "1", "5", "18446744073709551615", "18446744073709551616"); + auto level = GENERATE(Execution::Reporter::Level::Info, Execution::Reporter::Level::Warning, Execution::Reporter::Level::Error); + std::istringstream input{ std::string{ response } + "\n3\n" "next\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetStyle(VisualStyle::NoVT); + reporter.SetLevelMask(Execution::Reporter::Level::All, false); + reporter.SetLevelMask(level); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + const Resource::LocString invalid{ AppInstaller::Utility::LocIndString{ std::string_view{ "Try again" } } }; + + REQUIRE(reporter.PromptForIntegerResponseWithinRange(message, 2, 4, level, invalid) == uint64_t{3}); + REQUIRE(input.peek() == 'n'); + REQUIRE(output.str() == "Number: Try again\nNumber: "); +} + +TEST_CASE("ReporterPromptForIntegerResponseWithinRange_CancelRetry", "[PromptFlow]") +{ + auto response = GENERATE("1", "5"); + std::istringstream input{ std::string{ response } + "\n3\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetStyle(VisualStyle::NoVT); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + const Resource::LocString invalid{ AppInstaller::Utility::LocIndString{ std::string_view{ "Try again" } } }; + auto isCancelled = [&]() { return output.str().find("Try again\n") != std::string::npos; }; + + REQUIRE_FALSE(reporter.PromptForIntegerResponseWithinRange(message, 2, 4, Execution::Reporter::Level::Info, invalid, isCancelled)); + REQUIRE(input.peek() == '3'); + REQUIRE(output.str() == "Number: Try again\n"); +} + +TEST_CASE("ReporterPromptForIntegerResponseWithinRange_InvalidRange", "[PromptFlow]") +{ + std::istringstream input{ "1\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(GENERATE(false, true)); + const Resource::LocString message{ AppInstaller::Utility::LocIndString{ std::string_view{ "Number:" } } }; + + REQUIRE_THROWS_HR(reporter.PromptForIntegerResponseWithinRange(message, 2, 1), E_INVALIDARG); + REQUIRE(input.peek() == '1'); + REQUIRE(output.str().empty()); +} + +TEST_CASE("ReporterReadLine", "[PromptFlow]") +{ + auto response = GENERATE("", " text \t", "0", "invalid", "99999999999999999999999999"); + std::istringstream input{ std::string{ response } + '\n' + "next\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + reporter.SetLevelMask(Execution::Reporter::Level::Info, GENERATE(false, true)); + + REQUIRE(reporter.ReadLine() == response); + REQUIRE(input.peek() == 'n'); + REQUIRE(output.str().empty()); +} + +TEST_CASE("ReporterReadLine_InputFailure", "[PromptFlow]") +{ + std::istringstream input; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + + SECTION("EOF") + { + REQUIRE_THROWS_HR(reporter.ReadLine(), APPINSTALLER_CLI_ERROR_PROMPT_INPUT_ERROR); + } + SECTION("Redirected streams") + { + reporter.SetConsoleStreamsForTest(false); + REQUIRE_THROWS_HR(reporter.ReadLine(), HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); + } + SECTION("Cancelled input failure") + { + int checks = 0; + REQUIRE_FALSE(reporter.ReadLine([&]() { return ++checks == 3; })); + } + SECTION("Aborted stream read") + { + struct AbortedInputBuffer : std::streambuf + { + int_type underflow() override + { + SetLastError(ERROR_OPERATION_ABORTED); + return traits_type::eof(); + } + } buffer; + std::istream abortedInput{ &buffer }; + Execution::Reporter abortedReporter{ output, abortedInput }; + abortedReporter.SetConsoleStreamsForTest(true); + REQUIRE_FALSE(abortedReporter.ReadLine([]() { return false; })); + } + REQUIRE(output.str().empty()); +} + +TEST_CASE("ReporterReadLine_CancelBeforeRead", "[PromptFlow]") +{ + auto cancelOnCheck = GENERATE(1, 2, 3); + std::istringstream input{ "invalid\n2\n" }; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + int checks = 0; + auto isCancelled = [&]() + { + if (++checks == cancelOnCheck) + { + context.Terminate(E_ABORT); + } + return context.IsTerminated(); + }; + + REQUIRE_FALSE(context.Reporter.ReadLine(isCancelled)); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + REQUIRE(input.peek() == (cancelOnCheck >= 3 ? '2' : 'i')); + REQUIRE(output.str().empty()); +} + +TEST_CASE("ReporterReadLine_CancelPendingRead", "[PromptFlow]") +{ + struct PipeInputBuffer : std::streambuf + { + wil::unique_handle ReadHandle; + wil::unique_handle WriteHandle; + wil::unique_event ReadStarted{ wil::EventOptions::ManualReset }; + char Character = 0; + + PipeInputBuffer() + { + THROW_IF_WIN32_BOOL_FALSE(CreatePipe(ReadHandle.put(), WriteHandle.put(), nullptr, 0)); + } + + int_type underflow() override + { + ReadStarted.SetEvent(); + DWORD count = 0; + if (!ReadFile(ReadHandle.get(), &Character, 1, &count, nullptr) || !count) + { + return traits_type::eof(); + } + setg(&Character, &Character, &Character + 1); + return traits_type::to_int_type(Character); + } + }; + + bool integerPrompt = GENERATE(false, true); + PipeInputBuffer buffer; + std::istream input{ &buffer }; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + wil::unique_event finished{ wil::EventOptions::ManualReset }; + bool timedOut = false; + std::thread cancel([&]() + { + if (buffer.ReadStarted.wait(5000)) + { + context.Cancel(AppInstaller::CancelReason::CtrlCSignal); + } + }); + std::thread watchdog([&]() + { + if (!finished.wait(5000)) + { + timedOut = true; + DWORD written = 0; + LOG_IF_WIN32_BOOL_FALSE(WriteFile(buffer.WriteHandle.get(), "\n", 1, &written, nullptr)); + } + }); + auto join = wil::scope_exit([&]() + { + finished.SetEvent(); + cancel.join(); + watchdog.join(); + }); + + auto isCancelled = [&]() { return context.IsTerminated(); }; + if (integerPrompt) + { + REQUIRE_FALSE(context.Reporter.PromptForIntegerResponse(Resource::String::NumberedSelectionPrompt(2), + Execution::Reporter::Level::Info, Resource::String::NumberedSelectionInvalid, isCancelled)); + } + else + { + REQUIRE_FALSE(context.Reporter.ReadLine(isCancelled)); + } + finished.SetEvent(); + cancel.join(); + watchdog.join(); + join.release(); + REQUIRE_FALSE(timedOut); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + REQUIRE(output.str() == (integerPrompt ? Resource::String::NumberedSelectionPrompt(2).get() + " " : std::string{})); +} + +TEST_CASE("ReporterReadLine_ConsoleCancellationWaitsForHandler", "[PromptFlow]") +{ + bool integerPrompt = GENERATE(false, true); + BOOL readSucceeded = GENERATE(FALSE, TRUE); + std::istringstream input; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetInputStreamFileTypeForTest(FILE_TYPE_CHAR); + context.Reporter.SetStyle(VisualStyle::NoVT); + wil::unique_event readAborted{ wil::EventOptions::ManualReset }; + wil::unique_event readReturned{ wil::EventOptions::ManualReset }; + size_t readCount = 0; + TestHook::SetReadConsole_Override readConsoleOverride{ [&](wchar_t*, DWORD, DWORD* charactersRead) + { + ++readCount; + *charactersRead = 0; + readAborted.SetEvent(); + SetLastError(ERROR_OPERATION_ABORTED); + return readSucceeded; + } }; + + bool cancelled = false; + std::exception_ptr exception; + auto isCancelled = [&]() { return context.IsTerminated(); }; + std::thread reader([&]() + { + try + { + if (integerPrompt) + { + cancelled = !context.Reporter.PromptForIntegerResponse(Resource::String::NumberedSelectionPrompt(2), + Execution::Reporter::Level::Info, Resource::String::NumberedSelectionInvalid, isCancelled); + } + else + { + cancelled = !context.Reporter.ReadLine(isCancelled); + } + } + catch (...) + { + exception = std::current_exception(); + } + readReturned.SetEvent(); + }); + auto join = wil::scope_exit([&]() + { + context.Cancel(AppInstaller::CancelReason::Abort); + reader.join(); + }); + + bool aborted = readAborted.wait(5000); + bool returnedBeforeCancellation = readReturned.wait(100); + context.Cancel(AppInstaller::CancelReason::CtrlCSignal); + reader.join(); + join.release(); + if (exception) + { + std::rethrow_exception(exception); + } + + REQUIRE(aborted); + REQUIRE_FALSE(returnedBeforeCancellation); + REQUIRE(cancelled); + REQUIRE(readCount == size_t{1}); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + REQUIRE(output.str() == (integerPrompt ? Resource::String::NumberedSelectionPrompt(2).get() + " " : std::string{})); +} + +TEST_CASE("ReporterReadLine_ConsoleCancellationAlreadyRecorded", "[PromptFlow]") +{ + BOOL readSucceeded = GENERATE(FALSE, TRUE); + std::istringstream input; + std::ostringstream output; + TestContext context{ output, input }; + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetInputStreamFileTypeForTest(FILE_TYPE_CHAR); + TestHook::SetReadConsole_Override readConsoleOverride{ [&](wchar_t*, DWORD, DWORD* charactersRead) + { + context.Terminate(E_ABORT); + *charactersRead = 0; + SetLastError(ERROR_OPERATION_ABORTED); + return readSucceeded; + } }; + + REQUIRE_FALSE(context.Reporter.ReadLine([&]() { return context.IsTerminated(); })); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + REQUIRE(output.str().empty()); +} + +TEST_CASE("PackageSelection_ConsoleStreams", "[PackageSelection][PromptFlow]") +{ + Execution::Reporter reporter; + DWORD mode = 0; + bool consoleStreams = GetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), &mode) && + GetConsoleMode(GetStdHandle(STD_OUTPUT_HANDLE), &mode); + REQUIRE(reporter.CanPrompt() == consoleStreams); + + reporter.SetConsoleStreamsForTest(GENERATE(false, true)); + Execution::Reporter clone{ reporter, Execution::Reporter::clone_t{} }; + REQUIRE(clone.CanPrompt() == reporter.CanPrompt()); +} + +TEST_CASE("PackageSelection_ReporterUnavailable", "[PackageSelection][PromptFlow]") +{ + auto level = GENERATE(Execution::Reporter::Level::Info, Execution::Reporter::Level::Warning); + std::istringstream input{ "1\n" }; + std::ostringstream output; + Execution::Reporter reporter{ output, input }; + reporter.SetConsoleStreamsForTest(true); + + SECTION("Redirected streams") + { + reporter.SetConsoleStreamsForTest(false); + } + SECTION("Hidden output level") + { + reporter.SetLevelMask(level, false); + } + SECTION("Non-output channel") + { + reporter.SetChannel(GENERATE(Execution::Reporter::Channel::Completion, Execution::Reporter::Channel::Json, + Execution::Reporter::Channel::Disabled)); + } + + REQUIRE_FALSE(reporter.CanPrompt(level)); + REQUIRE_FALSE(reporter.PromptForIntegerResponse(Resource::String::NumberedSelectionPrompt(2), level)); + REQUIRE_FALSE(reporter.PromptForIntegerResponseWithinRange(Resource::String::NumberedSelectionPrompt(2), 0, 2, level)); + REQUIRE(input.peek() == '1'); + REQUIRE(output.str().empty()); +} + +TEST_CASE("PackageSelection_FeatureDisabled", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + if (GENERATE(false, true)) + { + settings.Set(false); + } + std::istringstream input{ "0\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Args.AddArg(Execution::Args::Type::Query, TSR::TestQuery_ReturnTwo.Query); + OverrideForOpenSource(context, CreateTestSource({ TSR::TestQuery_ReturnTwo })); + + SECTION("Install") + { + if (GENERATE(false, true)) + { + context.Args.AddArg(Execution::Args::Type::Silent); + } + context.Args.AddArg(Execution::Args::Type::Force); + InstallCommand({}).Execute(context); + } + SECTION("Show") + { + ShowCommand({}).Execute(context); + } + SECTION("Show versions") + { + context.Args.AddArg(Execution::Args::Type::ListVersions); + ShowCommand({}).Execute(context); + } + SECTION("Download") + { + DownloadCommand({}).Execute(context); + } + + INFO(output.str()); + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND); + REQUIRE_FALSE(context.Contains(Execution::Data::Package)); + REQUIRE_FALSE(context.Contains(Execution::Data::Manifest)); + REQUIRE(input.peek() == '0'); + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) == std::string::npos); + const std::string refinement = Resource::LocString{ Resource::String::PackageSelectionRefine }.get(); + REQUIRE_FALSE(refinement.empty()); + REQUIRE(output.str().find(refinement) != std::string::npos); +} + +TEST_CASE("PackageSelection_CommandCancel", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + settings.Set(true); + std::istringstream input{ "0\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Args.AddArg(Execution::Args::Type::Query, TSR::TestQuery_ReturnTwo.Query); + OverrideForOpenSource(context, CreateTestSource({ TSR::TestQuery_ReturnTwo })); + + SECTION("Install") + { + bool silent = GENERATE(false, true); + if (silent) + { + context.Args.AddArg(Execution::Args::Type::Silent); + } + context.Args.AddArg(Execution::Args::Type::Force); + InstallCommand({}).Execute(context); + } + SECTION("Show") + { + ShowCommand({}).Execute(context); + } + SECTION("Show versions") + { + context.Args.AddArg(Execution::Args::Type::ListVersions); + ShowCommand({}).Execute(context); + } + SECTION("Download") + { + DownloadCommand({}).Execute(context); + } + + INFO(output.str()); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + REQUIRE_FALSE(context.Contains(Execution::Data::Package)); + REQUIRE_FALSE(context.Contains(Execution::Data::Manifest)); + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) != std::string::npos); +} + +TEST_CASE("PackageSelection_CommandContinue", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + settings.Set(true); + std::istringstream input{ "1\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Args.AddArg(Execution::Args::Type::Query, TSR::TestQuery_ReturnTwo.Query); + context.Args.AddArg(Execution::Args::Type::Version, "1.0.0.0"sv); + OverrideForOpenSource(context, CreateTestSource({ TSR::TestQuery_ReturnTwo })); + + auto checkSelection = [](TestContext& selectedContext) + { + REQUIRE(selectedContext.Get()->GetProperty(AppInstaller::Repository::PackageProperty::Id) == + "AppInstallerCliTest.TestExeInstaller"); + REQUIRE(selectedContext.Get().Version == "1.0.0.0"); + REQUIRE(selectedContext.Args.GetArg(Execution::Args::Type::Version) == "1.0.0.0"); + selectedContext.Terminate(E_ABORT); + }; + + SECTION("Install") + { + bool silent = GENERATE(false, true); + if (silent) + { + context.Args.AddArg(Execution::Args::Type::Silent); + } + context.Args.AddArg(Execution::Args::Type::Force); + context.Override({ Workflow::InstallSinglePackage, checkSelection, 1 }); + InstallCommand({}).Execute(context); + REQUIRE(context.Args.Contains(Execution::Args::Type::Silent) == silent); + } + SECTION("Show") + { + context.Override({ Workflow::ShowManifestInfo, checkSelection, 1 }); + ShowCommand({}).Execute(context); + } + SECTION("Download") + { + context.Override({ Workflow::SetDownloadDirectory, checkSelection, 1 }); + DownloadCommand({}).Execute(context); + } + + INFO(output.str()); + REQUIRE_TERMINATED_WITH(context, E_ABORT); + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) != std::string::npos); +} + +TEST_CASE("PackageSelection_MultipleQueries", "[PackageSelection][workflow][MultiQuery]") +{ + TestUserSettings settings; + settings.Set(true); + std::istringstream input{ "1\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Args.AddArg(Execution::Args::Type::Force); + context.Args.AddArg(Execution::Args::Type::MultiQuery, TSR::TestQuery_ReturnTwo.Query); + context.Args.AddArg(Execution::Args::Type::MultiQuery, "MissingPackage"sv); + OverrideForOpenSource(context, CreateTestSource({ TSR::TestQuery_ReturnTwo })); + context.Override({ Workflow::GetSearchRequestForSingle, [](TestContext& subContext) + { + subContext.Reporter.SetConsoleStreamsForTest(true); + Workflow::GetSearchRequestForSingle(subContext); + } }); + + InstallCommand({}).Execute(context); + INFO(output.str()); + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_NOT_ALL_QUERIES_FOUND_SINGLE); + REQUIRE(input.peek() == '1'); + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) == std::string::npos); +} + +TEST_CASE("PackageSelection_SearchResult", "[PackageSelection][SourcePriority][workflow]") +{ + TestUserSettings settings; + settings.Set(true); + auto width = GENERATE(size_t{20}, size_t{120}); + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{width} }; + auto operation = GENERATE(OperationType::Install, OperationType::Show, OperationType::Download); + auto manifest = AppInstaller::Manifest::YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); + std::vector versions{ manifest }; + auto firstSource = std::make_shared(); + auto secondSource = std::make_shared(); + auto lowPriority = std::make_shared(); + firstSource->Details.Name = "FirstSource"; + secondSource->Details.Name = "SecondSource"; + SearchResult result; + result.Matches.emplace_back(TestCompositePackage::Make(versions, firstSource), + PackageMatchFilter{ PackageMatchField::Id, MatchType::Exact, manifest.Id }); + result.Matches.emplace_back(TestCompositePackage::Make(versions, secondSource), + PackageMatchFilter{ PackageMatchField::Id, MatchType::Exact, manifest.Id }); + auto expectedPackage = result.Matches[1].Package; + bool expectPrompt = true; + bool expectSecondSource = true; + + SECTION("Same identity across sources") + { + } + SECTION("Single source") + { + result.Matches[1].Package = TestCompositePackage::Make(versions, firstSource); + expectedPackage = result.Matches[1].Package; + expectSecondSource = false; + } + SECTION("Multiple available sources for a candidate") + { + auto package = TestCompositePackage::Make(versions, firstSource); + package->Available.emplace_back(TestPackage::Make(versions, secondSource)); + result.Matches[0].Package = package; + } + SECTION("Unique source priority") + { + secondSource->Details.Priority = 1; + expectPrompt = false; + } + SECTION("Priority tie") + { + firstSource->Details.Priority = 1; + secondSource->Details.Priority = 1; + auto excluded = TestCompositePackage::Make(versions, lowPriority); + result.Matches.insert(result.Matches.begin(), ResultMatch{ excluded, + PackageMatchFilter{ PackageMatchField::Id, MatchType::Exact, manifest.Id } }); + } + SECTION("Single match") + { + result.Matches.erase(result.Matches.begin()); + expectPrompt = false; + } + + std::istringstream input{ "2\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + context.Add(std::move(result)); + context << EnsureOneMatchFromSearchResult(operation, PackageSelectionBehavior::Prompt); + + INFO(output.str()); + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Get() == expectedPackage); + const std::string title = Resource::LocString{ Resource::String::PackageSelectionTitle }.get(); + REQUIRE_FALSE(title.empty()); + REQUIRE((output.str().find(title + "\n\n") != std::string::npos) == expectPrompt); + REQUIRE((output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) != std::string::npos) == expectPrompt); + if (expectPrompt) + { + auto tableStart = output.str().find("\n# "); + REQUIRE(tableStart != std::string::npos); + auto tableEnd = output.str().find("\n\n", tableStart + 1); + REQUIRE(tableEnd != std::string::npos); + auto tableText = output.str().substr(tableStart + 1, tableEnd - tableStart - 1); + REQUIRE(tableText.find("\n1 ") != std::string::npos); + REQUIRE(tableText.find("\n2 ") != std::string::npos); + std::istringstream tableStream{ tableText }; + std::string line; + size_t lineCount = 0; + while (std::getline(tableStream, line)) + { + REQUIRE(AppInstaller::Utility::UTF8ColumnWidth(line) < width); + ++lineCount; + } + REQUIRE(lineCount == size_t{4}); + if (width == 120) + { + REQUIRE(tableText.find(manifest.DefaultLocalization.Get()) != std::string::npos); + REQUIRE(tableText.find(manifest.Id) != std::string::npos); + REQUIRE(tableText.find(Resource::LocString{ Resource::String::SearchVersion }.get()) == std::string::npos); + REQUIRE(tableText.find(manifest.Version) == std::string::npos); + REQUIRE(tableText.find(Resource::LocString{ Resource::String::SearchSource }.get()) != std::string::npos); + REQUIRE(tableText.find("FirstSource") != std::string::npos); + REQUIRE((tableText.find("SecondSource") != std::string::npos) == expectSecondSource); + } + else + { + REQUIRE(tableText.find("\xE2\x80\xA6") != std::string::npos); + } + } + else + { + REQUIRE(input.peek() == '2'); + } + REQUIRE(firstSource->CountOfCallsRequiringManifestData == 0); + REQUIRE(secondSource->CountOfCallsRequiringManifestData == 0); +} + +TEST_CASE("PackageSelection_CandidateRowIdentity", "[PackageSelection][workflow]") +{ + struct PrimaryPackage : TestCompositePackage + { + using TestCompositePackage::TestCompositePackage; + + LocIndString GetProperty(PackageProperty property) const override + { + return Available.at(1)->GetProperty(property); + } + }; + + TestUserSettings settings; + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + auto operation = GENERATE(OperationType::Install, OperationType::Show, OperationType::Download); + settings.Set(true); + bool differentName = GENERATE(false, true); + bool differentId = GENERATE(false, true); + bool missingSourceName = GENERATE(false, true); + auto manifest = AppInstaller::Manifest::YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); + manifest.Id = "Public.App"; + manifest.Version = "1.0"; + manifest.DefaultLocalization.Add("PublicName"); + auto firstSource = std::make_shared(); + auto secondSource = std::make_shared(); + firstSource->Details.Name = "FirstSource"; + secondSource->Details.Name = "SecondSource"; + auto package = std::make_shared(std::vector{ manifest }, firstSource); + + std::string secondName = differentName ? "PrivateName" : "PublicName"; + std::string secondId = differentId ? "Private.App" : "Public.App"; + manifest.Id = secondId; + manifest.Version = "2.0"; + manifest.DefaultLocalization.Add(secondName); + package->Available.emplace_back(TestPackage::Make(std::vector{ manifest }, secondSource)); + if (missingSourceName) + { + firstSource->Details.Name.clear(); + } + + SearchResult result; + result.Matches.emplace_back(package, PackageMatchFilter{ PackageMatchField::Id, MatchType::Exact, secondId }); + manifest.Id = "Other.App"; + manifest.Version = "4.0"; + manifest.DefaultLocalization.Add("OtherName"); + result.Matches.emplace_back(TestCompositePackage::Make(std::vector{ manifest }, firstSource), + PackageMatchFilter{ PackageMatchField::Id, MatchType::Exact, manifest.Id }); + + std::istringstream input{ "1\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + context.Add(std::move(result)); + context << EnsureOneMatchFromSearchResult(operation, PackageSelectionBehavior::Prompt); + + INFO(output.str()); + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Get() == package); + auto tableStart = output.str().find("\n# "); + REQUIRE(tableStart != std::string::npos); + auto tableEnd = output.str().find("\n\n", tableStart + 1); + REQUIRE(tableEnd != std::string::npos); + std::istringstream tableStream{ output.str().substr(tableStart + 1, tableEnd - tableStart - 1) }; + std::string line; + REQUIRE(static_cast(std::getline(tableStream, line))); + REQUIRE(static_cast(std::getline(tableStream, line))); + std::vector> expectedRows{ + { "1", secondName, secondId }, + { "2", "OtherName", "Other.App" } + }; + if (!missingSourceName) + { + expectedRows[0].emplace_back("FirstSource"); + expectedRows[1].emplace_back("FirstSource"); + } + for (const auto& expectedRow : expectedRows) + { + REQUIRE(static_cast(std::getline(tableStream, line))); + std::istringstream rowStream{ line }; + std::vector fields; + std::string field; + while (rowStream >> field) + { + fields.emplace_back(std::move(field)); + } + REQUIRE(fields == expectedRow); + } + REQUIRE_FALSE(std::getline(tableStream, line)); + REQUIRE(firstSource->CountOfCallsRequiringManifestData == 0); + REQUIRE(secondSource->CountOfCallsRequiringManifestData == 0); +} + +TEST_CASE("PackageSelection_SharedAmbiguityTables", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + settings.Set(true); + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + bool withSource = GENERATE(false, true); + bool available = GENERATE(false, true); + bool prompt = GENERATE(false, true); + CAPTURE(withSource, available, prompt); + auto source = std::make_shared(); + auto manifest = AppInstaller::Manifest::YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); + std::istringstream input{ "2\n" }; + std::ostringstream output; + std::ostringstream expectedOutput; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + Execution::Reporter expectedReporter{ expectedOutput, input }; + expectedReporter.SetStyle(VisualStyle::NoVT); + std::vector header{ Resource::String::SearchName, Resource::String::SearchId }; + if (withSource) + { + header.emplace_back(Resource::String::SearchSource); + } + if (prompt) + { + header.insert(header.begin(), Resource::LocString{ AppInstaller::Utility::LocIndString{ "#"sv } }); + expectedReporter.Info() << Resource::String::PackageSelectionTitle << std::endl << std::endl; + } + else + { + expectedReporter.Warn() << (withSource ? Resource::String::MultiplePackagesFound : Resource::String::MultipleInstalledPackagesFound) << std::endl; + } + Execution::TableOutputBase expectedTable{ expectedReporter, std::move(header) }; + SearchResult result; + for (size_t i = 1; i <= 2; ++i) + { + auto name = "Package" + std::to_string(i); + manifest.Id = "Test." + name; + manifest.DefaultLocalization.Add(name); + auto package = available ? TestCompositePackage::Make(std::vector{ manifest }, source) : + TestCompositePackage::Make(manifest, TestPackage::MetadataMap{}, std::vector{}, source); + result.Matches.emplace_back(package, PackageMatchFilter{ PackageMatchField::Id, MatchType::Exact, manifest.Id }); + std::vector line{ name, manifest.Id }; + if (withSource) + { + line.emplace_back(available ? source->Details.Name : ""); + } + if (prompt) + { + line.insert(line.begin(), std::to_string(i)); + } + expectedTable.OutputLine(std::move(line)); + } + auto expectedPackage = result.Matches[1].Package; + context.Add(std::move(result)); + context << EnsureOneMatchFromSearchResult(withSource ? OperationType::Install : OperationType::Uninstall, + prompt ? PackageSelectionBehavior::Prompt : PackageSelectionBehavior::Disabled); + expectedTable.Complete(); + if (prompt) + { + expectedReporter.Info() << std::endl << Resource::String::NumberedSelectionPrompt(2) << ' ' << + Resource::String::PackageSelectionSelected(AppInstaller::Utility::LocIndView{ "Package2"sv }, + AppInstaller::Utility::LocIndView{ "Test.Package2"sv }) << std::endl; + REQUIRE_FALSE(context.IsTerminated()); + REQUIRE(context.Get() == size_t{1}); + REQUIRE(context.Get() == expectedPackage); + } + else + { + expectedReporter.Info() << Resource::String::PackageSelectionRefine << std::endl; + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND); + REQUIRE_FALSE(context.Contains(Execution::Data::Package)); + REQUIRE_FALSE(context.Contains(Execution::Data::SelectedIndex)); + REQUIRE(input.peek() == '2'); + } + + REQUIRE(output.str() == expectedOutput.str()); + REQUIRE(source->CountOfCallsRequiringManifestData == 0); +} + +TEST_CASE("PackageSelection_AmbiguityRefinement", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + bool enabled = GENERATE(false, true); + settings.Set(bool{ enabled }); + auto operation = GENERATE(OperationType::Install, OperationType::Uninstall, OperationType::Export); + auto selectionBehavior = GENERATE(PackageSelectionBehavior::Disabled, PackageSelectionBehavior::Prompt); + bool installed = operation != OperationType::Install; + bool truncated = GENERATE(false, true); + CAPTURE(enabled, operation, selectionBehavior, truncated); + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + auto source = CreateTestSource({ TSR::TestQuery_ReturnTwo }); + auto result = source->Search({}); + result.Truncated = truncated; + std::istringstream input{ "2\n" }; + std::ostringstream output; + std::ostringstream expectedOutput; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + context.Args.AddArg(Execution::Args::Type::DisableInteractivity); + context.Add(std::move(result)); + Execution::Reporter expectedReporter{ expectedOutput, input }; + expectedReporter.SetStyle(VisualStyle::NoVT); + expectedReporter.Warn() << (installed ? Resource::String::MultipleInstalledPackagesFound : Resource::String::MultiplePackagesFound) << std::endl; + std::vector header{ Resource::String::SearchName, Resource::String::SearchId }; + std::vector firstRow{ "AppInstaller Test Exe Installer", "AppInstallerCliTest.TestExeInstaller" }; + std::vector secondRow{ "MSIX SDK", "microsoft.msixsdk" }; + if (!installed) + { + header.emplace_back(Resource::String::SearchSource); + firstRow.emplace_back(source->Details.Name); + secondRow.emplace_back(source->Details.Name); + } + Execution::TableOutputBase table{ expectedReporter, std::move(header) }; + table.OutputLine(std::move(firstRow)); + table.OutputLine(std::move(secondRow)); + table.Complete(); + if (truncated) + { + expectedReporter.Info() << '<' << Resource::String::SearchTruncated << '>' << std::endl; + } + auto refinementId = operation == OperationType::Export ? + Resource::String::PackageSelectionRefineForExport : Resource::String::PackageSelectionRefine; + const std::string refinement = Resource::LocString{ refinementId }.get(); + REQUIRE_FALSE(refinement.empty()); + REQUIRE(refinement.find("--source") != std::string::npos); + if (operation == OperationType::Export) + { + REQUIRE(refinement.find("--package-id") != std::string::npos); + REQUIRE(refinement.find("--id") == std::string::npos); + REQUIRE(refinement.find("--exact") == std::string::npos); + } + else + { + REQUIRE(refinement.find("--id") != std::string::npos); + REQUIRE(refinement.find("--exact") != std::string::npos); + } + expectedReporter.Info() << refinementId << std::endl; + + context << EnsureOneMatchFromSearchResult(operation, selectionBehavior); + + REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND); + REQUIRE_FALSE(context.Contains(Execution::Data::Package)); + REQUIRE(input.peek() == '2'); + REQUIRE(output.str() == expectedOutput.str()); +} + +TEST_CASE("PackageSelection_PartialSearchFailureDoesNotPrompt", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + settings.Set(true); + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + auto source = CreateTestSource({ TSR::TestQuery_ReturnTwo }); + auto result = source->Search({}); + bool truncated = GENERATE(false, true); + result.Truncated = truncated; + result.Failures.push_back({ "BrokenSource", std::make_exception_ptr(wil::ResultException(E_FAIL)) }); + std::istringstream input{ "2\n" }; + std::ostringstream output; + std::ostringstream expectedOutput; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + context.Reporter.SetStyle(VisualStyle::NoVT); + context.SetFlags(Execution::ContextFlag::ShowSearchResultsOnPartialFailure); + context.Add(std::move(result)); + Execution::Reporter expectedReporter{ expectedOutput, input }; + expectedReporter.SetStyle(VisualStyle::NoVT); + expectedReporter.Info() << std::endl << Resource::String::SearchFailureErrorListMatches << std::endl; + Execution::TableOutput<3> table{ expectedReporter, + { Resource::String::SearchName, Resource::String::SearchId, Resource::String::SearchSource } }; + table.OutputLine({ "AppInstaller Test Exe Installer", "AppInstallerCliTest.TestExeInstaller", source->Details.Name }); + table.OutputLine({ "MSIX SDK", "microsoft.msixsdk", source->Details.Name }); + table.Complete(); + if (truncated) + { + expectedReporter.Info() << '<' << Resource::String::SearchTruncated << '>' << std::endl; + } + + context << HandleSearchResultFailures; + + REQUIRE(context.GetTerminationHR() == E_FAIL); + REQUIRE_FALSE(context.Contains(Execution::Data::SelectedIndex)); + REQUIRE(input.peek() == '2'); + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) == std::string::npos); + auto tableStart = output.str().find(expectedOutput.str()); + REQUIRE(tableStart != std::string::npos); + REQUIRE(output.str().substr(tableStart) == expectedOutput.str()); +} + +TEST_CASE("PackageSelection_Unavailable", "[PackageSelection][workflow]") +{ + TestUserSettings settings; + settings.Set(true); + std::istringstream input{ "2\n" }; + std::ostringstream output; + TestContext context{ output, input }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Reporter.SetConsoleStreamsForTest(true); + auto source = CreateTestSource({ TSR::TestQuery_ReturnTwo }); + auto result = source->Search({}); + EnsureOneMatchFromSearchResult ensureOneMatch{ OperationType::Install, PackageSelectionBehavior::Prompt }; + HRESULT expectedError = APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND; + + SECTION("Default workflow") + { + auto operation = GENERATE(OperationType::Install, OperationType::Show, OperationType::Download, + OperationType::Upgrade, OperationType::Uninstall, OperationType::Repair, OperationType::Export, + OperationType::Pin, OperationType::Search, OperationType::List, OperationType::Completion); + ensureOneMatch = EnsureOneMatchFromSearchResult(operation); + } + SECTION("Context disabled") + { + context.SetFlags(Execution::ContextFlag::DisableInteractivity); + } + SECTION("Argument disabled") + { + context.Args.AddArg(Execution::Args::Type::DisableInteractivity); + } + SECTION("Setting disabled") + { + settings.Set(true); + } + SECTION("Silent with interactivity disabled") + { + context.Args.AddArg(Execution::Args::Type::Silent); + context.Args.AddArg(Execution::Args::Type::DisableInteractivity); + } + SECTION("Redirected streams") + { + context.Reporter.SetConsoleStreamsForTest(false); + } + SECTION("Hidden output") + { + context.Reporter.SetChannel(Execution::Reporter::Channel::Disabled); + } + SECTION("Truncated results") + { + result.Truncated = true; + } + SECTION("No matches") + { + result.Matches.clear(); + expectedError = APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND; + } + + context.Add(std::move(result)); + context << ensureOneMatch; + INFO(output.str()); + REQUIRE_TERMINATED_WITH(context, expectedError); + REQUIRE_FALSE(context.Contains(Execution::Data::Package)); + REQUIRE(input.peek() == '2'); + REQUIRE(output.str().find(Resource::String::NumberedSelectionPrompt(2).get()) == std::string::npos); +} + TEST_CASE("PromptFlow_InteractivityDisabled", "[PromptFlow][workflow]") { TestCommon::TempFile installResultPath("TestExeInstalled.txt"); diff --git a/src/AppInstallerCLITests/TableOutput.cpp b/src/AppInstallerCLITests/TableOutput.cpp index 2d5480083f..cbe1b0b1e1 100644 --- a/src/AppInstallerCLITests/TableOutput.cpp +++ b/src/AppInstallerCLITests/TableOutput.cpp @@ -19,6 +19,126 @@ namespace } } +TEST_CASE("TableOutput_DynamicMatchesTyped", "[tableoutput]") +{ + std::ostringstream typedOutput; + std::ostringstream dynamicOutput; + std::istringstream input; + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{GENERATE(size_t{20}, size_t{120})} }; + Reporter typedReporter(typedOutput, input); + Reporter dynamicReporter(dynamicOutput, input); + typedReporter.SetStyle(AppInstaller::Settings::VisualStyle::NoVT); + dynamicReporter.SetStyle(AppInstaller::Settings::VisualStyle::NoVT); + TableOutput<3> typed(typedReporter, { MakeHeader("Name"), MakeHeader("Empty"), MakeHeader("Id") }); + TableOutputBase dynamic(dynamicReporter, { MakeHeader("Name"), MakeHeader("Empty"), MakeHeader("Id") }); + typed.OutputLine({ "LongPackageName", "", "test.id" }); + dynamic.OutputLine({ "LongPackageName", "", "test.id" }); + typed.OutputLine({ "OtherPackageName", "", "other.id" }); + dynamic.OutputLine({ "OtherPackageName", "", "other.id" }); + bool showLineNumbers = GENERATE(false, true); + typed.Complete(showLineNumbers); + dynamic.Complete(showLineNumbers); + + REQUIRE_FALSE(dynamic.IsEmpty()); + REQUIRE(dynamic.GetRowCount() == size_t{2}); + REQUIRE(dynamicOutput.str() == typedOutput.str()); + dynamic.Complete(showLineNumbers); + REQUIRE(dynamicOutput.str() == typedOutput.str()); +} + +TEST_CASE("TableOutput_DynamicInvalidDimensions", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + Reporter reporter(output, input); + REQUIRE_THROWS_HR(TableOutputBase(reporter, {}), E_INVALIDARG); + TableOutputBase table(reporter, { MakeHeader("Name") }); + REQUIRE_THROWS_HR(table.OutputLine({ "Name", "Extra" }), E_INVALIDARG); + REQUIRE(table.IsEmpty()); + REQUIRE(output.str().empty()); +} + +TEST_CASE("TableOutput_RowCount", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + Reporter reporter(output, input); + TableOutput<2> table(reporter, { MakeHeader("Choice"), MakeHeader("Source") }); + REQUIRE(table.GetRowCount() == size_t{0}); + + table.OutputLine({ "1", "FirstSource" }); + table.OutputLine({ "", "SecondSource" }); + table.OutputLine({ "2", "ThirdSource" }); + table.OutputLine({ "", "" }); + REQUIRE(table.GetRowCount() == size_t{4}); + REQUIRE(output.str().empty()); + + table.Complete(GENERATE(false, true)); + REQUIRE(table.GetRowCount() == size_t{4}); +} + +TEST_CASE("TableOutput_LineNumbers", "[tableoutput]") +{ + std::ostringstream output; + std::ostringstream expectedOutput; + std::istringstream input; + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{GENERATE(size_t{20}, size_t{120})} }; + Reporter reporter(output, input); + Reporter expectedReporter(expectedOutput, input); + reporter.SetStyle(AppInstaller::Settings::VisualStyle::NoVT); + expectedReporter.SetStyle(AppInstaller::Settings::VisualStyle::NoVT); + TableOutput<3> table(reporter, { MakeHeader("Name"), MakeHeader("Empty"), MakeHeader("Source") }); + TableOutput<4> expected(expectedReporter, { MakeHeader("#"), MakeHeader("Name"), MakeHeader("Empty"), MakeHeader("Source") }); + size_t count = GENERATE(size_t{1}, size_t{9}, size_t{10}, size_t{99}, size_t{100}); + for (size_t i = 1; i <= count; ++i) + { + auto number = std::to_string(i); + table.OutputLine({ "Package" + number, "", "Source" }); + expected.OutputLine({ number, "Package" + number, "", "Source" }); + } + + REQUIRE(table.GetRowCount() == count); + REQUIRE(output.str().empty()); + table.Complete(true); + expected.Complete(); + REQUIRE(output.str() == expectedOutput.str()); + REQUIRE(table.GetRowCount() == count); + table.Complete(true); + REQUIRE(output.str() == expectedOutput.str()); +} + +TEST_CASE("TableOutput_LineNumbersNotTruncated", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{GENERATE(size_t{1}, size_t{4}, size_t{8})} }; + Reporter reporter(output, input); + reporter.SetStyle(AppInstaller::Settings::VisualStyle::NoVT); + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Source") }); + size_t count = GENERATE(size_t{10}, size_t{100}); + + for (size_t i = 0; i < count; ++i) + { + table.OutputLine({ "Package", "Source" }); + } + table.Complete(true); + + std::istringstream lines{ output.str() }; + std::string line; + REQUIRE(static_cast(std::getline(lines, line))); + REQUIRE(line.find('#') == 0); + REQUIRE(static_cast(std::getline(lines, line))); + for (size_t i = 1; i <= count; ++i) + { + REQUIRE(static_cast(std::getline(lines, line))); + auto number = std::to_string(i); + REQUIRE(line.find(number + std::string(std::to_string(count).size() - number.size() + 1, ' ')) == 0); + } + REQUIRE_FALSE(std::getline(lines, line)); + REQUIRE(table.GetRowCount() == count); +} + // Test that all rows are buffered and column widths account for values beyond the first 50 rows. // In the old sizing-buffer design, a row at position 55 with a longer value than any of the // first 50 rows would be truncated. The new design buffers every row so no value is clipped. @@ -107,7 +227,7 @@ TEST_CASE("TableOutput_Empty_ProducesNoOutput", "[tableoutput]") TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); REQUIRE(table.IsEmpty()); - table.Complete(); + table.Complete(GENERATE(false, true)); REQUIRE(output.str().empty()); } @@ -199,7 +319,7 @@ TEST_CASE("TableOutput_ManyRowsBuffered", "[tableoutput]") }); } - table.Complete(); + table.Complete(GENERATE(false, true)); REQUIRE_FALSE(table.IsEmpty()); diff --git a/src/AppInstallerCLITests/TestHooks.h b/src/AppInstallerCLITests/TestHooks.h index 38dcb01c6e..396eafc9e1 100644 --- a/src/AppInstallerCLITests/TestHooks.h +++ b/src/AppInstallerCLITests/TestHooks.h @@ -79,6 +79,9 @@ namespace AppInstaller namespace CLI::Execution { void TestHook_SetConsoleWidth_Override(std::optional* value); + + using ReadConsoleFunction = std::function; + void TestHook_SetReadConsole_Override(ReadConsoleFunction* value); } namespace CLI::Workflow @@ -388,6 +391,22 @@ namespace TestHook std::optional m_width; }; + struct SetReadConsole_Override + { + SetReadConsole_Override(AppInstaller::CLI::Execution::ReadConsoleFunction function) : m_function(std::move(function)) + { + AppInstaller::CLI::Execution::TestHook_SetReadConsole_Override(&m_function); + } + + ~SetReadConsole_Override() + { + AppInstaller::CLI::Execution::TestHook_SetReadConsole_Override(nullptr); + } + + private: + AppInstaller::CLI::Execution::ReadConsoleFunction m_function; + }; + struct SetGetFontRegistryRoot_Override { SetGetFontRegistryRoot_Override(std::function function) diff --git a/src/AppInstallerCommonCore/ExperimentalFeature.cpp b/src/AppInstallerCommonCore/ExperimentalFeature.cpp index c814afb965..b59905f0b9 100644 --- a/src/AppInstallerCommonCore/ExperimentalFeature.cpp +++ b/src/AppInstallerCommonCore/ExperimentalFeature.cpp @@ -65,6 +65,8 @@ namespace AppInstaller::Settings return userSettings.Get(); case ExperimentalFeature::Feature::Font: return userSettings.Get(); + case ExperimentalFeature::Feature::InteractivePackageSelection: + return userSettings.Get(); default: THROW_HR(E_UNEXPECTED); } @@ -98,6 +100,8 @@ namespace AppInstaller::Settings return ExperimentalFeature{ "Resume", "resume", "https://aka.ms/winget-settings", Feature::Resume }; case Feature::Font: return ExperimentalFeature{ "Font", "fonts", "https://aka.ms/winget-settings", Feature::Font }; + case Feature::InteractivePackageSelection: + return ExperimentalFeature{ "Interactive Package Selection", "interactivePackageSelection", "https://aka.ms/winget-settings", Feature::InteractivePackageSelection }; default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h index 2dc097f548..436196cdc3 100644 --- a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h +++ b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h @@ -25,6 +25,7 @@ namespace AppInstaller::Settings DirectMSI = 0x1, Resume = 0x2, Font = 0x4, + InteractivePackageSelection = 0x8, Max, // This MUST always be after all experimental features // Features listed after Max will not be shown with the features command diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index f438d6ed8e..e000c6adda 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -102,6 +102,7 @@ namespace AppInstaller::Settings EFDirectMSI, EFResume, EFFonts, + EFInteractivePackageSelection, // Telemetry TelemetryDisable, // Install behavior @@ -194,6 +195,7 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFResume, bool, bool, false, ".experimentalFeatures.resume"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFFonts, bool, bool, false, ".experimentalFeatures.fonts"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFInteractivePackageSelection, bool, bool, false, ".experimentalFeatures.interactivePackageSelection"sv); // Telemetry SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); // Install behavior diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 31ea8e46ae..5b6ee9cec7 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -317,6 +317,7 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) WINGET_VALIDATE_PASS_THROUGH(EFResume) WINGET_VALIDATE_PASS_THROUGH(EFFonts) + WINGET_VALIDATE_PASS_THROUGH(EFInteractivePackageSelection) WINGET_VALIDATE_PASS_THROUGH(AnonymizePathForDisplay) WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) WINGET_VALIDATE_PASS_THROUGH(InteractivityDisable)