Added system architecture to winget --info.
#1937
Conversation
ghost
added
Area-Architecture
Co-authored-by: JohnMcPMS <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -173,6 +173,8 @@ namespace AppInstaller::CLI | |||
| info << std::endl << | |||
| "Windows: "_liv << Runtime::GetOSVersion() << std::endl; | |||
|
|
|||
| info << Resource::String::Architecture << ": "_liv << Utility::ToString(Utility::GetSystemArchitecture()) << std::endl; | |||
There was a problem hiding this comment.
We haven't enforced the output rule yet, but I think the Utility::ToString(Utility::GetSystemArchitecture()) needs to be wrapped inside LocIndString
There was a problem hiding this comment.
Utility::ToString(Architecture) could probably be changed to return a loc independent view. If it turns out we need to localize it in the future, that can be its own problem.
| @@ -1272,4 +1272,7 @@ Please specify one of them using the `--source` option to proceed.</value> | |||
| <data name="InvalidArgumentWithoutQueryError" xml:space="preserve"> | |||
| <value>The arguments provided can only be used with a query.</value> | |||
| </data> | |||
| <data name="Architecture" xml:space="preserve"> | |||
There was a problem hiding this comment.
nit: shall we match the variable name with the value so we don't misuse this in the future?
|
Sorry for the delay, I made it localization independent when the command is called. I tried to do as @JohnMcPMS suggested and just change the type to LocIndView, but since LocIndView is also defined inside of that project, it seemed to create a circular dependency somehow (although since LocIndView wasn't relying on anything in AppInstallerArchitecture.h, I'm not sure why). Does anyone have any ideas on how to fix that? |
I just tried adding |
|
It was the missing header :( Thanks @yao-msft. |
| { | ||
| switch (architecture) | ||
| { | ||
| case Architecture::Neutral: | ||
| return "Neutral"sv; | ||
| return LocIndView("Neutral"sv); |
There was a problem hiding this comment.
To make code a bit cleaner and consistent with other places. We may want to use our custom string literals(_liv).
using namespace literals;
switch (architecture)
{
case Architecture::Neutral:
return "Neutral"_liv;
case Architecture::X86:
return "X86"_liv;
case Architecture::X64:
return "X64"_liv;
case Architecture::Arm:
return "Arm"_liv;
case Architecture::Arm64:
return "Arm64"_liv;
}
return "Unknown"_liv;
There was a problem hiding this comment.
I tried to use them but was missing the namespace. Oops, my header game is weak this week.
|
Does this work in the Sandbox? I submitted an Issue for "Screen To Gif". I wanted to make sure we're checking the host architecture, for this scenario or at least be aware of limitations. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sorry I didn't get to this sooner, yes it works in the Sandbox. |
Resolves #1925 (yes, I know it was my issue. just had to leave myself a reminder :D)
This PR adds the system architecture to
winget --info, cutting out the first question we usually ask when troubleshooting.Microsoft Reviewers: Open in CodeFlow