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

Experiment with more 'native' binding to JsonRpc #39

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Sep 12, 2022

I've been thinking about this for a while, and issues like #35 have hammered home that this library is hiding too much of the underlying transport from users. I think we would have a lot more room to operate/fix problems without needing new versions of this library if it became more pluggable. To that end, I'm proposing a breaking set of changes that I think would reduce the size and maintenance burden of this library while still making it easy for others to build on top of it.

The primary changes are

  • moving to explicitly matching names for LSP requests and notifications (to make RPC binding easier)
  • changing return types to be Task-based for requests and EventHandler-based for notifications (again to make binding more seamless for the underlying JsonRpc library)
  • adding in Progress<T> and CancellationToken parameters for the members
  • changing the 'startup' helper methods to be more of a three-phase operation:
    • pass in some bare minimum set of data (stdin, stdout, server/client implementations) and we give you back JsonRpc instance(s) that are configured for communication
    • you can then customize the JsonRpc instance(s) however you like - this library can provide helpers here for common tasks like logging, etc
    • you can then pass the JsonRpc instance(s) into a helper here that starts them and provides an awaitable hook for you to bind

I think this will reduce a lot of conceptual friction, and make contributing new fixes to this library even easier.

@baronfel baronfel changed the title Experiment with more ' Experiment with more 'native' binding to JsonRpc Sep 12, 2022
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.

1 participant