n-riesco / ijavascript

IJavascript is a javascript kernel for the Jupyter notebook
Other
2.19k stars 185 forks source link

Implement responding to `is_complete_request` message, to handle terminal usage #97

Closed shreevatsa closed 7 years ago

shreevatsa commented 7 years ago

After installing IJavascript and trying it from the jupyter console, I see the following behaviour: for every command (after the first one), I have to hit Enter three times (instead of once), for the input to be accepted:

~/jupyter% jupyter console --kernel javascript
KERNEL: SHELL_SOCKET: Unhandled message type: history_request
Jupyter console 5.1.0

IJavascript v5.0.17
https://github.com/n-riesco/ijavascript

In [1]: x = 42KERNEL: SHELL_SOCKET: Unhandled message type: is_complete_request
/Users/srajagopalan/jupyter/lib/python2.7/site-packages/jupyter_console/ptshell.py:599: UserWarning: The kernel did not respond to an is_complete_request. Setting `use_kernel_is_complete` to False.
  warn('The kernel did not respond to an is_complete_request. '
In [1]: x = 42
Out[1]: 42

In [2]: x = 7
      : 
      : 
Out[2]: 7

In [3]:                                                                                                                                                                                                                                       
Do you really want to exit ([y]/n)? 
Shutting down kernel

It appears that jupyter sends an is_complete message to determine whether the input (like x = 7 above) is complete (because it may be incomplete, like f = function() {). And this kernel (ijavascript) does not support responding to the is_complete message.

shreevatsa commented 7 years ago

Note: I have also filed a bug against jupyter_console, to see if there's a possibility of improving their handling of cases where the kernel does not support the is_complete message. But the best solution would be for this kernel to implement it.

rgbkrk commented 7 years ago

Note for implementation - with the vm API, we could use vm.createScript API to check if the code segment is complete:

> vm = require('vm')
> vm.createScript('h')
ContextifyScript {}
> vm.createScript('h = ')
evalmachine.<anonymous>:1
h =
  ^
SyntaxError: Unexpected end of input

Which I suppose the code would look roughly like:

const code = isCompleteMessage.content.code;

try {
  vm.createScript(code);
  respond('complete');
} catch(e) {
  respond('incomplete'); // or invalid or unknown, depending on error
}
rgbkrk commented 7 years ago

Alternatively, could also use new vm.Script(code), which has the same thrown errors yet actually occurs in the documentation. 😉

n-riesco commented 7 years ago

@rgbkrk I experimented with vm when I started IJavascript (see the repository history). I came across enough issues that I decided to develop the nel package.

I'd like to implement is_complete_request using a parser, because I could use such a solution with other kernels (e.g. jp-coffeescript and jp-babel).

@shreevatsa IJavascript doesn't implement is_complete_request at the moment (that's the error message you're seeing). There are two PRs attempting to implement is_complete_request here and here. I would merge https://github.com/n-riesco/ijavascript/pull/81 if the author would address the feedback in my review. I won't merge #81 because it breaks IJavascript's and jp-kernel's design.

There are a number of issues in IJavascript that I'm in the process of addressing and implementing is_complete_request is one of them. The way I had planned to address it involves a new API for jp-kernel, see here.

As a quick fix (and until I implement the new API) I'm going to stop IJavascript from printing the error message KERNEL: SHELL_SOCKET: Unhandled message type: is_complete_request. I haven't tested it, but I think this should fix the problem you're seeing in the console.

rgbkrk commented 7 years ago

What issues did you run into in vm that we could address in nodejs directly?

/cc @captainsafia

n-riesco commented 7 years ago

@rgbkrk I've tried your suggestion and I didn't find any problems. Thanks!

@shreevatsa I've just published IJavascript v5.0.18 (it should fix this issue). Thank you for testing IJavascript in the console!

shreevatsa commented 7 years ago

I can confirm that it works very well, thank you!

(Would be nice to have it for jp-babel as well, when you get the time. But I believe you want to do some refactoring first before touching that code, or something like that...)

n-riesco commented 7 years ago

@shreevatsa I've just released jp-babel@1.0.0 that brings it in sync with IJavascript and implements is_complete_request. Enjoy!