Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upShow cmd extension openurls of manifest #507
Conversation
…homepage or license data.
…empting to get it (cause issue when installer wasn't present in the context)
…ed arguments homepage and license.
…ck to the context process. adjusted some validation on the url retrieval to ensure that text in the url values starts with an actual url http value.
src/AppInstallerCLICore/Workflows/GetRequestedUrlFromManifest.cpp
Outdated
Show resolved
Hide resolved
src/AppInstallerCLICore/Workflows/GetRequestedUrlFromManifest.cpp
Outdated
Show resolved
Hide resolved
src/AppInstallerCLICore/Workflows/GetRequestedUrlFromManifest.cpp
Outdated
Show resolved
Hide resolved
| { | ||
| strstr << " '" << arg << '\''; | ||
| } | ||
| return strstr.str(); |
JohnMcPMS
Jul 28, 2020
Member
Not sure why/how, but this change isn't needed.
Not sure why/how, but this change isn't needed.
| const auto& manifest = context.Get<Execution::Data::Manifest>(); | ||
| Workflow::ManifestComparator manifestComparator(context.Args); | ||
| const auto selectedLocalization = manifestComparator.GetPreferredLocalization(manifest); | ||
| if (context.Args.Contains(Execution::Args::Type::Homepage)) { |
JohnMcPMS
Jul 28, 2020
Member
{ [](start = 67, length = 2)
More style things, we use braces on new lines except in rare one line cases:
if (condition)
{
}
{ [](start = 67, length = 2)
More style things, we use braces on new lines except in rare one line cases:
if (condition)
{
}
| context.Reporter.Info() << Resource::String::UrlNotFound << std::endl; | ||
| } | ||
| else { | ||
| if (!selectedLocalization.Homepage.empty()) { |
JohnMcPMS
Jul 28, 2020
Member
Don't need to test again.
Don't need to test again.
| if (!selectedLocalization.LicenseUrl.empty()) { | ||
| if (Utility::IsUrlRemote(selectedLocalization.LicenseUrl)) { | ||
| context.Reporter.Info() << Resource::String::ShowLicenseOpeningUrl << ": " << selectedLocalization.LicenseUrl << std::endl; | ||
| context.Add<Execution::Data::ShowUrls>({ selectedLocalization.LicenseUrl.c_str() }); |
JohnMcPMS
Jul 28, 2020
Member
context.AddExecution::Data::ShowUrls({ selectedLocalization.LicenseUrl.c_str() }); [](start = 24, length = 84)
This second time will overwrite the first vector. Would probably be easiest to always insert an empty vector at the beginning, then always add here:
At top:
context.AddExecution::Data::ShowUrls({});
When adding:
context.GetExecution::Data::ShowUrls().emplace_back(selectedLocalization.LicenseUrl);
context.AddExecution::Data::ShowUrls({ selectedLocalization.LicenseUrl.c_str() }); [](start = 24, length = 84)
This second time will overwrite the first vector. Would probably be easiest to always insert an empty vector at the beginning, then always add here:
At top:
context.AddExecution::Data::ShowUrls({});
When adding:
context.GetExecution::Data::ShowUrls().emplace_back(selectedLocalization.LicenseUrl);
| { | ||
| if (context.Contains(Execution::Data::ShowUrls)) { | ||
| std::vector<std::string>& vectorOfUrls = context.Get<Execution::Data::ShowUrls>(); | ||
| for (auto url : vectorOfUrls) |
JohnMcPMS
Jul 28, 2020
Member
auto [](start = 17, length = 4)
const auto& (or auto const& if you want to get ahead of the curve of us old timers and our west const).
auto [](start = 17, length = 4)
const auto& (or auto const& if you want to get ahead of the curve of us old timers and our west const).
| @@ -36,6 +38,8 @@ namespace AppInstaller::Settings | |||
| return ExperimentalFeature{ "Argument Sample", "experimentalArg", "https://aka.ms/winget-settings", Feature::ExperimentalArg }; | |||
| case Feature::ExperimentalMSStore: | |||
| return ExperimentalFeature{ "Microsoft Store Support", "experimentalMSStore", "https://aka.ms/winget-settings", Feature::ExperimentalMSStore }; | |||
| case Feature::BrowseOrOpenRemoteUrl: | |||
| return ExperimentalFeature{ "Open Url", "experimentalBrowseOrOpenRemoteUrl", "https://aka.ms/winget-settings", Feature::BrowseOrOpenRemoteUrl }; | |||
JohnMcPMS
Jul 28, 2020
Member
experimentalBrowseOrOpenRemoteUrl [](start = 53, length = 33)
Can we just start naming these without experimental* at the front? My fault for not catching the last one. So just "browseOrOpenRemoteUrl".
Also this should have changes to /doc/Settings.md to describe it.
Tagging @yao-msft as I forgot to mention this in his PR for experimentalMSStore.
experimentalBrowseOrOpenRemoteUrl [](start = 53, length = 33)
Can we just start naming these without experimental* at the front? My fault for not catching the last one. So just "browseOrOpenRemoteUrl".
Also this should have changes to /doc/Settings.md to describe it.
Tagging @yao-msft as I forgot to mention this in his PR for experimentalMSStore.


Summary of the Pull Request
Adds subcommand's "--homepage" and "--license" to the show command that lookup, validates (has http at start) and will open the url in the default brower based on the HomepageUrl and LicenseUrl respectively.
References
This PR replaces #503.
Fixes #501
PR Checklist
Detailed Description of the Pull Request / Additional comments
So from my previous PR #503, I learned alot and upon discovering that workflows were being extracted into single seperate files of the same namespace I did just that, pulled the methods and logic out into two seperate workflow files.
Open Url Workflow
This file and it's methods simply look for the URL's in question from the context and opens them as a shell execute command. this method of call is equivelent to opening the run box and typing in a URL and clicking ok. it opens the url in your system's default browswer.
Get Requested Url From Manifest
This file and it's methods take the context's current search result and attempts to identify if only one manifest was retrieved. if not, it says so and outputs the search results allowing the user to refine or pick specificly.
if it only finds one manifest, it then extracts the url from the manifest starting and adds it as a data value back into the context for use in the Open Url Workflow.
Validation Steps Performed
Test 1: Default Show
Command Performed
Expectation
Display's the Mainfest information in human readable format
Result (Includes Command Output)
Test 2: Show - Hompeage found - Open App Homepage
Command Performed
wingetdev show git --homepageExpectation
Result (Includes Command Output)
Test 3: Show - Hompeage found, License - Open App Homepage, Open License
Command Performed
Expectation
Result (Includes Command Output)
Test 4: Show - Hompeage NOT found, License is found - States Homepage not found, Open License (License is 404 for some reason)
Command Performed
Expectation
Result (Includes Command Output)
Test 5: Show - Hompeage NOT found - States Homepage not found
Command Performed
wingetdev show Playstation.PSnow --homepageExpectation
Result (Includes Command Output)
Test 6: Show - License NOT found - States License not found
Command Performed
wingetdev show Wings3D.Wings3D --licenseExpectation
Result (Includes Command Output)
Microsoft Reviewers: Open in CodeFlow