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

Find a way to bring back AstVisitor2 usage #276

Closed
daviwil opened this issue Aug 23, 2016 · 5 comments
Closed

Find a way to bring back AstVisitor2 usage #276

daviwil opened this issue Aug 23, 2016 · 5 comments
Labels
Area-IntelliSense Issue-Enhancement A feature request (enhancement).

Comments

@daviwil
Copy link
Contributor

daviwil commented Aug 23, 2016

After the recent design of the PSES host to be loaded as a module inside of powershell.exe, it appears that one of our multi-version PowerShell support tricks don't work any longer. When a user running PSv3 or PSv4 loads up Microsoft.PowerShell.EditorServices.dll it crashes due to the use of AstVisitor2 in the FindSymbolsVisitor2 class. For now I'm just going to comment out this class so that I can get a fix out for those impacted by the 0.7.0 release.

In the near future we may have to split out features that only work in specific PS versions into separate DLLs that are loaded and registered dynamically depending on the PS version present.

@daviwil daviwil added the Issue-Enhancement A feature request (enhancement). label Aug 23, 2016
@daviwil daviwil added this to the Backlog milestone Aug 23, 2016
daviwil added a commit that referenced this issue Aug 23, 2016
This change resolves some compatibility issues with PowerShell v3 and v4.
The new PowerShellEditorServices.psm1 file was using the ::new()
constructor syntax that is only available in PowerShell v5 and higher.
Also, the AstVisitor2 class was causing issues with Add-Type when being
loaded into the older versions of PowerShell.

Issue #276 tracks the need for bringing AstVisitor2 usage back after a
more appropriate multi-version support strategy is designed.

Related to PowerShell/vscode-powershell#248
daviwil added a commit that referenced this issue Aug 23, 2016
This change resolves some compatibility issues with PowerShell v3 and v4.
The new PowerShellEditorServices.psm1 file was using the ::new()
constructor syntax that is only available in PowerShell v5 and higher.
Also, the AstVisitor2 class was causing issues with Add-Type when being
loaded into the older versions of PowerShell.

Issue #276 tracks the need for bringing AstVisitor2 usage back after a
more appropriate multi-version support strategy is designed.

Related to PowerShell/vscode-powershell#248
@rkeithhill
Copy link
Contributor

I was looking at adding support for class and enum symbols but it requires AstVisitor2. I ran across your comment in the commented out FindSymbolsVisitor2 class.

    // TODO: Restore this when we figure out how to support multiple
    //       PS versions in the new PSES-as-a-module world (issue #276)

If we are already selectively loaded assemblies based on the platform e.g.:

if (!$PSVersionTable.PSEdition -or $PSVersionTable.PSEdition -eq "Desktop") {
    Add-Type -Path "$PSScriptRoot/bin/Desktop/Microsoft.PowerShell.EditorServices.dll"
    Add-Type -Path "$PSScriptRoot/bin/Desktop/Microsoft.PowerShell.EditorServices.Host.dll"
}
else {
    Add-Type -Path "$PSScriptRoot/bin/Core/Microsoft.PowerShell.EditorServices.dll"
    Add-Type -Path "$PSScriptRoot/bin/Core/Microsoft.PowerShell.EditorServices.Host.dll"
}

Why not continue to build different assemblies for v3, v4 and v5 and load those depending on the PSVersion?

@daviwil
Copy link
Contributor Author

daviwil commented Feb 21, 2017

That's what I'll end up doing. I'm going to be doing a big refactoring of the PSES codebase once I get 0.10.0 out, part of that will be restructuring the assemblies to enable some version-specific features without causing the assembly loader to throw exceptions.

@TylerLeonhardt
Copy link
Member

Looks like this one might be bundled into the whole "support for v3 and v4 or not" debate.

@fflaten
Copy link
Contributor

fflaten commented Aug 15, 2022

Fixed by PowerShell/vscode-powershell#1310, right?
Will move 4 of 6 visitors to AstVisitor2 in #1886

@fflaten
Copy link
Contributor

fflaten commented Feb 7, 2023

@andschwa Ready to close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IntelliSense Issue-Enhancement A feature request (enhancement).
Projects
None yet
Development

No branches or pull requests

6 participants