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

How to chain more than one call? #6

Closed
nschoe opened this issue Oct 17, 2016 · 19 comments
Closed

How to chain more than one call? #6

nschoe opened this issue Oct 17, 2016 · 19 comments

Comments

@nschoe
Copy link

nschoe commented Oct 17, 2016

Hi,
I've seen #5 but this one is slightly different (at least I think so). I find the library very nice, but I'm failing at making a simple menu with 2 screens (the 2nd screen depending on the output of the 1st), which seems like a basic use of enquirer.

So have I missed something here?
Here is my use case: I want to first ask the user for the name of an argument, and then make him chose between a predefined set of arguments (the use case for this is templating, but that doesn't matter)

Here is my code:

'use strict';

const Enquirer = require('enquirer')
const inspect = require ('util').inspect

for (let i = 3; i > 0; i--) {
    let enquirer = new Enquirer ()

    enquirer.register ('radio', require ('prompt-radio'))

    enquirer.question ('argName', 'Name for the input argument (' + i + ')?')
    enquirer.question ('argType', 'Type of argument (' + i + ')?', {
        type: 'radio',
        choices: ['STRING', 'INT32', 'BOOL']
    })

    enquirer.ask()
    .then (answers => {
        console.log ('Argument ' + i)
        console.log (inspect (answers))
        console.log ('--')
    })
}

And when I run it, I've got:

? Name for the input argument (3)? Foo
? Name for the input argument (2)? Foo
? Name for the input argument (1)? Foo
? Type of argument (3)? INT32
? Type of argument (2)? INT32
? Type of argument (1)? INT32
Argument 3
{ argName: 'Foo', argType: [ 'INT32' ] }
--
Argument 2
{ argName: 'Foo', argType: [ 'INT32' ] }
--
Argument 1
{ argName: 'Foo', argType: [ 'INT32' ] }
--

I've got the first Name input for the argument (1)?, then I answered Foo, then I was presented the radio choice, I selected INT32 with <Space> and when I pressed entered, it dumped all of this.

I did not have a second (and neither a third) choice. How can I achieve this?

@nschoe
Copy link
Author

nschoe commented Oct 17, 2016

Just for information, then I tried to change my function so that it worked with a recursive function, note that unlike #5, I use a let enquirer = new Enquirer () inside the recursive function, so that .ask() is never called twice on the same enquirer, but a new one gets created at each function call.

'use strict';

const Enquirer = require('enquirer')
const inspect = require ('util').inspect

function query (i) {
    let enquirer = new Enquirer ()

    enquirer.register ('radio', require ('prompt-radio'))

    enquirer.question ('argName', 'Name for the input argument (' + i + ')?')
    enquirer.question ('argType', 'Type of argument (' + i + ')?', {
        type: 'radio',
        choices: ['STRING', 'INT32', 'BOOL']
    })

    enquirer.ask()
    .then (answers => {
        console.log ('Argument ' + i)
        console.log (inspect (answers))
        console.log ('--')

        if (i > 0) {
            console.log ('Asking for argument ' + i + ' now...')
            query (i-1)
        }
    })
}

query (3)

This time, I've got this on my console:

? Name for the input argument (3)? Foo
? Type of argument (3)? INT32
Argument 3
{ argName: 'Foo', argType: [ 'INT32' ] }
--
Asking for argument 2 now...
^C% 

So, as before, the first set of questions is ok: I'm asked for the name, then the type, but when I validate, this time it hangs, like in #5, but this is weird because it seems to be on a completely new Enquirer instance...

Any chance?

@jonschlinkert
Copy link
Member

with a recursive function

Thanks for the detailed explanation. We had this issue a couple of weeks ago and I'm struggling to remember how we resolved it. e.g. I thought this was fixed, but apparently it isn't.

That said, I have a general idea of what's causing it but I'm not sure if I know enough to explain it yet. Let me look into it and I'll come back with updates as soon as I learn what's happening.

thank you for creating the issue!

any thoughts @doowb?

@nschoe
Copy link
Author

nschoe commented Oct 17, 2016

Hum thanks.
But this is somehow weird to me: given the library's nature (making prompts to ask questions interactively to the user), it seems like a basic use case to chain several questions.

How come you did not run into this problem sooner?
Anyway I'm not criticizing: this is an awesome work. But I'm just wondering if I'm missing something :-)

@doowb
Copy link
Member

doowb commented Oct 17, 2016

Just off the top of my head... enquirer.ask() returns a promise, which is async... meaning that it doesn't resolve until the user responses to the prompt. So you're update to use the query function and call it in the .then method makes sense. (The first example was trying to ask all 3 questions at the same time).

As to why it's not showing the other prompts, I'll have to look into this a little closer because we have examples that are asking multiple questions that work fine.

Just to be sure, do you have all of the latest code? I usually delete my node_modules folder, and run npm cache clear && npm install to make sure npm hasn't cached something that might have been fixed in a dependency.

I'll dig into it a little more in a bit.

@jonschlinkert
Copy link
Member

jonschlinkert commented Oct 17, 2016

How come you did not run into this problem sooner?

The reason actually makes sense (if it's what I think it is). After publishing enquirer I decided to refactor all of the individual prompts to make them completely self-encapsulated. Since each prompt needed to be able to control when and how the readline interface is creates, I had to refactor the logic around that to ensure that prompts and enquirer could both instantiate and terminate the interface when needed. I'm guessing this is where the issue is

edit: and thank you for the kind words :)

@nschoe
Copy link
Author

nschoe commented Oct 17, 2016

Hi @doowb , thanks for answering.
As for the npm cache: I have just discovered and tried enqurier, so the install dates back a few minutes :-)

