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

Upgraded to latest lsp #1562

Merged
merged 14 commits into from
Aug 20, 2019
Merged

Upgraded to latest lsp #1562

merged 14 commits into from
Aug 20, 2019

Conversation

david-driscoll
Copy link
Member

@mholo65 This upgrade went pretty well things seem to working.. I dunno if might help with your other issue.

@bjorkstromm
Copy link
Member

@david-driscoll I tried the alpha bits yesterday, with no success. Actually it didn’t even initialize using the C# LSP Client (version 0.12.1). Saw another exception too bubble up regarding converting ”empty json rpc response” to a ”initialized response”.. Or something like that. Can look again with this PR instead.

@bjorkstromm
Copy link
Member

Seeing this with C# LSP Client when updating Omnisharp to use LSP libs 0.13.0-alpha*

Finding descriptor for initialized
Starting: Routing Notification initialized
Converting params for Notification initialized to OmniSharp.Extensions.LanguageServer.Protocol.Models.InitializedParams
Failed to handle request initialized - System.ArgumentException: Object of type 'OmniSharp.Extensions.JsonRpc.EmptyRequest' cannot be converted to type 'OmniSharp.Extensions.LanguageServer.Protocol.Models.InitializedParams'.
   at System.RuntimeType.TryChangeType(Object value, Binder binder, CultureInfo culture, Boolean needsSpecialCast)
   at System.Reflection.MethodBase.CheckArguments(Object[] parameters, Binder binder, BindingFlags invokeAttr, CultureInfo culture, Signature sig)
   at System.Reflection.RuntimeMethodInfo.InvokeArgumentsCheck(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.HandleNotification(IMediator mediator, IHandlerDescriptor handler, Object params, CancellationToken token)
   at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.<RouteNotification>d__5.MoveNext()

Initialized params is empty, so don't know what happened here. https://microsoft.github.io/language-server-protocol/specification#initialized

Will look into this, and also try to fix the issues seen in #1505 and #1525 as these are not fixed yet with updating LSP libs.

@bjorkstromm bjorkstromm changed the title Upgraded to latest lsp-0.13 alpha for testing Upgraded to latest lsp Jul 21, 2019
@bjorkstromm bjorkstromm marked this pull request as ready for review July 21, 2019 21:24
@bjorkstromm
Copy link
Member

Fixes #1505
Fixes #1525

@vincentwoo
Copy link

Eagerly awaiting this merge! Was also running into the didChange handler failures.

Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

LGTM

@vincentwoo
Copy link

How's this coming along? Any blockers?

@sebasmonia
Copy link

Surprised this wasn't included in the last release :(

@filipw
Copy link
Member

filipw commented Aug 20, 2019

I chatted with @mholo65 yesterday and we updated the test from #1583 from Linux since it's unstable on Azure DevOps with the LSP changes in. We can investigate that separately next and it shouldn't hold up this PR, the test still runs on Mac and Windows for now.

Should be good to go now.

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.

5 participants