Skip to content

Support prepare-commit-msg hooks #2795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DCVortexxx wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support prepare-commit-msg hooks #2795

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 115 additions & 5 deletions GitUpKit/Views/GICommitViewController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@

@implementation GICommitViewController {
NSString* _headCommitMessage;
NSUInteger _prepareCommitMessageHookGeneration;
BOOL _prepareCommitMessageHookRunning;
NSError* _prepareCommitMessageHookError;
NSString* _automaticallyPreparedCommitMessage;
}

#if DEBUG
Expand Down Expand Up @@ -103,6 +107,95 @@ - (void)_updateInterface {
}
}

- (NSArray*)_prepareCommitMessageHookArgumentsWithMessagePath:(NSString*)path amendCommitSHA1:(NSString*)sha1 {
if (_amendButton.state) {
return sha1 ? @[ path, @"commit", sha1 ] : @[ path, @"commit" ];
}
if (self.repository.state == kGCRepositoryState_Merge) {
return @[ path, @"merge" ];
}
return @[ path ];
}
Comment on lines +110 to +118

@greptile-apps greptile-apps Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing squash and template source types for prepare-commit-msg

Git defines five source types for prepare-commit-msg: message (from -m/-F), template, merge, squash, and commit (amend). This implementation only handles merge, amend (commit), and the default (no source). Squash commits — which Git generates during interactive rebase — are a common use case for prepare-commit-msg hooks. If GitUp drives squash commits, those paths will invoke the hook with only the path argument instead of path squash, producing incorrect hook behavior for hooks that branch on the source type.


- (NSString*)_messageByInsertingPreparedMessage:(NSString*)preparedMessage beforeMessage:(NSString*)message {
if (!preparedMessage.length || [preparedMessage isEqualToString:message]) {
return message;
}
if (!message.length) {
return preparedMessage;
}
if ([preparedMessage hasSuffix:@"\n"] || [message hasPrefix:@"\n"]) {
return [preparedMessage stringByAppendingString:message];
}
return [preparedMessage stringByAppendingFormat:@"\n%@", message];
}

- (void)_applyPreparedCommitMessage:(NSString*)preparedMessage replacingInitialMessage:(NSString*)initialMessage {
NSString* currentMessage = _messageTextView.string;
NSString* message;
BOOL replacesInitialMessage = [currentMessage isEqualToString:initialMessage];
if (replacesInitialMessage) {
message = preparedMessage;
} else if (initialMessage.length && [currentMessage hasPrefix:initialMessage]) {
NSString* userMessage = [currentMessage substringFromIndex:initialMessage.length];
message = [self _messageByInsertingPreparedMessage:preparedMessage beforeMessage:userMessage];
} else {
message = [self _messageByInsertingPreparedMessage:preparedMessage beforeMessage:currentMessage];
}
_messageTextView.string = message;
_automaticallyPreparedCommitMessage = replacesInitialMessage ? message : nil;
}

- (void)_cancelPrepareCommitMessageHook {
++_prepareCommitMessageHookGeneration;
_prepareCommitMessageHookRunning = NO;
_prepareCommitMessageHookError = nil;
_automaticallyPreparedCommitMessage = nil;
}

- (void)_runPrepareCommitMessageHookWithInitialMessage:(NSString*)initialMessage {
if (![self.repository pathForHookWithName:@"prepare-commit-msg"]) {
return;
}

NSUInteger generation = ++_prepareCommitMessageHookGeneration;
_prepareCommitMessageHookRunning = YES;
_prepareCommitMessageHookError = nil;
_automaticallyPreparedCommitMessage = nil;

NSString* repositoryPath = self.repository.repositoryPath;
NSString* messagePath = [repositoryPath stringByAppendingPathComponent:@"COMMIT_EDITMSG"];
NSString* sha1 = nil;
if (_amendButton.state) {
GCCommit* headCommit = nil;
[self.repository lookupHEADCurrentCommit:&headCommit branch:NULL error:NULL];
sha1 = headCommit.SHA1;
}
NSArray* arguments = [self _prepareCommitMessageHookArgumentsWithMessagePath:messagePath amendCommitSHA1:sha1];

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
NSError* error = nil;
NSString* preparedMessage = nil;
GCRepository* repository = [[GCRepository alloc] initWithExistingLocalRepository:repositoryPath error:&error];
if (repository && [initialMessage writeToFile:messagePath atomically:YES encoding:NSUTF8StringEncoding error:&error] && [repository runHookWithName:@"prepare-commit-msg" arguments:arguments standardInput:nil error:&error]) {
preparedMessage = [[NSString alloc] initWithContentsOfFile:messagePath encoding:NSUTF8StringEncoding error:&error];
}

dispatch_async(dispatch_get_main_queue(), ^{
if (generation != _prepareCommitMessageHookGeneration) {
return;
}
_prepareCommitMessageHookRunning = NO;
_prepareCommitMessageHookError = error;
if (preparedMessage) {
[self _applyPreparedCommitMessage:preparedMessage replacingInitialMessage:initialMessage];
} else if (error) {
[self presentError:error];
}
Comment on lines +184 to +194

@greptile-apps greptile-apps Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Hook failure shows the error twice

When prepare-commit-msg fails, [self presentError:error] is called here in the async completion block, and _prepareCommitMessageHookError is also stored. If the user dismisses that alert and then clicks Commit, createCommitFromHEADWithMessage: checks _prepareCommitMessageHookError and calls [self presentError:_prepareCommitMessageHookError] a second time. The user sees the identical error dialog twice with no indication of how to recover other than navigating away and back.

Consider showing the error only in one place: either eagerly (here, and not re-checking in createCommitFromHEADWithMessage:), or lazily (only in createCommitFromHEADWithMessage:, silently storing the failure). The current code does both.

});
});
Comment on lines +176 to +196

