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

Add support for running the app directly #2

Closed
1j01 opened this issue Apr 16, 2014 · 13 comments
Closed

Add support for running the app directly #2

1j01 opened this issue Apr 16, 2014 · 13 comments

Comments

@1j01
Copy link
Contributor

1j01 commented Apr 16, 2014

I've added this functionality here: 1j01@b0d35f2 but without any tests or anything. Also "really" it should be added to a new NwRunner.

@steffenmllr
Copy link
Contributor

Great work! this would also be nice for the cli version. Something like nwbuild --run myapp.
Would you like to add tests for it and send a PR? Trying to get this whole thing shipped by the end of next week.

@1j01
Copy link
Contributor Author

1j01 commented Apr 16, 2014

My implementation actually just downloads node-webkit for all versions in the options, instead of the one for your current platform. It could completely fail if for some reason in the uncommon case that your current platform wasn't included.

@steffenmllr
Copy link
Contributor

I would just download the current platform. So if you are on a mac just download the mac version and ignore the --platform settings if it get's called with --run

@ivantodorovich
Copy link
Contributor

👍

@steffenmllr
Copy link
Contributor

The first version is published to npm - should add tests at some point :) Tell me what you think

@ivantodorovich
Copy link
Contributor

Thanks!
El abr 20, 2014 11:08 AM, "Steffen Müller" notifications@github.com
escribió:

The first version is published to npm - should add tests at some point :)
Tell me what you think


Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-40895626
.

@1j01
Copy link
Contributor Author

1j01 commented Apr 21, 2014

So now it downloads only the current platform, but only in the CLI. I'm using the module directly, so running it would still download the platforms in the options. Ideally, you should be able to do

nwb = new NwBuilder({
    files: 'app/**/*',
    platforms: ['mac'] // (doesn't contain current platform)
});

nwb.run(); // only dl current platform (only windows for instance)
setTimeout(function(){
    nwb.build(); // dl chosen platforms (only mac)
}, require("timespan")("5 years").ms());

Changing options.platforms only works for a NwBuilder instance that won't be used again, so it wouldn't work for the above (somewhat ridiculous) case to just move the switch(process.platform){...} into run (which would be nice and simple)
Rather downloadNodeWebkit should be given an argument (platforms) and be called with options.platforms when building and the result* of the switch when running.
*I'm used to coffeescript.

@steffenmllr
Copy link
Contributor

hmm why are you not using the CLI version for running nw? It wasn't designed to be used programmatically. What exactly are you trying to do? I don't really get it from the code above.

@1j01
Copy link
Contributor Author

1j01 commented Apr 22, 2014

hmm why are you not using the CLI version for running nw?

Well it didn't exist when I started using the module and also using a cakefile/gulpfile lets you do more with one command, like compiling coffeescript before building/running the app. And not just with a command, but with a keyboard shortcut (^b) in Sublime Text.

It wasn't designed to be used programmatically.

It should be, as it's a real use case, and a recommended one: "For Gulp, just use the module"?
The CLI should just be an interface to the module, and the module should do the choosing of the downloading of the relevant versions and whatnot.

@adam-lynch
Copy link
Contributor

My team definitely will be using this only programmatically

@1j01
Copy link
Contributor Author

1j01 commented May 30, 2014

I'd like to make a pull request with the small change of moving the platform-choosing out of the CLI and into the API
I think I know enough git to do that now :P

@mmacaulay
Copy link

Got this working, although I found the use of the files option when called with run to be a little weird (especially since it needs to be a glob). Seems like you should be able to just pass in a dir when in run mode and have it create the files object that gets passed to checkFiles. Won't need to do a replace on options.files in the runApp either, just pass in the dir and spawn against that.

Thoughts? I can put together a PR...

@dgattey
Copy link

dgattey commented Aug 6, 2014

Put together a PR and let's check it out.

@1j01 1j01 closed this as completed Aug 16, 2014
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

6 participants