diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 72aba15720..0de3433dcc 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -561,6 +561,7 @@ swgus SYD SYG systemnotsupported +tableoutput Tagit TARG taskhostw diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index 6b2029d84d..3793f7c768 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -47,6 +47,11 @@ The WinGet MCP server's existing tools have been extended with new parameters to The PowerShell module now automatically uses `GH_TOKEN` or `GITHUB_TOKEN` environment variables to authenticate GitHub API requests. This significantly increases the GitHub API rate limit, preventing failures in CI/CD pipelines. Use `-Verbose` to see which token is being used. +### Improved `list` output when redirected + +- `winget list` (and similar table commands) no longer truncates output when stdout is redirected to a file or variable — column widths are now computed from the full result set. +- Spinner and progress bar output are suppressed when no console is attached, keeping redirected output clean. + ## Bug Fixes * Fixed the `useLatest` property in the DSC v3 `Microsoft.WinGet/Package` resource schema to emit a boolean default (`false`) instead of the incorrect string `"false"`. diff --git a/src/AppInstallerCLICore/ChannelStreams.cpp b/src/AppInstallerCLICore/ChannelStreams.cpp index b152fd2acd..7095df2fd3 100644 --- a/src/AppInstallerCLICore/ChannelStreams.cpp +++ b/src/AppInstallerCLICore/ChannelStreams.cpp @@ -8,8 +8,23 @@ namespace AppInstaller::CLI::Execution { using namespace VirtualTerminal; - size_t GetConsoleWidth() +#ifndef AICLI_DISABLE_TEST_HOOKS + static std::optional* s_consoleWidthOverride = nullptr; + + void TestHook_SetConsoleWidth_Override(std::optional* value) + { + s_consoleWidthOverride = value; + } +#endif + + std::optional GetConsoleWidth() { +#ifndef AICLI_DISABLE_TEST_HOOKS + if (s_consoleWidthOverride) + { + return *s_consoleWidthOverride; + } +#endif CONSOLE_SCREEN_BUFFER_INFO consoleInfo{}; if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &consoleInfo)) { @@ -17,7 +32,7 @@ namespace AppInstaller::CLI::Execution } else { - return 120; + return std::nullopt; } } diff --git a/src/AppInstallerCLICore/ChannelStreams.h b/src/AppInstallerCLICore/ChannelStreams.h index 4a1d66cdc1..8e168e380e 100644 --- a/src/AppInstallerCLICore/ChannelStreams.h +++ b/src/AppInstallerCLICore/ChannelStreams.h @@ -5,14 +5,16 @@ #include "VTSupport.h" #include +#include #include #include namespace AppInstaller::CLI::Execution { - // Gets the current console width. - size_t GetConsoleWidth(); + // Gets the current console width, or std::nullopt if stdout is not attached to a console + // (e.g. redirected to a file or pipe). Callers that receive nullopt should not truncate output. + std::optional GetConsoleWidth(); // The base stream for all channels. struct BaseStream diff --git a/src/AppInstallerCLICore/ExecutionProgress.cpp b/src/AppInstallerCLICore/ExecutionProgress.cpp index 562e293007..56d38ab190 100644 --- a/src/AppInstallerCLICore/ExecutionProgress.cpp +++ b/src/AppInstallerCLICore/ExecutionProgress.cpp @@ -161,7 +161,15 @@ namespace AppInstaller::CLI::Execution } else { - m_out << '\r' << std::string(GetConsoleWidth(), ' ') << '\r'; + auto consoleWidth = GetConsoleWidth(); + if (consoleWidth.has_value()) + { + m_out << '\r' << std::string(*consoleWidth, ' ') << '\r'; + } + else + { + m_out << '\n'; + } } } diff --git a/src/AppInstallerCLICore/ExecutionReporter.cpp b/src/AppInstallerCLICore/ExecutionReporter.cpp index ab5c1d259c..16222345f9 100644 --- a/src/AppInstallerCLICore/ExecutionReporter.cpp +++ b/src/AppInstallerCLICore/ExecutionReporter.cpp @@ -56,9 +56,15 @@ namespace AppInstaller::CLI::Execution m_out(outStream), m_in(inStream) { - auto sixelSupported = [&]() { return SixelsSupported(); }; - m_spinner = IIndefiniteSpinner::CreateForStyle(*m_out, ConsoleModeRestore::Instance().IsVTEnabled(), VisualStyle::Accent, sixelSupported); - m_progressBar = IProgressBar::CreateForStyle(*m_out, ConsoleModeRestore::Instance().IsVTEnabled(), VisualStyle::Accent, sixelSupported); + // Only create spinner and progress bar when stdout is attached to a console. + // When output is redirected to a file or pipe, suppress all progress output + // so it does not appear in the redirected stream. + if (GetConsoleWidth().has_value()) + { + auto sixelSupported = [&]() { return SixelsSupported(); }; + m_spinner = IIndefiniteSpinner::CreateForStyle(*m_out, ConsoleModeRestore::Instance().IsVTEnabled(), VisualStyle::Accent, sixelSupported); + m_progressBar = IProgressBar::CreateForStyle(*m_out, ConsoleModeRestore::Instance().IsVTEnabled(), VisualStyle::Accent, sixelSupported); + } SetProgressSink(this); } @@ -146,7 +152,7 @@ namespace AppInstaller::CLI::Execution { m_style = style; - if (m_channel == Channel::Output) + if (m_channel == Channel::Output && GetConsoleWidth().has_value()) { auto sixelSupported = [&]() { return SixelsSupported(); }; m_spinner = IIndefiniteSpinner::CreateForStyle(*m_out, ConsoleModeRestore::Instance().IsVTEnabled(), style, sixelSupported); diff --git a/src/AppInstallerCLICore/TableOutput.h b/src/AppInstallerCLICore/TableOutput.h index 6916602e40..03a3e1bfe4 100644 --- a/src/AppInstallerCLICore/TableOutput.h +++ b/src/AppInstallerCLICore/TableOutput.h @@ -20,8 +20,9 @@ namespace AppInstaller::CLI::Execution using header_t = std::array; using line_t = std::array; - TableOutput(Reporter& reporter, header_t&& header, size_t sizingBuffer = 50) : - m_reporter(reporter), m_sizingBuffer(sizingBuffer) + TableOutput(Reporter& reporter, header_t&& header) : + m_reporter(reporter), + m_hasConsole(GetConsoleWidth().has_value()) { for (size_t i = 0; i < FieldCount; ++i) { @@ -35,15 +36,11 @@ namespace AppInstaller::CLI::Execution { m_empty = false; - if (m_buffer.size() < m_sizingBuffer) - { - m_buffer.emplace_back(std::move(line)); - } - else - { - EvaluateAndFlushBuffer(); - OutputLineToStream(line); - } + // 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)); } void Complete() @@ -71,10 +68,10 @@ namespace AppInstaller::CLI::Execution Reporter& m_reporter; std::array m_columns; - size_t m_sizingBuffer; std::vector m_buffer; bool m_bufferEvaluated = false; bool m_empty = true; + bool m_hasConsole = false; void EvaluateAndFlushBuffer() { @@ -127,13 +124,14 @@ namespace AppInstaller::CLI::Execution totalRequired += m_columns[i].MaxLength + (m_columns[i].SpaceAfter ? 1 : 0); } - size_t consoleWidth = GetConsoleWidth(); + auto consoleWidthOpt = GetConsoleWidth(); - // If the total space would be too big, shrink them. - // We don't want to use the last column, lest we auto-wrap - if (totalRequired >= consoleWidth) + // 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 - consoleWidth) + 1; + size_t extra = (totalRequired - *consoleWidthOpt) + 1; while (extra) { @@ -151,7 +149,7 @@ namespace AppInstaller::CLI::Execution extra -= 1; } - totalRequired = consoleWidth - 1; + totalRequired = *consoleWidthOpt - 1; } // Header line diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp index 224a3a31bf..3290b03340 100644 --- a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -356,7 +356,7 @@ namespace AppInstaller::CLI::Workflow truncated = true; } - if (Utility::LimitOutputLines(lines, GetConsoleWidth(), maxLines)) + if (Utility::LimitOutputLines(lines, GetConsoleWidth().value_or(120), maxLines)) { truncated = true; } @@ -767,7 +767,7 @@ namespace AppInstaller::CLI::Workflow if (messageData.ShowDescription && !description.empty()) { constexpr size_t maximumDescriptionLines = 3; - size_t consoleWidth = GetConsoleWidth(); + size_t consoleWidth = GetConsoleWidth().value_or(120); std::vector lines = Utility::SplitIntoLines(description, maximumDescriptionLines + 1); bool wasLimited = Utility::LimitOutputLines(lines, consoleWidth, maximumDescriptionLines); diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 09b6e2b100..7a5f2ef16e 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -113,7 +113,7 @@ namespace AppInstaller::CLI::Workflow { // Using a height of 4 arbitrarily; allow width up to the entire console. UINT imageHeightCells = 4; - UINT imageWidthCells = static_cast(Execution::GetConsoleWidth()); + UINT imageWidthCells = static_cast(Execution::GetConsoleWidth().value_or(120)); icon.RenderSizeInCells(imageWidthCells, imageHeightCells); icon.RenderTo(outputStream); diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 387398354d..f87f626911 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -326,6 +326,7 @@ + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 39ecacf9be..a6e9192a24 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -230,6 +230,9 @@ Source Files\Common + + Source Files\Common + Source Files diff --git a/src/AppInstallerCLITests/TableOutput.cpp b/src/AppInstallerCLITests/TableOutput.cpp new file mode 100644 index 0000000000..2d5480083f --- /dev/null +++ b/src/AppInstallerCLITests/TableOutput.cpp @@ -0,0 +1,252 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestCommon.h" +#include "TestHooks.h" +#include +#include +#include + +using namespace AppInstaller::CLI; +using namespace AppInstaller::CLI::Execution; +using namespace AppInstaller::Utility; + +namespace +{ + Resource::LocString MakeHeader(std::string text) + { + return Resource::LocString{ LocIndString{ std::move(text) } }; + } +} + +// 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. +TEST_CASE("TableOutput_AllRowsBuffered_NoTruncation", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + + Reporter reporter(output, input); + + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + + // 54 rows with a short first-column value + for (int i = 0; i < 54; ++i) + { + table.OutputLine({ "ShortValue", "id" + std::to_string(i) }); + } + + // Row 55: first column is much longer than any previous row + std::string longValue(50, 'A'); + table.OutputLine({ longValue, "id54" }); + + table.Complete(); + + std::string result = output.str(); + + // The full long value must appear; the ellipsis character (U+2026, UTF-8: E2 80 A6) + // must not appear anywhere, confirming no truncation occurred. + REQUIRE(result.find(longValue) != std::string::npos); + REQUIRE(result.find("\xE2\x80\xA6") == std::string::npos); +} + +// Test that every data row has its second column starting at the same horizontal position, +// i.e., column 1 is consistently padded regardless of the individual value lengths. +TEST_CASE("TableOutput_ColumnsAligned", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + + Reporter reporter(output, input); + + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + + table.OutputLine({ "Short", "id.short" }); + table.OutputLine({ "MediumValue", "id.medium" }); + table.OutputLine({ "LongerValueHere","id.longer" }); + + table.Complete(); + + std::string result = output.str(); + std::istringstream ss(result); + std::vector lines; + std::string line; + while (std::getline(ss, line)) + { + if (!line.empty()) lines.push_back(line); + } + + // Expect: header, separator, 3 data rows = 5 lines minimum + REQUIRE(lines.size() >= 5); + + // The "id." prefix of the second-column value should start at the same position in every data row. + size_t pos0 = lines[2].find("id."); + size_t pos1 = lines[3].find("id."); + size_t pos2 = lines[4].find("id."); + + REQUIRE(pos0 != std::string::npos); + REQUIRE(pos0 == pos1); + REQUIRE(pos0 == pos2); +} + +// Test that calling Complete() on an empty table produces no output. +TEST_CASE("TableOutput_Empty_ProducesNoOutput", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{120} }; + + Reporter reporter(output, input); + + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + + REQUIRE(table.IsEmpty()); + table.Complete(); + REQUIRE(output.str().empty()); +} + +// Test that the console width override works and that TableOutput truncates the widest column +// (with ellipsis) when the console is too narrow to fit all columns. +TEST_CASE("TableOutput_ConsoleWidth_TruncatesWhenNarrow", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + // Simulate a console that is too narrow for the content. + // Column 0 max = 20 ("VeryLongPackageName0"), column 1 max = 6 ("pkg.id"). + // SpaceAfter on col 0 = true -> totalRequired = 20 + 1 + 6 = 27. + // With width = 20, extra = (27 - 20) + 1 = 8, so col 0 shrinks to 12. + // "VeryLongPackageName0" (20 chars) > 12 -> ellipsis in output. + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{20} }; + + Reporter reporter(output, input); + + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + table.OutputLine({ "VeryLongPackageName0", "pkg.id" }); + table.Complete(); + + std::string result = output.str(); + REQUIRE(result.find("\xE2\x80\xA6") != std::string::npos); // ellipsis present = truncation occurred +} + +// Test that with a wide enough console, values are output in full (no ellipsis). +TEST_CASE("TableOutput_ConsoleWidth_NoTruncationWhenWide", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::optional{200} }; + + Reporter reporter(output, input); + + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + std::string longValue(50, 'A'); + table.OutputLine({ longValue, "pkg.id" }); + table.Complete(); + + std::string result = output.str(); + REQUIRE(result.find(longValue) != std::string::npos); + REQUIRE(result.find("\xE2\x80\xA6") == std::string::npos); +} + +// Test that with no-console override (nullopt), content is never truncated regardless of length. +TEST_CASE("TableOutput_NoConsoleOverride_NeverTruncates", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::nullopt }; // simulate redirected output + + Reporter reporter(output, input); + + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + std::string longValue(200, 'B'); + table.OutputLine({ longValue, "pkg.id" }); + table.Complete(); + + std::string result = output.str(); + REQUIRE(result.find(longValue) != std::string::npos); + REQUIRE(result.find("\xE2\x80\xA6") == std::string::npos); +} + +// Test that a large number of rows can be buffered and output in a single table without +// any truncation or missing rows. +TEST_CASE("TableOutput_ManyRowsBuffered", "[tableoutput]") +{ + // At the time of creating this test, there were ~12k unique package IDs in the community repository + constexpr size_t RowCount = 25000; + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::nullopt }; // no console: no truncation + + Reporter reporter(output, input); + + TableOutput<3> table(reporter, { MakeHeader("Name"), MakeHeader("Id"), MakeHeader("Version") }); + + for (size_t i = 0; i < RowCount; ++i) + { + table.OutputLine({ + "Package.Name." + std::to_string(i), + "pkg.id." + std::to_string(i), + "1.0." + std::to_string(i) + }); + } + + table.Complete(); + + REQUIRE_FALSE(table.IsEmpty()); + + std::string result = output.str(); + + // Spot-check first, middle, and last rows + REQUIRE_FALSE(result.empty()); + REQUIRE(result.find("pkg.id.0") != std::string::npos); + REQUIRE(result.find("pkg.id." + std::to_string(RowCount / 2)) != std::string::npos); + REQUIRE(result.find("pkg.id." + std::to_string(RowCount - 1)) != std::string::npos); + + // Count newlines to verify all rows were written (header + separator + RowCount data rows) + size_t lineCount = std::count(result.begin(), result.end(), '\n'); + REQUIRE(lineCount == RowCount + 2); // +2 for header and separator lines + + // No truncation ellipsis anywhere + REQUIRE(result.find("\xE2\x80\xA6") == std::string::npos); +} + +// Test that when output is redirected (no console), triggering progress/spinner produces no +// spinner characters in the output stream — only the table content is present. +TEST_CASE("TableOutput_Redirected_NoSpinnerOutput", "[tableoutput]") +{ + std::ostringstream output; + std::istringstream input; + + TestHook::SetConsoleWidth_Override widthOverride{ std::nullopt }; // simulate redirected output + + Reporter reporter(output, input); + + // Simulate a spinner lifecycle that would emit characters if a spinner were active. + reporter.BeginProgress(); + reporter.SetProgressMessage("Searching..."); + reporter.EndProgress(true); + + // Now output a table with known content. + TableOutput<2> table(reporter, { MakeHeader("Name"), MakeHeader("Id") }); + table.OutputLine({ "TestPackage", "test.pkg" }); + table.Complete(); + + std::string result = output.str(); + + // The table data must be present. + REQUIRE(result.find("TestPackage") != std::string::npos); + REQUIRE(result.find("test.pkg") != std::string::npos); + + // Spinner frames written with \r must not appear. + // The character spinner uses '-', '\', '|', '/' preceded by '\r'. + REQUIRE(result.find('\r') == std::string::npos); +} diff --git a/src/AppInstallerCLITests/TestHooks.h b/src/AppInstallerCLITests/TestHooks.h index 44c5b2304a..ed0f23ef9c 100644 --- a/src/AppInstallerCLITests/TestHooks.h +++ b/src/AppInstallerCLITests/TestHooks.h @@ -75,6 +75,11 @@ namespace AppInstaller void TestHook_SetScanArchiveResult_Override(bool* status); } + namespace CLI::Execution + { + void TestHook_SetConsoleWidth_Override(std::optional* value); + } + namespace CLI::Workflow { void TestHook_SetEnableWindowsFeatureResult_Override(std::optional&& result); @@ -346,6 +351,24 @@ namespace TestHook std::optional info)> m_downloadFunction; }; + struct SetConsoleWidth_Override + { + // Pass std::nullopt to simulate no console (redirected output); + // pass a size_t value to simulate a console of that width. + SetConsoleWidth_Override(std::optional width) : m_width(width) + { + AppInstaller::CLI::Execution::TestHook_SetConsoleWidth_Override(&m_width); + } + + ~SetConsoleWidth_Override() + { + AppInstaller::CLI::Execution::TestHook_SetConsoleWidth_Override(nullptr); + } + + private: + std::optional m_width; + }; + struct SetGetFontRegistryRoot_Override { SetGetFontRegistryRoot_Override(std::function function)