-
Notifications
You must be signed in to change notification settings - Fork 32.8k
feat: offer to move into Applications folder on macOS #249345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
I lean towards NO. And then we can add a setting if users complain. |
src/vs/code/electron-main/app.ts
Outdated
|
||
if (response === 0) { | ||
try { | ||
const result = app.moveToApplicationsFolder({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its unclear to me what moveToApplicationsFolder
does under the hood? Will it trigger our shutdown handlers, for example configure files.hotExit
to off
, have a dirty file and trigger the move, would it still confirm shutdown and allow to veto, given there is a dirty file not saved on shutdown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
underneath the equivalent of app.quit
is called from the runtime to exit the application which should invoke all shutdown related handlers for graceful exit, I haven't tested the above flow, will confirm.
src/vs/code/electron-main/app.ts
Outdated
localize({ key: 'move', comment: ['&& denotes a mnemonic'] }, "&&Move to Applications"), | ||
localize({ key: 'doNotMove', comment: ['&& denotes a mnemonic'] }, "&&Do not Move") | ||
], | ||
message: localize('moveToApplicationsFolderWarning', "{0} works best when run from the Applications folder", this.productService.nameLong), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it'd be nice to say something about the reason, e.g. "Auto-updating {0} works best..." or "{0} will not be able to auto-update unless you move it to the Applications folder"
7df3620
to
eceb76f
Compare
eceb76f
to
4a00af9
Compare
Fixes #213909
Detect when application on macOS is installed outside the Applications folder and offer choice to move. The detection will bail early under the following cases,
There are 3 cases this path should cover,
In this case, once user confirms the choice to move
app.moveToApplicationsFolder
will quit the current instance (which follows app.quit path in the runtime so all shutdown veto paths will be respected), copy the app to destination, trash the source app, wait for the application to exit and relaunch from the destinationconflictType === exists
In this case, once user confirms the choice to move
app.moveToApplicationsFolder
delete the version under Applications folder and follow the same order of steps as 1)conflictType === existsAndRunning
In this case, due to our singleton logic any new instance sharing the same user-data-dir will merge to the running instance and hence no action is needed. When using different user-data-dir the launch would happen from cli in which case the detection will bail early as mentioned before.
Question: