nelu-kernelu icon indicating copy to clipboard operation
nelu-kernelu copied to clipboard

Support for top-level await

Open legraphista opened this issue 6 years ago • 2 comments

Hi! Awesome project!

I've taken the liberty to add support for await at the outer most scope. I've also added some features here and there like

  • console.log/warn/error to use kernel.print
  • syntax error messages
  • use util.inspect to print returned nested objects

image

If this looks right to you, i can issue a PR with the changes :grinning:

Here's the diff: https://github.com/3Nigma/nelu-kernelu/compare/master...legraphista:master

legraphista avatar Oct 10 '19 15:10 legraphista

Thanks, @legraphista ! 💓

Here's my quick thoughts on your proposals/works:

  1. we chose to code Jupyter's log streaming functionality and make it available through kernel.print instead of console.log to differentiate between terminal streams and UI ones when dealing with complex user-side logic. I would like to keep them apart mostly for semantic reasons (the js console is not to be confused with the Jupyter one). Now I do understand that this forces the developer to bind to the kernel's API which could make their code harder to execute in other non nelu-kernelu environments (where kernel is not available, for instance). This could be fixed with some glue logic adapters (kernel.print to console.log). Having said this, you do raise an important point here, and I think it's worth the effort investigating other ways of doing this without forcing the user to change their logic. In this sense, we are thinking of providing a command-line flag (eg. --log-to-ui) that would allow console.logs to get dumped into the UI instead of the process terminal. I'm curious of what your thoughts might be regarding this solution.
  2. With regards to syntax error messages - I like it! We'll review the changes and carry on from there.
  3. And with what util.inspect is concerned, we have issue #3 opened so go ahead and and open a PR, if you like/can. I think @RaduMilici will be more than happy to review it 👍

Let's continue this discussion in the upcoming days, but, yet again, thank you for your interest in the project. It's nice seeing people actually putting it to some good use/effort. 🙇

3Nigma avatar Oct 13 '19 08:10 3Nigma

Hey @3Nigma!

Thanks for the reply! Allow me to quickly go through the talking points:

  1. I understand the need for a differentiation between kernel.print and console.log , but my first experience was a bit odd. I used Jupyter a lot with Python, and there you use the standard print function to output to the interface, therefore my expectation was that I could also use the standard console.log to print to the interface. One could argue that a new user that hasn't seen/used Jupyter before and sets up this kernel, would expect the same behavior. The flag --log-to-ui would be a good addition, though my suggestion would be to separate the console output of the kernel itself from the console output the user intended to see in the interface. Another small issue i see with the flag is that people don't fully read documentation, and they might skim through the part that enables the console.log interface behavior and then be confused when it doesn't work. Maybe it could be enabled by default?

  2. Thank you! I've also noticed a branch PR that uses babel, so maybe we could switch acorn out for the babel AST parser.

  3. ~I'll open a PR with just that fix as soon as I can.~ https://github.com/3Nigma/nelu-kernelu/pull/10

legraphista avatar Oct 13 '19 08:10 legraphista