Resolve worksheet ArgumentCompleter Fixes:#1590

[pbossman]

This will update the argument completer for Import-Excel

This change will use Get-ExcelSheetInfo instead of Open-ExcelPackage to get the worksheet names

[dfinke]

hI @jhoneill, wanted to get your thoughts on this. - Thank you

[jhoneill]

Not sure what's happened with the readonly parameter on open package which is the root cause if the issue.

This looks like it will work and Get-ExcelSheetInfo has a little less overhead. So LGTM - without testing it for myself.

[dfinke]

@jhoneill I saw the ReadOnly missing and know it was there. - Investigating

@pbossman Do you have the script that shows the issue? I was answering a question within the last week and the completer "seemed" to be working. With ReadOnly missing, it should throw and I want to see if we are dropping the error on the floor.

[pbossman]

@dfinke:

To start: I discovered the issue by trying to tab-complete the worksheet names of a known file, I even tried a sample.xlsx file
sample.xlsx

Import-Excel C:\sample.xlsx -WorksheetName

which then shows the current folder items, not the sheet names

Per your feedback, I added some code to catch the error explicitly and Write-Host.
Adding an explicit try\catch seems to show that there is actually an error,
but the error is not being caught by anything.

    $xlPath = $fakeBoundParameter['Path']
    if (Test-Path -Path $xlPath) {
        Try {
            $xlpkg = Open-ExcelPackage -ReadOnly -Path $xlPath
        } catch {
            Write-Host "Caught Error" -ForegroundColor Yellow
        }
        $WorksheetNames = $xlPkg.Workbook.Worksheets.Name
        Close-ExcelPackage -nosave -ExcelPackage $xlpkg

My guess is that this non-terminating behavior is internal to PowerShell ArgumentCompleter logic. A terminating error would create some interesting user behavior.

You can reproduce the weird behavior be adding a completer into your current scope.
I was able to reproduce the weird user behavior directly by running Register-ArgumentCompleter external to the module.

Register-ArgumentCompleter -CommandName Import-Excel -ParameterName WorksheetName -ScriptBlock {
    param($commandName, $parameterName, $wordToComplete, $commandAst, $fakeBoundParameter)

    $xlPath = $fakeBoundParameter['Path']
    Write-Host "Trying" -ForegroundColor Cyan
    if (Test-Path -Path $xlPath) {
        Try {
            $xlpkg = Open-ExcelPackage -ReadOnly -Path $xlPath
        } catch {
            Write-Host "Caught Error" -ForegroundColor Cyan
        }
        $WorksheetNames = $xlPkg.Workbook.Worksheets.Name
        Close-ExcelPackage -nosave -ExcelPackage $xlpkg
        $WorksheetNames.where( { $_ -like "*$wordToComplete*" }) | foreach-object {
            New-Object -TypeName System.Management.Automation.CompletionResult -ArgumentList "'$_'",
            $_ , ([System.Management.Automation.CompletionResultType]::ParameterValue) , $_
        }
    }

}

In summary:
As you mentioned, it looks like the error is just being dropped on the floor and not caught by default.
Why this is happening?
Or how to avoid it?

I defer to brighter minds

As I've shown, the error can be explicitly caught\thrown, but to what end?

[dfinke]

Thanks @pbossman for checking that out. Fixing the feature would be great.

More troubling is that Open-ExcelPackage no longer has -ReadOnly that both James and I remember. :-)

Did a git log -S'ReadOnly'. I did not see anything but git is not something I spend time with learning.

I look at your PR and take if for a spin

[dfinke]

I guess one anecdotal test should be enough :-D

Funny, the Get-ExcelSheetInfo is a really early implementation uses the FileStream with Read.

@jhoneill Do any of your writeups on completers etc have an example of unit testing something like this?

Guess it may be simple. The function body only uses the $fakeBoundParameter for the $Path.

function WorksheetArgumentCompleter {
    param($commandName, $parameterName, $wordToComplete, $commandAst, $fakeBoundParameter)
}

[dfinke]

Thanks Phil for the drill down and the fix. I've got a couple PRs still in review I want to include, so I'll publish this soon.