search

gruntjs / grunt-contrib-connect

Start a static web server.
http://gruntjs.com
MIT License
714 stars 146 forks source link

Fixed: options.open.{appName, callback} #230

Closed bitnot closed 5 years ago

bitnot commented 7 years ago

Fixes #229 by passing open:appName config property to opn and calling open:callback.

bitnot commented 7 years ago

@vladikoff could you kindly take a look?

intih commented 7 years ago

Good stuff 👍 Would like to see this merged as open.appName not working is an issue for me.

A suggestion for this PR: is it worth updating the readme to point to opn instead of open?

Also, opn allows you to specify a string or array for options.app - array to pass arguments to the app so the documentation should probably reflect that 😄

XhmikosR commented 6 years ago

This needs a rebase, no unrelated style changes and no unrelated version changes.

bitnot commented 5 years ago

Thanks for looking into this @XhmikosR! Not sure if I should update the version and change log, please advise.

XhmikosR commented 5 years ago

@bitnot: no you shouldn't update any meta files, this is done when a new release is made.

XhmikosR commented 5 years ago

@bitnot: can you skip README.md inclusion? We usually update it when we make the release.

bitnot commented 5 years ago

@XhmikosR I was a bit concerned users may try passing opn-style config with app instead of appName and with unsupported options, so felt need to clarify in the ReadMe. Thanks for merging!

XhmikosR commented 5 years ago

Will be in v2.0.0 when that is made.

Maybe you could add a test?

bitnot commented 5 years ago

I am not very good with node, but I will try to add a test.

Any release date in mind?

XhmikosR commented 5 years ago

It doesn't depend on me otherwise I'd made the release already. I don't have access to this npm package, so when https://github.com/gruntjs/grunt-contrib-connect/pull/252 is merged.