n-riesco / ijavascript

IJavascript is a javascript kernel for the Jupyter notebook
Other
2.18k stars 187 forks source link

Any way to prompt the user ? #122

Closed JocelynDelalande closed 6 years ago

JocelynDelalande commented 6 years ago

With python notebooks, one could use input() to prompt the user, it appears as a text-box where the user can type something and validate with Enter.

Any hints on how to achieve the same with ijavascript kernel ?

n-riesco commented 6 years ago

The short answer is that I haven't implemented stdin messages in IJavascript yet (stdin messages is what Jupyter uses to prompt the user).

Because of the asynchronous nature of Javascript/Node.js, the equivalent to Python's input() would either need a callback or return a Promise. Would that be OK for your needs?

JocelynDelalande commented 6 years ago

Nicolas Riesco notifications@github.com wrote:

The short answer is that I haven't implemented stdin messages in IJavascript yet (stdin messages is what Jupyter uses to prompt the user).

That is what I found out searching afterwards. Thanks for your confirmation :-). I also found that implementing this myself was a bit out of my league for now.

Because of the asynchronous nature of Javascript/Node.js, the equivalent to Python's input() would either need a callback or return a Promise. Would that be OK for your needs?

It is ok respect to the nature of NodeJS.

However, because you are speaking of my needs, What would be realy useful to me would be to have something that could be used a simpler manner, like python input().

If you implement stdin messages, do you know if something like prompt-sync¹ work out of the box ?

¹ https://www.npmjs.com/package/prompt-sync of the box ?

EDIT: sorry for badly formatted message, github seems to have disabled markdown syntax from email replies, which is a shame :-(

n-riesco commented 6 years ago

It is ok respect to the nature of NodeJS.

However, because you are speaking of my needs, What would be realy useful to me would be to have something that could be used a simpler manner, like python input().

Until Node.js provides a way to read stdin synchronously, I don't see how this could be achieved in a simple way or without a busy loop.

Once I get https://github.com/n-riesco/nel/pull/8 merged, I could implement a $$.input(prompt), that could be used like this:

$$.input("Please, type your name and press enter: ")
    .then((name) => {
        // do something with name
    });

If you implement stdin messages, will something like prompt-sync work out of the box ?

This package accesses the terminal directly (i.e. instead of accessing process.stdin, it accesses process.stdin.fd or /dev/tty depending on your platform). This means IJavascript would have to emulate process.stdin as a TTY or as a file descriptor. I don't think such an effort would be justified just to give access to Jupyter stdin messages.

Another issue with prompt-sync (I haven't tested it, I'm speaking just by looking at the source code) is that it achieves a synchronous input() by means of a busy loop (i.e. 100% CPU use until the user presses enter).

JocelynDelalande commented 6 years ago

Nicolas Riesco notifications@github.com wrote:

It is ok respect to the nature of NodeJS.

However, because you are speaking of my needs, What would be realy useful to me would be to have something that could be used a simpler manner, like python input().

Until Node.js provides a way to read stdin synchronously, I don't see how this could be achieved in a simple way or without a busy loop.

You are right, and my needs are quite special regarding JavaScript way to do.

Once I get https://github.com/n-riesco/nel/pull/8 merged, I could implement a $$.input(prompt), that could be used like this:

$$.input("Please, type your name and press enter: ")
    .then((name) => {
        // do something with name
    });

That would be neat :-).

I will write myself a busy loop around $$.input and add it to the startup script. That is largely suboptimal but might be ok for my needs (which are more educational than performance related).

If you implement stdin messages, will something like prompt-sync work out of the box ?

This package accesses the terminal directly (i.e. instead of accessing process.stdin, it accesses process.stdin.fd or /dev/tty depending on your platform). This means IJavascript would have to emulate process.stdin as a TTY or as a file descriptor. I don't think such an effort would be justified just to give access to Jupyter stdin messages.

Another issue with prompt-sync (I haven't tested it, I'm speaking just by looking at the source code) is that it achieves a synchronous input() by means of a busy loop (i.e. 100% CPU use until the user presses enter).

For the record this one seems cleaner https://www.npmjs.com/package/syncprompt (but still, accesses tty dev directly).

Thanks for your quick and detailed answers :-).

Any idea on of when the implementation of the stdin messages will land in ijavascript ? (I have no will to pressure kind FLOSS benevolent contributor, I just want to know if I may count on it, or if I have to find some other temporary solution meanwhile).

