From b6d6e0d5c21d45a22b30d2b52a9a4d2a0121870d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:20:08 +0100 Subject: [PATCH 1/8] Bug fix: Correct dot notation misuse. As some Apple employees continue to advocate: Using dot notation for anything else than documented (!) synthesized properties and calling non-behavioural messages can be dangerous. Methods that look like setters/getters but aren't connected to a documented property may introduce private state change inside the class which can further bring all sorts of subtle and hard to debug problems. The Objective-C 2.0 Language Guide now also mentions this. --- PBGitCommitController.m | 2 +- PBGitIndex.m | 2 +- PBGitRevList.mm | 4 ++-- PBWebHistoryController.m | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/PBGitCommitController.m b/PBGitCommitController.m index e11c24d..11ee9f2 100644 --- a/PBGitCommitController.m +++ b/PBGitCommitController.m @@ -108,7 +108,7 @@ - (IBAction) commit:(id) sender { - if ([[NSFileManager defaultManager] fileExistsAtPath:[repository.fileURL.path stringByAppendingPathComponent:@"MERGE_HEAD"]]) { + if ([[NSFileManager defaultManager] fileExistsAtPath:[[[repository fileURL] path] stringByAppendingPathComponent:@"MERGE_HEAD"]]) { [[repository windowController] showMessageSheet:@"Cannot commit merges" infoText:@"GitX cannot commit merges yet. Please commit your changes from the command line."]; return; } diff --git a/PBGitIndex.m b/PBGitIndex.m index bd9bd56..5f2cdf2 100644 --- a/PBGitIndex.m +++ b/PBGitIndex.m @@ -155,7 +155,7 @@ NSString *PBGitIndexOperationFailed = @"PBGitIndexOperationFailed"; [commitSubject appendString:[commitMessage substringToIndex:newLine.location]]; NSString *commitMessageFile; - commitMessageFile = [repository.fileURL.path stringByAppendingPathComponent:@"COMMIT_EDITMSG"]; + commitMessageFile = [[[repository fileURL] path] stringByAppendingPathComponent:@"COMMIT_EDITMSG"]; [commitMessage writeToFile:commitMessageFile atomically:YES encoding:NSUTF8StringEncoding error:nil]; diff --git a/PBGitRevList.mm b/PBGitRevList.mm index a849de6..dc08df6 100644 --- a/PBGitRevList.mm +++ b/PBGitRevList.mm @@ -87,10 +87,10 @@ using namespace std; else [arguments addObjectsFromArray:[rev parameters]]; - NSString *directory = rev.workingDirectory ? rev.workingDirectory.path : repository.fileURL.path; + NSString *directory = rev.workingDirectory ? [rev.workingDirectory path] : [[repository fileURL] path]; NSTask *task = [PBEasyPipe taskForCommand:[PBGitBinary path] withArgs:arguments inDir:directory]; [task launch]; - NSFileHandle* handle = [task.standardOutput fileHandleForReading]; + NSFileHandle* handle = [[task standardOutput] fileHandleForReading]; int fd = [handle fileDescriptor]; __gnu_cxx::stdio_filebuf buf(fd, std::ios::in); diff --git a/PBWebHistoryController.m b/PBWebHistoryController.m index 1934b73..e8638be 100644 --- a/PBWebHistoryController.m +++ b/PBWebHistoryController.m @@ -112,7 +112,7 @@ contextMenuItemsForElement:(NSDictionary *)element // Every ref has a class name of 'refs' and some other class. We check on that to see if we pressed on a ref. if ([[node className] hasPrefix:@"refs "]) { NSString *selectedRefString = [[[node childNodes] item:0] textContent]; - for (PBGitRef *ref in historyController.webCommit.refs) + for (PBGitRef *ref in [historyController.webCommit refs]) { if ([[ref shortName] isEqualToString:selectedRefString]) return [contextMenuDelegate menuItemsForRef:ref commit:historyController.webCommit]; From 22ba33f5758408d83a56460f501f7eca087b5c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:22:53 +0100 Subject: [PATCH 2/8] Disable/re-enable Push, Pull and Rebase buttons depending on selected branch validity. If "All branches" or "Local branches" is selected, the buttons will be disabled. Actions from the context menu continue to work as they implicitly set the target branch through the identity of the clicked label patch. Any other branch will re-enable the buttons. --- PBRefController.h | 1 + PBRefController.m | 38 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/PBRefController.h b/PBRefController.h index c2f2f7d..d9312fd 100644 --- a/PBRefController.h +++ b/PBRefController.h @@ -46,6 +46,7 @@ - (BOOL) fetchImpl:(NSString *)refName; - (void) showMessageSheet:(NSString *)title message:(NSString *)msg; +- (void) toggleToolbarItems:(NSToolbar *)tb matchingLabels:(NSArray *)labels enabledState:(BOOL)state; @end diff --git a/PBRefController.m b/PBRefController.m index f04b17b..ab00320 100644 --- a/PBRefController.m +++ b/PBRefController.m @@ -182,6 +182,25 @@ return success; } +- (void) toggleToolbarItems:(NSToolbar *)tb matchingLabels:(NSArray *)labels enabledState:(BOOL)state { + NSArray * tbItems = [tb items]; + + /* if labels is nil, assume all toolbar items */ + if (!labels) { + for (NSToolbarItem * curItem in tbItems) { + [curItem setEnabled:state]; + } + } else { + for (NSToolbarItem * curItem in tbItems) { + for (NSString * curLabel in labels) { + if ([[curItem label] isEqualToString:curLabel]) { + [curItem setEnabled:state]; + } + } + } + } +} + # pragma mark Tableview delegate methods - (BOOL)tableView:(NSTableView *)tv writeRowsWithIndexes:(NSIndexSet *)rowIndexes toPasteboard:(NSPasteboard*)pboard @@ -479,8 +498,23 @@ - (void) selectCurrentBranch { PBGitRevSpecifier *rev = historyController.repository.currentBranch; - if (rev) - [branchPopUp setTitle:[rev description]]; + NSToolbar * tb = historyController.viewToolbar; + NSArray * tbLabels = [NSArray arrayWithObjects:@"Push", @"Pull", @"Rebase", nil]; + + if (rev) { + [branchPopUp setTitle:[rev description]]; + + if ([[rev description] isEqualToString:@"All branches"] || + [[rev description] isEqualToString:@"Local branches"]) + { + [self toggleToolbarItems:tb matchingLabels:tbLabels enabledState:NO]; + } else { + [self toggleToolbarItems:tb matchingLabels:tbLabels enabledState:YES]; + } + } else { + /* just in case, re-enable all toolbar buttons */ + [self toggleToolbarItems:tb matchingLabels:nil enabledState:YES]; + } } @end From 4cdf6348cae6089673893eb5930b1463c555dcab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:24:04 +0100 Subject: [PATCH 3/8] Replace pre-10.6 workaround with public - [NSSavePanel setShowsHiddenFiles:] method. --- PBPrefsWindowController.m | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/PBPrefsWindowController.m b/PBPrefsWindowController.m index 254adc4..7e9ee1a 100644 --- a/PBPrefsWindowController.m +++ b/PBPrefsWindowController.m @@ -44,7 +44,7 @@ [openPanel setAllowsMultipleSelection:NO]; [openPanel setTreatsFilePackagesAsDirectories:YES]; [openPanel setAccessoryView:gitPathOpenAccessory]; - //[[openPanel _navView] setShowsHiddenFiles:YES]; + //[openPanel setShowsHiddenFiles:YES]; gitPathOpenPanel = openPanel; } @@ -54,9 +54,10 @@ - (IBAction) showHideAllFiles: sender { - /* FIXME: This uses undocumented OpenPanel features to show hidden files! */ - NSNumber *showHidden = [NSNumber numberWithBool:[sender state] == NSOnState]; - [[gitPathOpenPanel valueForKey:@"_navView"] setValue:showHidden forKey:@"showsHiddenFiles"]; + //NSNumber *showHidden = [NSNumber numberWithBool:[sender state] == NSOnState]; + //[[gitPathOpenPanel valueForKey:@"_navView"] setValue:showHidden forKey:@"showsHiddenFiles"]; + BOOL showHidden = ([sender state] == NSOnState); + [gitPathOpenPanel setShowsHiddenFiles:showHidden]; } @end From 1a7b4cb4c78e95b24cf356ca7a0c2f9a4116e8f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:26:07 +0100 Subject: [PATCH 4/8] Update branch menu after deletion of a branch. --- PBRefController.m | 1 + 1 file changed, 1 insertion(+) diff --git a/PBRefController.m b/PBRefController.m index ab00320..084d5be 100644 --- a/PBRefController.m +++ b/PBRefController.m @@ -52,6 +52,7 @@ [historyController.repository removeBranch:[[PBGitRevSpecifier alloc] initWithRef:[sender ref]]]; [[sender commit] removeRef:[sender ref]]; [commitController rearrangeObjects]; + [self updateBranchMenu]; } } From 1ade0f0620271d752ec1192f4e506b9f0e38584d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:28:50 +0100 Subject: [PATCH 5/8] Remove completely wrong TODO comment. A deletion execution dialog should block the UI until it's complete if there could potentionally be state change occurring which has different outcome for pre-deletion or post-deletion finite state. A middle ground solution would be to use a modal sheet attached to a specific document which then only blocks this particular document. --- PBRefController.m | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/PBRefController.m b/PBRefController.m index 084d5be..58f65dc 100644 --- a/PBRefController.m +++ b/PBRefController.m @@ -40,8 +40,7 @@ NSString *ref_desc = [NSString stringWithFormat:@"%@ %@", [[sender ref] type], [[sender ref] shortName]]; NSString *question = [NSString stringWithFormat:@"Are you sure you want to remove the %@?", ref_desc]; int choice = NSRunAlertPanel([NSString stringWithFormat:@"Delete %@?", ref_desc], question, @"Delete", @"Cancel", nil); - // TODO: Use a non-modal alert here, so we don't block all the GitX windows - + if(choice) { int ret = 1; [historyController.repository outputForArguments:[NSArray arrayWithObjects:@"update-ref", @"-d", [[sender ref] ref], nil] retValue: &ret]; From 2c6fd97b5c61ed4ce3f93ff2a2476b5ee91ec32d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:30:18 +0100 Subject: [PATCH 6/8] Cosmetic: Move showMessageSheet convenience method to the proper pragma mark section. --- PBRefController.m | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/PBRefController.m b/PBRefController.m index 58f65dc..62a9d57 100644 --- a/PBRefController.m +++ b/PBRefController.m @@ -131,7 +131,8 @@ [self showMessageSheet:@"Pull from Remote" message:PBMissingRemoteErrorMessage]; return success; } - NSString *rval = [historyController.repository outputInWorkdirForArguments:[NSArray arrayWithObjects:@"pull", remote, refName, nil] retValue: &ret]; + NSArray * args = [NSArray arrayWithObjects:@"pull", remote, refName, nil]; + NSString *rval = [historyController.repository outputInWorkdirForArguments:args retValue: &ret]; if (ret) { NSString *info = [NSString stringWithFormat:@"There was an error pulling from the remote repository.\n\n%d\n%@", ret, rval]; [[historyController.repository windowController] showMessageSheet:@"Pulling from remote failed" infoText:info]; @@ -149,7 +150,7 @@ BOOL success = NO; NSString *remote = [[[historyController repository] config] valueForKeyPath:[NSString stringWithFormat:@"branch.%@.remote", refName]]; if (!remote) { - [self showMessageSheet:@"Pull Rebase from Remote" message:PBMissingRemoteErrorMessage]; + [self showMessageSheet:@"Pull from Remote and Rebase" message:PBMissingRemoteErrorMessage]; return success; } NSString *rval = [[historyController repository] outputInWorkdirForArguments:[NSArray arrayWithObjects:@"pull", @"--rebase", remote, refName, nil] retValue: &ret]; @@ -283,6 +284,8 @@ } # pragma mark Add ref methods + + -(void)addRef:(id)sender { [errorMessage setStringValue:@""]; @@ -293,21 +296,7 @@ contextInfo:NULL]; } -- (void) showMessageSheet:(NSString *)title message:(NSString *)msg { - - [[NSAlert alertWithMessageText:title - defaultButton:@"OK" - alternateButton:nil - otherButton:nil - informativeTextWithFormat:msg] - beginSheetModalForWindow:[[historyController view] window] - modalDelegate:self - didEndSelector:nil - contextInfo:nil]; - - return; -} - +// MARK: Buttons -(void)rebaseButton:(id)sender { NSString *refName = [[[[historyController repository] currentBranch] simpleRef] refForSpec]; @@ -354,6 +343,23 @@ // NSLog([NSString stringWithFormat:@"Fetch hit for %@!", refName]); } +// MARK: Sheets + +- (void) showMessageSheet:(NSString *)title message:(NSString *)msg { + + [[NSAlert alertWithMessageText:title + defaultButton:@"OK" + alternateButton:nil + otherButton:nil + informativeTextWithFormat:msg] + beginSheetModalForWindow:[[historyController view] window] + modalDelegate:self + didEndSelector:nil + contextInfo:nil]; + + return; +} + -(void)saveSheet:(id) sender { NSString *branchName = [@"refs/heads/" stringByAppendingString:[newBranchName stringValue]]; From 12f26bc36f2f08fecdd8afd093c5e61a95882211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Sun, 8 Nov 2009 16:33:38 +0100 Subject: [PATCH 7/8] Update height for split view partials. Update toolbar item sizes to work against the visual inconsistency where the items where off by 1 pixel causing toolbar jumps between history and commit view. --- PBGitHistoryView.xib | 34 +++++++++++++++++----------------- RepositoryWindow.xib | 38 +++++++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/PBGitHistoryView.xib b/PBGitHistoryView.xib index 27b1680..a8972b7 100644 --- a/PBGitHistoryView.xib +++ b/PBGitHistoryView.xib @@ -22,7 +22,7 @@ YES - + @@ -168,7 +168,7 @@ 256 - {852, 228} + {852, 308} YES @@ -352,7 +352,7 @@ 0 - {{0, 17}, {852, 228}} + {{0, 17}, {852, 308}} @@ -395,7 +395,7 @@ - {852, 245} + {852, 325} 560 @@ -409,7 +409,7 @@ 274 - {{0, 246}, {852, 152}} + {{0, 326}, {852, 72}} YES @@ -507,7 +507,7 @@ 256 - {124, 152} + {124, 72} YES @@ -561,7 +561,7 @@ 0 - {{1, 1}, {124, 152}} + {{1, 1}, {124, 72}} @@ -571,11 +571,11 @@ 256 - {{125, 1}, {15, 152}} + {{125, 1}, {15, 72}} _doScroller: - 0.94736842105263153 + 0.73684210526315785 @@ -589,7 +589,7 @@ 0.99801189999999995 - {141, 154} + {141, 74} 18 @@ -611,7 +611,7 @@ 2322 - {610, 98} + {610, 70} @@ -803,7 +803,7 @@ - {{1, 1}, {693, 152}} + {{1, 1}, {693, 72}} @@ -817,7 +817,7 @@ 256 - {{694, 1}, {15, 152}} + {{694, 1}, {15, 72}} _doScroller: @@ -835,7 +835,7 @@ 0.94565220000000005 - {{142, 0}, {710, 154}} + {{142, 0}, {710, 74}} 18 @@ -844,13 +844,13 @@ - {852, 154} + {852, 74} YES 2 - {852, 152} + {852, 72} Tree @@ -3131,7 +3131,7 @@ TG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA PBGitRevisionCell com.apple.InterfaceBuilder.CocoaPlugin - {{841, 929}, {616, 227}} + {{617, 880}, {616, 227}} com.apple.InterfaceBuilder.CocoaPlugin {{132, 614}, {616, 0}} com.apple.InterfaceBuilder.CocoaPlugin diff --git a/RepositoryWindow.xib b/RepositoryWindow.xib index c8a5253..aa09779 100644 --- a/RepositoryWindow.xib +++ b/RepositoryWindow.xib @@ -81,9 +81,11 @@ View selector - + 268 {{0, 14}, {87, 25}} + + 3 YES @@ -141,9 +143,11 @@ Clone Clone a repository - + 268 {{1, 14}, {36, 25}} + + YES -2080244224 @@ -184,9 +188,11 @@ - + 265 {{0, 14}, {183, 22}} + + YES 343014976 @@ -279,9 +285,11 @@ Create Branch - + 268 {{21, 14}, {40, 25}} + + YES -2080244224 @@ -320,9 +328,11 @@ Branch - + 268 {{0, 14}, {134, 26}} + + YES -2076049856 @@ -411,9 +421,11 @@ ZW1vdGUgY29uZmlndXJlZCBmb3IgdGhlIGN1cnJlbnRseSBzZWxlY3RlZCBicmFuY2ggZnJvbSB0aGUg YnJhbmNoIHBvcHVwIG1lbnUuIAoKTm90ZTogdGhlIGJyYW5jaCBoYXMgdG8gYmUgc3BlY2lmaWMuIAon QWxsIGJyYW5jaGVzJyBvciAnTG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA - + 268 {{0, 14}, {35, 25}} + + YES -2080244224 @@ -456,9 +468,11 @@ bGVjdGVkIGJyYW5jaCBmcm9tIHRoZSBicmFuY2ggcG9wdXAgbWVudQoKTm90ZTogdGhlIGJyYW5jaCBo YXMgdG8gYmUgc3BlY2lmaWMuIAonQWxsIGJyYW5jaGVzJyBvciAnTG9jYWwgYnJhbmNoZXMnIHdpbGwg bm90IHdvcms - + 268 {{0, 14}, {35, 25}} + + YES -2080244224 @@ -505,9 +519,11 @@ ZXMgdGhhdCBpbmZvcm1hdGlvbiB0byBhdm9pZCByZWJhc2luZyBub24tbG9jYWwgY2hhbmdlcwoKTm90 ZTogdGhlIGxvY2FsIGJyYW5jaCBoYXMgdG8gYmUgc3BlY2lmaWMuIAonQWxsIGJyYW5jaGVzJyBvciAn TG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA - + 268 {{5, 14}, {35, 25}} + + YES -2080244224 @@ -1006,9 +1022,9 @@ TG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA YES com.apple.InterfaceBuilder.CocoaPlugin - {{180, 303}, {850, 550}} + {{519, 294}, {850, 550}} com.apple.InterfaceBuilder.CocoaPlugin - {{180, 303}, {850, 550}} + {{519, 294}, {850, 550}} {{15, 196}, {850, 418}} @@ -1016,7 +1032,7 @@ TG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA {3.40282e+38, 3.40282e+38} {213, 107} - {{297, 853}, {616, 0}} + {{636, 844}, {616, 0}} com.apple.InterfaceBuilder.CocoaPlugin {{132, 614}, {616, 0}} com.apple.InterfaceBuilder.CocoaPlugin From 37dcdf2c11795c3fe697e297594b55a0cb3ae2f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Berg?= Date: Mon, 9 Nov 2009 00:03:21 +0100 Subject: [PATCH 8/8] Adjust split view height of the commit webview. --- PBGitHistoryView.xib | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/PBGitHistoryView.xib b/PBGitHistoryView.xib index a8972b7..d902642 100644 --- a/PBGitHistoryView.xib +++ b/PBGitHistoryView.xib @@ -21,9 +21,9 @@ YES - - + + YES @@ -366,18 +366,18 @@ _doScroller: - 37 - 0.19473679999999999 + 0.95129870129870131 -2147483392 - {{0, 196}, {837, 15}} + {{0, 310}, {852, 15}} + YES 1 _doScroller: - 0.21932109999999999 + 0.9988276670574443 @@ -398,7 +398,7 @@ {852, 325} - 560 + 688 @@ -611,7 +611,7 @@ 2322 - {610, 70} + {610, 98} @@ -819,9 +819,10 @@ 256 {{694, 1}, {15, 72}} + YES _doScroller: - 0.96938775510204078 + 0.73469387755102045 @@ -838,7 +839,7 @@ {{142, 0}, {710, 74}} - 18 + 82 @@ -925,7 +926,7 @@ 15 2 - {{196, 408}, {346, 102}} + {{196, 408}, {380, 102}} 603979776 New Branch Sheet NSWindow @@ -940,7 +941,7 @@ 266 - {{177, 60}, {149, 22}} + {{144, 60}, {216, 22}} YES @@ -968,13 +969,13 @@ 268 - {{17, 62}, {155, 17}} + {{17, 62}, {122, 17}} YES 68288064 272630784 - Create a branch named: + New branch name: @@ -984,7 +985,7 @@ 289 - {{236, 12}, {96, 32}} + {{270, 12}, {96, 32}} YES @@ -1004,7 +1005,7 @@ 289 - {{140, 12}, {96, 32}} + {{174, 12}, {96, 32}} YES @@ -1041,7 +1042,7 @@ - {346, 102} + {380, 102} {{0, 0}, {1680, 1028}} @@ -2510,10 +2511,10 @@ TG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA YES - + @@ -3085,9 +3086,9 @@ TG9jYWwgYnJhbmNoZXMnIHdpbGwgbm90IHdvcmsuA com.apple.WebKitIBPlugin - {{350, 207}, {346, 102}} + {{394, 535}, {380, 102}} com.apple.InterfaceBuilder.CocoaPlugin - {{350, 207}, {346, 102}} + {{394, 535}, {380, 102}}