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

Support for top-level await #9

Open
legraphista opened this issue Oct 10, 2019 · 2 comments
Open

Support for top-level await #9

legraphista opened this issue Oct 10, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@legraphista
Copy link
Contributor

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 😀

Here's the diff: master...legraphista:master

@3Nigma
Copy link
Owner

3Nigma commented Oct 13, 2019

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 [Feature] - kernel.print should print objects more friendlier #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 3Nigma added the enhancement New feature or request label Oct 13, 2019
@legraphista
Copy link
Contributor Author

legraphista commented Oct 13, 2019

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. fix: kernel.print no longer abbreviates objects #10

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

No branches or pull requests

2 participants