I perfectly understand why the first example doesn't work (indeed tries to ask the three questions at once), but the second code snippet should be working.

By the way, yes there are situations where several questions can be asked, but it's when you call enquierer.question() several times in a row, then you make one enquirer.ask(). Starting here, it seems to chain the questions just fine. So... why not in my example? :-)

Also from the top of my head, isn't it related to this where you somehow disable the finish() function?
I've just looked at the code, so pardon me if it's not that at all.

@jonschlinkert
Copy link
Member

jonschlinkert commented Oct 17, 2016

isn't it related to this where you somehow disable the finish() function?

It's called here, but I'm thinking the bug is in prompt-base and is related to how the events are handled.

We had to make changes to allow prompts themselves to terminate a readline session. This was a little trickier than I thought it would be, because it's not at all implicit when a user plans to chain prompts - when the interface should just be terminated after the first prompt. To clarify, this is a lot easier to do when enquirer is used for flow control, I specifically mean when a single prompt is chained. The point is that even though you're using enquirer directly, I might have broken the flow control here when I made the changes to the prompts.

edit: and thanks for working through this with us

@nschoe
Copy link
Author

nschoe commented Oct 17, 2016

Okay, I'll take a look at prompt-base, but I'm not sure I'll be able to find anything faster than you guys.
Thanks fro the reactivity.

@jonschlinkert
Copy link
Member

hey no worries, we're not expecting you to do any debugging or looking into it at all. Just your feedback on whether something is working for you etc. is great

@nschoe
Copy link
Author

nschoe commented Oct 17, 2016

Yeah don't worry, but if I can help...

By the way, I've just isolated prompt-base and run this simple snippet:

'use strict';

const Prompt = require ('prompt-base')
const inspect = require ('util').inspect

let prompt = new Prompt({
    name: 'name',
    message: 'Argument name?'
})

prompt.run()
.then (answer => {
    console.log ('answers: ' + inspect (answer))
    let prompt2 = new Prompt({
        name: 'type',
        message: 'Argument type?'
    })

    return prompt2.run()
})
.then( answer2 => {
    console.log ('Answer2: ' + inspect (answer2))
})
.catch( err => {
    console.error ('Err: ' + err)
})

And this works okay for me, here's the output of the console:

? Argument name? Foo
answers: 'Foo'
? Argument type? String
Answer2: 'String'

So it does ask for the first argument's name then the argument's type.
At least, just with prompt-base, I can ask several questions.


EDIT: I've just confirmed that I have no problems asking severals questions with prompt-base, I've build a slightly more advanced version:

'use strict';

const Prompt = require ('prompt-base')
const inspect = require ('util').inspect

let names = []

function query(n) {
    let prompt = new Prompt({
        name: 'name',
        message: 'Argument #' + n + '\'s name?'
    })

    return prompt.run()
    .then (answer => {
        if (answer !== '') {
            console.log (`'${answer}' registered for argument #${n}`)
            names.push (answer)
            return query(++n)
        }
    })
}

query (1)
.then( () => {
    console.log ('Questions finished, you\'ve supplied ' + names.length + ' arguments:\n' + inspect (names))
})
.catch( err => {
    console.error ('Err: ' + err)
})

And the console session:

? Argument #1's name? foo
'foo' registered for argument #1
? Argument #2's name? bar
'bar' registered for argument #2
? Argument #3's name? quux
'quux' registered for argument #3
? Argument #4's name? 
Questions finished, you've supplied 3 arguments:
[ 'foo', 'bar', 'quux' ]

