Skip to content

Commit

Permalink
Fix bad interaction with never opening window + terminate on last window
Browse files Browse the repository at this point in the history
Previously, if you configure MacVim to never open an untitled window on
launch, *and* terminate MacVim on last window closing, you could get an
odd behavior where MacVim will close itself soon after launch, usually
when you fiddle with "About MacVim" or the preference pane. This isn't
too big a deal but could potentially make it hard to change the
preference back, and it's hard to know if a future macOS update will
further break this behavior causing MacVim to keep terminating itself on
launch (the termination behavior relies on the
`applicationShouldTerminateAfterLastWindowClosed` API which is
controlled by the OS).

To fix this, simply make it so that the preference pane doesn't allow
both settings to be set at once. If the user is trying to do so, set the
other setting to be something sane. Also, in the
`applicationShouldTerminateAfterLastWindowClosed` behavior, make sure we
add additional protection so that it will not return true when we are
set to never open untitled window (this should only be the case if the
user manually set it using `defaults` because we are now already
protecting against this in the preference pane logic). This should be
fine because these two setting don't really make sense for the user
anyway. It doesn't seem to make a lot of sense for the user to want this
behavior.

Note that I'm doing manual checking in preference UI instead of using
some sort of key-value listening of NSUserDefaults because I'm afraid of
some unintentional infinite recursion going on where the settings keep
setting each other back and forth. By only doing this at preference pane
changes this should not happen.

Fix macvim-dev#1257
  • Loading branch information
ychin committed Aug 4, 2022
1 parent aeafa39 commit f6ba7dd
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/MacVim/Base.lproj/Preferences.xib
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="17701" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="20037" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
<dependencies>
<deployment version="1090" identifier="macosx"/>
<plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="17701"/>
<plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="20037"/>
<capability name="documents saved in the Xcode 8 format" minToolsVersion="8.0"/>
</dependencies>
<objects>
Expand Down Expand Up @@ -63,6 +63,7 @@
</column>
</cells>
<connections>
<action selector="openUntitledWindowChanged:" target="-2" id="S8l-uX-tEl"/>
<binding destination="58" name="selectedTag" keyPath="values.MMUntitledWindow" id="171"/>
</connections>
</matrix>
Expand Down Expand Up @@ -175,6 +176,7 @@
</menu>
</popUpButtonCell>
<connections>
<action selector="lastWindowClosedChanged:" target="-2" id="IcT-7v-gxr"/>
<binding destination="58" name="selectedIndex" keyPath="values.MMLastWindowClosedBehavior" id="968"/>
</connections>
</popUpButton>
Expand Down
11 changes: 11 additions & 0 deletions src/MacVim/MMAppController.m
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,17 @@ - (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]);
Expand Down
30 changes: 30 additions & 0 deletions src/MacVim/MMPreferenceController.m
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,34 @@ - (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];
}
}
}

@end

0 comments on commit f6ba7dd

Please sign in to comment.