Rebrand AppUserModelID - Ensure Pulsar is separated as it's own App Icon on Windows #315
Conversation
Co-authored-by: DeeDeeG <DeeDeeG@users.noreply.github.com>
|
I'm approving, LGTM. 👍 Optional info in this comment, just thought I'd post my findings about where the "Squirrel" in the ID seems to have come from. Looking into what sort of "ID" if any is used by Squirrel (click to expand):Using And supposedly, Squirrel just uses the app name?
From: https://www.electron.build/configuration/nsis#guid-vs-application-name Anecdotally, I have a number of Atom update files around on my windows machine, in couple places under This PR got me curious again, since I've always wondered about that A fair guess would be "it just has to be a unique ID, the Squirrel part is not meaningful or even accurate, and it is just a bit of a red herring/not what it seems to be." As it often does, trawling though the Trawling through the git blame, here are my notes (click to expand):Looks like a mishap with meaningful indentation in CoffeeScript. See: atom/atom@b483fc6 It appears there was an attempt to nest the app ID logic in the code handling the Squirrel code path. I think it shouldn't have been nested, given that it works when it's not nested, and because that makes more sense for the persistent running ID to have not much if anything to do with Squirrel. Indeed, when decaffeinated, this code becomes not nested. See: atom/atom@e4946a1#diff-7493904054a0d148d0518afb746d400c755837b07604c684e6fed26cee3ddd35R28-R39 I think the answer is: the ID having "Squirrel" in it was always just a coincidence, a bit of a misnomer, and the result of reading too much into / having a little too much faith in "nesting" whitespace, or an "attempt at meaningful whitespace gone wrong" mishap. Hopefully that was an entertaining tangent. Tangential thoughts on "release channels" and this bit of code So given that info, I'm thinking this ID can be whatever we want it to be. And given the code comments, to the extent we bring back the "release channels" idea, it looks like we should retain the different IDs per "release channel" so the channels get their own TaskBar icons on Windows. As of the moment, if we have only one release channel, it's kind of a moot point. Although it'd be nice if the version running from source was distinct from (had a separate TaskBar icon from) the packaged version. (I think it is -- I think the "from source" is "dev" channel, and the |
|
I gotta say @DeeDeeG I think this was one of the better reviews I've seen recently. Answers a few questions I had myself, and is just an interesting deep dive into how it came about, so thanks for writing this up, seriously. But yeah like you said, it's nice this will out of the box provide different icons for the packaged and source running apps, and if we did add release channels, it's not a burden to support the feature here until we do. And otherwise I'd make the change to ensure this always happens, but considering we are not using squirrel nor do I think there's any push for us to do so, we could probably leave this as is then. Thanks for the review, I'll go ahead and merge! |
|
@DeeDeeG I've discovered why this was here, and why it was It's named this only to allow Squirrel AutoUpdates to still pin themselves to the TaskBar. It makes sense if you think about it, this string is how Windows identifies the application (which side note, with this PR now being merged this means everyone that uses Windows will have to repin their Icon. Especially considering the bug for windows task bar icons being broken this will very likely result in a few issues.) But it seems Squirrel by default creates a shortcut icon with an Application User Model ID of But here if you want the docs read through this to see why it was the way it was. |
|
Hmm, I thought there might be some risk it was really squirrel related. And I vaguely recalled seeing It's hard to piece everything together with 100% completeness or confidence. If you feel there is a reason to revert this, then I would understand. Otherwise, if this is a thing where some folks will be confused by it, and we just need to tell them to re-pin, I'll keep that in mind and be ready to advise folks to re-pin. |
Yeah honestly I feel like us getting away from the Atom pinned task icon, or concurrently running Atom instances in the task bar is worth it. Where the only downside currently is that during an upgrade users need to repin. Now when/if we eventually use Squirrel again we will have to modify this value again, but again the downside doesn't really concern me. Since the fix is so simple, and would likely only affect a few users |
By rebranding this value, it ensures that if a User were to run both Atom and Pulsar at the same time, that they will remain separate items within the Task Bar.
Without this change running both items simultaneous (or if a user has Atom/Pulsar pinned) the programs would overlap each other within the Task Bar.
But with it rebranding ensures they stay as separate applications.