From: Wincent Colaiuta <win@wincent.com>
Date: Sun, 09 Dec 2007 12:22:32 GMT
Subject: Re: [PATCH 3/3] Fix commit-msg hook to allow editing
Message-ID: <5B3413C6-48E3-44CD-9577-622C9A93230A@wincent.com>
In-Reply-To: <7vtzmss58h.fsf@gitster.siamese.dyndns.org>

El 9/12/2007, a las 8:21, Junio C Hamano escribió:

> IOW, how about something like this?
>
> ---
>
> builtin-commit.c |    9 +++++----
> builtin-tag.c    |    2 ++
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/builtin-commit.c b/builtin-commit.c
> index 2032ca3..30a9deb 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -787,16 +787,17 @@ int cmd_commit(int argc, const char **argv,  
> const char *prefix)
> 		char index[PATH_MAX];
> 		const char *env[2] = { index, NULL };
> 		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> -		launch_editor(git_path(commit_editmsg), &sb, env);
> -	} else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
> -		rollback_index_files();
> -		die("could not read commit message");
> +		launch_editor(git_path(commit_editmsg), NULL, env);
> 	}
> 	if (!no_verify &&
> 	    run_hook(index_file, "commit-msg", git_path(commit_editmsg))) {
> 		rollback_index_files();
> 		exit(1);
> 	}
> +	if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
> +		rollback_index_files();
> +		die("could not read commit message");
> +	}
>
> 	/* Truncate the message just before the diff, if any. */
> 	p = strstr(sb.buf, "\ndiff --git a/");
> diff --git a/builtin-tag.c b/builtin-tag.c
> index 729389b..9f966fc 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -53,6 +53,8 @@ void launch_editor(const char *path, struct strbuf  
> *buffer, const char *const *e
> 			die("There was a problem with the editor %s.", editor);
> 	}
>
> +	if (!buffer)
> +		return;
> 	if (strbuf_read_file(buffer, path, 0) < 0)
> 		die("could not read message file '%s': %s",
> 		    path, strerror(errno));

Excellent. Reads much better. I hadn't considered changing the strbuf  
API, but looking at the callers of launch_editor this is a safe  
change. Ack.

Cheers,
Wincent