The Wayback Machine - https://web.archive.org/web/20200930185229/https://github.com/microsoft/winget-cli/pull/507
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show cmd extension openurls of manifest #507

Open
wants to merge 9 commits into
base: master
from

Conversation

@skyhoshi
Copy link

@skyhoshi skyhoshi commented Jul 22, 2020

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

  • Closes #501 : REPLACES PR: #503
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
    • NOTE: All existing tests pass, no new tests were added. ([🟥][🔰] -> help wanted | Needed [🏳️])
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #501

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.

  1. OpenUrlWorkflow.(h|cpp)
  2. GetRequestedUrlFromManifest.(h|cpp)

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

wingetdev show git

Expectation

Display's the Mainfest information in human readable format

Result (Includes Command Output)

PS C:\> wingetdev show git
Found Git [Git.Git]
Version: 2.27.0
Publisher: Git
Description: Git version control system.
Homepage: https://git-scm.com/
License: GNU General Public License, version 2
License Url: https://github.com/git-for-windows/git/blob/master/COPYING
Installer:
  Type: Inno
  Download Url: https://github.com/git-for-windows/git/releases/download/v2.27.0.windows.1/Git-2.27.0-64-bit.exe
  SHA256: 5974db8c52b32f5e575ee021e8b47948892ce0e760095eef98c31e3bbd5167b6

Test 2: Show - Hompeage found - Open App Homepage

Command Performed

wingetdev show git --homepage

Expectation

  1. Display's the Mainfest for the app information in human readable format
  2. If the homepage exists (which it does for git) it will automatically open the homepage in the system's default browser.

Result (Includes Command Output)

PS C:\> wingetdev show git --homepage
Found Git [Git.Git]
Version: 2.27.0
Publisher: Git
Description: Git version control system.
Homepage: https://git-scm.com/
License: GNU General Public License, version 2
License Url: https://github.com/git-for-windows/git/blob/master/COPYING
Installer:
  Type: Inno
  Download Url: https://github.com/git-for-windows/git/releases/download/v2.27.0.windows.1/Git-2.27.0-64-bit.exe
  SHA256: 5974db8c52b32f5e575ee021e8b47948892ce0e760095eef98c31e3bbd5167b6
Opening Homepage Url: https://git-scm.com/
<Opens Homepage Default Browser with URL>

Test 3: Show - Hompeage found, License - Open App Homepage, Open License

Command Performed

wingetdev show git --homepage --License

Expectation

  1. Display's the Mainfest for the app information in human readable format
  2. If the homepage exists (which it does for git) it will automatically open the homepage in the system's default browser.
  3. If the License exists (which it does for git) it will automatically open the license page in the system's default brower.

Result (Includes Command Output)

PS C:\> wingetdev show git --homepage --License
Found Git [Git.Git]
Version: 2.27.0
Publisher: Git
Description: Git version control system.
Homepage: https://git-scm.com/
License: GNU General Public License, version 2
License Url: https://github.com/git-for-windows/git/blob/master/COPYING
Installer:
  Type: Inno
  Download Url: https://github.com/git-for-windows/git/releases/download/v2.27.0.windows.1/Git-2.27.0-64-bit.exe
  SHA256: 5974db8c52b32f5e575ee021e8b47948892ce0e760095eef98c31e3bbd5167b6
Opening Homepage Url: https://git-scm.com/
Opening License Url: https://github.com/git-for-windows/git/blob/master/COPYING
<Opens Homepage Default Browser with URL>
<Opens License Default Browser with URL>

Test 4: Show - Hompeage NOT found, License is found - States Homepage not found, Open License (License is 404 for some reason)

Command Performed

wingetdev show Playstation.PSnow --homepage --License

Expectation

  1. Display's the Mainfest for the app information in human readable format
  2. No Homepage is found. States it did not find a homepage url for this manifest
  3. If the License exists (which it does for Playstation.PsNow) it will automatically open the license page in the system's default brower.

Result (Includes Command Output)

