5CYTH3 / karm_parser

A parser / maybe compiler that I am proud of
1 stars 2 forks source link

Add a basic REPL #22

Closed RHL120 closed 7 months ago

RHL120 commented 8 months ago

Basic description

This PR adds a small REPL similar to the one found in python. It is uses the termion library. It is based on a simplified version of the Elm architecture (it is basically the same method, except we do not send any events from the update function). This makes the REPL flexible across TUI libraries, (the logic is strictly in the update function, if we were to change the TUI library, we would only have to change the show and the run functions).

Some problems

5CYTH3 commented 8 months ago

Will make an in-depth review ASAP. Probably during the week. Great job from what I've seen.

RHL120 commented 8 months ago

Will make an in-depth review ASAP. Probably during the week. Great job from what I've seen.

Hello there, the last commit generates the AST for the inserted command. I tried to test it out using the input found at src/parser.rs:338, but I got the following result: SyntaxError -> Expected: [;], Got: Some(IDENT) at col 3, line 1. I tried running the test, and I got the same output. Can you confirm whether or not this is a bug in the parser? If it is, I can take a look at it.

5CYTH3 commented 8 months ago

Will make an in-depth review ASAP. Probably during the week. Great job from what I've seen.

Hello there, the last commit generates the AST for the inserted command. I tried to test it out using the input found at src/parser.rs:338, but I got the following result: SyntaxError -> Expected: [;], Got: Some(IDENT) at col 3, line 1. I tried running the test, and I got the same output. Can you confirm whether or not this is a bug in the parser? If it is, I can take a look at it.

I believe this is after I made changes on the parser, I need to look that up. Don't worry, I will try to fix that asap. You can try to run basic interpretation unless the bug comes from the parsing of functions.

RHL120 commented 8 months ago

Will make an in-depth review ASAP. Probably during the week. Great job from what I've seen.

Hello there, the last commit generates the AST for the inserted command. I tried to test it out using the input found at src/parser.rs:338, but I got the following result: SyntaxError -> Expected: [;], Got: Some(IDENT) at col 3, line 1. I tried running the test, and I got the same output. Can you confirm whether or not this is a bug in the parser? If it is, I can take a look at it.

I believe this is after I made changes on the parser, I need to look that up. Don't worry, I will try to fix that asap. You can try to run basic interpretation unless the bug comes from the parsing of functions.

Cool, I will look into it and add a CI

UPDATE

Hmm, it would seem to me like you did not just update the parser, you updated the entire syntax. For example, you used the keyword "fn" in your test, however "fn", as far as I can tell, appears no where in the lexer's code (it looks like you have replaced it with "lam"). I tested out the string lam x :: hello -> x and it gave me a correct AST. After taking a quick look at the parser, I was not able to find a way to evaluate a function.

5CYTH3 commented 8 months ago

Okay, I was worried that this was not the only change that broke the test. ATM, there's nothing to properly evaluate a function, because I didn't plan on making a JIT Compiler/Interpreter for this project. You can just check if the AST produced is what you expected to be parsed haha, I misused the "interpretation" term

RHL120 commented 8 months ago

Okay, I was worried that this was not the only change that broke the test. ATM, there's nothing to properly evaluate a function, because I didn't plan on making a JIT Compiler/Interpreter for this project. You can just check if the AST produced is what you expected to be parsed haha, I misused the "interpretation" term

How were you planning to implement the _shell function, then? Anyway, I hope this will help you play around with the parser, type checker, IR, etc. I do not think there is any features to add, so if I will leave it as is. I will be wating for your reviews! Have a great night!

5CYTH3 commented 8 months ago

I think the _shell function was only kind of a placeholder, but this will be implemented later on, I didn't plan it until recently tbh haha. Tysm for what've been done, I get back to you about it asap :)

5CYTH3 commented 8 months ago

Okay so, thanks for your contribution @RHL120, this PR is very useful and saves me a lot of time. I'm only worried about the fact that Termion cannot run on windows... I will give myself a day or two just to think about what I want to do, but I think your solution is very efficient and elegant so there is a high chance that I approve this PR in the next few days.

RHL120 commented 8 months ago

or your contribution @RHL120, this PR is very use

Hello, if you give me enough time I can completely decouple the main code from termion. This will allow us to write windows specific code.

5CYTH3 commented 7 months ago

After quite some time, Merge of the PR :)