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

feat: add last evaluate result at _ variable in relp #135

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yazaldefilimone
Copy link

I don’t know, but I think this is not a good approach for that. I see that it is not possible to access this property using this._ or globalThis._. I don’t know how to set it global ctx with changes only in the REPL file.

runner/repl.js Outdated

function setLastEvalExp(prev, ret) {
const lastEvalExp = `let _ = ${ret != null ? ret : "undefined"};\n`;
if (/let _ = .*;\n/.test(prev)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this break after subsequent runs? If we're going with this approach it should be var not let.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t understand what you are saying.

are you referring to redeclaring the let statement? In this case, we only declare it once for each sequence (just change a assign expression)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you just do _ = instead of having to deal with let/var and check every time/etc?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, makes sense i’ve made the change...

runner/repl.js Outdated Show resolved Hide resolved
runner/repl.js Outdated

function setLastEvalExp(prev, ret) {
const lastEvalExp = `let _ = ${ret != null ? ret : "undefined"};\n`;
if (/let _ = .*;\n/.test(prev)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you just do _ = instead of having to deal with let/var and check every time/etc?

@BobVarioa
Copy link
Contributor

I personally don't really like this approach, I feel like this should be done with some AST manipulation in compiler.js that just checks if we're running in a REPL context. Something that essentially checks if the last statement of main is a ExpressionStatement (or something else which can be put in an Expression), and then replacing that last statement with an assignment to _.
If we were to stay with this approach, there needs to be a lot more serialization logic, JSON.stringify is very lossy.

@yazaldefilimone
Copy link
Author

I personally don't really like this approach, I feel like this should be done with some AST manipulation in compiler.js that just checks if we're running in a REPL context. Something that essentially checks if the last statement of main is a ExpressionStatement (or something else which can be put in an Expression), and then replacing that last statement with an assignment to _. If we were to stay with this approach, there needs to be a lot more serialization logic, JSON.stringify is very lossy.

ummmmm

In this case:

const age = 19

the last exp in ast became:

_ = const age = 19

@yazaldefilimone
Copy link
Author

yazaldefilimone commented Jul 31, 2024

I personally don't really like this approach, I feel like this should be done with some AST manipulation in compiler.js that just checks if we're running in a REPL context. Something that essentially checks if the last statement of main is a ExpressionStatement (or something else which can be put in an Expression), and then replacing that last statement with an assignment to _. If we were to stay with this approach, there needs to be a lot more serialization logic, JSON.stringify is very lossy.

I think if you want manipulate ast directly you nneed suport vistor pattern... maybe bacoume ast to do that...

I personally don't really like this approach, I feel like this should be done with some AST manipulation in compiler.js that just checks if we're running in a REPL context. Something that essentially checks if the last statement of main is a ExpressionStatement (or something else which can be put in an Expression), and then replacing that last statement with an assignment to _. If we were to stay with this approach, there needs to be a lot more serialization logic, JSON.stringify is very lossy.

ummmmm

In this case:

const age = 19

the last exp in ast became:

_ = const age = 19

I think if you want to manipulate the ast directly, wee need to support the visitor pattern in ast or another algorithm. This ensures proper handling of cases like the last exp is const declarations or other...

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

Successfully merging this pull request may close these issues.

3 participants