Replace the app's details on windows (i.e. override the node.rc) #9
Conversation
- Added some documentations for the new feature and for the icon feature
|
i had to set it up pretty quick so it can answer my need which was urgent. i think that in future version it's worth doing a more sophisticated regex based find-replace on the node.rc file. because the one that i've implemented is limited. |
|
This looks like a pretty good start. Thanks! Do you need this right away, or are you working off of your fork? I'd like to add these options to the cli in some way, but will need some time to figure out how to handle the multiple-field configuration. |
There was a problem hiding this comment.
Thanks for doing the work on this @unbalanced! We have been wanting this configuration as well, but haven't had the time to get it added.
| } | ||
|
|
||
| if (options.LegalTrademarks) { | ||
| file.contents = file.contents.replace(`VALUE "LegalCopyright", "${options.LegalCopyright}"`, `VALUE "LegalCopyright", "${options.LegalCopyright}"\r\n VALUE "LegalTrademarks", "${options.LegalTrademarks}"`) |
There was a problem hiding this comment.
Just curious, why is this one different than all the others? Is LegalTrademarks not in the file normally? The way it's written, you cannot provide options.LegalTrademarks without also providing options.LegalCopyright
There was a problem hiding this comment.
Your first assumption is correct https://github.com/nodejs/node/commits/master/src/res/node.rc
There was a problem hiding this comment.
There was a problem hiding this comment.
Yep...
As i mentioned above, i moved quick with this feature because of a tight schedule.
I think we can iterate and create a better more sophisticated regex based find-replace.
Another limitation is that currently the node.rc file is being saved/cached with your values, so if you change them you might want to revert to the original file or run a clean build (we can overcome this limitation with a better find replace rules).
There was a problem hiding this comment.
Thanks for the clarification. I agree that some refactoring could be done in this file, but it's a great start. Thanks again for the contribution!
| "InternalName": "blah-blah", | ||
| "LegalCopyright": "(C) Blah Blah Inc.", | ||
| "LegalTrademarks": "(TM) Blah Blah Inc." | ||
| } |
There was a problem hiding this comment.
We can probably just add these values to the configuration example above, rather than duplicating all of the other settings and their defaults. Thoughts @calebboyd?
|
@calebboyd i don't mind to work with my fork for now. Regarding the CLI, maybe it should stay simple and all complex features (e.g. icon, node.rc, plugins, etc) should stay available only via the Programmatic API. So CLI will be used only for "basic integrations" |
also added some documentation for the new feature and the icon replacement