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

Improve support for later Python versions #13

Merged
merged 4 commits into from
Mar 12, 2019

Conversation

MatthewWilkes
Copy link
Contributor

Hi!

This PR adds testing for newer Python versions, as well as a workaround for a CPython bug in Python 3.6.6 and up.

It works for me locally, but there are a lot of tox environments and I have only run a subset of them. I'll fix any errors that come up, but if you have any concerns please do holler.

Cheers, love the software,

Matt

@ionelmc
Copy link
Owner

ionelmc commented Mar 4, 2019

So this seems like doubling down on a wrong way. Maybe we should stop using RW wrappers regardless of version. And since there's output pre-processing anyway I'd just pass the socket instance to the LF2CRLF_FileWrapper - seems more clean and encapsulated.

@MatthewWilkes
Copy link
Contributor Author

I agree, it does seem horrid to go further on this. That said, reliably getting lines out will also be a fair chunk of code so I don't think it will be simpler overall.

An alternative would be simply to live with the bug, or make the tests resistant to it. It will only affect users who copy/paste or otherwise automate interacting with remote-pdb, normal commands will still work. There's certainly an argument for just documenting that some versions of Python may drop lines in rare circumstances.

@MatthewWilkes MatthewWilkes force-pushed the python3-compatibility branch from 16fc7df to 393197d Compare March 5, 2019 19:09
@ionelmc
Copy link
Owner

ionelmc commented Mar 5, 2019

Well I guess I need to write an integration test to see how bad this bug is. I'll get around to it ... uh ... I have a terrible flu/cold, soon-ish.

MatthewWilkes and others added 4 commits March 10, 2019 00:40
when data is sent. This prevented multiple rapidly entered commands
from all being received, causing failing tests and the inability
to paste commands into the telnet window.
@ionelmc ionelmc force-pushed the python3-compatibility branch from 393197d to b565296 Compare March 9, 2019 22:42
@ionelmc
Copy link
Owner

ionelmc commented Mar 9, 2019

So I added a test + simpler change, lets see how it works on CI ...

@ionelmc ionelmc merged commit b565296 into ionelmc:master Mar 12, 2019
@MatthewWilkes
Copy link
Contributor Author

Sorry, I missed your last couple of messages, had a bit of a crazy time myself. Thanks so much for fixing this up, I really appreciate it!

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