dthree / vorpal

Node's framework for interactive CLIs
http://vorpal.js.org
MIT License
5.64k stars 280 forks source link

Fixed missing attach for parse function. #166

Closed alansouzati closed 8 years ago

alansouzati commented 8 years ago

Fixes https://github.com/dthree/vorpal/issues/164.

It turns out that if I add attach function everything works as expected because now UI module has a parent inside the prompt function.

dthree commented 8 years ago

Good find. :+1:

alansouzati commented 8 years ago

Cool, thanks for the fast follow up. Could please release a new version of vorpal? I'm kind of dependent on this fix to release our new CLI 💃

dthree commented 8 years ago

Pushed to v1.11.3. :tada:

alansouzati commented 8 years ago

🍻

ruyadorno commented 8 years ago

@alansouzati and @dthree just a small follow up from this PR 😊

I manage an internal cli app that used to rely on vorpal.parse mechanism to just forward commands to its vorpal actions and exit the process right away.

After this PR landed, this "command forwarding" flow is not so nice as it used to be, as the process will be kept alive by having an attached ui and users now need an extra CTRL+C to exit the app after a command ran - in the meantime I found a solution by monkey-patching vorpal.ui.attachto preserve the same functionality as before. e.g:

if (process.argv.length > 2) {
    cli.ui.attach = () => {};
    cli.parse(process.argv);
}

So my question is: Was our previous usage a non-standard and therefore non-supported way of consuming the vorpal API? Is there a better/correct syntax to implement the same functionality (non-interactive commands that just execute their logic and exit right after).

Any insights would be very much appreciated, thanks for your time!

alansouzati commented 8 years ago

I'm facing the same issue. See this PR I sent:

https://github.com/dthree/vorpal/pull/169

dthree commented 8 years ago

Hmmm... okay. I see this. More in the PR.