PowerShell / PSReadLine

A bash inspired readline implementation for PowerShell
BSD 2-Clause "Simplified" License
3.74k stars 295 forks source link

Tab Completion uses data that has gone "stale" #1150

Closed jhoneill closed 4 years ago

jhoneill commented 4 years ago

Environment data

PS version: 5.1.18362.145 PSReadline version: 2.0.0-beta2 os: 10.0.18362.1 (WinBuild.160101.0800) PS file version: 10.0.18362.1 (WinBuild.160101.0800) HostName: ConsoleHost BufferWidth: 120 BufferHeight: 3000

and

PS version: 6.2.3 PSReadline version: 2.0.0-beta3 os: 10.0.18362.1 (WinBuild.160101.0800) PS file version: 6.2.3.0 HostName: ConsoleHost (Windows Terminal) BufferWidth: 119 BufferHeight: 28

Steps to reproduce or exception report

I have a function which replaces the CD alias. When I load the function with an argument completer , the argument completer does not properly IF PSRreadline is loaded. Unloading PSReadline fixes the problem. Further investigation shows that the parameters of the alias are also remembered when they should not be.

Test 1. Start a new instance of Powershell.exe or pwsh.exe run these two commands

del alias:cd
get-alias cd 

So the alias has been removed Now type Cd - [tab] parameters for the extinct "CD" alias cycle through.

Remove-Module PSReadLine type Cd - [tab] again nothing completes

Test 2 Save this file as test.ps1

DEL ALIAS:CD
Function CD  {
[CmdletBinding()]
Param ($Par)

$par 
}
[void] [System.Reflection.Assembly]::LoadWithPartialName("system.drawing")
Function ColorCompletion {
    param($commandName, $parameterName, $wordToComplete, $commandAst, $fakeBoundParameter)
    [System.Drawing.KnownColor].GetFields() | Where-Object {$_.IsStatic -and $_.name -like "$wordToComplete*" } |
        Sort-Object name | ForEach-Object { [System.Management.Automation.CompletionResult]::new( $_.name )
    }
}
Register-ArgumentCompleter -CommandName CD  -ParameterName par -ScriptBlock $Function:ColorCompletion

Start a new instance of Powershell.exe or pwsh.exe and load it with . test.ps1

Type CD [tab] and the list of directories cycles through as CD would at start time.

Remove-Module PSReadLine type Cd [tab] and now KnownColors cycle

msftrncs commented 4 years ago

I'll look in to this. I'm curious, as I have been working on the code in PowerShell that provides the completions (in order to correct quoting and escaping). I wasn't aware that PSReadLine cache'd any results.

jhoneill commented 4 years ago

I was surprised that it seems to cache. Re-importing the module with -force clears the cache.

msftrncs commented 4 years ago

Interesting. PSReadLine does not cache completions.

Instead, it appears that there is some issue with how PSReadLine starts in the PowerShell session when PowerShell first starts, and causes it to not see everything that goes on later. If PSReadLine is imported after PowerShell session has been established, it seems that the completions are always up to date.

@jhoneill, can you confirm, that if PSReadLine is removed, and then reimported, and then you perform the reproduction steps, the issue does not occur at all?

msftrncs commented 4 years ago

Or possibly, this apply applies to alias's that are part of the default list of aliases, and only if the alias was in existence at the time PSReadLine was imported.

remove-module PSReadLine
import-module PSReadLine

del alias:cd
# completion will still find CD's parameters

remove-module PSReadLine
import-module PSReadLine

new-alias cd set-location

# now it finds the parameters

del alias:cd

# now it doesn't.

in a new session:

remove-module psreadline
del alias:cd
new-alias cd set-location
import-module psreadline
del alias:cd
# does not find CD's parameters
new-alias cd set-location
# finds them now
jhoneill commented 4 years ago

@msftrncs Yes, I get the same results. -Option Allscope seems to be what makes things happen differently; I asked myself how your second example could works if it is recreating the alias...

