From 094418700ebf4332bdc3679d5b38a35f69ec2b69 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Fri, 4 Nov 2022 18:26:23 -0700 Subject: [PATCH 1/2] Revert "Fix bad interaction with never opening window + terminate on last window" This reverts commit f6ba7dd40be20b3af73b01f5bd3ce157bc7dbd28. This change was reverted as part of #1338 as we want to implement a different solution. --- src/MacVim/Base.lproj/Preferences.xib | 6 ++---- src/MacVim/MMAppController.m | 11 ---------- src/MacVim/MMPreferenceController.m | 30 --------------------------- 3 files changed, 2 insertions(+), 45 deletions(-) diff --git a/src/MacVim/Base.lproj/Preferences.xib b/src/MacVim/Base.lproj/Preferences.xib index d9e24bfcda..228c305069 100644 --- a/src/MacVim/Base.lproj/Preferences.xib +++ b/src/MacVim/Base.lproj/Preferences.xib @@ -1,8 +1,8 @@ - + - + @@ -67,7 +67,6 @@ - @@ -180,7 +179,6 @@ - diff --git a/src/MacVim/MMAppController.m b/src/MacVim/MMAppController.m index 12e743f482..eab4df0202 100644 --- a/src/MacVim/MMAppController.m +++ b/src/MacVim/MMAppController.m @@ -548,17 +548,6 @@ - (void)application:(NSApplication *)sender openFiles:(NSArray *)filenames - (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)sender { - if (MMUntitledWindowNever == - [[NSUserDefaults standardUserDefaults] - integerForKey:MMUntitledWindowKey]) { - // Sanity protection: If we never open a new window on application launch, there could - // be an issue here where we immediately terminate MacVim. Because of that, we always - // return false regardless of what MMLastWindowClosedBehavior is. Note that the user - // should not be able to set these two conflicting options together in the preference pane - // but it's possible to do so in the terminal by calling "defaults" manually. - return false; - } - return (MMTerminateWhenLastWindowClosed == [[NSUserDefaults standardUserDefaults] integerForKey:MMLastWindowClosedBehaviorKey]); diff --git a/src/MacVim/MMPreferenceController.m b/src/MacVim/MMPreferenceController.m index e93540df16..a556626496 100644 --- a/src/MacVim/MMPreferenceController.m +++ b/src/MacVim/MMPreferenceController.m @@ -167,36 +167,6 @@ - (IBAction)fontPropertiesChanged:(id)sender [[MMAppController sharedInstance] refreshAllFonts]; } --(IBAction)lastWindowClosedChanged:(id)sender -{ - // Sanity checking for terminate when last window closed + not opening an untitled window. - // This results in a potentially awkward situation wehre MacVim will close itself since it - // doesn't have any window opened when launched. - // Note that the potentially bad behavior is already protected against for in applicationShouldTerminateAfterLastWindowClosed:, - // but this sanity checking is to make sure the user can see that in an explicit fashion. - NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults]; - if ([defaults integerForKey:MMLastWindowClosedBehaviorKey] == MMTerminateWhenLastWindowClosed) { - if ([defaults integerForKey:MMUntitledWindowKey] == MMUntitledWindowNever) { - [defaults setInteger:MMUntitledWindowOnOpen forKey:MMUntitledWindowKey]; - } - } -} - --(IBAction)openUntitledWindowChanged:(id)sender -{ - // Sanity checking for terminate when last window closed + not opening an untitled window. - // This results in a potentially awkward situation wehre MacVim will close itself since it - // doesn't have any window opened when launched. - // Note that the potentially bad behavior is already protected against for in applicationShouldTerminateAfterLastWindowClosed:, - // but this sanity checking is to make sure the user can see that in an explicit fashion. - NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults]; - if ([defaults integerForKey:MMLastWindowClosedBehaviorKey] == MMTerminateWhenLastWindowClosed) { - if ([defaults integerForKey:MMUntitledWindowKey] == MMUntitledWindowNever) { - [defaults setInteger:MMHideWhenLastWindowClosed forKey:MMLastWindowClosedBehaviorKey]; - } - } -} - - (IBAction)smoothResizeChanged:(id)sender { [[MMAppController sharedInstance] refreshAllResizeConstraints]; From cabb4957faf4e620c21cb5bbf6306a5f4a4a54d6 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Fri, 4 Nov 2022 18:47:18 -0700 Subject: [PATCH 2/2] Allow never opening window + terminate on last window, with guards Previously we disabled this combo in f6ba7dd40be20 because when implemented naively, it causes an issue where just opening an About MacVim or Settings window could immediately cause MacVim to exit (because macOS determined that there were no non-auxillary window open). This was awkward and potentially made it hard to change the setting back, and exact behavior depended on OS behavior. However, it seems like there are legit use case for this combo of settings. Change it so that we allow setting both of them again, but add checks so that `applicationShouldTerminateAfterLastWindowClosed:` will only return `YES` if we have opened at least one Vim window before. This gives the user a chance to open a window first, so using Settings etc wouldn't immediately terminate the app. Fix #1338 --- src/MacVim/MMAppController.h | 2 ++ src/MacVim/MMAppController.m | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/MacVim/MMAppController.h b/src/MacVim/MMAppController.h index d3e2fae121..a8acd9c219 100644 --- a/src/MacVim/MMAppController.h +++ b/src/MacVim/MMAppController.h @@ -49,6 +49,8 @@ int numChildProcesses; NSMutableDictionary *inputQueues; int processingFlag; + + BOOL hasShownWindowBefore; #if !DISABLE_SPARKLE #if USE_SPARKLE_1 diff --git a/src/MacVim/MMAppController.m b/src/MacVim/MMAppController.m index eab4df0202..8e8af1c87e 100644 --- a/src/MacVim/MMAppController.m +++ b/src/MacVim/MMAppController.m @@ -548,6 +548,15 @@ - (void)application:(NSApplication *)sender openFiles:(NSArray *)filenames - (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)sender { + if (!hasShownWindowBefore) { + // If we have not opened a window before, never return YES. This can + // happen when MacVim is not configured to open window at launch. We + // want to give the user a chance to open a window first. Otherwise + // just opening the About MacVim or Settings windows could immediately + // terminate the app (since those are not proper app windows), + // depending if the OS feels like invoking this method. + return NO; + } return (MMTerminateWhenLastWindowClosed == [[NSUserDefaults standardUserDefaults] integerForKey:MMLastWindowClosedBehaviorKey]); @@ -888,6 +897,8 @@ - (void)windowControllerWillOpen:(MMWindowController *)windowController [NSApp activateIgnoringOtherApps:YES]; shouldActivateWhenNextWindowOpens = NO; } + + hasShownWindowBefore = YES; } - (void)setMainMenu:(NSMenu *)mainMenu