Add explicit icon path during --createShortcut on install

[jeffbargmann]

As-is, this module creates shortcuts that point the the EXE in the version folder. This means that all shortcuts have invalid icons after each upgrade, and all shortcuts must be updated. Unfortunately, Windows' shell caches some of this information, does fancy stuff for shortcut-to-exe matching, etc. Further, the user may have copied a shortcut, in which case it will just go bad.

A better solution is to use Squirrel's --icon flag and copy an icon to a known place. Squirrel already uses app.ico in the Update.exe directory for uninstall icon, so seems like a good place. (Though-- that icon will fail to be placed if it doesn't download at time of install, oddly.)

NOTE: This is a breaking change so not really eligible for merge :( What do you recommend / Any review suggestions?

---

This change is

[mention-bot]

@JBLatenight, thanks for your PR! By analyzing the annotation information on this pull request, we identified @imlucas, @Vj3k0 and @kangas to be potential reviewers

[jeffbargmann]

By the way looks like your CI is jammed up on rake call?

Thanks for providing this lib!

[jeffbargmann]

Ugh. Squirrel has line "if (zp.IconUrl != null && !File.Exists(targetIco))" in UpdateManager.InstallHelpers.cs, wherein it decides to bail entirely on the Uninstall entry having an icon if the icon already exists. https://github.com/Squirrel/Squirrel.Windows/blob/913b3e730070f2d0461902cd65a6f0cba1f2fcb5/src/Squirrel/UpdateManager.InstallHelpers.cs#L53 Would PR in to adjust that behavior, but that routine's dependency on a remote url for uninstall icon warrants more work than just this fix o.O (Flaky connection on install == no uninstall icon??)

Anywho -- updated to use app's name as icon name (or _app if conflict)

[kangas]

Adding an options object seems ok. Let's clearly document options using JSDoc

http://usejsdoc.org/tags-param.html

[kangas]

@JBLatenight thanks for submitting this! I have seen those shortcut icons go bad and I had no idea why it was happening. Great catch.

Breaking change is fine, not a concern.

Please "npm i" and run "npm run check". I see a number of linter errors currently on this branch:

$ npm run check

> [email protected] check /Users/kangas/workspace/electron-squirrel-startup
> mongodb-js-precommit


index.js
  16:2   error    Split 'var' declarations into multiple statements                      one-var
  17:0   warning  Line 17 exceeds the maximum line length of 100                         max-len
  21:37  error    Expected '===' and instead saw '=='                                    eqeqeq
  27:4   warning  Unexpected sync method: 'writeFileSync'                                no-sync
  27:33  warning  Unexpected sync method: 'readFileSync'                                 no-sync
  37:4   error    Split 'var' declarations into multiple statements                      one-var
  50:6   error    Expected an assignment or function call and instead saw an expression  no-unused-expressions
  55:6   error    Expected an assignment or function call and instead saw an expression  no-unused-expressions
  60:6   error    Expected an assignment or function call and instead saw an expression  no-unused-expressions

[kangas]

Update: please rebase on latest master, then "npm i" to get our latest & greatest linter rules.

  › Running eslint on 1 files…
  ✖ 12  eslint errors detected
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✖ 1 of 3 check(s) failed:
  Please fix the 12 error(s) below.


      /Users/kangas/workspace/electron-squirrel-startup/index.js
        16:3   error    Split 'var' declarations into multiple statements                      one-var
        17:20  error    Strings must use singlequote                                           quotes
        21:5   error    Expected space(s) after "if"                                           keyword-spacing
        21:37  error    Expected '===' and instead saw '=='                                    eqeqeq
        23:1   error    Trailing spaces not allowed                                            no-trailing-spaces
        25:5   warning  Unexpected sync method: 'writeFileSync'                                no-sync
        25:34  warning  Unexpected sync method: 'readFileSync'                                 no-sync
        27:5   error    Expected space(s) after "catch"                                        keyword-spacing
        35:5   error    Split 'var' declarations into multiple statements                      one-var
        41:1   error    Trailing spaces not allowed                                            no-trailing-spaces
        43:7   error    Expected space(s) after "if"                                           keyword-spacing
        46:7   error    Expected an assignment or function call and instead saw an expression  no-unused-expressions
        51:7   error    Expected an assignment or function call and instead saw an expression  no-unused-expressions
        56:7   error    Expected an assignment or function call and instead saw an expression  no-unused-expressions

      ✖ 14 problems (12 errors, 2 warnings)

[imlucas]

@JBLatenight

By the way looks like your CI is jammed up on rake call?

Thanks for pointing this out! Travis setup sorted in #11

[jeffbargmann]

Hey @kangas sorry I've been off on other work.

Just checked this out and it seems the project has conflicting lint rules; Each time I fix one-var I get a no-mixed-requires, and vice versa. Thoughts?

eslint/eslint#3091
eslint/eslint#6175

[imlucas]

Cleaning up old PR's. Please reopen if this should be reconsidered.