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

Address breaking changes in Omnisharp lib and depend on DocumentUri more #1300

Conversation

TylerLeonhardt
Copy link
Member

Two main parts

Address breaking changes

  • LanguageClient totally redesigned
  • A few namespaces changed
  • ServiceProvider disposes of all Services so PowerShellContextService needs to not be a Disposable

Depend on DocumentUri more

  • ScriptFile.DocumentUri is now an actual DocumentUri type instead of a string

@TylerLeonhardt TylerLeonhardt requested a review from rjmholt as a code owner May 27, 2020 21:08
@@ -147,7 +146,7 @@ internal static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic)
EnsureEngineSettingsCurrent();

// Create a cancellation token source that will cancel if we do or if the caller does
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment here is no longer true with the CreateLinked method removed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the comment. This is no longer needed because it's not really possible to cancel a didChange notification.

Comment on lines 55 to 58
var capabilities = typeof(ICapability).Assembly.GetExportedTypes()
.Where(z => typeof(ICapability).IsAssignableFrom(z))
.Where(z => z.IsClass && !z.IsAbstract);
foreach (var item in capabilities)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the vars here? Possible to change them to explicit types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. Also added a comment that this code is temporary.

@TylerLeonhardt
Copy link
Member Author

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 2
- Added 1
           

Complexity increasing per file
==============================
- test/PowerShellEditorServices.Test.E2E/Processes/StdioServerProcess.cs  4
- test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs  2
- test/PowerShellEditorServices.Test.E2E/Processes/ServerProcess.cs  2
         

Clones removed
==============
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs  -3
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentSymbolHandler.cs  -2
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs  -2
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/HoverHandler.cs  -1
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/FoldingRangeHandler.cs  -2
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/DefinitionHandler.cs  -2
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs  -1
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs  -1
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/ReferencesHandler.cs  -1
+ src/PowerShellEditorServices/Services/Workspace/Handlers/WorkspaceSymbolsHandler.cs  -1
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/DocumentHighlightHandler.cs  -1
         

See the complete overview on Codacy

@TylerLeonhardt TylerLeonhardt merged commit 512df9e into PowerShell:master May 27, 2020
@TylerLeonhardt TylerLeonhardt deleted the take-2-fix-lang-features-disappear branch May 27, 2020 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants