[repl] experimental repl prototype #21396
Conversation
devsnek
added
wip
nodejs-github-bot
added
the
lib / src
devsnek
removed
the
lib / src
|
|
||
| this._paused = false; | ||
| this.transformBuffer = transformBuffer; | ||
| this.completionList = undefined; |
There was a problem hiding this comment.
Should the default of this.completionList be [] instead of undefined?
|
Would this make #20977 easier 😮 ? |
| r = result; | ||
| }); | ||
| return r; | ||
| }; |
There was a problem hiding this comment.
You probably want to cache the returned function(?)
| session.post(n, params, (err, result) => { | ||
| if (err) { | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| throw new Error(err.message); |
There was a problem hiding this comment.
It's still bothering me quite a bit that this assumes post will call the callback synchronously, which is not the case for some commands. Now that the REPL pipeline uses promises, can we instead make this function return a promise?
| if (value === -1) { | ||
| process.exit(0); | ||
| } | ||
| process._tickCallback(); |
There was a problem hiding this comment.
same... 😢
this really stumped me
| }).runInThisContext({ | ||
| displayErrors: true, | ||
| breakOnSigint: true, | ||
| }); |
There was a problem hiding this comment.
Have you considered using Inspector's Runtime.evaluate instead of vm? Top-level await would basically come for free if you do that.
There was a problem hiding this comment.
yes I have been experimenting with it, it's been a bit difficult to work with though with regard to handling inspection and stuff.
|
@benjamingr I was trying to implement that but it thought literally every single evaluation would have side effects so I left it out. |
|
Have you considered publishing that separately as an npm module and iterate on that (accepting PRs especially the Windows ones, fixing bugs,.etc) until moving this into core? This seems to be a pretty good start, but I think publishing it as an npm module would probably help getting more usage stats and iterate faster without going through our own release cylcle. We can put it under the foundation to get more traction. |
|
@joyeecheung sounds like a good idea, what would i need to do to move the repo (https://github.com/devsnek/node-repl-prototype) into the foundation? |
|
@devsnek I have an WIP step-by-step guide in nodejs/admin#68 although you can forget about the Travis part now that we can enable the github app of travis |
The overall design of the REPL code has been rethought. the pipeline is now all async to support future tooling we may want to introduce. autocomplete is revamped. there's a spot in the api to install a method that highlights the current input buffer as the user types but but i wasn't sure how to integrate a library that highlights into core. an effort to better separate the ui from the repl has been made. the repl itself is not aware of things like command history. its also purposeful that this does not expose a public api.
there are still some things missing (the most annoying of which is probably persistent history) but i think this is a pretty good start.
what i'm hoping for right now is not getting this merged but getting feedback about how this can be made better.
some stuff has been duplicated to allow for the existing repl and the experimental repl to continue developing separately.
someone suggested that i integrate microsoft lsp into the repl but i'm not quite sure how to go about that so if you're familiar with it lemme know.
/cc @nodejs/repl
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes