Re: [PATCH] builtin-commit: re-read file index before launching editor

> From: Samuel Yvon <samuelyvon9@gmail.com>
>
> Changes made within a pre-commit hook are not reflected within
> the editor (for instance, with `git commit --verbose`) because
> the index is re-read after the editor has been used.
>
> This has the consequence of not displaying the actual changes made
> by the pre-commit hook, but committing them anyways.
>
> While it is often argued that the hook's purpose is not to automatically
> write content to the repository, it is acknowledged that using the
> pre-commit to apply mechanical fixes on top of the existing changes
> is a supported use case, along with verifying the content.
>
> I think not seeing these mechanical fixes before commiting is incorrect.
> A developer might expect the commit to look a certain way but if the
> pre-commit goes wrong the invalid changes will go unnoticed until
> committed.
>
> Signed-off-by: Samuel Yvon <samuelyvon9@gmail.com>
> ---
>     builtin-commit: Re-read file index before launching editor
>     
>     Changes made within a pre-commit hook are not reflected within the
>     editor (for instance, with git commit --verbose) because the index is
>     re-read after the editor has been used.
>     
>     This has the consequence of not displaying the actual changes made by
>     the pre-commit hook, but committing them anyways.
>     
>     While it is often argued that the hook's purpose is not to automatically
>     write content to the repository, it is acknowledged that using the
>     pre-commit to apply mechanical fixes on top of the existing changes is a
>     supported use case, along with verifying the content.
>     
>     I think not seeing these mechanical fixes before commiting is incorrect.
>     A developer might expect the commit to look a certain way but if the
>     pre-commit goes wrong the invalid changes will go unnoticed until
>     committed.
>     
>     I had a small exchange in the Google Group before submitting this Pr.
>     Here is a link for cross-referencing:
>     https://groups.google.com/g/git-mentoring/c/FsP83I9mN6c
>     
>     As a side note, I do not know who manages the Github Repo but the
>     following description threw me off a little bit:
>     
>     Git Source Code Mirror - This is a publish-only repository and all pull requests are ignored. 
>     
>     
>     since after looking deeper it seems the PRs are not ignored.
>     
>     Signed-off-by: Samuel Yvon samuelyvon9@gmail.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1127%2FSamuelYvon%2Fmaint-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1127/SamuelYvon/maint-v1
> Pull-Request: https://github.com/git/git/pull/1127
>
>  builtin/commit.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 7c9b1e7be3a..e75b11d1c60 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -728,8 +728,17 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  	/* This checks and barfs if author is badly specified */
>  	determine_author_info(author_ident);
>  
> -	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
> -		return 0;
> +	if (!no_verify && find_hook("pre-commit")) {
> +		if(run_commit_hook(use_editor, index_file, "pre-commit", NULL))
> +			return 0;
> +
> +		/*
> +		 * Re-read the index as pre-commit hook could have updated it,
> +		 * and write it out as a tree.
> +		 */
> +		discard_cache();
> +		read_cache_from(index_file);
> +	}
>  
>  	if (squash_message) {
>  		/*
> @@ -1051,14 +1060,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  		return 0;
>  	}
>  
> -	if (!no_verify && find_hook("pre-commit")) {
> -		/*
> -		 * Re-read the index as pre-commit hook could have updated it,
> -		 * and write it out as a tree.  We must do this before we invoke
> -		 * the editor and after we invoke run_status above.
> -		 */
> -		discard_cache();
> -	}
>  	read_cache_from(index_file);
>  
>  	if (update_main_cache_tree(0)) {
>
> base-commit: 5fbd2fc5997dfa4d4593a862fe729b1e7a89bcf8