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

Generalize implementation of the transport #1847

Merged
merged 7 commits into from
Sep 18, 2021
Merged

Conversation

rchl
Copy link
Member

@rchl rchl commented Sep 13, 2021

Extracted the JSON RPC-specific handling into a separate processor
and used composition concept to pass and use it within the transport.

That should allow re-using the transport implementation in the file
watcher to implement a transport that uses plain lines of text rather
than JSON RPC.

Not using abstract classes anymore as that is not compatible with
generic types.

Extracted the JSON RPC-specific handling into a separate processor
and used composition concept to pass and use it within the transport.

That should allow re-using the transport implementation in the file
watcher to implement a transport that uses plain lines of text rather
than JSON RPC.

Not using abstract classes anymore as that is not compatible with
generic types.
plugin/core/transports.py Outdated Show resolved Hide resolved
@rwols
Copy link
Member

rwols commented Sep 14, 2021

I probably want to rewrite this part of the code once we can use py38. How would that work when other packages use this code?

@rchl
Copy link
Member Author

rchl commented Sep 14, 2021

I probably want to rewrite this part of the code once we can use py38. How would that work when other packages use this code?

The LSP-file-watcher-chokidar is tightly tied to LSP so I imagine those would have to be updated at the same time.

Comment on lines +65 to +70
headers = http.client.parse_headers(reader) # type: ignore
try:
body = reader.read(int(headers.get("Content-Length")))
except TypeError:
# Expected error on process stopping. Stop the read loop.
raise StopLoopError()
Copy link
Member Author

@rchl rchl Sep 15, 2021

Choose a reason for hiding this comment

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

Changed so that explicit action is used to stop the read loop instead of relying on TypeError doing it.

We could maybe just raise StopLoopError() earlier by checking if headers.get("Content-Length") is None.

@rwols rwols merged commit 8ff07b0 into main Sep 18, 2021
@rwols rwols deleted the fix/generic-transport branch September 18, 2021 20:23
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