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

Fixed windows-specific problem in subprocess_repl.read_bytes() #307

Open
wants to merge 10 commits into
base: merges
Choose a base branch
from

Conversation

simonzack
Copy link
Contributor

Fixed windows-specific problem in subprocess_repl.read_bytes(), so that more than 1 byte is returned on every read, by converting the pipe to an asynchronous one.

This issue comes up as a bug at some places, for example open an ipython repl prompt, then enter some non-existent variable name to generate an error. Then the color escape sequences are not stripped even when the option is set to True in SublimeREPL, as the color stripping function only has a single byte as input instead of all the output bytes.

This fixes the aforementioned problem.

…at more than 1 byte is returned on every read, by converting the pipe to an asynchronous one
@ghost ghost assigned wuub Dec 27, 2013
@wuub
Copy link
Owner

wuub commented Dec 27, 2013

@simonzack thanks :)

Are you using Sublime Text 2 or 3? Did you test both and are all repls working as before?
I'm trying to decide how much testing should I do before releasing this ;)

@simonzack
Copy link
Contributor Author

I think it should work fine after the new commits.
On sublime text 3 build 3059, the repls I've tested are: python, ipython, haskell (ghc), node.js, windows shell, and powershell.
I also tested sublime text 2's windows shell.
They are all the shells I have, hope it works for all the other ones :)

@simonzack
Copy link
Contributor Author

Any updates on this? Thanks.

@gacelita gacelita changed the base branch from master to merges January 27, 2017 20:08
@gacelita
Copy link
Collaborator

Please solve the conflicts in repl.js, since I lack the knowledge to do so, and then we'll merge this in merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants