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

Wrong type on server.start_io #496

Closed
OhioDschungel6 opened this issue Sep 13, 2024 · 3 comments · Fixed by #497
Closed

Wrong type on server.start_io #496

OhioDschungel6 opened this issue Sep 13, 2024 · 3 comments · Fixed by #497

Comments

@OhioDschungel6
Copy link

The server.start_io method has 2 optional arguments which are typed with TextIO.
Entering an TextIO object crashes the language server.

Entering a BinaryIO object makes everything work.
This seems reasonable as the default case also uses an BinaryIO object (see line 239):
transport = StdOutTransportAdapter(stdin or sys.stdin.buffer, stdout or sys.stdout.buffer)

sys.stdin.buffer has type BinaryIO

Solution:
in server.py change
def start_io(self, stdin: Optional[TextIO] = None, stdout: Optional[TextIO] = None):
to
def start_io(self, stdin: Optional[BinaryIO] = None, stdout: Optional[BinaryIO] = None):

tombh added a commit that referenced this issue Sep 14, 2024
Although this is strictly an API change, I think it's actually just a
fix because passing an actual `TextIO` object causes an error.

Fixes: #496
tombh added a commit that referenced this issue Sep 14, 2024
Although this is strictly an API change, I think it's actually just a
fix because passing an actual `TextIO` object causes an error.

Fixes: #496
@tombh
Copy link
Collaborator

tombh commented Sep 14, 2024

Like this? #497

@OhioDschungel6
Copy link
Author

Yes exactly :) Thanks!

tombh added a commit that referenced this issue Sep 17, 2024
Although this is strictly an API change, I think it's actually just a
fix because passing an actual `TextIO` object causes an error.

Fixes: #496
@tombh
Copy link
Collaborator

tombh commented Sep 17, 2024

Merged into main ✅ Please let us know if there's any more problems.

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 a pull request may close this issue.

2 participants