-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add interactive shell to WASM demo #643
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
Conversation
palaviv
commented
Mar 9, 2019
Codecov Report
@@ Coverage Diff @@
## master #643 +/- ##
==========================================
+ Coverage 40.81% 41.02% +0.21%
==========================================
Files 76 77 +1
Lines 17345 17558 +213
Branches 4474 4516 +42
==========================================
+ Hits 7079 7204 +125
- Misses 8366 8446 +80
- Partials 1900 1908 +8
Continue to review full report at Codecov.
|
Hi @palaviv , very cool stuff! Surprisingly simple change to achieve this! |
Seems like adding the backspace and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good! I've made some suggestions but other than that it's good to go!
term.on("data", (data) => { | ||
const code = data.charCodeAt(0); | ||
if (code == 13) { // CR | ||
if (input[input.length - 1] == ':') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use .trimEnd()
here in case of spaces at the end of a line? Also, there might be a function that has multiple lines in its body, so it'd be good to keep a boolean of whether or not we're in a block and if so wait to end input until there's an empty line, a la CPython:
>>>>> def a():
..... print('1')
..... print('2')
.....
>>>>>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to do this in a different PR. We currently don't support this in the REPL as well... (#626)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!