From 33991ec19036d955252cb83eba88936d1699867b Mon Sep 17 00:00:00 2001 From: Pieter de Bie Date: Thu, 28 May 2009 16:28:52 +0100 Subject: [PATCH 1/5] GitCommitController: clean up index functions This adds some comments in the right place and makes the code somewhat easier to follow. --- PBGitCommitController.m | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/PBGitCommitController.m b/PBGitCommitController.m index 9297ca6..7f33394 100644 --- a/PBGitCommitController.m +++ b/PBGitCommitController.m @@ -91,24 +91,26 @@ - (void) refresh:(id) sender { + if (![repository workingDirectory]) + return; + + // Mark all files for deletion. We'll undo the files we want to keep later for (PBChangedFile *file in files) file.shouldBeDeleted = YES; self.status = @"Refreshing index…"; - if (![repository workingDirectory]) { - //if ([[repository outputForCommand:@"rev-parse --is-inside-work-tree"] isEqualToString:@"false"]) { - return; - } - self.busy++; + // If self.busy reaches 0, all tasks have finished + self.busy = 0; + // Refresh the index, necessary for the next methods (that's why it's blocking) + // FIXME: Make this non-blocking. This call can be expensive in large repositories [repository outputInWorkdirForArguments:[NSArray arrayWithObjects:@"update-index", @"-q", @"--unmerged", @"--ignore-missing", @"--refresh", nil]]; - self.busy--; NSNotificationCenter *nc = [NSNotificationCenter defaultCenter]; [nc removeObserver:self]; - // Other files + // Other files (not tracked, not ignored) NSArray *arguments = [NSArray arrayWithObjects:@"ls-files", @"--others", @"--exclude-standard", @"-z", nil]; NSFileHandle *handle = [repository handleInWorkDirForArguments:arguments]; [nc addObserver:self selector:@selector(readOtherFiles:) name:NSFileHandleReadToEndOfFileCompletionNotification object:handle]; @@ -121,7 +123,7 @@ self.busy++; [handle readToEndOfFileInBackgroundAndNotify]; - // Cached files + // Staged files handle = [repository handleInWorkDirForArguments:[NSArray arrayWithObjects:@"diff-index", @"--cached", @"-z", [self parentTree], nil]]; [nc addObserver:self selector:@selector(readCachedFiles:) name:NSFileHandleReadToEndOfFileCompletionNotification object:handle]; self.busy++; @@ -133,6 +135,9 @@ [self refresh:nil]; } +// This method is called for each of the three processes from above. +// If all three are finished (self.busy == 0), then we can delete +// all files previously marked as deletable - (void) doneProcessingIndex { [self willChangeValueForKey:@"files"]; @@ -157,6 +162,7 @@ BOOL added = NO; // Check if file is already in our index + // FIXME: this is O(N^2) for (PBChangedFile *file in files) { if ([[file path] isEqualToString:line]) { file.shouldBeDeleted = NO; @@ -341,12 +347,16 @@ if (reverse) [array addObject:@"--reverse"]; - int ret; + int ret = 1; NSString *error = [repository outputForArguments:array inputString:hunk - retValue: &ret]; + retValue:&ret]; + + // FIXME: show this error, rather than just logging it if (ret) NSLog(@"Error: %@", error); - [self refresh:self]; // TODO: We should do this smarter by checking if the file diff is empty, which is faster. + + // TODO: We should do this smarter by checking if the file diff is empty, which is faster. + [self refresh:self]; } @end From cf80a6aa09900cc739639ef244770b9865b7eb7a Mon Sep 17 00:00:00 2001 From: Pieter de Bie Date: Thu, 28 May 2009 16:58:20 +0100 Subject: [PATCH 2/5] CommitController: Use a dictionary lookup when refreshing index This way, we can accurately change the status of all existing files, and keep the same files. Now, what is needed is a way to do the same for the 'other changes', and then we can remove the shouldBeRemoved stuff --- PBGitCommitController.m | 65 ++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/PBGitCommitController.m b/PBGitCommitController.m index 7f33394..8407e99 100644 --- a/PBGitCommitController.m +++ b/PBGitCommitController.m @@ -191,42 +191,54 @@ - (void) addFilesFromLines:(NSArray *)lines cached:(BOOL) cached { + NSMutableDictionary *dictionary = [NSMutableDictionary dictionaryWithCapacity:[lines count]/2]; + + // Fill the dictionary with the new information NSArray *fileStatus; - int even = 0; + BOOL even = FALSE; for (NSString *line in lines) { if (!even) { - even = 1; + even = TRUE; fileStatus = [line componentsSeparatedByString:@" "]; continue; } - even = 0; + even = FALSE; + [dictionary setObject:fileStatus forKey:line]; + } + + // Iterate over all existing files + for (PBChangedFile *file in files) { + NSArray *fileStatus = [dictionary objectForKey:file.path]; + // Object found, this is still a cached / uncached thing + if (fileStatus) { + NSString *mode = [[fileStatus objectAtIndex:0] substringFromIndex:1]; + NSString *sha = [fileStatus objectAtIndex:2]; + file.shouldBeDeleted = NO; + + if (cached) { + file.hasCachedChanges = YES; + file.commitBlobSHA = sha; + file.commitBlobMode = mode; + } else + file.hasUnstagedChanges = YES; + + [dictionary removeObjectForKey:file.path]; + } else { // Object not found, let's remove it from the changes + if (cached) + file.hasCachedChanges = NO; + else if (file.status != NEW) + file.hasUnstagedChanges = NO; + } + } + + // Do new files + for (NSString *path in [dictionary allKeys]) { + NSArray *fileStatus = [dictionary objectForKey:path]; NSString *mode = [[fileStatus objectAtIndex:0] substringFromIndex:1]; NSString *sha = [fileStatus objectAtIndex:2]; - BOOL isNew = YES; - // If the file is already added, we shouldn't add it again - // but rather update it to incorporate our changes - for (PBChangedFile *file in files) { - if ([file.path isEqualToString:line]) { - if (cached && file.hasCachedChanges) { // if we're listing cached files - file.shouldBeDeleted = NO; // and the matching file in files had cached changes - file.commitBlobSHA = sha; // we don't delete it - file.commitBlobMode = mode; - isNew = NO; - break; - } else if ((!cached) && file.hasUnstagedChanges) { // if we're listing unstaged files and the - file.shouldBeDeleted = NO; // matching file in files had unstaged changes - isNew = NO; // we don't delete it - break; - } - } - } - - if (!isNew) - continue; - - PBChangedFile *file = [[PBChangedFile alloc] initWithPath:line]; + PBChangedFile *file = [[PBChangedFile alloc] initWithPath:path]; if ([[fileStatus objectAtIndex:4] isEqualToString:@"D"]) file.status = DELETED; else if([[fileStatus objectAtIndex:0] isEqualToString:@":000000"]) @@ -242,7 +254,6 @@ [files addObject: file]; } - } - (void) readUnstagedFiles:(NSNotification *)notification From a6b7c0c2a6b3b682429a5c0c7d86e29af4c46397 Mon Sep 17 00:00:00 2001 From: Pieter de Bie Date: Thu, 28 May 2009 17:36:24 +0100 Subject: [PATCH 3/5] GitCommitController: Also use dictionary lookup for untracked files We fake the index entry for the untracked files, and then use the same method as we used earlier for adding the files. This allows us to delete untracked files without problem, and is generally a nicer implementation. --- PBGitCommitController.m | 99 ++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 51 deletions(-) diff --git a/PBGitCommitController.m b/PBGitCommitController.m index 8407e99..282a59a 100644 --- a/PBGitCommitController.m +++ b/PBGitCommitController.m @@ -11,6 +11,14 @@ #import "PBChangedFile.h" #import "PBWebChangesController.h" + +@interface PBGitCommitController (PrivateMethods) +- (NSArray *) linesFromNotification:(NSNotification *)notification; +- (void) doneProcessingIndex; +- (NSMutableDictionary *)dictionaryForLines:(NSArray *)lines; +- (void) addFilesFromDictionary:(NSMutableDictionary *)dictionary cached:(BOOL)cached tracked:(BOOL)tracked; +@end + @implementation PBGitCommitController @synthesize files, status, busy, amend; @@ -94,10 +102,6 @@ if (![repository workingDirectory]) return; - // Mark all files for deletion. We'll undo the files we want to keep later - for (PBChangedFile *file in files) - file.shouldBeDeleted = YES; - self.status = @"Refreshing index…"; // If self.busy reaches 0, all tasks have finished @@ -143,10 +147,11 @@ [self willChangeValueForKey:@"files"]; if (!--self.busy) { self.status = @"Ready"; - NSArray *filesToBeDeleted = [files filteredArrayUsingPredicate:[NSPredicate predicateWithFormat:@"shouldBeDeleted == 1"]]; - for (PBChangedFile *file in filesToBeDeleted) { + for (PBChangedFile *file in files) { + if (!file.hasCachedChanges && !file.hasUnstagedChanges) { NSLog(@"Deleting file: %@", [file path]); [files removeObject:file]; + } } } [self didChangeValueForKey:@"files"]; @@ -156,40 +161,19 @@ { [unstagedFilesController setAutomaticallyRearrangesObjects:NO]; NSArray *lines = [self linesFromNotification:notification]; - for (NSString *line in lines) { - if ([line length] == 0) + NSMutableDictionary *dictionary = [[NSMutableDictionary alloc] initWithCapacity:[lines count]]; + // We fake this files status as good as possible. + NSArray *fileStatus = [NSArray arrayWithObjects:@":000000", @"100644", @"0000000000000000000000000000000000000000", @"0000000000000000000000000000000000000000", @"A", nil]; + for (NSString *path in lines) { + if ([path length] == 0) continue; - - BOOL added = NO; - // Check if file is already in our index - // FIXME: this is O(N^2) - for (PBChangedFile *file in files) { - if ([[file path] isEqualToString:line]) { - file.shouldBeDeleted = NO; - added = YES; - file.status = NEW; - file.hasCachedChanges = NO; - file.hasUnstagedChanges = YES; - break; - } - } - - if (added) - continue; - - // File does not exist yet, so add it - PBChangedFile *file =[[PBChangedFile alloc] initWithPath:line]; - file.status = NEW; - file.hasCachedChanges = NO; - file.hasUnstagedChanges = YES; - [files addObject: file]; + [dictionary setObject:fileStatus forKey:path]; } - [unstagedFilesController setAutomaticallyRearrangesObjects:YES]; - [unstagedFilesController rearrangeObjects]; + [self addFilesFromDictionary:dictionary cached:NO tracked:NO]; [self doneProcessingIndex]; } -- (void) addFilesFromLines:(NSArray *)lines cached:(BOOL) cached +- (NSMutableDictionary *)dictionaryForLines:(NSArray *)lines { NSMutableDictionary *dictionary = [NSMutableDictionary dictionaryWithCapacity:[lines count]/2]; @@ -206,28 +190,39 @@ even = FALSE; [dictionary setObject:fileStatus forKey:line]; } + return dictionary; +} +- (void) addFilesFromDictionary:(NSMutableDictionary *)dictionary cached:(BOOL)cached tracked:(BOOL)tracked +{ // Iterate over all existing files for (PBChangedFile *file in files) { NSArray *fileStatus = [dictionary objectForKey:file.path]; // Object found, this is still a cached / uncached thing if (fileStatus) { - NSString *mode = [[fileStatus objectAtIndex:0] substringFromIndex:1]; - NSString *sha = [fileStatus objectAtIndex:2]; - file.shouldBeDeleted = NO; + if (tracked) { + NSString *mode = [[fileStatus objectAtIndex:0] substringFromIndex:1]; + NSString *sha = [fileStatus objectAtIndex:2]; - if (cached) { - file.hasCachedChanges = YES; - file.commitBlobSHA = sha; - file.commitBlobMode = mode; - } else + if (cached) { + file.hasCachedChanges = YES; + file.commitBlobSHA = sha; + file.commitBlobMode = mode; + } else + file.hasUnstagedChanges = YES; + } else { + // Untracked file, set status to NEW, only unstaged changes + file.hasCachedChanges = NO; file.hasUnstagedChanges = YES; - + file.status = NEW; + } [dictionary removeObjectForKey:file.path]; } else { // Object not found, let's remove it from the changes if (cached) file.hasCachedChanges = NO; - else if (file.status != NEW) + else if (tracked && file.status != NEW) // Only remove it if it's not an untracked file. We handle that with the other thing + file.hasUnstagedChanges = NO; + else if (!tracked && file.status == NEW) file.hasUnstagedChanges = NO; } } @@ -235,8 +230,6 @@ // Do new files for (NSString *path in [dictionary allKeys]) { NSArray *fileStatus = [dictionary objectForKey:path]; - NSString *mode = [[fileStatus objectAtIndex:0] substringFromIndex:1]; - NSString *sha = [fileStatus objectAtIndex:2]; PBChangedFile *file = [[PBChangedFile alloc] initWithPath:path]; if ([[fileStatus objectAtIndex:4] isEqualToString:@"D"]) @@ -246,8 +239,10 @@ else file.status = MODIFIED; - file.commitBlobSHA = sha; - file.commitBlobMode = mode; + if (cached) { + file.commitBlobSHA = [[fileStatus objectAtIndex:0] substringFromIndex:1]; + file.commitBlobMode = [fileStatus objectAtIndex:2]; + } file.hasCachedChanges = cached; file.hasUnstagedChanges = !cached; @@ -259,14 +254,16 @@ - (void) readUnstagedFiles:(NSNotification *)notification { NSArray *lines = [self linesFromNotification:notification]; - [self addFilesFromLines:lines cached:NO]; + NSMutableDictionary *dic = [self dictionaryForLines:lines]; + [self addFilesFromDictionary:dic cached:NO tracked:YES]; [self doneProcessingIndex]; } - (void) readCachedFiles:(NSNotification *)notification { NSArray *lines = [self linesFromNotification:notification]; - [self addFilesFromLines:lines cached:YES]; + NSMutableDictionary *dic = [self dictionaryForLines:lines]; + [self addFilesFromDictionary:dic cached:YES tracked:YES]; [self doneProcessingIndex]; } From f9ff15cc6bf3c807425321581002f676985dd222 Mon Sep 17 00:00:00 2001 From: Pieter de Bie Date: Thu, 28 May 2009 17:36:40 +0100 Subject: [PATCH 4/5] PBChangedFile: remove shouldBeDeleted boolean We don't need this anymore --- PBChangedFile.h | 3 +-- PBChangedFile.m | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/PBChangedFile.h b/PBChangedFile.h index f4d53fd..7453db3 100644 --- a/PBChangedFile.h +++ b/PBChangedFile.h @@ -19,7 +19,6 @@ typedef enum { NSString *path; BOOL hasCachedChanges; BOOL hasUnstagedChanges; - BOOL shouldBeDeleted; // Index and HEAD stuff, to be used to revert changes NSString *commitBlobSHA; @@ -31,7 +30,7 @@ typedef enum { @property (copy) NSString *path, *commitBlobSHA, *commitBlobMode; @property (assign) PBChangedFileStatus status; -@property (assign) BOOL hasCachedChanges, hasUnstagedChanges, shouldBeDeleted; +@property (assign) BOOL hasCachedChanges, hasUnstagedChanges; - (NSImage *)icon; - (NSString *)indexInfo; diff --git a/PBChangedFile.m b/PBChangedFile.m index bbea198..e2bb7e8 100644 --- a/PBChangedFile.m +++ b/PBChangedFile.m @@ -11,7 +11,7 @@ @implementation PBChangedFile -@synthesize path, status, hasCachedChanges, hasUnstagedChanges, commitBlobSHA, commitBlobMode, shouldBeDeleted; +@synthesize path, status, hasCachedChanges, hasUnstagedChanges, commitBlobSHA, commitBlobMode; - (id) initWithPath:(NSString *)p { From 0d8ba8c2632e129fe7627f86d907c8b470c1bf20 Mon Sep 17 00:00:00 2001 From: Pieter de Bie Date: Thu, 28 May 2009 17:40:17 +0100 Subject: [PATCH 5/5] Rename 'CachedChanges" to "StagedChanges" for greater consistency --- PBChangedFile.h | 4 ++-- PBChangedFile.m | 2 +- PBGitCommitController.m | 30 +++++++++++++++--------------- PBGitIndexController.m | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/PBChangedFile.h b/PBChangedFile.h index 7453db3..bffc110 100644 --- a/PBChangedFile.h +++ b/PBChangedFile.h @@ -17,7 +17,7 @@ typedef enum { @interface PBChangedFile : NSObject { NSString *path; - BOOL hasCachedChanges; + BOOL hasStagedChanges; BOOL hasUnstagedChanges; // Index and HEAD stuff, to be used to revert changes @@ -30,7 +30,7 @@ typedef enum { @property (copy) NSString *path, *commitBlobSHA, *commitBlobMode; @property (assign) PBChangedFileStatus status; -@property (assign) BOOL hasCachedChanges, hasUnstagedChanges; +@property (assign) BOOL hasStagedChanges, hasUnstagedChanges; - (NSImage *)icon; - (NSString *)indexInfo; diff --git a/PBChangedFile.m b/PBChangedFile.m index e2bb7e8..4cbeabb 100644 --- a/PBChangedFile.m +++ b/PBChangedFile.m @@ -11,7 +11,7 @@ @implementation PBChangedFile -@synthesize path, status, hasCachedChanges, hasUnstagedChanges, commitBlobSHA, commitBlobMode; +@synthesize path, status, hasStagedChanges, hasUnstagedChanges, commitBlobSHA, commitBlobMode; - (id) initWithPath:(NSString *)p { diff --git a/PBGitCommitController.m b/PBGitCommitController.m index 282a59a..a546881 100644 --- a/PBGitCommitController.m +++ b/PBGitCommitController.m @@ -16,7 +16,7 @@ - (NSArray *) linesFromNotification:(NSNotification *)notification; - (void) doneProcessingIndex; - (NSMutableDictionary *)dictionaryForLines:(NSArray *)lines; -- (void) addFilesFromDictionary:(NSMutableDictionary *)dictionary cached:(BOOL)cached tracked:(BOOL)tracked; +- (void) addFilesFromDictionary:(NSMutableDictionary *)dictionary staged:(BOOL)staged tracked:(BOOL)tracked; @end @implementation PBGitCommitController @@ -32,7 +32,7 @@ [commitMessageView setTypingAttributes:[NSDictionary dictionaryWithObject:[NSFont fontWithName:@"Monaco" size:12.0] forKey:NSFontAttributeName]]; [unstagedFilesController setFilterPredicate:[NSPredicate predicateWithFormat:@"hasUnstagedChanges == 1"]]; - [cachedFilesController setFilterPredicate:[NSPredicate predicateWithFormat:@"hasCachedChanges == 1"]]; + [cachedFilesController setFilterPredicate:[NSPredicate predicateWithFormat:@"hasStagedChanges == 1"]]; [unstagedFilesController setSortDescriptors:[NSArray arrayWithObjects: [[NSSortDescriptor alloc] initWithKey:@"status" ascending:false], @@ -148,7 +148,7 @@ if (!--self.busy) { self.status = @"Ready"; for (PBChangedFile *file in files) { - if (!file.hasCachedChanges && !file.hasUnstagedChanges) { + if (!file.hasStagedChanges && !file.hasUnstagedChanges) { NSLog(@"Deleting file: %@", [file path]); [files removeObject:file]; } @@ -169,7 +169,7 @@ continue; [dictionary setObject:fileStatus forKey:path]; } - [self addFilesFromDictionary:dictionary cached:NO tracked:NO]; + [self addFilesFromDictionary:dictionary staged:NO tracked:NO]; [self doneProcessingIndex]; } @@ -193,7 +193,7 @@ return dictionary; } -- (void) addFilesFromDictionary:(NSMutableDictionary *)dictionary cached:(BOOL)cached tracked:(BOOL)tracked +- (void) addFilesFromDictionary:(NSMutableDictionary *)dictionary staged:(BOOL)staged tracked:(BOOL)tracked { // Iterate over all existing files for (PBChangedFile *file in files) { @@ -204,22 +204,22 @@ NSString *mode = [[fileStatus objectAtIndex:0] substringFromIndex:1]; NSString *sha = [fileStatus objectAtIndex:2]; - if (cached) { - file.hasCachedChanges = YES; + if (staged) { + file.hasStagedChanges = YES; file.commitBlobSHA = sha; file.commitBlobMode = mode; } else file.hasUnstagedChanges = YES; } else { // Untracked file, set status to NEW, only unstaged changes - file.hasCachedChanges = NO; + file.hasStagedChanges = NO; file.hasUnstagedChanges = YES; file.status = NEW; } [dictionary removeObjectForKey:file.path]; } else { // Object not found, let's remove it from the changes - if (cached) - file.hasCachedChanges = NO; + if (staged) + file.hasStagedChanges = NO; else if (tracked && file.status != NEW) // Only remove it if it's not an untracked file. We handle that with the other thing file.hasUnstagedChanges = NO; else if (!tracked && file.status == NEW) @@ -239,13 +239,13 @@ else file.status = MODIFIED; - if (cached) { + if (staged) { file.commitBlobSHA = [[fileStatus objectAtIndex:0] substringFromIndex:1]; file.commitBlobMode = [fileStatus objectAtIndex:2]; } - file.hasCachedChanges = cached; - file.hasUnstagedChanges = !cached; + file.hasStagedChanges = staged; + file.hasUnstagedChanges = !staged; [files addObject: file]; } @@ -255,7 +255,7 @@ { NSArray *lines = [self linesFromNotification:notification]; NSMutableDictionary *dic = [self dictionaryForLines:lines]; - [self addFilesFromDictionary:dic cached:NO tracked:YES]; + [self addFilesFromDictionary:dic staged:NO tracked:YES]; [self doneProcessingIndex]; } @@ -263,7 +263,7 @@ { NSArray *lines = [self linesFromNotification:notification]; NSMutableDictionary *dic = [self dictionaryForLines:lines]; - [self addFilesFromDictionary:dic cached:YES tracked:YES]; + [self addFilesFromDictionary:dic staged:YES tracked:YES]; [self doneProcessingIndex]; } diff --git a/PBGitIndexController.m b/PBGitIndexController.m index 84da6ca..dab2cd6 100644 --- a/PBGitIndexController.m +++ b/PBGitIndexController.m @@ -58,7 +58,7 @@ for (PBChangedFile *file in files) { file.hasUnstagedChanges = NO; - file.hasCachedChanges = YES; + file.hasStagedChanges = YES; } [self resumeTrackingIndex]; } @@ -85,7 +85,7 @@ for (PBChangedFile *file in files) { file.hasUnstagedChanges = YES; - file.hasCachedChanges = NO; + file.hasStagedChanges = NO; } [self resumeTrackingIndex]; }