JocelynDelalande commented 6 years ago

@n-riesco please let me know if there is anything I could do to ease the implementation of this feature (I'm not very familiar with nodejs, but I would happilly test & provide feedback, for example).

n-riesco commented 6 years ago

@JocelynDelalande sorry about the silence. I'm already working on this feature. Here's a progress update:

I don't think I'll be able to work this week on this issue (for sure, not before the weekend)

JocelynDelalande commented 6 years ago

@n-riesco thanks for the detailed update, I'll wait for next episode :)

n-riesco commented 6 years ago

@JocelynDelalande I was hoping to finish everything today, but I won't. The input functionality is already in NEL (and tested).

There is very little left to do! I'm sorry is taking this long.

JocelynDelalande commented 6 years ago

@n-riesco I am the one being sorry repeatedly asking :x

Do you think you will have something by wednesday, or I better try to do it myself for that delay ?

n-riesco commented 6 years ago

I've just published a new version of NEL with the $$.input() functionality.

I'll try to update jp-kernel by tomorrow evening, but of course I can't promise anything.

What is left to do in jp-kernel should be easy.

JocelynDelalande commented 6 years ago

@n-riesco ok, thanks :-)

JocelynDelalande commented 6 years ago

Actually, to serve my own needs, I did a quick implementation in jp-kernel, it works :-).

I could show how I did (next weekend), but poorly experimented as I am in NodeJS, I think this is too far from your code standards, but maybe, it could gain you some time as a starting point, who knows…

n-riesco commented 6 years ago

@JocelynDelalande I'm sorry I didn't manage to do it by Wednesday (I got sidetracked by a bug in jmp and another one in jp-kernel).

I'll update jp-kernel soon (I'd like to do it today, but again no promises).

n-riesco commented 6 years ago

@JocelynDelalande Finally! I've just release https://github.com/n-riesco/jp-kernel/releases/tag/v0.1.5 . To pick up the new version, you'll have uninstall and install ijavascript again.

Thank you for chasing me until now. I'm sure it would've taken me longer otherwise!

Let me know if you come across any issues.

JocelynDelalande commented 6 years ago

I will give a look at it :-).

For the record, my own (probably ugly) implementation is here : 

https://github.com/JocelynDelalande/jp-kernel/commit/d1cd7bdec6582546cc9267a659f76a52f2f8f195

(It gave me the oportunity to dig into your code… nice code, congrats !)

I will happily move to your implementation asap.

Btw, I will PR something to make configurable that kind of code wrapping hack.

n-riesco commented 6 years ago

Btw, I will PR something to make configurable that kind of code wrapping hack.

You can already do that with the option transpile passed to the Kernel construtor. That's how jp-babel and jp-coffeescript are implemented.

JocelynDelalande commented 6 years ago

Awesome :-)

JocelynDelalande commented 6 years ago

(back on the issue !)

You can already do that with the option transpile passed to the Kernel construtor. That's how jp-babel and jp-coffeescript are implemented.

So, from what I understand, the clean way to do that would be to start a new package and to specify the transpile option. There seems to be no way to do that with config only.

Am I right or is there a way to specify a transpile function with config only ?

n-riesco commented 6 years ago

Am I right or is there a way to specify a transpile function with config only ?

Haven't tested it, but you could use --startup-script=path to run a script on startup and redefine vm.runInThisContext (this is the function NEL uses to evaluate code); like this:

var runInThisContext = vm.runInThisContext;

vm.runInThisContext = function(code) {
    runInThisContext("Fiber(function() {" + code + "}).run();");
}
n-riesco commented 6 years ago

BTW, note that wrapping the code of an execution request inside a function, i.e.: "function() {" + code + "}" will break the ability to define variables in the global context.

This is rather inconvenient (this would mean that an import or a global definition inside one execution cell wouldn't be visible inside another execution cell).

JocelynDelalande commented 6 years ago

Thanks for your answer, I will try that :-).

BTW, note that wrapping the code of an execution request inside a function, i.e.: "function() {" + code + "}" will break the ability to define variables in the global context.

Yes (not an issue in my precise case).

JocelynDelalande commented 6 years ago

Haven't tested it, but you could use --startup-script=path to run a script on startup and redefine vm.runInThisContext (this is the function NEL uses to evaluate code); like this:

I can confirm it does work :-)