@greptile-apps greptile-apps Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Strong capture of self in nested dispatch blocks

The outer dispatch_async block accesses _prepareCommitMessageHookGeneration and _prepareCommitMessageHookRunning via implicit self->_ivar syntax, capturing self strongly. The inner dispatch_get_main_queue block does the same. This is safe against crashes (the generation check guards stale application), but it extends the view controller's lifetime until the background task finishes, even after the view has been dismissed and ownership released. The idiomatic ObjC pattern is to capture a __weak reference, then create a __strong local at the top of the inner block and bail early if it is nil.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

}

- (void)viewDidLoad {
[super viewDidLoad];

Expand All @@ -127,8 +220,11 @@ - (void)viewWillAppear {
}

_messageTextView.string = message;
_automaticallyPreparedCommitMessage = nil;
[_messageTextView.undoManager removeAllActions];
[_messageTextView selectAll:nil];

[self _runPrepareCommitMessageHookWithInitialMessage:message];
}

[self _updateInterface];
Expand All @@ -137,6 +233,7 @@ - (void)viewWillAppear {
- (void)viewDidDisappear {
[super viewDidDisappear];

[self _cancelPrepareCommitMessageHook];
_headCommitMessage = nil;
}

Expand All @@ -154,15 +251,20 @@ - (void)didCreateCommit:(GCCommit*)commit {
[_otherMessageTextView.undoManager removeAllActions];

_amendButton.state = NSControlStateValueOff;
_automaticallyPreparedCommitMessage = nil;

[_delegate commitViewController:self didCreateCommit:commit];
}

- (IBAction)toggleAmend:(id)sender {
if (_amendButton.state && (_messageTextView.string.length == 0)) {
BOOL shouldReplaceMessage = (_messageTextView.string.length == 0) || [_messageTextView.string isEqualToString:_automaticallyPreparedCommitMessage];
if (_amendButton.state && shouldReplaceMessage) {
_messageTextView.string = _headCommitMessage;
[_messageTextView.undoManager removeAllActions];
[_messageTextView selectAll:nil];
[self _runPrepareCommitMessageHookWithInitialMessage:_headCommitMessage];
} else {
[self _cancelPrepareCommitMessageHook];
}
}

Expand All @@ -173,14 +275,23 @@ @implementation GICommitViewController (Extensions)
- (void)createCommitFromHEADWithMessage:(NSString*)message {
NSError* error;

if (_prepareCommitMessageHookRunning) {
[self presentAlertWithType:kGIAlertType_Caution title:NSLocalizedString(@"The prepare-commit-msg hook is still running", nil) message:NSLocalizedString(@"Please wait for it to finish before committing.", nil)];
return;
}
if (_prepareCommitMessageHookError) {
[self presentError:_prepareCommitMessageHookError];
return;
}
Comment on lines +278 to +285

@greptile-apps greptile-apps Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 _prepareCommitMessageHookError never cleared on failed commit attempt

After a hook failure the user is blocked from committing, but createCommitFromHEADWithMessage: shows the error and returns without clearing _prepareCommitMessageHookError. Every subsequent commit attempt will show the same error and abort. The only implicit recovery paths are toggling Amend (the else branch calls _cancelPrepareCommitMessageHook) or navigating away (which calls viewDidDisappear). Neither path is surfaced to the user in the error message, so they are left with no visible way forward.

At minimum, the error should be cleared after being presented here, or the alert should include a recovery suggestion (e.g., a "Retry" action that re-runs the hook).


if (![self.repository runHookWithName:@"pre-commit" arguments:nil standardInput:nil error:&error]) {
[self presentError:error];
return;
}

NSString* hook = [self.repository pathForHookWithName:@"commit-msg"];
if (hook) {
NSString* path = [NSTemporaryDirectory() stringByAppendingPathComponent:[[NSProcessInfo processInfo] globallyUniqueString]];
NSString* commitMsgHookPath = [self.repository pathForHookWithName:@"commit-msg"];
if (commitMsgHookPath) {
NSString* path = [self.repository.repositoryPath stringByAppendingPathComponent:@"COMMIT_EDITMSG"];
if (![message writeToFile:path atomically:YES encoding:NSUTF8StringEncoding error:&error]) {
[self presentError:error];
return;
Expand All @@ -194,7 +305,6 @@ - (void)createCommitFromHEADWithMessage:(NSString*)message {
[self presentError:error];
return;
}
[[NSFileManager defaultManager] removeItemAtPath:path error:NULL];
}

GCCommit* newCommit;
Expand Down
Loading