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

PSDefaultParameterValues on Get-Help for ShowWindow triggers help window on mouse over of cmdlet #2121

Open
jrich523 opened this issue Aug 5, 2019 · 8 comments
Labels
Area-UI Issue-Bug A bug to squash. Up for Grabs Will shepherd PRs.

Comments

@jrich523
Copy link

jrich523 commented Aug 5, 2019

System Details

  • Windows 10 build 17134
  • VS Code 1.26.1
  • PowerShell extension v2019.5.0
  • Output from $PSVersionTable
    Name Value

PSVersion 5.1.17134.858
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.17134.858
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Issue Description

With the following set, i get a help window pop up on mouse over of cmdlets/functions
$PSDefaultParameterValues.'Get-Help:ShowWindow' = $true

Expected Behaviour

Nothing

Actual Behaviour

Help window pops up

Attached Logs

event log that triggers it:

2019-08-05 10:59:47.487 [VERBOSE] tid:23 in 'WriteMessage' C:\PowerShellEditorServices\src\PowerShellEditorServices.Protocol\MessageProtocol\MessageWriter.cs: line 88
    Writing Response 'textDocument/hover' with id 10
    
    {
      "jsonrpc": "2.0",
      "id": "10",
      "result": {
        "contents": [
          {
            "language": "PowerShell",
            "value": "function Set-PSReadlineOption"
          }
        ],
        "range": {
          "start": {
            "line": 42,
            "character": 0
          },
          "end": {
            "line": 42,
            "character": 20
          }
        }
      }
    }```

@ghost ghost added the Needs: Triage Maintainer attention needed! label Aug 5, 2019
@corbob
Copy link
Contributor

corbob commented Aug 5, 2019

So I was thinking it would be a simple addition to here: https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices/Language/CommandHelpers.cs#L88-L90 whereby we add a parameter of ShowWindow and set it to $false. However, in PowerShell 7 (and presumably 6) this will likely fail: Get-Help : A parameter cannot be found that matches parameter name 'showwindow'.

@SydneyhSmith
Copy link
Collaborator

@jrich523 thanks for opening this issue, are you able to provide us with a screen shot so we can better understand the issue? Thanks!

@SydneyhSmith SydneyhSmith added Needs-Repro-Info and removed Needs: Triage Maintainer attention needed! labels Aug 6, 2019
@corbob
Copy link
Contributor

corbob commented Aug 7, 2019

@SydneyhSmith here's a gif of the issue (hopefully it makes sense). Notice when mousing over Write-Host a PowerShell icon appears in the task bar.

Get-Help-ShowWindow

I'm fairly certain the code that's causing it is https://github.com/PowerShell/PowerShellEditorServices/blob/master/src/PowerShellEditorServices/Language/CommandHelpers.cs#L88-L90, however as this would only be an issue in Windows PowerShell and possibly PowerShell 7 (if -ShowWindow is brought to it, it's not in preview 2) is it worth the overhead of checking versions and appending -ShowWindow:$false if it's less than 6? Do we then also need to add -Online:$false and any other parameters that might be loaded in a profile?

@jrich523 as another workaround: in your profile, you could look at $Host and if it's Visual Studio Code Host then don't set the default.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Aug 7, 2019
@SydneyhSmith
Copy link
Collaborator

@corbob thanks for the excellent gif and explanation...we agree that that is likely the best way forward...I'll mark this as up for grabs in case anyone has some time to tackle this issue

@SydneyhSmith SydneyhSmith added Area-UI Issue-Bug A bug to squash. Up for Grabs Will shepherd PRs. and removed Needs-Repro-Info Needs: Maintainer Attention Maintainer attention needed! labels Aug 8, 2019
@KirkMunro
Copy link
Collaborator

KirkMunro commented Aug 9, 2019

@SydneyhSmith and @corbob There is a broader issue that is worth addressing here that I've run into a number of times, and that you'll probably run into again if you don't address it properly.

$PSDefaultParameterValues is meant for customization of PowerShell behavior for commands that users invoke. It is not meant to modify behavior of tools that are built using PowerShell. That is why PowerShell 5 added a change so that module scope receives its own $PSDefaultParameterValues -- so that user customization does not impact modules (which are supposed to work like black box utilities). While that change helps, it's only for PowerShell 5+, and it's only helping modules. It does not help other tools/programs that work with PowerShell at an SDK level, outside of a module, yet such tools are also meant to function like a black box without changes in behavior based on how a user customizes their PowerShell commands.

Other preference variables such as $ErrorActionPreference, $WarningPreference, etc. can also impact tools that internally invoke PowerShell if tool authors are not careful.

Command and variable breakpoints can and will impact tools that internally invoke PowerShell if tool authors are not careful. For example, in VS Code, invoke this: Set-PSBreakpoint -Command Get-Help, and then move your mouse over a command. The integrated console crashes.

Solving these problems is not trivial because PowerShell itself does not do a great job making it easy to solve these problems. PowerShell needs the following so that tools can be written and used as black boxes:

  • proper support for invocation of commands using the SDK while keeping those commands hidden from a debugger (equivalent to the System.Diagnostics.DebuggerHidden attribute, but without having to run individual commands as a script just to be able to use the capability that is provided by that attribute)
  • proper support for running commands/scripts using the SDK without influence from user preference variables

I'll open issues for those in PowerShell/PowerShell, but in the meantime you should consider having an internal helper method that protects against this, by doing all of the following:

  1. Explicitly use all of the common parameters that impact preference with their default values when you invoke any PowerShell command via the SDK.

  2. Get the functionality you need directly from the PowerShell SDK without invoking PowerShell whenever and wherever possible.

  3. Disable and then re-enable breakpoints around invocations, or invoke scripts that use the DebuggerHidden attribute instead of commands. For example, invoke something like this instead of using PSCommand.AddCommand, PSCommand.AddArgument, and PSCommand.AddParameter:

    // Assume you have some resource string defined as follows:
    //     [CmdletBinding()]
    //     [System.Diagnostics.DebuggerHidden()]
    //     param()
    //     {0}{1}
    
    // Warning: This is just rough and untested...I just typed it into this issue to give you
    // an idea what I had in mind.
    private PSCommand GetSafeCommand(string command, Dictionary<string,object> parameters)
    {
        var parameterString = new StringBuilder();
        foreach (var key in parameters.Keys)
        {
            var value = parameters[key];
            if (value is string && ((string)value).Contains(" "))
            {
                value = $"'{value}'";
            }
            parameterString.Append($" -{key}:{value}");
        }
        foreach (var key in commonParameterDefaults.Keys)
        {
            if (!parameters.Keys.Contains(key))
            {
                var value = commonParameterDefaults[key];
                if (value is string && ((string)value).Contains(" "))
                {
                    value = $"'{value}'";
                }
                parameterString.Append($" -{key}:{value}");
            }
        }
        string script = String.Format(
            GetScriptResourceString(),
            command,
            parameterString.ToString());
        return new PSCommand().AddScript(script, true);
    }
  4. Rename PSDefaultParameterValues via the SDK before you run your command, and then rename it back after you run your command. I call out doing this via the SDK because of PowerShell Issue #10302. If this cannot be done via the SDK (I don't recall if it can), then do it in PowerShell as part of the script generated above if you're working in the global scope, which I think you are (that script is configured to run in the current scope).

I think that covers most if not all of what you should be thinking about here. If you have any questions, let me know.

@KirkMunro
Copy link
Collaborator

When I posted this issue in the PowerShell repo, @BrucePay raised a good question:

Why not just use a separate runspace.

If the PowerShell commands you are invoking are not dependent on the state of the current runspace, nor on modules loaded in the current runspace, then that would be a great solution to this problem, because the runspace you use for "internal" commands would never pick up on user preference variables or state. In the case of the OP on this issue, invoking Get-Help in a separate, internal runspace would avoid the problem entirely, and you'd never have to worry about Get-Help picking up on $PSDefaultParameterValues customizations.

It's been a little while since I worked on PowerShell tools though, but my gut says that while this approach would solve some problems, for PowerShell tools there is still the broader problem of working with PowerShell when you want to work with data in the user's runspace (getting variable values, etc.). For those cases, using the SDK without invoking any PowerShell would be the best alternative.

If vscode-powershell has PowerShell commands that it needs to invoke in the current user's runspace that perform tasks that are not supported by the SDK, that would be very good to know.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Aug 12, 2019
@SeeminglyScience
Copy link
Collaborator

SeeminglyScience commented Aug 12, 2019

When I posted this issue in the PowerShell repo, @BrucePay raised a good question:

Why not just use a separate runspace.

That would miss help from commands that were loaded from:

  1. Dot sourced files
  2. Imported modules that are not in PSModulePath
  3. The user $profile

It would also use a different help cache which would (slightly) hurt performance.

If vscode-powershell has PowerShell commands that it needs to invoke in the current user's runspace that perform tasks that are not supported by the SDK, that would be very good to know.

If we're invoking PowerShell then we pretty much always need state.

That said, the integrated console is a special case. Most SDK consumers won't need to share a runspace with an active prompt. I imagine most will completely own their runspace, and probably won't even invoke the user's $profile. The answer "use a separate runspace" is the right one, it just doesn't apply to PSES.

@KirkMunro
Copy link
Collaborator

Thanks @SeeminglyScience, that's what I thought. The separate runspace idea seemed to simple, and for PowerShell tools like this (and PowerGUI and PowerWF and PowerSE in the past, which all had this same challenge), runspace state is central to what those tools do, in which case it is better to work around the problem where possible, perhaps using something like the method logic I shared earlier, while moving the PowerShell SDK forward to support such things in a native way.

@SydneyhSmith SydneyhSmith removed the Needs: Maintainer Attention Maintainer attention needed! label Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UI Issue-Bug A bug to squash. Up for Grabs Will shepherd PRs.
Projects
None yet
Development

No branches or pull requests

5 participants