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

Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow #10899

Merged
merged 23 commits into from Nov 1, 2019

Conversation

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Oct 25, 2019

PR Summary

Add back GraphicalHost assembly code to enable Out-GridView, Show-Command, and Get-Help -ShowWindow on Windows only.

Some of the original code was changed for updated copyright and some style issues. Because of a difference in the Windows build system vs dotnet msbuild on embedding resources, had to change resgen to work with what XAML expects. I would focus the code review on source code that was already in this repo that had to be changed to make this work. The original WPF code is largely unchanged except where needed to get it to compile.

No automated tests so I did some manual validation for all three commands.

In the future, we'll probably want to split this out as a separate module, but currently part of the code is in SMA.dll and the rest is in GraphicalHost.dll. Removing the SMA.dll code is not trivial as we need to expose new public APIs for the module in how the cmdlet talks to the WPF window. Also the cmdlet is part of Utility module which is a breaking change in itself.

Note the source folder is the namespace name as resgen uses that to produce the path to the resources and the code expects that path.

PR Context

Fix #10599
Fix #9778

PR Checklist

@SteveL-MSFT SteveL-MSFT requested review from daxian-dbw and TravisEz13 Oct 25, 2019
@SteveL-MSFT SteveL-MSFT changed the title WIP: Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow Oct 25, 2019
build.psm1 Show resolved Hide resolved
build.psm1 Show resolved Hide resolved
build.psm1 Show resolved Hide resolved
@SteveL-MSFT SteveL-MSFT changed the title Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow WIP: Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow Oct 26, 2019
@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 26, 2019

Marking it WIP as I work through the Codacy and Codefactor issues

@Jaykul

This comment has been minimized.

Copy link

Jaykul commented Oct 26, 2019

I still think this should be an external module 😞

@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 26, 2019

@Jaykul did you read the PR description above?

assets/files.wxs Show resolved Hide resolved
@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 26, 2019

@PoshChan please retry linux

@PoshChan

This comment has been minimized.

Copy link
Collaborator

PoshChan commented Oct 26, 2019

@SteveL-MSFT, successfully started retry of PowerShell-CI-Linux

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Oct 26, 2019

@iSazonov did you have a script to fix all the documentation issues previously? also making the using directives alphabetical.

For second #9490

Fixing doc issues was done manually. I can do this in follow PR.

@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 26, 2019

@iSazonov Seems like some of the doc rules can be scriptable, let me give it a shot since there's so many. Thanks for the using script.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Oct 26, 2019

For doc issues I used many Regex queries in VS Code and Notepad++.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

iSazonov commented Oct 26, 2019

I would focus the code review on source code that was already in this repo that had to be changed to make this work. The original WPF code is largely unchanged except where needed to get it to compile.

Maybe add original code in first PR and then add new commits in follow PRs? It is simplify code review.

@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 26, 2019

@iSazonov I'm ok deferring the code cleanup. However, it's a large amount of code. I'd rather focus this effort on the changes to files outside of the graphicalhost code base since that code base is from Windows PowerShell and has been working for some time. I don't want to invest in improving GraphicalHost, this is purely a parity with Windows PowerShell effort. I'd rather invest in a cross platform version of the tools over time.

@SteveL-MSFT SteveL-MSFT changed the title WIP: Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow Add GraphicalHost assembly to enable Out-GridView, Show-Command, and Get-Help -ShowWindow Oct 26, 2019
Copy link
Collaborator

iSazonov left a comment

Reviewed commit by commit.

assets/files.wxs Show resolved Hide resolved
@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 29, 2019

@sdwheeler does Show-Command also need a new fwlink?

assets/files.wxs Show resolved Hide resolved
@sdwheeler

This comment has been minimized.

Copy link
Collaborator

sdwheeler commented Oct 29, 2019

@SteveL-MSFT Show-Command https://go.microsoft.com/fwlink/?linkid=2109589

The Show-Command cmdlet lets you create a Windows PowerShell command in a command window. You can use the features of the command window to run the command or have it return the command to you. Show-Command is a very useful teaching and learning tool. Show-Command works on all command types, including cmdlets, functions, workflows and CIM commands. Without parameters, Show-Command displays a command window that lists all available commands in all installed modules. To find the commands in a module, select the module from the Modules drop-down list. To select a command, click the command name. To use the command window, select a command, either by using the Name or by clicking the command name in the Commands list. Each parameter set is displayed on a separate tab. Asterisks indicate the mandatory parameters. To enter values for a parameter, type the value in the text box or select the value from the drop-down box. To add a switch parameter, click to select the parameter check box. When you're ready, you can click Copy to copy the command that you've created to the clipboard or click Run to run the command. You can also use the PassThru parameter to return the command to the host program, such as the Windows PowerShell console. To cancel the command selection and return to the view that displays all commands, press Ctrl and click the selected command. In the Windows PowerShell Integrated Scripting Environment (ISE), a variation of the Show-Command window is displayed by default. For information about using this command window, see the Windows PowerShell ISE Help topics. This cmdlet was introduced in Windows PowerShell 3.0.
@adityapatwardhan

This comment has been minimized.

Copy link
Member

adityapatwardhan commented Oct 30, 2019

@SteveL-MSFT please resolve merge conflict.

Also, open a PR in the PowerShell-Native repo to add the new assembly in the trusted assemblies list:

https://github.com/PowerShell/PowerShell-Native/blob/d4acbd519d1c77e07abc894db42853055ef65f55/src/powershell-native/nativemsh/pwrshcommon/pwrshcommon.cpp#L684-L686

GitHub
Contribute to PowerShell/PowerShell-Native development by creating an account on GitHub.
@SteveL-MSFT

This comment has been minimized.

Copy link
Member Author

SteveL-MSFT commented Oct 31, 2019

@adityapatwardhan does it make sense to add it to trusted assemblies since it can't be used remotely?

@adityapatwardhan

This comment has been minimized.

Copy link
Member

adityapatwardhan commented Nov 1, 2019

@SteveL-MSFT merging this PR. I can confirm with @PaulHigin if the change in trusted assemblies must be made. It does not block this PR.

@adityapatwardhan adityapatwardhan merged commit ca68d9d into PowerShell:master Nov 1, 2019
6 of 8 checks passed
6 of 8 checks passed
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
CodeFactor 899 issues found.
Details
PowerShell-CI-linux #PR-10899-20191030.01 succeeded
Details
PowerShell-CI-macos #PR-10899-20191030.01 succeeded
Details
PowerShell-CI-static-analysis #PR-10899-20191030.01 succeeded
Details
PowerShell-CI-windows #PR-10899-20191030.01 succeeded
Details
WIP Ready for review
Details
license/cla All CLA requirements met.
Details
@adityapatwardhan adityapatwardhan added this to the 7.0.0-preview.6 milestone Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.