CLI: Initial support for version format option#40238
CLI: Initial support for version format option#40238AmelBawa-msft wants to merge 5 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial wslc version command support for selectable output formats (table/json), wiring version collection into the CLI execution context and exposing it via a new task/service/model flow.
Changes:
- Introduces
VersionService+VersionInfomodel to provide client/server version data. - Adds
VersionTasksto emit version output as table (default) or JSON via--format. - Updates
VersionCommandexecution and adds an E2E help test forwslc version.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/wslc/e2e/WSLCE2EVersionTests.cpp | Adds E2E test coverage for version --help. |
| src/windows/wslc/tasks/VersionTasks.h | Declares tasks for fetching and rendering version info. |
| src/windows/wslc/tasks/VersionTasks.cpp | Implements format selection and output for version. |
| src/windows/wslc/services/VersionService.h | Declares service API for version info/string. |
| src/windows/wslc/services/VersionService.cpp | Implements version string + static version info instance. |
| src/windows/wslc/services/VersionModel.h | Adds ClientVersion/ServerVersion/VersionInfo model + JSON support. |
| src/windows/wslc/core/ExecutionContextData.h | Adds Data::Version storage mapping to the execution context. |
| src/windows/wslc/commands/VersionCommand.h | Switches VersionCommand to expose arguments (format). |
| src/windows/wslc/commands/VersionCommand.cpp | Wires version execution through the new tasks and adds --format. |
| src/windows/wslc/commands/RootCommand.cpp | Routes --version to VersionService::GetVersionString(). |
|
@copilot fix the VersionCommand_HasNoArguments test method in WSLCCLICommandUnitTests.cpp, in this PR I added one argument to the version command. |
…dded in PR Agent-Logs-Url: https://github.com/microsoft/WSL/sessions/309f2df8-2ce9-4f99-8b04-2d5c7fbeb50c Co-authored-by: AmelBawa-msft <104940545+AmelBawa-msft@users.noreply.github.com>
Fixed in commit 36d00ce. Renamed the test to |
dkbennett
left a comment
dkbennett
left a comment
There was a problem hiding this comment.
Overall structure and approach looks good, I only have issues with the table output, which looks like it needs loc for table format and should use the same table output as other commands. Also a note about the help change that will cause a test failure.
| std::wstringstream options; | ||
| options << L"The following options are available:\r\n" | ||
| << L" --format Output formatting (json or table) (Default: table)\r\n" | ||
| << L" -h,--help Shows help about the selected command\r\n" |
There was a problem hiding this comment.
This will be a test failure now that help flag has been changed to -?
| } | ||
| case FormatType::Table: | ||
| { | ||
| PrintMessage(MultiByteToWide(versionInfo.ToString())); |
There was a problem hiding this comment.
Should use the table output format all the other tables use.
| { | ||
| std::stringstream ss; | ||
| ss << "Server:\n"; | ||
| ss << WSLC_VERSION_ROW(" Linux kernel", Kernel); |
There was a problem hiding this comment.
These strings should be loc'd for table output in the CLI.
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed