Re: [PATCH] Port git commit to C.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 8, 2007, 23:14 UTC
- Message-ID
- <7vy7d8z5w9.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1194541140-3062-1-git-send-email-krh@redhat.com>
A huge diff.
I'll quote the difference between this version and the version previously parked on 'pu', and comment, after re-reverting what you reverted from the parked version (which has minor tweaks such as removing git-runstatus from .gitignore I made previously).
Show 16 quoted lines
> builtin-commit.c | 88 +++++++++++++++++++++++++++---------------------------
> 1 files changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/builtin-commit.c b/builtin-commit.c
> index f108e90..669cc6b 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -70,25 +70,22 @@ static char *prepare_index(const char **files, const char *prefix)
> struct tree *tree;
> struct lock_file *next_index_lock;
>
> + if (interactive) {
> + interactive_add();
> + return get_index_file();
> + }
> +So interactive bypasses everything else, which makes sense.
What happens when the user aborts interactive_add, wishing to abort the whole commit process?
Show 34 quoted lines
> @@ -267,36 +264,41 @@ static int message_is_empty(struct strbuf *sb, int start)
>
> static void determine_author_info(struct strbuf *sb)
> {
> - char *p, *eol;
> - char *name = NULL, *email = NULL;
> + char *name, *email, *date;
>
> - if (force_author) {
> - const char *eoname = strstr(force_author, " <");
> - const char *eomail = strchr(force_author, '>');
> + name = getenv("GIT_AUTHOR_NAME");
> + email = getenv("GIT_AUTHOR_EMAIL");
> + date = getenv("GIT_AUTHOR_DATE");
>
> - if (!eoname || !eomail)
> - die("malformed --author parameter\n");
> - name = xstrndup(force_author, eoname - force_author);
> - email = xstrndup(eoname + 2, eomail - eoname - 2);
> - /* REVIEW: drops author date from amended commit on --amend --author=<author> */
> - strbuf_addf(sb, "author %s\n",
> - fmt_ident(name, email,
> - getenv("GIT_AUTHOR_DATE"), 1));
> - free(name);
> - free(email);
> - } else if (use_message) {
> - p = strstr(use_message_buffer, "\nauthor");
> - if (!p)
> + if (use_message) {
> + const char *a, *lb, *rb, *eol;
> +
> + a = strstr(use_message_buffer, "\nauthor ");
> + if (!a)
> die("invalid commit: %s\n", use_message);You know in what sense it is invalid here and ...
Show 9 quoted lines
> - p++;
> - eol = strchr(p, '\n');
> - if (!eol)
> +
> + lb = strstr(a + 8, " <");
> + rb = strstr(a + 8, "> ");
> + eol = strchr(a + 8, '\n');
> + if (!lb || !rb || !eol)
> die("invalid commit: %s\n", use_message);... here. Would it be more helpful to say "No author line", and "Cannot parse author line", I wonder. Probably too much.
Show 19 quoted lines
> - strbuf_add(sb, p, eol + 1 - p);
> - } else {
> - strbuf_addf(sb, "author %s\n", git_author_info(1));
> + name = xstrndup(a + 8, lb - (a + 8));
> + email = xstrndup(lb + 2, rb - (lb + 2));
> + date = xstrndup(rb + 2, eol - (rb + 2));
> }
> +
> + if (force_author) {
> + const char *lb = strstr(force_author, " <");
> + const char *rb = strchr(force_author, '>');
> +
> + if (!lb || !rb)
> + die("malformed --author parameter\n");
> + name = xstrndup(force_author, lb - force_author);
> + email = xstrndup(lb + 2, rb - (lb + 2));
> + }
> +
> + strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, 1));I see a slight leak here of name/email/date depending on how they are obtained. Probably it is too much hassle to deal with for too little gain.