mattwparas / steel

An embedded scheme interpreter in Rust
Apache License 2.0
1.05k stars 49 forks source link

Steel REPL Cursor "Bug" #144

Closed partisani closed 6 months ago

partisani commented 7 months ago

Not sure what this is about, but the title says it all, see: a

mattwparas commented 7 months ago

Interesting - could you provide some details about your environment? Any help with trying to reproduce it?

partisani commented 7 months ago

Windows 11, tried with/without WSL, using Windows Terminal/cmd.exe. I built the interpreter in release mode following the instructions in the readme ¯\_(ツ)_/¯

mattwparas commented 7 months ago

Thanks, I'll try to reproduce today. I'll boot up a windows machine and see what happens

mattwparas commented 7 months ago

Would you look at that, I was able to reproduce - I'll investigate

mattwparas commented 7 months ago

Looks to be this: https://github.com/kkawakam/rustyline/issues/562 - I can get in a fix later today

mattwparas commented 7 months ago

The relevant line is here https://github.com/mattwparas/steel/blob/master/crates/steel-repl/src/repl.rs#L98

Specifically, changing the prompt to this on windows will work:

let mut prompt = "λ > ";

Might need a to_string() there, but can't test it quite this second. Feel free to make the change locally and see if it works for you

mattwparas commented 7 months ago

Just pushed a commit that will fix this for windows: https://github.com/mattwparas/steel/commit/904906bdbb72a22b06630a88a2f0baf17d04ea1f

Sorry for the trouble! I'll leave this open for a few days, let me know if its fixed for you

partisani commented 6 months ago

There was an error, i am working on a fix (just a single line, also my first contribution).

partisani commented 6 months ago

See, in line 100 you set prompt to a &str, but in line 103 you set it to a String, on linux there won't be any problem, because in line 163 it's also set to a String, but on windows prompt is a &str, so it will give a mismatched types error.

mattwparas commented 6 months ago

ah yes, so it does need the to_string()

partisani commented 6 months ago

a

Yes. I really felt the need to say that it works using itself.