Exit on parse

[alansouzati]

This PR aims to exit (detach the UI) after parser execution is complete.

[dthree]

Not sure this is a good move - this is not always the desired behavior. I can name a few Vorpal apps in which the app doesn't exit after parsing.

[alansouzati]

I see your point. See this stackoverflow that you answered:

http://stackoverflow.com/questions/33489201/is-it-possible-to-create-a-vorpal-application-which-the-commands-output-goes-di

You can have the app exit after completing the command simply by omitting vorpal.show(). Because there is no prompt, Node realizes there is nothing left to do, and the process will exit naturally.

The issue is that after my PR has been merged, there is a UI attached to the parse function. So Node will not exit naturally.

Any recommended way to fix this problem?

[dthree]

Okay... trying to find the best way to solve this.

I think we need to add in options to parse - something like:

vorpal.parse(process.argv, {survive: true});

With this, the default behavior will be to exit, and if a flag is passed, the exit will be omitted.

I don't really like survive though. Got a better name?

[ruyadorno]

👍 I like the idea of having options/config for parsing

keepAlive is certainly better than survive

[dthree]

Hahaha right? Just woke up, mind not functioning.

@alansouzati would you be willing to add a keepAlive flag to your PR?

[alansouzati]

You bet, great idea guys. Will add that tomorrow.

Alan

On Aug 6, 2016, at 9:34 AM, dc notifications@github.com wrote:

Hahaha right? Just woke up, mind not functioning.

@alansouzati would you be willing to add a keepAlive flag to your PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[alansouzati]

Added the keepAlive, I believe we are good to go now 💃

[alansouzati]

fixed :)

[ruyadorno]

@alansouzati tried your branch with the mentioned fix from above but I still have one annoying issue, which is that the interactive shell prefix gets printed twice when I execute a command, e.g:

ad help
local@macbookpro~$

  Commands:

    help [command...]           Provides help for a given command.
    exit                        Exits application.
    ...

local@macbookpro~$

where macbookpro is my Hostname :)

[ruyadorno]

I think that's probably due to the fact that the ui attaches itself and then executes an exit command but I'd like your inputs on it before jumping into any conclusions...

[dthree]

@alansouzati if we don't resolve this shortly, I'm going to have to revert that last PR. Willing to look into additional fixes for a little though.

[alansouzati]

Yeah. I have the same issue.

Well, I believe printing the current delimiter is less impactful than not supporting prompt for inline commands.

See this issue for reference:

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

[alansouzati]

I can try and investigate why the delimiter in being printed in this case, but I believe this is a separate issue from "exit on parse".

[ruyadorno]

👍 makes sense,

@dthree IMHO it's preferable to merge this the way it is instead of reverting, a fix for the extra delimiters printed can wait more

[alansouzati]

In the case of the delimiter, I believe if keepAlive is false we should never print it.

[dthree]

Okay. I would like to see if we could figure this delimiter thing out though... don't want to merge this and then have it forgotten about.

[alansouzati]

Do you agree with this statement: "if keepAlive is false we should never print the delimiter." ?

[ruyadorno]

@dthree in a second thought, maybe a revert is really a good idea in this case

seeing #172 it seems other people are suffering from the ui attach change as a breaking change

adding the keepAlive options is at best a new feature and should yield a new minor version but to be honest even with this option it still a breaking change in the API for many consuming applications out there and a new major version release would probably be better for everyone

[alansouzati]

agreed with @ruyadorno. maybe we need a major release as it is a breaking change.

But I believe we don't necessarily need to revert the change in Git to revert it in NPM.

[alansouzati]

I'm pushing the fix for the delimiter here for you guys to evaluate.

[alansouzati]

Ok, can you take a look now? If keepAlive is false no delimiter will be printed.

[dthree]

But I believe we don't necessarily need to revert the change in Git to revert it in NPM.

The git repo should always reflect NPM, for users' sake.

[ruyadorno]

@alansouzati just updated my project with the latest changes and I can confirm that this fixes the extra delimiters printed issue 👍

thank you so much for all the work on this 😊

[alansouzati]

thanks for trying it out @ruyadorno.

It seems that we will need to wait until @dthree figures out the side effects of this change.

[ruyadorno]

@alansouzati I have yet another very tricky situation, I actually have a mixed of interactive and non-interactive (keepAlive:false) commands in my cli app - @dthree is that supposed to be a supported use case?

in this situation it would be nice to have a way to define the keepAlive option from within the command definition instead...

[alansouzati]

hey @dthree any updates on this?

I have few folks complaining about the grommet new sample-app hanging the UI. I really want to avoid a three step process like

1. grommet
2. new sample-app
3. exit

[alansouzati]

Actually no rush anymore. I added my fork branch in my package.json for now.

https://github.com/grommet/grommet-cli/blob/master/package.json#L27