Improved Windows Builds

[confused-Techie]

This PR modifies the Windows Installation Configuration:

• Allows the user to select which directory to install the Pulsar App to. Resolves Windows: Bad install folder and forced #277
• Additionally since the install folder is configurable this invalidates/ Resolves Windows installation directory incorrect #229
• Additionally configures a Desktop Shortcut as well as Start Menu shortcut

This PR has fully been tested and confirmed to work on Windows 10

[confused-Techie]

@pulsar-edit/core This PR is now ready for review

[Spiker985]

For the Uninstall Display Name, do we want to shorten in up to Pulsar? Since that's the app name, and Pulsar-Edit is just the org?

[confused-Techie]

For the Uninstall Display Name, do we want to shorten in up to Pulsar? Since that's the app name, and Pulsar-Edit is just the org?

Yeah we absolutely could. if we think it looks better sure thing. And honestly now that I consider it, the control panel already shows the Publisher in the same table when you uninstall, so there's no reason we need to include it in the title.

Also as a side note, the Publisher on Windows does show Pulsar Community for some reason

[Spiker985]

Also, how does allowing the change of install directory affect having a per-user install? Or does it not, because it's being executed as the invoker of the exe anyway?

[Spiker985]

Also as a side note, the Publisher on Windows does show Pulsar Community for some reason

Bet you it grabs it from the Author field in package.json

[confused-Techie]

Also, how does allowing the change of install directory affect having a per-user install? Or does it not, because it's being executed as the invoker of the exe anyway?

IIRC a per user install means it installs into AppData since anywhere else it's now global on the system.

So if we allow the directory to be changed it's now possible to have it be a global install, or per user.

I had hoped that setting asInvoker would mean that during installation it would start as the user only, allowing non-admins to still install, and have it only install into AppData since a non-admin would need admin access to install globally.

But, and I'm not sure if this is because I'm an admin on my system, I still got a UAC prompt when running the installer with asInvoker set

[confused-Techie]

Also as a side note, the Publisher on Windows does show Pulsar Community for some reason

Bet you it grabs it from the Author field in package.json

Should probably fix that huh

[Spiker985]

But, and I'm not sure if this is because I'm an admin on my system) I still got a UAC prompt when running the installer with asInvoker set

If you told it "no" to the UAC prompt, did it get angry, or carry on it's way? Because if this change fully requires admin, I feel like we'd need to see if we can stop that from happening. I think that requiring admin is a divergence of expectation 🤔

[confused-Techie]

But, and I'm not sure if this is because I'm an admin on my system) I still got a UAC prompt when running the installer with asInvoker set

If you told it "no" to the UAC prompt, did it get angry, or carry on it's way? Because if this change fully requires admin, I feel like we'd need to see if we can stop that from happening. I think that requiring admin is a divergence of expectation 🤔

Looks like without approval on the UAC prompt it does in fact fail. So I do agree it's a change in expectation, but that's essentially what those linked issues are calling for.

I even tried to explain this on one, but they remained steadfast that it's best to install into these other locations.

Possibly should we open up a poll? I didn't realize before it would fail without admin, and honestly I'm not super on board with that idea.

[Spiker985]

But, and I'm not sure if this is because I'm an admin on my system) I still got a UAC prompt when running the installer with asInvoker set

If you told it "no" to the UAC prompt, did it get angry, or carry on it's way? Because if this change fully requires admin, I feel like we'd need to see if we can stop that from happening. I think that requiring admin is a divergence of expectation thinking

Looks like without approval on the UAC prompt it does in fact fail. So I do agree it's a change in expectation, but that's essentially what those linked issues are calling for.

I even tried to explain this on one, but they remained steadfast that it's best to install into these other locations.

Possibly should we open up a poll? I didn't realize before it would fail without admin, and honestly I'm not super on board with that idea.

Yeah, honestly, if it allowed the choice of "machine-wide" versus "user" install types, and then did the prompt for UAC - I would be okay with it, but since it outright invalidates a non-admin install I would definitely put it to a vote

[mauricioszabo]

Yeah, I'm also not comfortable with not allowing the user to install without admin powers. If we don't find a way to fix this, I feel we don't need to address the issues - I mean, it's not like Pulsar is not working right?

[sertonix]

Yeah, I'm also not comfortable with not allowing the user to install without admin powers. If we don't find a way to fix this, I feel we don't need to address the issues - I mean, it's not like Pulsar is not working right?

Removing "perMachine": true as mentioned in my comments should resolve this issue.

[mauricioszabo]

Worth a try, @sertonix :)

[confused-Techie]

I've just tried @sertonix recommended changes to fix the admin only installation, so we can wait for the binaries to generate and see if this does work

[sertonix]

@confused-Techie @mauricioszabo The build worked for me.

[confused-Techie]

@sertonix fantastic work!

This PR now does exactly as I had originally hoped.

This allows users to install Machine wide (into Program Files) or per user (into AppData) and will only require admin permission as needed.

This still doesn't allow the user to install into any directory they would like, but I think is close enough for our needs.

Super glad @sertonix was able to find the solution here. Since this resolves everyones concerns if it looks good then lets merge, super rad

[Neustradamus]

@confused-Techie: Thanks for your PR!

With your changes, what is the default install place?

[confused-Techie]

@confused-Techie: Thanks for your PR!

With your changes, what is the default install place?

The default installation directory is a per-machine install. That is C:\Program Files\Pulsar but that does require admin privilege's. If the user does not have admin privilege's then they can install per user, which will be into AppData

[Neustradamus]

@confused-Techie: Can you create test build?

And after, it is needed to create an official new build with this fix!

[confused-Techie]

@confused-Techie: Can you create test build?

And after, it is needed to create an official new build with this fix!

Any of the latest Alpha builds should already have this change. They are built automatically from each merged PR. You can download them directly from our website.

But we are hoping to release anther beta when we can, but will likely still be some time.