REPL Ctrl-D termination, unclosed string handling and other refactors

brundonsmith / rust_lisp

A Rust-embeddable Lisp, with support for interop with native Rust functions

234 stars 20 forks source link

REPL Ctrl-D termination, unclosed string handling and other refactors #9

Closed alexmozaidze closed 2 years ago

alexmozaidze commented 2 years ago

After I've read your blog, I decided to check on parser.rs and found out how simple it actually is, which amazed me. I like the blogs, keep it up.

I did following things in this PR:

Made REPL close on Ctrl-D (at least on Linux)
Handled unclosed strings properly so it doesn't panic
Allowed single semicolon comments
Bumped Rust edition (2018 -> 2021)
Did some general refactoring

brundonsmith commented 2 years ago

Some thoughts:

I'm not a fan of clippy::clone_on_ref_ptr and the related changes; let's revert those
I'm not seeing where it checks for Ctrl+D; does .lines() do that automatically? If so that's pretty cool. It works, in any case, but I'd still like to know exactly why.
I think we want to return None instead of Some(&code[index..]) on line 81 in parser.rs. At least in the REPL this bubbles up as an Unrecognized expression instead of automatically closing the string, which I think is better.

Otherwise I think these changes look good

brundonsmith commented 2 years ago

Ah, one other thing- there are some |c: char|s in here that don't need the type annotation

alexmozaidze commented 2 years ago

Yeah, .lines() returns an iterator, and it's None when Ctrl-D is hit. I was quite surprised it worked so well.

alexmozaidze commented 2 years ago

I did the changes and simplified read_atom() by ommiting unclosed strings check; it is not needed because we drop unclosed string on tokenization step now.

brundonsmith commented 2 years ago

Merged and published to crates.io