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

On Windows, REPL fails to start if there's a non-ASCII character in .hy-history #1535

Closed
gilch opened this issue Mar 15, 2018 · 10 comments
Closed
Labels

Comments

@gilch
Copy link
Member

gilch commented Mar 15, 2018

At least on Windows, if you paste a Δ into the repl, you can get in a state where the repl refuses to start.
It gets saved into the .hy-history file properly. But if you re-run the repl while a Δ is in the history file, it gets corrupted as something like Δ upon exit.

Seems like it could be an encoding issue in pyreadline, which completely chokes on it. I think that's what we're using to save the repl history. Maybe we've set some options improperly, or maybe pyreadline itself has a bug. Not sure.

This may be just on Windows though, since it uses its own encoding in the console. If you're running a different OS, can you confirm?

Deleting the offending characters from .hy-history allows the repl to start again.

I'm wondering if we should mangle with X's instead of Δ's even on Python3 to avoid this kind of issue. Pasting these things in the repl would be very tempting.

@gilch gilch added the bug label Mar 15, 2018
@jgkamat
Copy link

jgkamat commented Mar 15, 2018

On my computer (debian 9), I don't see this issue. The Δ is saved properly in history, even after opening the hy repl up multiple times.

I'm using hy3 though, maybe this is only an issue on hy2 (since python string encoding was improved in python3)

@vodik
Copy link
Contributor

vodik commented Mar 15, 2018

Might not be a bad idea to add appveyor CI

@Kodiologist
Copy link
Member

I'm wondering if we should mangle with X's instead of Δ's even on Python3

That was my original suggestion, and I still support it, although it presumably wouldn't fix the underlying problem here.

@Kodiologist Kodiologist changed the title REPL fails to start On Windows, REPL fails to start if there's a non-ASCII character in .hy-history Mar 15, 2018
@Kodiologist
Copy link
Member

@gilch, would you veto a PR to change the delimiter to X on Python 3?

@gilch
Copy link
Member Author

gilch commented Apr 4, 2018

@Kodiologist, no. I might even approve it. But that doesn't solve this underlying problem.

@Kodiologist
Copy link
Member

Right, that's probably something to do with pyreadline, either a bug in pyreadline or a misconfiguration on our part.

@lafrenierejm
Copy link
Contributor

It looks like the current HEAD of master requires Python 3.7+. @gilch are you able to confirm whether this issue is still present? I don't have a Windows machine on-hand, so I'm not able to test it myself.

@Kodiologist
Copy link
Member

We actually run the test suite on Windows now. I think it would be pretty easy to add a test for this, if you'd like to take a shot.

@Kodiologist
Copy link
Member

Kodiologist commented Jan 25, 2022

In particular, GitHub Actions will run on your personal fork of Hy, which I've found to be a handy way to check that stuff works on Windows before opening a PR.

@Kodiologist
Copy link
Member

I looked into this. It turns out that we no longer use a history file on Windows, anyway, since we dropped support for the unmaintained and permanently broken pyreadline. In fact, chances are decent that this was a pyreadline bug.

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

No branches or pull requests

5 participants