So it really seems to work.

That being said, I'm sorry I have to leave. I hope we'll find the solution soon! A important tool of mine depends directly on this!
Cheers

@doowb
Copy link
Member

doowb commented Oct 17, 2016

As @jonschlinkert pointed out, the finish method is being called too soon and closing the interface to the readline prompt.

I think we had initially done this because using prompts multiple times was adding too many event listeners. This caused "earlier" prompts to still be listening for events on the UI event though they should be finished.

We added a .only method to help manage these listeners at the prompt level (during the refactor to allow prompts to be run independently) but haven't updated all of the prompts yet.

I think the change in enquirer is to remove the call to the .finish method to allow the readline interface to stay open until the process has ended (or an implementor calls the .finish method manually). Since the .finish method is already registered for the SIGINT event, this should be fine.

Then in prompt-checkbox (inherited by prompt-radio) we just need to use the .only method to ensure the listeners are working as expected.

I've tried these out locally and will push up changes in a little bit if there are no objections.

@nschoe thanks for the examples

@jonschlinkert
Copy link
Member

or an implementor calls the .finish method manually

It doesn't hurt for users to be able to do this, but this shouldn't "need" to be done in enquirer. It should only be necessary when individual prompts are used without enquirer AND the prompt is chained or nested.

In enquirer, the .prompt() method is used for running a single prompt, and .ask() (which calls the .prompt() method) is used to control flow for one or more prompts.

@doowb
Copy link
Member

doowb commented Oct 17, 2016

In enquirer, the .prompt() method is used for running a single prompt

Good point... the original example will run when using the .prompt method on enquirer because the .finish method is never called. @nschoe this would work like your prompt-base example above.

I think .finished still needs to be removed from .ask so the method can be used multiple times if necessary.

@doowb
Copy link
Member

doowb commented Oct 17, 2016

@nschoe please pull down the latest and try your example with the query function. This should be working now.

@nschoe
Copy link
Author

nschoe commented Oct 18, 2016

Hi @doowb, @jonschlinkert ,

thanks for the amazing work again, I'm happy that you seemed to have worked it out. I'm pulling the latest and trying again right now, I'll come back to you.

In the meantime, reading your message, what I understand is that we have two functions .prompt() and .ask(), one of them should be called when using the prompts in standalone mode (it calls .finish() afterward) and the other is to be used when we intend to chain to calls and manually call .finish() at the end, is that right?

@nschoe
Copy link
Author

nschoe commented Oct 18, 2016

Okay sorry for the delay I was dragged urgently by someone.
So I have pulled the latest and tried by example with the recursive function, it seems to work, here is the console session:

? Name for the input argument (2)? foo
? Type of argument (2)? BOOL
Argument 2
{ argName: 'foo', argType: [ 'BOOL' ] }
--
Asking for argument 1 now...
? Type of argument (2)? BOOL, BOOL
? Name for the input argument (1)? bar
? Type of argument (2)? BOOL, BOOL, STRING
? Type of argument (1)? INT32
Argument 1
{ argName: 'bar', argType: [ 'INT32' ] }
--
Asking for argument 0 now...
? Type of argument (2)? BOOL, BOOL, STRING, STRING
? Type of argument (1)? INT32, INT32
? Name for the input argument (0)? quux
? Type of argument (2)? BOOL, BOOL, STRING, STRING, STRING
? Type of argument (1)? INT32, INT32, INT32
? Type of argument (0)? STRING
Argument 0
{ argName: 'quux', argType: [ 'STRING' ] }
--

It is a bit bloated on the output, but at least it works: it asks me for several chained questions. It's only at the end that the output get a bit scrambled. Do you have ideas as to why?

Anyway, thanks a lot guys for the amazing work and fast reactivity!

As far as I'm concerned, you can close, I'm just leaving this opened to have a chance to discuss the scrambled display.

@doowb
Copy link
Member

doowb commented Oct 18, 2016

There was a bug in prompt-checkbox (used by prompt-radio) that I fixed yesterday causing the extra output. Get the latest and it should be good.

@nschoe
Copy link
Author

nschoe commented Oct 18, 2016

Awesome, thanks!
Thanks again for the good work and fast answers, @doowb, @jonschlinkert !

What about the issue on enquirer/prompt-confirm#1, any idea?

@jonschlinkert
Copy link
Member

closing per #6 (comment)

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

No branches or pull requests

3 participants