Skip to content

Commit

Permalink
Added additional safety net for remote operations.
Browse files Browse the repository at this point in the history
All remote operations confirmations that can result in data loss have additional guard to prevent accidental confirming; instead of taking simple return key press, these dialogs don't have default button, so can't be confirmed as easily. There's also preference option that can toggle this option on and off.
  • Loading branch information
tomaz committed Dec 11, 2015
1 parent c6ca85b commit aa8b820
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 17 deletions.
34 changes: 27 additions & 7 deletions GitUp/Application/en.lproj/MainMenu.xib
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ You must close and reopen any opened repositories in GitUp after changing this s
<rect key="frame" x="139" y="216" width="299" height="58"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<color key="backgroundColor" name="controlColor" catalog="System" colorSpace="catalog"/>
<size key="cellSize" width="299" height="18"/>
<size key="cellSize" width="276" height="18"/>
<size key="intercellSpacing" width="4" height="2"/>
<buttonCell key="prototype" type="radio" title="Radio" imagePosition="left" alignment="left" inset="2" id="S4o-dE-O99">
<behavior key="behavior" changeContents="YES" doesNotDimImage="YES" lightByContents="YES"/>
Expand Down Expand Up @@ -252,13 +252,13 @@ You must close and reopen any opened repositories in GitUp after changing this s
</subviews>
</view>
</tabViewItem>
<tabViewItem label="{500, 206}" identifier="advanced" id="Qdl-lQ-O4q">
<tabViewItem label="{500, 265}" identifier="advanced" id="Qdl-lQ-O4q">
<view key="view" id="8CG-H0-Xhk">
<rect key="frame" x="0.0" y="0.0" width="500" height="400"/>
<autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/>
<subviews>
<popUpButton verticalHuggingPriority="750" id="7N4-S9-iWL">
<rect key="frame" x="136" y="248" width="200" height="26"/>
<rect key="frame" x="136" y="189" width="200" height="26"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<popUpButtonCell key="cell" type="push" title="&lt;RELEASE CHANNEL&gt;" bezelStyle="rounded" alignment="left" lineBreakMode="truncatingTail" state="on" borderStyle="borderAndBezel" imageScaling="proportionallyDown" inset="2" autoenablesItems="NO" selectedItem="xdx-IX-FK6" id="Sdr-Ep-kPW">
<behavior key="behavior" lightByBackground="YES" lightByGray="YES"/>
Expand All @@ -276,7 +276,7 @@ You must close and reopen any opened repositories in GitUp after changing this s
</connections>
</popUpButton>
<textField horizontalHuggingPriority="251" verticalHuggingPriority="750" id="mi3-Eg-51i">
<rect key="frame" x="12" y="254" width="120" height="17"/>
<rect key="frame" x="12" y="195" width="120" height="17"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" scrollable="YES" lineBreakMode="clipping" allowsUndo="NO" sendsActionOnEndEditing="YES" alignment="right" title="Release Channel:" id="M0A-b6-dhN">
<font key="font" metaFont="system"/>
Expand All @@ -294,7 +294,7 @@ You must close and reopen any opened repositories in GitUp after changing this s
</textFieldCell>
</textField>
<textField verticalHuggingPriority="750" horizontalCompressionResistancePriority="250" id="PtA-hG-Qmn">
<rect key="frame" x="137" y="210" width="345" height="34"/>
<rect key="frame" x="137" y="151" width="345" height="34"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" controlSize="small" sendsActionOnEndEditing="YES" alignment="left" title="The &quot;Continuous&quot; release channel installs builds directly from GitUp Continuous Integration. Use at your own risks!" id="qB2-E4-Yuu">
<font key="font" metaFont="smallSystem"/>
Expand All @@ -303,7 +303,7 @@ You must close and reopen any opened repositories in GitUp after changing this s
</textFieldCell>
</textField>
<textField horizontalHuggingPriority="251" verticalHuggingPriority="750" id="w2N-io-xkS">
<rect key="frame" x="137" y="295" width="130" height="17"/>
<rect key="frame" x="137" y="236" width="130" height="17"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" scrollable="YES" lineBreakMode="clipping" allowsUndo="NO" sendsActionOnEndEditing="YES" alignment="left" title="Automatically check" id="X1v-BK-dAQ">
<font key="font" metaFont="system"/>
Expand All @@ -312,7 +312,7 @@ You must close and reopen any opened repositories in GitUp after changing this s
</textFieldCell>
</textField>
<popUpButton verticalHuggingPriority="750" id="ZYX-Yt-1fl">
<rect key="frame" x="271" y="289" width="145" height="26"/>
<rect key="frame" x="271" y="230" width="145" height="26"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<popUpButtonCell key="cell" type="push" title="Never" bezelStyle="rounded" alignment="left" lineBreakMode="truncatingTail" borderStyle="borderAndBezel" imageScaling="proportionallyDown" inset="2" autoenablesItems="NO" altersStateOfSelectedItem="NO" selectedItem="KgR-3b-gT5" id="Pdg-S3-YXJ">
<behavior key="behavior" lightByBackground="YES" lightByGray="YES"/>
Expand Down Expand Up @@ -408,6 +408,26 @@ You must close and reopen any opened repositories in GitUp after changing this s
</binding>
</connections>
</popUpButton>
<button id="Xxa-CO-ELY">
<rect key="frame" x="136" y="294" width="346" height="18"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<buttonCell key="cell" type="check" title="Allow quick confirmation of dangerous operations" bezelStyle="regularSquare" imagePosition="left" state="on" inset="2" id="uJq-iS-sPr">
<behavior key="behavior" changeContents="YES" doesNotDimImage="YES" lightByContents="YES"/>
<font key="font" metaFont="system"/>
</buttonCell>
<connections>
<binding destination="AR1-cq-KNa" name="value" keyPath="values.GIMapViewController_AllowReturnKeyForDangerousRemoteOperations" id="c8T-9g-4KN"/>
</connections>
</button>
<textField verticalHuggingPriority="750" horizontalCompressionResistancePriority="250" id="Ad9-ef-Jqv">
<rect key="frame" x="137" y="262" width="345" height="28"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/>
<textFieldCell key="cell" controlSize="small" sendsActionOnEndEditing="YES" alignment="left" title="This allows dangerous operations on remotes, like force push, to be confirmed by pressing the Return key in dialogs." id="LQY-Xb-bQ5">
<font key="font" metaFont="smallSystem"/>
<color key="textColor" name="secondaryLabelColor" catalog="System" colorSpace="catalog"/>
<color key="backgroundColor" name="controlColor" catalog="System" colorSpace="catalog"/>
</textFieldCell>
</textField>
</subviews>
</view>
</tabViewItem>
Expand Down
1 change: 1 addition & 0 deletions GitUp/GitUp.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@
indentWidth = 2;
sourceTree = "<group>";
tabWidth = 2;
usesTabs = 0;
};
E2C338AC19F8562F00063D95 /* Products */ = {
isa = PBXGroup;
Expand Down
3 changes: 2 additions & 1 deletion GitUpKit/Utilities/GIAppKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
typedef NS_ENUM(NSUInteger, GIAlertType) {
kGIAlertType_Note = 0,
kGIAlertType_Caution,
kGIAlertType_Stop
kGIAlertType_Stop,
kGIAlertType_Danger
};

extern NSString* const GICommitMessageViewUserDefaultKey_ShowInvisibleCharacters;
Expand Down
2 changes: 1 addition & 1 deletion GitUpKit/Utilities/GIAppKit.m
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ - (void)setType:(GIAlertType)type {
switch (type) {
case kGIAlertType_Note: self.icon = [[NSBundle bundleForClass:[GILayoutManager class]] imageForResource:@"icon_alert_note"]; break; // TODO: Image is not cached
case kGIAlertType_Caution: self.icon = [[NSBundle bundleForClass:[GILayoutManager class]] imageForResource:@"icon_alert_caution"]; break; // TODO: Image is not cached
case kGIAlertType_Stop: self.icon = [[NSBundle bundleForClass:[GILayoutManager class]] imageForResource:@"icon_alert_stop"]; break; // TODO: Image is not cached
case kGIAlertType_Stop: case kGIAlertType_Danger: self.icon = [[NSBundle bundleForClass:[GILayoutManager class]] imageForResource:@"icon_alert_stop"]; break; // TODO: Image is not cached
}
}

Expand Down
5 changes: 4 additions & 1 deletion GitUpKit/Utilities/GIViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,10 @@ - (void)confirmUserActionWithAlertType:(GIAlertType)type
alert.messageText = title;
alert.informativeText = message;
alert.showsSuppressionButton = key ? YES : NO;
[alert addButtonWithTitle:button];
NSButton* defaultButton = [alert addButtonWithTitle:button];
if (type == kGIAlertType_Danger) {
defaultButton.keyEquivalent = @"";
}
[alert addButtonWithTitle:NSLocalizedString(@"Cancel", nil)];
[self presentAlert:alert completionHandler:^(NSModalResponse returnCode) {

Expand Down
19 changes: 12 additions & 7 deletions GitUpKit/Views/GIMapViewController+Operations.m
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#define kUserDefaultsKey_SkipPushBranchWarning kUserDefaultsPrefix "SkipPushBranchWarning"
#define kUserDefaultsKey_SkipPushLocalBranchToRemoteWarning kUserDefaultsPrefix "SkipPushLocalBranchToRemoteWarning"
#define kUserDefaultsKey_SkipFetchRemoteBranchesWarning kUserDefaultsPrefix "SkipFetchRemoteBranchesWarning"
#define kUserDefaultsKey_AllowReturnKeyForDangerousRemoteOperations kUserDefaultsPrefix "AllowReturnKeyForDangerousRemoteOperations"

@interface GIMapViewController (Internal)
- (void)_promptForCommitMessage:(NSString*)message withTitle:(NSString*)title button:(NSString*)button block:(void (^)(NSString* message))block;
Expand All @@ -40,6 +41,10 @@ - (void)_promptForCommitMessage:(NSString*)message withTitle:(NSString*)title bu
return [message stringByTrimmingCharactersInSet:[NSCharacterSet newlineCharacterSet]];
}

static inline GIAlertType _AlertTypeForDangerousRemoteOperations() {
return ([[NSUserDefaults standardUserDefaults] boolForKey:kUserDefaultsKey_AllowReturnKeyForDangerousRemoteOperations] ? kGIAlertType_Stop : kGIAlertType_Danger);
}

@implementation GIMapViewController (Operations)

- (BOOL)_checkClean {
Expand Down Expand Up @@ -456,7 +461,7 @@ - (void)deleteLocalBranch:(GCHistoryLocalBranch*)branch {

}]) {
if ([upstream isKindOfClass:[GCHistoryRemoteBranch class]]) {
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"Do you also want to delete the upstream remote branch \"%@\" from its remote?", nil), upstream.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Delete Remote Branch", nil)
Expand Down Expand Up @@ -924,7 +929,7 @@ - (void)_pushLocalBranch:(GCHistoryLocalBranch*)branch toRemote:(GCRemote*)remot
}

} else if (!force && [error.domain isEqualToString:GCErrorDomain] && (error.code == kGCErrorCode_NonFastForward)) {
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"The branch \"%@\" could not be fast-forwarded on the remote \"%@\". Do you want to attempt to force push?", nil), branch.name, remote ? remote.name : upstreamRemote.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Force Push", nil)
Expand Down Expand Up @@ -978,7 +983,7 @@ - (void)_pushAllLocalBranchesToRemote:(GCRemote*)remote force:(BOOL)force {
if (success) {
[self.windowController showOverlayWithStyle:kGIOverlayStyle_Informational format:NSLocalizedString(@"All branches were pushed to the remote \"%@\" successfully!", nil), remote.name];
} else if ([error.domain isEqualToString:GCErrorDomain] && (error.code == kGCErrorCode_NonFastForward)) {
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"Some branches could not be fast-forwarded on the remote \"%@\". Do you want to attempt to force push?", nil), remote.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Force Push", nil)
Expand All @@ -1000,7 +1005,7 @@ - (void)pushAllLocalBranchesToAllRemotes {
NSArray* remotes = [self.repository listRemotes:&localError];
if (remotes.count <= 1) {
GCRemote* remote = remotes[0];
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"Are you sure you want to push all branches to the remote \"%@\"?", nil), remote.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Push All Branches", nil)
Expand Down Expand Up @@ -1056,7 +1061,7 @@ - (void)pushAllTagsToAllRemotes {
NSArray* remotes = [self.repository listRemotes:&localError];
if (remotes.count <= 1) {
GCRemote* remote = remotes[0];
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"Are you sure you want to push all tags to the remote \"%@\"?", nil), remote.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Push All Tags", nil)
Expand Down Expand Up @@ -1103,7 +1108,7 @@ - (void)_deleteRemoteBranchFromRemote:(GCHistoryRemoteBranch*)branch {

// TODO: Delete upstream(s) in config if needed and put on undo stack
- (void)deleteRemoteBranch:(GCHistoryRemoteBranch*)branch {
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"Are you sure you want to delete the remote branch \"%@\" from its remote?", nil), branch.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Delete Branch", nil)
Expand All @@ -1116,7 +1121,7 @@ - (void)deleteRemoteBranch:(GCHistoryRemoteBranch*)branch {
}

- (void)deleteTagFromAllRemotes:(GCHistoryTag*)tag {
[self confirmUserActionWithAlertType:kGIAlertType_Stop
[self confirmUserActionWithAlertType:_AlertTypeForDangerousRemoteOperations()
title:[NSString stringWithFormat:NSLocalizedString(@"Are you sure you want to delete the tag \"%@\" from all remotes?", nil), tag.name]
message:NSLocalizedString(@"This action cannot be undone.", nil)
button:NSLocalizedString(@"Delete Tag", nil)
Expand Down

0 comments on commit aa8b820

Please sign in to comment.