macPlist option improvements

[bastimeyer]

This change makes it easier if you want to add a single value to the Info.plist file. Right now, the only choice you have is replacing the whole plist file, which is inconvenient.
I've changed the editPlist utility method, so you can use an object with custom values which will overwrite the parsed ones.

[bastimeyer]

Could I please get a response? 😕

[adam-lynch]

@steffenmllr?

[steffenmllr]

LGTM - it would be great if the object properties are documented in the readme

[bastimeyer]

@steffenmllr
Are you talking about the appName, appVersion, copyright, etc. properties? I don't think that these custom auxiliary properties are a good idea at all. I didn't touch those, but in my opinion these should be removed.
Right now, you're unable to set the CFBundleDisplayName property, because it will be overwritten by appName later on (see index.js#387 and utils.js#159).
It would be much better to create a default object with the needed CFBundleName, CFBundleDisplayName, CFBundleVersion, CFBundleShortVersionString and NSHumanReadableCopyright properties. Then it can be modified by all user defined properties instead of using auxiliary properties.

Do you want me to make further changes and update my pull request? (I've also seen, that I messed up some line endings in the readme file, so let me fix these too)

[bastimeyer]

PR is updated and squashed...
I would really appreciate it if you could merge these changes and bump the version of this and the grunt repo. Thanks!

[steffenmllr]

@adam-lynch could you please take a look at it and merge it if you are ok with it

[adam-lynch]

I have no idea about macPlist to be honest. Sorry for taking a few days to respond, was out of the country.

[majodev]

Would also love to see these changes within node-webkit-builder. Updating the Info.plist afterwards is a pain (as is replacing the whole file). This will make it easier to replace one single value.

[bastimeyer]

I have no idea about macPlist to be honest.

If the motivation of this pull request is still unclear, just have a look at the modified test case. These changes are actually fairly simple and make using the macPlist option much, much easier, so I don't know why nobody feels responsible to quickly look into this for a merge.
As you can see, I'm not the only one who's having some issues with the current implementation.
Thank you...

[adam-lynch]

@bastimeyer you have a point 😄

I had another there now. Looks good to me. One small thing I'd suggest is not having more than three params per function. So for getPlistOptions, if it was me I'd have it accept a single object argument, which would then contain appName, appVersion, copyright and custom as properties.

Also, for the README, how about something like this?

Pass a string if you want to supply exactly what will be written to the plist file. If a string isn't passed, a plist file will be generated from your package.json. Pass an object to overwrite or add properties to the generated plist file.

[bastimeyer]

PR is again updated... 87ee185

[adam-lynch]

Thanks 👍 Sorry for taking so long 😄

[bastimeyer]

Thanks! Would be nice if you could also bump the version and publish it on npm (also the grunt repo), so this can be used. 😃

[adam-lynch]

Will do tomorrow unless someone else gets to it before me.

Why are you suggesting to bump the grunt module's version though? Won't anyone installing it fresh / updating their install of the grunt module automatically get the latest version of the dependences (i.e. this module)? That's what I assume, unless we bump the version by its major version.

[bastimeyer]

installing it fresh

Then yes. But if you've already installed the current version, then you'd need to clear your npm cache first before installing/updating the package or else it will be copied from there. That's why I'm suggesting to bump the patch version number...

[adam-lynch]

K thanks

[adam-lynch]

Done. Both at 0.3.0 now