add ability to specify path to custom installer.nsi #51
Conversation
| @@ -27,6 +27,7 @@ var Builder = { | |||
| options = options || {}; | |||
| options.log = options.log || console.log; | |||
| options.out = options.out ? path.resolve( process.cwd(), options.out ) : process.cwd(); | |||
| options.nsiTemplate = options.nsiTemplate || path.join( __dirname, 'templates/installer.nsi.tpl' ); | |||
|
|
|||
There was a problem hiding this comment.
Can you please follow coding style here ( aligned = )?
There was a problem hiding this comment.
oops, missed this one, fixed
|
Except a little coding style issue this looks good to me. The only thing I'm asking myself is, if it is needed to pipe a custom NSI file trough the templating engine. It feels a bit weird - doesn't it? But maybe it's just me. :) |
|
it is a bit weird now that I think about it, but I think it's better to keep things consistent with the default template. and after all, enduser can easily hardcode things in it if he wants, templating is not that intrusive :) |
|
I just got my head around it a bit more. I think we can do it like that - but are there ways around setting a custom file? I'm thinking of several config options or even areas where user can place command easily. |
|
I think there are pros and cons to each approach. 1. Options and customisable areas in config:prossome level of control over things conshave to add every little bit as an option/area to the config (only a con if there are a lot more options, depends if the demand is high or low) 2. Custom file:proscomplete control conshave to know what you're doing (harder for the end user) i probably missed lots of things For me second approach is better, because digging in the source code and copying and adapting a template is fairly easy for me personally, and I kinda like the complete control thing. It also means that a problem is solved with minimum effort. Longterm i think combining approach 2 and 3 would probably be better in general, but it's gonna cost more time, and the answer is really "it depends". Up to you to decide :) |
|
+1 for @vxsx changes. Have to change .tpl file in electron-builder folder to add register settings. |
|
👍 |
|
did you have a chance to think about the path you want to take with this @stefanjudis ? |
| options = options || {}; | ||
| options.log = options.log || console.log; | ||
| options.out = options.out ? path.resolve( process.cwd(), options.out ) : process.cwd(); | ||
| options.nsiTemplate = options.nsiTemplate || path.join( __dirname, 'templates/installer.nsi.tpl' ); |
There was a problem hiding this comment.
As this windows specific this should be treated inside of the windows installer file, or?
|
Okay, okay. I see a few people want this - so let's do it. :) Sorry for the delay on my side. :( I made a few notes to the actual PR. One additional note I have is, that we should try to engage people using a different nsi template to think of their nsi template and ask themselves if it might be useful for others. There might be some additions which we can add to the "base"-template. Thanks a lot folks for your effort. :) |
|
@stefanjudis updated the PR. not entirely sure about what to add to the readme beside the config option though |
|
@vxsx Alright. Looks good. :) Thanks a lot. ;) |
add ability to specify path to custom installer.nsi - fix #50
|
@stefanjudis thank you :) |
#50