Re: [PATCH] Port git commit to C.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:48
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).
quoted hunk
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?
quoted hunk
@@ -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 ...
- 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.
- 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.