PS C:\> wingetdev show Playstation.PSnow --homepage --License
Found PlayStation Now [PlayStation.PSNow]
Version: 11.1.2
Publisher: Sony Interactive Entertainment
Description: Get instant access to a huge on demand game collection – the only place to play PlayStation exclusive titles on Windows PC.
License: Copyright (c) Sony Interactive Entertainment
License Url: https://www.playstation.com/legal/important-information-about-the-ps-now-service/
Installer:
  Type: Exe
  Download Url: https://download-psnow.playstation.com/downloads/psnow/pc/PlayStationNow-11.1.2.exe
  SHA256: 660eaa555fd3376174c1ebccceb5008099c1da19a4ddfd1076436eefa67fedfe
No Homepage found within manifest
Opening License Url: https://www.playstation.com/legal/important-information-about-the-ps-now-service/

<Opens License Default Browser with URL> (License is 404 for some reason)

Test 5: Show - Hompeage NOT found - States Homepage not found

Command Performed

wingetdev show Playstation.PSnow --homepage

Expectation

  1. Display's the Mainfest for the app information in human readable format
  2. No Homepage is found. States it did not find a homepage url for this manifest

Result (Includes Command Output)

Found PlayStation Now [PlayStation.PSNow]
Version: 11.1.2
Publisher: Sony Interactive Entertainment
Description: Get instant access to a huge on demand game collection – the only place to play PlayStation exclusive titles on Windows PC.
License: Copyright (c) Sony Interactive Entertainment
License Url: https://www.playstation.com/legal/important-information-about-the-ps-now-service/
Installer:
  Type: Exe
  Download Url: https://download-psnow.playstation.com/downloads/psnow/pc/PlayStationNow-11.1.2.exe
  SHA256: 660eaa555fd3376174c1ebccceb5008099c1da19a4ddfd1076436eefa67fedfe
No Homepage found within manifest

Test 6: Show - License NOT found - States License not found

Command Performed

wingetdev show Wings3D.Wings3D --license

Expectation

  1. Display's the Mainfest for the app information in human readable format
  2. No Homepage is found. States it did not find a homepage url for this manifest

Result (Includes Command Output)

PS C:\> wingetdev show Wings3D.Wings3D --license
Found Wings3D [Wings3D.Wings3D]
Version: 2.2.5
Publisher: Wings3D Team
Description: Wings 3D is an advanced subdivision modeler that is both powerful and easy to use.
Homepage: http://www.wings3d.com
License: BSD
Installer:
  Type: Nullsoft
  Download Url: https://sourceforge.net/projects/wings/files/wings/2.2.5/wings-x64-2.2.5.exe/download
  SHA256: d14f8b07c07d2bf6a6766a540be7a82b7e5b367c5ba88da54a5728c7f173a1c8
No License url found within manifest
Microsoft Reviewers: Open in CodeFlow
skyhoshi added 5 commits Jul 21, 2020
…empting to get it (cause issue when installer wasn't present in the context)
…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.
@skyhoshi skyhoshi requested a review from microsoft/windows-package-manager-admins as a code owner Jul 22, 2020
@msftbot msftbot bot added the Issue-Feature label Jul 22, 2020
skyhoshi added 4 commits Jul 23, 2020
{
strstr << " '" << arg << '\'';
}
return strstr.str();

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jul 28, 2020
Member

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)) {

This comment has been minimized.

@JohnMcPMS

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)
{
}

context.Reporter.Info() << Resource::String::UrlNotFound << std::endl;
}
else {
if (!selectedLocalization.Homepage.empty()) {

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jul 28, 2020
Member

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() });

This comment has been minimized.

@JohnMcPMS

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);

{
if (context.Contains(Execution::Data::ShowUrls)) {
std::vector<std::string>& vectorOfUrls = context.Get<Execution::Data::ShowUrls>();
for (auto url : vectorOfUrls)

This comment has been minimized.

@JohnMcPMS

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).

@@ -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 };

This comment has been minimized.

@JohnMcPMS

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.