remove-module psreadline
del alias:cd
new-alias cd set-location
import-module psreadline

Removing the alias stops tab expanding parameters - but the initial alias had option allscope and one created here doesn't, so it is not a recreation. . So ... let's try with allscope.

remove-module psreadline
del alias:cd
new-alias cd set-location -Option allscope
import-module psreadline

cd now gives the parameters for Get-content del alias:cd cd still gives the parameters for Get-content

I think a more accurate problem statement is aliases with the option "AllScope" set when PSReadline loads cause PSReadline to remember their tab behavior, even if the alias is reassigned later

msftrncs commented 4 years ago

Ahh … that might make more sense, Its not that PSReadLine is remembering CD's parameters, its that the scope that PSReadLine requests the completions under still has the CD alias, because you cannot remove the alias from all active scopes.

lzybkr commented 4 years ago

AllScope does mean aliases will be copied to the module scope of PSReadLIne and PSReadLine's implementation of PSConsoleHostReadLine is defined in module scope.

There are a couple of fixes. Aliases could probably be removed when the module is loaded by adding something like:

Get-Alias -Scope 0 | ForEach-Object { Remove-Item -Path alias:$_ -Force }

to the top of PSReadLine.psm1.

Another option would be to define PSConsoleHostReadLine in global scope instead of module scope. This might be better for performance, but PSReadLine would need an OnRemove handler to remove the global function - something like:

$MyInvocation.MyCommand.ScriptBlock.Module.OnRemove = {
    Remove-Item function:PSConsoleHostReadLine
}
jhoneill commented 4 years ago

@lzybkr I think you have found a a gap in my understanding of scopes ... I thought everything sees one common, shared global scope, and a change anywhere applied everywhere. Or is this PSReadline doing its own caching ? This also only seems to be for aliases, because if I load a proxy function which adds and removes parameters for select-string, the added ones show and the removed ones have gone. I think it's OK to say if you redefine the build-in aliases you should reload PSreadline . It's simple to do and not many people need to do it. I think that's better than removing (and restoring) aliases at load time.

lzybkr commented 4 years ago

Scopes are complicated. You can start with the official docs.

The issue is AllScope - that attribute copies items from a parent scope into any newly created child scope.

PSReadLine gets it's own scope and doesn't need those aliases, but they are visible to completion because PSReadLine module scope is active when invoking completion.

Removing the aliases would only happen in PSReadLine's module scope. The global aliases would still be visible.

Note that for complete correctness, PSReadLine should remove any AllScope item, not just aliases.

Alternatively, completion should default to global scope.

At any rate, it's probably not a big deal either way - doing nothing or applying one of my suggested fixes, because as you hint at - not many people would notice.

jhoneill commented 4 years ago

@lzybkr

The issue is AllScope - that attribute copies items from a parent scope into any newly created child scope.

Thank you, that was the thing that was confusing the hell out of me. So it gets copied into the module scope, and any changes after that are ignored. Which is what the help describes , but I think when I've tried playing with allscope (long ago, when I found I couldn't make CD Push-Location instead Set-Location c. V2 or V3) I was not creating child scopes but doing everything in the global scope. I need to go back and play some more , my understanding of scopes turns out to have been fine except for the blind-spot on allscope.

I wasted several hours because I was absolutely convinced my argument completer code was at fault. (I now have a working completer which turns ... into .... and .... into ...... and so on). To replace the completer which Set-Location assigns, I need a function , and it uses Pop-Location instead set. Only after seeing it work in VS Code integrated shell and not the a powershell terminal in VS code did I think of unloading psreadline... Documenting ones way out of this is of limited help, because I didn't look anywhere outside my own code for the problem, and one wouldn't look to blame psreadline. It's such an edge case that I'm not sure it needs to be fixed.
I'm going to leave this issue for the team to close, if there is easy fix, that's great. If you decide that there isn't a fix worth implementing, I won't be upset :-) Thanks again J

daxian-dbw commented 4 years ago

Based on the discussion here, I will close this issue as Won't Fix.