diff mbox series

git-gui: do not exit upon prepare-commit-msg hook failure

Message ID 20240711132542.9792-2-anthony@loiseau.fr (mailing list archive)
State New, archived
Headers show
Series git-gui: do not exit upon prepare-commit-msg hook failure | expand

Commit Message

Anthony Loiseau July 11, 2024, 1:25 p.m. UTC
prepare-commit-msg hook is fired as soon git-gui is started
and upon F5 in order to pre-fill commit message area.

Having it fatal, forcibly exiting app when this hook fails
rendered git-gui unusable in this case. Fix this by not
treating this hook as fatal, and not exiting app when failure
popup is dismissed.

This way, user can use git-gui when he/she dismisses failure popup
of a failed prepare-commit-msg hook.

Note that this commit does not deny user from commiting when
prepare-commit-msg failed. Message is simply not pre-filled.

Signed-off-by: Anthony Loiseau <anthony@loiseau.fr>
---
 git-gui/git-gui.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Johannes Sixt July 13, 2024, 8:50 p.m. UTC | #1
[j6t: removed Shawn from the Cc list]

Am 11.07.24 um 15:25 schrieb Anthony Loiseau:
> prepare-commit-msg hook is fired as soon git-gui is started
> and upon F5 in order to pre-fill commit message area.
> 
> Having it fatal, forcibly exiting app when this hook fails
> rendered git-gui unusable in this case. Fix this by not
> treating this hook as fatal, and not exiting app when failure
> popup is dismissed.

It is understandable that a Git GUI that cannot be started looks like a
problem that needs solving. However, I am not happy with the proposed
solution.

The first reason is that even if Git GUI can now run, the error message
still reappears every time when Rescan (F5) is used and must be
dismissed each time. (The version of Git GUI that I use does a rescan on
window activation and now runs into rescan loop after the dismission; I
can't even exit Git GUI anymore.)

> This way, user can use git-gui when he/she dismisses failure popup
> of a failed prepare-commit-msg hook.
> 
> Note that this commit does not deny user from commiting when
> prepare-commit-msg failed. Message is simply not pre-filled.

This is the second reason. As a first step, an attempt should be made to
fill the edit box with the unprocessed commit message.

I am unsure what next steps should be, though.

- Should we not allow to make a commit?
- Should we allow to edit the message and then permit a commit?

I can't tell, because I do not know for what purposes people use the
prepare-commit-msg hook.

Without these two points addressed, I consider it better that Git GUI
does not even start or exits.

> 
> Signed-off-by: Anthony Loiseau <anthony@loiseau.fr>
> ---
>  git-gui/git-gui.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 8fe7538e72..ab6caaf2ae 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1643,9 +1643,8 @@ proc prepare_commit_msg_hook_wait {fd_ph} {
>  	if {[eof $fd_ph]} {
>  		if {[catch {close $fd_ph}]} {
>  			ui_status [mc "Commit declined by prepare-commit-msg hook."]
> -			hook_failed_popup prepare-commit-msg $pch_error
> +			hook_failed_popup prepare-commit-msg $pch_error 0
>  			catch {file delete [gitdir PREPARE_COMMIT_MSG]}
> -			exit 1
>  		} else {
>  			load_message PREPARE_COMMIT_MSG
>  		}

Below this context is the same catch {file delete ...}. Since it is now
reached, the instance in the if-branch should be removed.

-- Hannes
diff mbox series

Patch

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 8fe7538e72..ab6caaf2ae 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1643,9 +1643,8 @@  proc prepare_commit_msg_hook_wait {fd_ph} {
 	if {[eof $fd_ph]} {
 		if {[catch {close $fd_ph}]} {
 			ui_status [mc "Commit declined by prepare-commit-msg hook."]
-			hook_failed_popup prepare-commit-msg $pch_error
+			hook_failed_popup prepare-commit-msg $pch_error 0
 			catch {file delete [gitdir PREPARE_COMMIT_MSG]}
-			exit 1
 		} else {
 			load_message PREPARE_COMMIT_MSG
 		}