Open
Conversation
This comment has been minimized.
This comment has been minimized.
JohnMcPMS
reviewed
Apr 14, 2026
Comment on lines
+19
to
+24
| #ifndef AICLI_DISABLE_TEST_HOOKS | ||
| // Overrides the value returned by GetConsoleWidth(). Pass nullptr to remove the override. | ||
| // Pass a pointer to std::nullopt to simulate no console; pass a pointer to a value to | ||
| // simulate a console of that width. | ||
| void TestHook_SetConsoleWidth_Override(std::optional<size_t>* value); | ||
| #endif |
Member
There was a problem hiding this comment.
We don't put the hooks into the headers here; only the test headers.
| else | ||
| { | ||
| m_out << '\r' << std::string(GetConsoleWidth(), ' ') << '\r'; | ||
| m_out << '\r' << std::string(GetConsoleWidth().value_or(0), ' ') << '\r'; |
Member
There was a problem hiding this comment.
Independent of other changes I think that it would be better to just \n. Since we should never hit this if progress is disabled whenever this is std::nullopt, more of a nit.
| // 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]") |
Member
There was a problem hiding this comment.
All tests should set width override to prevent using the console width they are run in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When winget list output is redirected to a file or variable:
Changes
ChannelStreams.h / ChannelStreams.cpp
TableOutput.h
non-console output.
Note
I assumed the 50-row sizing buffer was for performance reasons and to ensure the CLI would not take too much memory. A test on my machine showed a terminal width of 220 and roughly 200 packages. Assuming a terminal width of 500 characters on an ultrawide monitor, UTF-16 encoded, with 10k rows, that would be roughly 10 MB of buffered content. Given that 10k rows and 500 characters would be a statistical anomaly, I assumed that it would be safe to remove this sizing buffer even if it comes at a minor performance cost.
ExecutionReporter.cpp
ExecutionProgress.cpp, WorkflowBase.cpp, ConfigurationFlow.cpp
AppInstallerCLITests
Microsoft Reviewers: Open in CodeFlow