{"thread":{"id":"10810","subject":"[PATCH 0/6] Various (replacement) patches to builtin-commit","startedAt":"2007-11-11T17:35:04Z","lastAt":"2007-12-03T07:53:57Z","messageCount":24,"participants":["Johannes Schindelin","Pierre Habouzit","Kristian Høgsberg","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"59342","messageId":"Pine.LNX.4.64.0711111730580.4362@racer.site","threadId":"10810","inReplyTo":null,"subject":"[PATCH 0/6] Various (replacement) patches to builtin-commit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:35:04Z","receivedAt":"2007-11-11T17:35:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI did some more work on top of kh/commit (rebased to next, + Kristian's \nrefresh_cache() patch which is still waiting to be enhanced with \nrefresh_cache()-after-commit-with-paths).\n\nThey are in chronological order. In particular, all patches except 2/6 \nresurrect old behaviour.\n\nCiao,\nDscho\n"},{"id":"59343","messageId":"Pine.LNX.4.64.0711111735100.4362@racer.site","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111730580.4362@racer.site","subject":"[PATCH 1/6] builtin-commit: fix author date with --amend --author=","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:35:18Z","receivedAt":"2007-11-11T17:35:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen amending a commit with a new author, the author date is taken\nfrom the original commit.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-commit.c | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex a84a729..6be6def 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -527,6 +527,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t} else if (amend) {\n \t\tstruct commit_list *c;\n \t\tstruct commit *commit;\n+\t\tconst char *author;\n \n \t\treflog_msg = \"commit (amend)\";\n \t\tcommit = lookup_commit(head_sha1);\n@@ -536,6 +537,21 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tfor (c = commit->parents; c; c = c->next)\n \t\t\tstrbuf_addf(&sb, \"parent %s\\n\",\n \t\t\t\t sha1_to_hex(c->item->object.sha1));\n+\n+\t\t/* determine author date */\n+\t\tauthor = strstr(commit->buffer, \"\\nauthor\");\n+\t\tif (author && !memmem(commit->buffer,\n+\t\t\t\t\tauthor + 1 - commit->buffer,\n+\t\t\t\t\t\"\\n\\n\", 2)) {\n+\t\t\tconst char *email_end = strchr(author + 1, '>');\n+\t\t\tconst char *line_end = strchr(author + 1, '\\n');\n+\t\t\tif (email_end && line_end && email_end < line_end) {\n+\t\t\t\tchar *date = xstrndup(email_end + 1,\n+\t\t\t\t\tline_end - email_end - 1);\n+\t\t\t\tsetenv(\"GIT_AUTHOR_DATE\", date, 1);\n+\t\t\t\tfree(date);\n+\t\t\t}\n+\t\t}\n \t} else if (in_merge) {\n \t\tstruct strbuf m;\n \t\tFILE *fp;\n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59344","messageId":"Pine.LNX.4.64.0711111735280.4362@racer.site","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111730580.4362@racer.site","subject":"[PATCH 2/6] git status: show relative paths when run in a subdirectory","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:35:41Z","receivedAt":"2007-11-11T17:35:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nTo show the relative paths, the function formerly called quote_crlf()\n(now called quote_path()) takes the prefix as an additional argument.\n\nWhile at it, the static buffers were replaced by strbufs.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-commit.c | 13 ++++---\n builtin-runstatus.c | 1 +\n t/t7502-status.sh | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++\n wt-status.c | 69 ++++++++++++++++++++++++++-------------\n wt-status.h | 1 +\n 5 files changed, 146 insertions(+), 29 deletions(-)\n create mode 100755 t/t7502-status.sh\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 6be6def..5c7d4df 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -118,11 +118,12 @@ static char *prepare_index(const char **files, const char *prefix)\n \treturn next_index_lock->filename;\n }\n \n-static int run_status(FILE *fp, const char *index_file)\n+static int run_status(FILE *fp, const char *index_file, const char *prefix)\n {\n \tstruct wt_status s;\n \n \twt_status_prepare(&s);\n+\ts.prefix = prefix;\n \n \tif (amend) {\n \t\ts.amend = 1;\n@@ -140,7 +141,7 @@ static int run_status(FILE *fp, const char *index_file)\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n \n-static int prepare_log_message(const char *index_file)\n+static int prepare_log_message(const char *index_file, const char *prefix)\n {\n \tstruct stat statbuf;\n \tint commitable;\n@@ -216,7 +217,7 @@ static int prepare_log_message(const char *index_file)\n \tif (only_include_assumed)\n \t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n \n-\tcommitable = run_status(fp, index_file);\n+\tcommitable = run_status(fp, index_file, prefix);\n \n \tfclose(fp);\n \n@@ -409,7 +410,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \tindex_file = prepare_index(argv, prefix);\n \n-\tcommitable = run_status(stdout, index_file);\n+\tcommitable = run_status(stdout, index_file, prefix);\n \n \trollback_lock_file(&lock_file);\n \n@@ -503,8 +504,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\texit(1);\n \n-\tif (!prepare_log_message(index_file) && !in_merge) {\n-\t\trun_status(stdout, index_file);\n+\tif (!prepare_log_message(index_file, prefix) && !in_merge) {\n+\t\trun_status(stdout, index_file, prefix);\n \t\tunlink(commit_editmsg);\n \t\treturn 1;\n \t}\ndiff --git a/builtin-runstatus.c b/builtin-runstatus.c\nindex 2db25c8..8d167a9 100644\n--- a/builtin-runstatus.c\n+++ b/builtin-runstatus.c\n@@ -14,6 +14,7 @@ int cmd_runstatus(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_status_config);\n \twt_status_prepare(&s);\n+\ts.prefix = prefix;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tif (!strcmp(argv[i], \"--color\"))\ndiff --git a/t/t7502-status.sh b/t/t7502-status.sh\nnew file mode 100755\nindex 0000000..269b334\n--- /dev/null\n+++ b/t/t7502-status.sh\n@@ -0,0 +1,91 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Johannes E. Schindelin\n+#\n+\n+test_description='git-status'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\t: > tracked &&\n+\t: > modified &&\n+\tmkdir dir1 &&\n+\t: > dir1/tracked &&\n+\t: > dir1/modified &&\n+\tmkdir dir2 &&\n+\t: > dir1/tracked &&\n+\t: > dir1/modified &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\t: > untracked &&\n+\t: > dir1/untracked &&\n+\t: > dir2/untracked &&\n+\techo 1 > dir1/modified &&\n+\techo 2 > dir2/modified &&\n+\techo 3 > dir2/added &&\n+\tgit add dir2/added\n+'\n+\n+cat > expect << \\EOF\n+# On branch master\n+# Changes to be committed:\n+# (use \"git reset HEAD ...\" to unstage)\n+#\n+#\tnew file: dir2/added\n+#\n+# Changed but not updated:\n+# (use \"git add ...\" to update what will be committed)\n+#\n+#\tmodified: dir1/modified\n+#\n+# Untracked files:\n+# (use \"git add ...\" to include in what will be committed)\n+#\n+#\tdir1/untracked\n+#\tdir2/modified\n+#\tdir2/untracked\n+#\texpect\n+#\toutput\n+#\tuntracked\n+EOF\n+\n+test_expect_success 'status' '\n+\n+\tgit status > output &&\n+\tgit diff expect output\n+\n+'\n+\n+cat > expect << \\EOF\n+# On branch master\n+# Changes to be committed:\n+# (use \"git reset HEAD ...\" to unstage)\n+#\n+#\tnew file: ../dir2/added\n+#\n+# Changed but not updated:\n+# (use \"git add ...\" to update what will be committed)\n+#\n+#\tmodified: ../dir1/modified\n+#\n+# Untracked files:\n+# (use \"git add ...\" to include in what will be committed)\n+#\n+#\tuntracked\n+#\t../dir2/modified\n+#\t../dir2/untracked\n+#\t../expect\n+#\t../output\n+#\t../untracked\n+EOF\n+\n+test_expect_success 'status with relative paths' '\n+\n+\t(cd dir1 && git status) > output &&\n+\tgit diff expect output\n+\n+'\n+\n+test_done\ndiff --git a/wt-status.c b/wt-status.c\nindex 03b5ec4..0d25362 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -82,33 +82,46 @@ static void wt_status_print_trailer(struct wt_status *s)\n \tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n }\n \n-static const char *quote_crlf(const char *in, char *buf, size_t sz)\n+static char *quote_path(const char *in, int len,\n+\t\tstruct strbuf *out, const char *prefix)\n {\n-\tconst char *scan;\n-\tchar *out;\n-\tconst char *ret = in;\n+\tif (len > 0)\n+\t\tstrbuf_grow(out, len);\n+\tstrbuf_setlen(out, 0);\n+\n+\tif (prefix) {\n+\t\tint off = 0;\n+\t\twhile (prefix[off] && off < len && prefix[off] == in[off])\n+\t\t\tif (prefix[off] == '/') {\n+\t\t\t\tprefix += off + 1;\n+\t\t\t\tin += off + 1;\n+\t\t\t\tlen -= off + 1;\n+\t\t\t\toff = 0;\n+\t\t\t} else\n+\t\t\t\toff++;\n+\n+\t\tfor (; *prefix; prefix++)\n+\t\t\tif (*prefix == '/')\n+\t\t\t\tstrbuf_addstr(out, \"../\");\n+\t}\n \n-\tfor (scan = in, out = buf; *scan; scan++) {\n-\t\tint ch = *scan;\n-\t\tint quoted;\n+\tfor (; (len < 0 && *in) || len > 0; in++, len--) {\n+\t\tint ch = *in;\n \n \t\tswitch (ch) {\n \t\tcase '\\n':\n-\t\t\tquoted = 'n';\n+\t\t\tstrbuf_addstr(out, \"\\\\n\");\n \t\t\tbreak;\n \t\tcase '\\r':\n-\t\t\tquoted = 'r';\n+\t\t\tstrbuf_addstr(out, \"\\\\r\");\n \t\t\tbreak;\n \t\tdefault:\n-\t\t\t*out++ = ch;\n+\t\t\tstrbuf_addch(out, ch);\n \t\t\tcontinue;\n \t\t}\n-\t\t*out++ = '\\\\';\n-\t\t*out++ = quoted;\n-\t\tret = buf;\n \t}\n-\t*out = '\\0';\n-\treturn ret;\n+\n+\treturn out->buf;\n }\n \n static void wt_status_print_filepair(struct wt_status *s,\n@@ -116,10 +129,12 @@ static void wt_status_print_filepair(struct wt_status *s,\n {\n \tconst char *c = color(t);\n \tconst char *one, *two;\n-\tchar onebuf[PATH_MAX], twobuf[PATH_MAX];\n+\tstruct strbuf onebuf, twobuf;\n \n-\tone = quote_crlf(p->one->path, onebuf, sizeof(onebuf));\n-\ttwo = quote_crlf(p->two->path, twobuf, sizeof(twobuf));\n+\tstrbuf_init(&onebuf, 0);\n+\tstrbuf_init(&twobuf, 0);\n+\tone = quote_path(p->one->path, -1, &onebuf, s->prefix);\n+\ttwo = quote_path(p->two->path, -1, &twobuf, s->prefix);\n \n \tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n \tswitch (p->status) {\n@@ -151,6 +166,8 @@ static void wt_status_print_filepair(struct wt_status *s,\n \t\tdie(\"bug: unhandled diff status %c\", p->status);\n \t}\n \tfprintf(s->fp, \"\\n\");\n+\tstrbuf_release(&onebuf);\n+\tstrbuf_release(&twobuf);\n }\n \n static void wt_status_print_updated_cb(struct diff_queue_struct *q,\n@@ -205,8 +222,9 @@ static void wt_read_cache(struct wt_status *s)\n static void wt_status_print_initial(struct wt_status *s)\n {\n \tint i;\n-\tchar buf[PATH_MAX];\n+\tstruct strbuf buf;\n \n+\tstrbuf_init(&buf, 0);\n \twt_read_cache(s);\n \tif (active_nr) {\n \t\ts->commitable = 1;\n@@ -215,11 +233,12 @@ static void wt_status_print_initial(struct wt_status *s)\n \tfor (i = 0; i < active_nr; i++) {\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n \t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), \"new file: %s\",\n-\t\t\t\tquote_crlf(active_cache[i]->name,\n-\t\t\t\t\t buf, sizeof(buf)));\n+\t\t\t\tquote_path(active_cache[i]->name, -1,\n+\t\t\t\t\t &buf, s->prefix));\n \t}\n \tif (active_nr)\n \t\twt_status_print_trailer(s);\n+\tstrbuf_release(&buf);\n }\n \n static void wt_status_print_updated(struct wt_status *s)\n@@ -254,7 +273,9 @@ static void wt_status_print_untracked(struct wt_status *s)\n \tconst char *x;\n \tint i;\n \tint shown_header = 0;\n+\tstruct strbuf buf;\n \n+\tstrbuf_init(&buf, 0);\n \tmemset(&dir, 0, sizeof(dir));\n \n \tdir.exclude_per_dir = \".gitignore\";\n@@ -291,9 +312,11 @@ static void wt_status_print_untracked(struct wt_status *s)\n \t\t\tshown_header = 1;\n \t\t}\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n-\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), \"%.*s\",\n-\t\t\t\tent->len, ent->name);\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), \"%s\",\n+\t\t\t\tquote_path(ent->name, ent->len,\n+\t\t\t\t\t&buf, s->prefix));\n \t}\n+\tstrbuf_release(&buf);\n }\n \n static void wt_status_print_verbose(struct wt_status *s)\ndiff --git a/wt-status.h b/wt-status.h\nindex 7744932..f58ebcb 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -23,6 +23,7 @@ struct wt_status {\n \tint workdir_untracked;\n \tconst char *index_file;\n \tFILE *fp;\n+\tconst char *prefix;\n };\n \n int git_status_config(const char *var, const char *value);\n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59345","messageId":"Pine.LNX.4.64.0711111735470.4362@racer.site","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111730580.4362@racer.site","subject":"[PATCH 3/6] builtin-commit: fix --signoff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:35:58Z","receivedAt":"2007-11-11T17:35:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe Signed-off-by: line contained a spurious timestamp. The reason was\na call to git_committer_info(1), which automatically added the\ntimestamp.\n\nInstead, fmt_ident() was taught to interpret an empty string for the\ndate (as opposed to NULL, which still triggers the default behavior)\nas \"do not bother with the timestamp\", and builtin-commit.c uses it.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-commit.c | 29 ++++++++++++++++++-----------\n ident.c | 10 +++++++---\n t/t7500-commit.sh | 13 +++++++++++++\n 3 files changed, 38 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 5c7d4df..6b1507d 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -183,21 +183,28 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\tdie(\"could not open %s\\n\", git_path(commit_editmsg));\n \n \tstripspace(&sb, 0);\n-\tif (fwrite(sb.buf, 1, sb.len, fp) < sb.len)\n-\t\tdie(\"could not write commit template: %s\\n\",\n-\t\t strerror(errno));\n \n \tif (signoff) {\n-\t\tconst char *info, *bol;\n-\n-\t\tinfo = git_committer_info(1);\n-\t\tstrbuf_addch(&sb, '\\0');\n-\t\tbol = strrchr(sb.buf + sb.len - 1, '\\n');\n-\t\tif (!bol || prefixcmp(bol, sign_off_header))\n-\t\t\tfprintf(fp, \"\\n\");\n-\t\tfprintf(fp, \"%s%s\\n\", sign_off_header, git_committer_info(1));\n+\t\tstruct strbuf sob;\n+\t\tint i;\n+\n+\t\tstrbuf_init(&sob, 0);\n+\t\tstrbuf_addstr(&sob, sign_off_header);\n+\t\tstrbuf_addstr(&sob, fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n+ getenv(\"GIT_COMMITTER_EMAIL\"), \"\", 1));\n+\t\tstrbuf_addch(&sob, '\\n');\n+\n+\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n+\t\t\t; /* do nothing */\n+\t\tif (prefixcmp(sb.buf + i, sob.buf))\n+\t\t\tstrbuf_addbuf(&sb, &sob);\n+\t\tstrbuf_release(&sob);\n \t}\n \n+\tif (fwrite(sb.buf, 1, sb.len, fp) < sb.len)\n+\t\tdie(\"could not write commit template: %s\\n\",\n+\t\t strerror(errno));\n+\n \tstrbuf_release(&sb);\n \n \tif (in_merge && !no_edit)\ndiff --git a/ident.c b/ident.c\nindex 9b2a852..5be7533 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -224,13 +224,17 @@ const char *fmt_ident(const char *name, const char *email,\n \t}\n \n \tstrcpy(date, git_default_date);\n-\tif (date_str)\n-\t\tparse_date(date_str, date, sizeof(date));\n+\tif (date_str) {\n+\t\tif (*date_str)\n+\t\t\tparse_date(date_str, date, sizeof(date));\n+\t\telse\n+\t\t\tdate[0] = '\\0';\n+\t}\n \n \ti = copy(buffer, sizeof(buffer), 0, name);\n \ti = add_raw(buffer, sizeof(buffer), i, \" <\");\n \ti = copy(buffer, sizeof(buffer), i, email);\n-\ti = add_raw(buffer, sizeof(buffer), i, \"> \");\n+\ti = add_raw(buffer, sizeof(buffer), i, date[0] ? \"> \" : \">\");\n \ti = copy(buffer, sizeof(buffer), i, date);\n \tif (i >= sizeof(buffer))\n \t\tdie(\"Impossibly long personal identifier\");\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex abbf54b..13d5a0c 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' '\n \tcommit_msg_is \"standard input msg\"\n '\n \n+cat > expect << EOF\n+zort\n+Signed-off-by: C O Mitter \n+EOF\n+\n+test_expect_success '--signoff' '\n+\techo \"yet another content *narf*\" >> foo &&\n+\techo \"zort\" |\n+\t\tGIT_EDITOR=../t7500/add-content git commit -s -F - foo &&\n+\tgit cat-file commit HEAD | sed \"1,/^$/d\" > output &&\n+\tgit diff expect output\n+'\n+\n test_done\n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59346","messageId":"Pine.LNX.4.64.0711111736170.4362@racer.site","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111730580.4362@racer.site","subject":"[PATCH 4/6] builtin-commit --s: add a newline if the last line was no S-O-B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:36:27Z","receivedAt":"2007-11-11T17:36:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe rule is this: if the last line already contains the sign off by the\ncurrent committer, do nothing. If it contains another sign off, just\nadd the sign off of the current committer. If the last line does not\ncontain a sign off, add a new line before adding the sign off.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-commit.c | 5 ++++-\n t/t7500-commit.sh | 1 +\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 6b1507d..66d7e5e 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -196,8 +196,11 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \n \t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n \t\t\t; /* do nothing */\n-\t\tif (prefixcmp(sb.buf + i, sob.buf))\n+\t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n+\t\t\tif (prefixcmp(sb.buf + i, sign_off_header))\n+\t\t\t\tstrbuf_addch(&sb, '\\n');\n \t\t\tstrbuf_addbuf(&sb, &sob);\n+\t\t}\n \t\tstrbuf_release(&sob);\n \t}\n \ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex 13d5a0c..e0be2ed 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -95,6 +95,7 @@ test_expect_success 'commit message from file should override template' '\n \n cat > expect << EOF\n zort\n+\n Signed-off-by: C O Mitter \n EOF\n \n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59347","messageId":"Pine.LNX.4.64.0711111736310.4362@racer.site","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111730580.4362@racer.site","subject":"[PATCH 5/6] builtin-commit: resurrect behavior for multiple -m options","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:36:39Z","receivedAt":"2007-11-11T17:36:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen more than one -m option is given, the message does not replace\nthe previous, but is appended.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-commit.c | 26 ++++++++++++++++++++------\n 1 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 66d7e5e..069d180 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -30,13 +30,27 @@ static char *use_message_buffer;\n static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n static struct lock_file lock_file;\n \n-static char *logfile, *force_author, *message, *template_file;\n+static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify;\n \n static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n+struct strbuf message;\n+\n+static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct strbuf *buf = opt->value;\n+\tif (unset)\n+\t\tstrbuf_setlen(buf, 0);\n+\telse {\n+\t\tstrbuf_addstr(buf, arg);\n+\t\tstrbuf_addch(buf, '\\n');\n+\t\tstrbuf_addch(buf, '\\n');\n+\t}\n+\treturn 0;\n+}\n \n static struct option builtin_commit_options[] = {\n \tOPT__QUIET(&quiet),\n@@ -45,7 +59,7 @@ static struct option builtin_commit_options[] = {\n \n \tOPT_STRING('F', \"file\", &logfile, \"FILE\", \"read log from file\"),\n \tOPT_STRING(0, \"author\", &force_author, \"AUTHOR\", \"override author for commit\"),\n-\tOPT_STRING('m', \"message\", &message, \"MESSAGE\", \"specify commit message\"),\n+\tOPT_CALLBACK('m', \"message\", &message, \"MESSAGE\", \"specify commit message\", opt_parse_m),\n \tOPT_STRING('c', \"reedit-message\", &edit_message, \"COMMIT\", \"reuse and edit message from specified commit \"),\n \tOPT_STRING('C', \"reuse-message\", &use_message, \"COMMIT\", \"reuse message from specified commit\"),\n \tOPT_BOOLEAN('s', \"signoff\", &signoff, \"add Signed-off-by: header\"),\n@@ -150,8 +164,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tFILE *fp;\n \n \tstrbuf_init(&sb, 0);\n-\tif (message) {\n-\t\tstrbuf_add(&sb, message, strlen(message));\n+\tif (message.len) {\n+\t\tstrbuf_addbuf(&sb, &message);\n \t} else if (logfile && !strcmp(logfile, \"-\")) {\n \t\tif (isatty(0))\n \t\t\tfprintf(stderr, \"(reading log message from standard input)\\n\");\n@@ -321,7 +335,7 @@ static int parse_and_validate_options(int argc, const char *argv[])\n \targc = parse_options(argc, argv, builtin_commit_options,\n \t\t\t builtin_commit_usage, 0);\n \n-\tif (logfile || message || use_message)\n+\tif (logfile || message.len || use_message)\n \t\tno_edit = 1;\n \tif (edit_flag)\n \t\tno_edit = 0;\n@@ -346,7 +360,7 @@ static int parse_and_validate_options(int argc, const char *argv[])\n \t\tf++;\n \tif (f > 1)\n \t\tdie(\"Only one of -c/-C/-F can be used.\");\n-\tif (message && f > 0)\n+\tif (message.len && f > 0)\n \t\tdie(\"Option -m cannot be combined with -c/-C/-F.\");\n \tif (edit_message)\n \t\tuse_message = edit_message;\n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59348","messageId":"Pine.LNX.4.64.0711111736440.4362@racer.site","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111730580.4362@racer.site","subject":"[PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:36:52Z","receivedAt":"2007-11-11T17:36:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe function log_tree_commit() does not break the line, so we have to\ndo it ourselves.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-commit.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 069d180..3739bfc 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -493,6 +493,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \tprintf(\"Created %scommit \", initial_commit ? \"initial \" : \"\");\n \n \tlog_tree_commit(&rev, commit);\n+\tprintf(\"\\n\");\n }\n \n int git_commit_config(const char *k, const char *v)\n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59362","messageId":"20071111194228.GC13200@artemis.corp","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111736310.4362@racer.site","subject":"Re: [PATCH 5/6] builtin-commit: resurrect behavior for multiple -m options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-11T19:42:28Z","receivedAt":"2007-11-11T19:42:28Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Nov 11, 2007 at 05:36:39PM +0000, Johannes Schindelin wrote:\n> \n> When more than one -m option is given, the message does not replace\n> the previous, but is appended.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin-commit.c | 26 ++++++++++++++++++++------\n> 1 files changed, 20 insertions(+), 6 deletions(-)\n> \n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index 66d7e5e..069d180 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -30,13 +30,27 @@ static char *use_message_buffer;\n> static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n> static struct lock_file lock_file;\n> \n> -static char *logfile, *force_author, *message, *template_file;\n> +static char *logfile, *force_author, *template_file;\n> static char *edit_message, *use_message;\n> static int all, edit_flag, also, interactive, only, amend, signoff;\n> static int quiet, verbose, untracked_files, no_verify;\n> \n> static int no_edit, initial_commit, in_merge;\n> const char *only_include_assumed;\n> +struct strbuf message;\n\n Unless I'm mistaken `static` keywords are missign for`message` and\n`only_include_assumed`.\n\n And you _have_ to initialize message with STRBUF_INIT (remember of the\nslop).\n\n> +static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tstruct strbuf *buf = opt->value;\n> +\tif (unset)\n> +\t\tstrbuf_setlen(buf, 0);\n> +\telse {\n> +\t\tstrbuf_addstr(buf, arg);\n> +\t\tstrbuf_addch(buf, '\\n');\n> +\t\tstrbuf_addch(buf, '\\n');\n> +\t}\n> +\treturn 0;\n> +}\n\n I believe such a callback could live in parse-options.[hc]. The need\nto aggregate all string arguments into a strbuf looks generic enough to\nme. Why are you adding two '\\n' btw ? Isn't one enough ?\n\n Oh and last nitpicking, strbuf_addstr(buf, \"\\n\\n\"); is more efficient\nthan the two addchar (the strlen it generates is inlined).\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"59374","messageId":"Pine.LNX.4.64.0711112039130.4362@racer.site","threadId":"10810","inReplyTo":"20071111194228.GC13200@artemis.corp","subject":"Re: [PATCH 5/6] builtin-commit: resurrect behavior for multiple -m options","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T20:42:48Z","receivedAt":"2007-11-11T20:42:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Nov 2007, Pierre Habouzit wrote:\n\n> On Sun, Nov 11, 2007 at 05:36:39PM +0000, Johannes Schindelin wrote:\n> > \n> > When more than one -m option is given, the message does not replace\n> > the previous, but is appended.\n> > \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > builtin-commit.c | 26 ++++++++++++++++++++------\n> > 1 files changed, 20 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/builtin-commit.c b/builtin-commit.c\n> > index 66d7e5e..069d180 100644\n> > --- a/builtin-commit.c\n> > +++ b/builtin-commit.c\n> > @@ -30,13 +30,27 @@ static char *use_message_buffer;\n> > static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n> > static struct lock_file lock_file;\n> > \n> > -static char *logfile, *force_author, *message, *template_file;\n> > +static char *logfile, *force_author, *template_file;\n> > static char *edit_message, *use_message;\n> > static int all, edit_flag, also, interactive, only, amend, signoff;\n> > static int quiet, verbose, untracked_files, no_verify;\n> > \n> > static int no_edit, initial_commit, in_merge;\n> > const char *only_include_assumed;\n> > +struct strbuf message;\n> \n> Unless I'm mistaken `static` keywords are missign for`message` and\n> `only_include_assumed`.\n\nOh yeah. Will fix.\n\n> And you _have_ to initialize message with STRBUF_INIT (remember of the\n> slop).\n\nNot in this case, since I do not use message.buf as long as message.len == \n0. But I agree it would be cleaner to just use STRBUF_INIT.\n\n> > +static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n> > +{\n> > +\tstruct strbuf *buf = opt->value;\n> > +\tif (unset)\n> > +\t\tstrbuf_setlen(buf, 0);\n> > +\telse {\n> > +\t\tstrbuf_addstr(buf, arg);\n> > +\t\tstrbuf_addch(buf, '\\n');\n> > +\t\tstrbuf_addch(buf, '\\n');\n> > +\t}\n> > +\treturn 0;\n> > +}\n> \n> I believe such a callback could live in parse-options.[hc]. The need\n> to aggregate all string arguments into a strbuf looks generic enough to\n> me. Why are you adding two '\\n' btw ? Isn't one enough ?\n\nWell, this empty line is needed to stay backwards compatible. It was \nadded to pass the test that Junio added to 'next'. As such, this function \nis not really generic enough, right?\n\n> Oh and last nitpicking, strbuf_addstr(buf, \"\\n\\n\"); is more efficient\n> than the two addchar (the strlen it generates is inlined).\n\nWell, I meant to mention it in the cover letter. My preference is to do \naway with the extra empty line. But this might break existing setups \ndepending on that behaviour.\n\nCiao,\nDscho\n"},{"id":"59388","messageId":"20071111221134.GD13200@artemis.corp","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711112039130.4362@racer.site","subject":"Re: [PATCH 5/6] builtin-commit: resurrect behavior for multiple -m options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-11T22:11:34Z","receivedAt":"2007-11-11T22:11:34Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Nov 11, 2007 at 08:42:48PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 11 Nov 2007, Pierre Habouzit wrote:\n> \n> > On Sun, Nov 11, 2007 at 05:36:39PM +0000, Johannes Schindelin wrote:\n> > > \n> > > When more than one -m option is given, the message does not replace\n> > > the previous, but is appended.\n> > > \n> > > Signed-off-by: Johannes Schindelin \n> > > ---\n> > > builtin-commit.c | 26 ++++++++++++++++++++------\n> > > 1 files changed, 20 insertions(+), 6 deletions(-)\n> > > \n> > > diff --git a/builtin-commit.c b/builtin-commit.c\n> > > index 66d7e5e..069d180 100644\n> > > --- a/builtin-commit.c\n> > > +++ b/builtin-commit.c\n> > > @@ -30,13 +30,27 @@ static char *use_message_buffer;\n> > > static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n> > > static struct lock_file lock_file;\n> > > \n> > > -static char *logfile, *force_author, *message, *template_file;\n> > > +static char *logfile, *force_author, *template_file;\n> > > static char *edit_message, *use_message;\n> > > static int all, edit_flag, also, interactive, only, amend, signoff;\n> > > static int quiet, verbose, untracked_files, no_verify;\n> > > \n> > > static int no_edit, initial_commit, in_merge;\n> > > const char *only_include_assumed;\n> > > +struct strbuf message;\n> > \n> > Unless I'm mistaken `static` keywords are missign for`message` and\n> > `only_include_assumed`.\n> \n> Oh yeah. Will fix.\n> \n> > And you _have_ to initialize message with STRBUF_INIT (remember of the\n> > slop).\n> \n> Not in this case, since I do not use message.buf as long as message.len == \n> 0. But I agree it would be cleaner to just use STRBUF_INIT.\n\n I know that you don't use it, but who knows where this code will be\nheaded in its future right ? :) It's just good practice.\n\n> > > +static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n> > > +{\n> > > +\tstruct strbuf *buf = opt->value;\n> > > +\tif (unset)\n> > > +\t\tstrbuf_setlen(buf, 0);\n> > > +\telse {\n> > > +\t\tstrbuf_addstr(buf, arg);\n> > > +\t\tstrbuf_addch(buf, '\\n');\n> > > +\t\tstrbuf_addch(buf, '\\n');\n> > > +\t}\n> > > +\treturn 0;\n> > > +}\n> > \n> > I believe such a callback could live in parse-options.[hc]. The need\n> > to aggregate all string arguments into a strbuf looks generic enough to\n> > me. Why are you adding two '\\n' btw ? Isn't one enough ?\n> \n> Well, this empty line is needed to stay backwards compatible. It was \n> added to pass the test that Junio added to 'next'. As such, this function \n> is not really generic enough, right?\n\n Well, you can make it generic enough using the defval to point to a\ndelimiter to use between lines :) But it's probably an overkill, and yes\nif the \"\\n\\n\" is to be kept, then it's not generic \"enough\".\n\n> Well, I meant to mention it in the cover letter. My preference is to do \n> away with the extra empty line. But this might break existing setups \n> depending on that behaviour.\n\n Right.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"59389","messageId":"20071111221314.GE13200@artemis.corp","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711112039130.4362@racer.site","subject":"Re: [PATCH 5/6] builtin-commit: resurrect behavior for multiple -m options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-11T22:13:14Z","receivedAt":"2007-11-11T22:13:14Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Nov 11, 2007 at 08:42:48PM +0000, Johannes Schindelin wrote:\n> Well, I meant to mention it in the cover letter. My preference is to do \n> away with the extra empty line. But this might break existing setups \n> depending on that behaviour.\n\n In fact I believe what matters is that if there is more than one -m,\nyou have a \\n\\n between the first and the second one, else it'll break\nsubjects, and that sucks, so I'd say we have to stay with \"\\n\\n\" at\nleast for the first aggregation, and I'm unsure if it's worth the hassle\nto count how many times we aggregated to use '\\n' then.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"59554","messageId":"1194896334.2869.7.camel@hinata.boston.redhat.com","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111735100.4362@racer.site","subject":"Re: [PATCH 1/6] builtin-commit: fix author date with --amend --author=","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-12T19:38:54Z","receivedAt":"2007-11-12T19:38:54Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Sun, 2007-11-11 at 17:35 +0000, Johannes Schindelin wrote:\n> When amending a commit with a new author, the author date is taken\n> from the original commit.\n\nThe new determine_author_info() should fix this. There was a problem\nearlier that Junio pointed out and I sent a patch to update\ndetermine_author_info() to do the right thing for amend commits. The\ntest suite still pass without this patch, and if you look carefully a\ndetermine_author_info(), you can see it does the right thing:\n\n 1) Default to getenv for name, email and date\n\n 2) If a commit has been specified (-c, -C or --amend), we parse the\nauthor name, email and date from use_message_buffer, which holds the\ncommit, overriding the values from getenv.\n\n 3) If --author has be passed, we parse name and email from the\nargument and override whatever name and email the two previous steps\ncame up with.\n\nThen we add the author line to the commit buffer under construction\nbased on these values.\n\nI suggest we back this patch out.\n\ncheers,\nKristian\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin-commit.c | 16 ++++++++++++++++\n> 1 files changed, 16 insertions(+), 0 deletions(-)\n> \n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index a84a729..6be6def 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -527,6 +527,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t} else if (amend) {\n> \t\tstruct commit_list *c;\n> \t\tstruct commit *commit;\n> +\t\tconst char *author;\n> \n> \t\treflog_msg = \"commit (amend)\";\n> \t\tcommit = lookup_commit(head_sha1);\n> @@ -536,6 +537,21 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t\tfor (c = commit->parents; c; c = c->next)\n> \t\t\tstrbuf_addf(&sb, \"parent %s\\n\",\n> \t\t\t\t sha1_to_hex(c->item->object.sha1));\n> +\n> +\t\t/* determine author date */\n> +\t\tauthor = strstr(commit->buffer, \"\\nauthor\");\n> +\t\tif (author && !memmem(commit->buffer,\n> +\t\t\t\t\tauthor + 1 - commit->buffer,\n> +\t\t\t\t\t\"\\n\\n\", 2)) {\n> +\t\t\tconst char *email_end = strchr(author + 1, '>');\n> +\t\t\tconst char *line_end = strchr(author + 1, '\\n');\n> +\t\t\tif (email_end && line_end && email_end < line_end) {\n> +\t\t\t\tchar *date = xstrndup(email_end + 1,\n> +\t\t\t\t\tline_end - email_end - 1);\n> +\t\t\t\tsetenv(\"GIT_AUTHOR_DATE\", date, 1);\n> +\t\t\t\tfree(date);\n> +\t\t\t}\n> +\t\t}\n> \t} else if (in_merge) {\n> \t\tstruct strbuf m;\n> \t\tFILE *fp;\n"},{"id":"59561","messageId":"Pine.LNX.4.64.0711122011540.4362@racer.site","threadId":"10810","inReplyTo":"1194896334.2869.7.camel@hinata.boston.redhat.com","subject":"Re: [PATCH 1/6] builtin-commit: fix author date with --amend --author=","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-12T20:12:06Z","receivedAt":"2007-11-12T20:12:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 12 Nov 2007, Kristian H?gsberg wrote:\n\n> On Sun, 2007-11-11 at 17:35 +0000, Johannes Schindelin wrote:\n> > When amending a commit with a new author, the author date is taken\n> > from the original commit.\n> \n> The new determine_author_info() should fix this. There was a problem\n> earlier that Junio pointed out and I sent a patch to update\n> determine_author_info() to do the right thing for amend commits. The\n> test suite still pass without this patch, and if you look carefully a\n> determine_author_info(), you can see it does the right thing:\n> \n> 1) Default to getenv for name, email and date\n> \n> 2) If a commit has been specified (-c, -C or --amend), we parse the\n> author name, email and date from use_message_buffer, which holds the\n> commit, overriding the values from getenv.\n> \n> 3) If --author has be passed, we parse name and email from the\n> argument and override whatever name and email the two previous steps\n> came up with.\n> \n> Then we add the author line to the commit buffer under construction\n> based on these values.\n> \n> I suggest we back this patch out.\n\nI agree.\n\nCiao,\nDscho\n"},{"id":"59603","messageId":"7v8x530ylq.fsf@gitster.siamese.dyndns.org","threadId":"10810","inReplyTo":"1194896334.2869.7.camel@hinata.boston.redhat.com","subject":"Re: [PATCH 1/6] builtin-commit: fix author date with --amend --author=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-13T00:37:37Z","receivedAt":"2007-11-13T00:37:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristian Høgsberg writes:\n\n> I suggest we back this patch out.\n\nOk. My mistake. Your \"Add testcase for amending and fixing\nauthor in git commit\" does contain test for this.\n\nI need to amend the commit log message of that one, though ;-)\n"},{"id":"61602","messageId":"20071201222106.GA27102@coredump.intra.peff.net","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0711111736440.4362@racer.site","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-01T22:21:06Z","receivedAt":"2007-12-01T22:21:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 11, 2007 at 05:36:52PM +0000, Johannes Schindelin wrote:\n\n> The function log_tree_commit() does not break the line, so we have to\n> do it ourselves.\n\nJohannes,\n\nCan you explain the rationale for this change in more detail?\n\nWhen I run builtin-commit from the tip of next, I always get an extra\nnewline (as compared to the shell behavior):\n\n-- >8 --\n$ git version &&\n mkdir test && cd test && git init &&\n touch file && git add file && git commit -m added &&\n echo content >file && git commit -a -m updated &&\n echo done\ngit version 1.5.3.6.2090.g4ece0\nInitialized empty Git repository in .git/\nCreated initial commit b3cbe63: added\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 file\n\nCreated commit 7a6b446: updated\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndone\n-- 8< --\n\nwhere the shell behavior omits the extra newlines. Is there some other\ninput for which log_tree_commit actually needs the newline?\n\n-Peff\n"},{"id":"61604","messageId":"Pine.LNX.4.64.0712012238510.27959@racer.site","threadId":"10810","inReplyTo":"20071201222106.GA27102@coredump.intra.peff.net","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-01T22:41:47Z","receivedAt":"2007-12-01T22:41:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 1 Dec 2007, Jeff King wrote:\n\n> On Sun, Nov 11, 2007 at 05:36:52PM +0000, Johannes Schindelin wrote:\n> \n> > The function log_tree_commit() does not break the line, so we have to\n> > do it ourselves.\n> \n> Johannes,\n> \n> Can you explain the rationale for this change in more detail?\n\nBasically, I ran a test case in which the shell script was different from \nthe builtin version, and this was the patch that fixed it for me.\n\nMaybe it should have been\n\n\tif (log_tree_commit(&rev, commit))\n\t\tprintf(\"\\n\");\n\nat the end of print_summary() instead. Can you try if that fixes it for \nyou?\n\nCiao,\nDscho\n"},{"id":"61621","messageId":"20071202054030.GA393@coredump.intra.peff.net","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0712012238510.27959@racer.site","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-02T05:40:30Z","receivedAt":"2007-12-02T05:40:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 01, 2007 at 10:41:47PM +0000, Johannes Schindelin wrote:\n\n> Basically, I ran a test case in which the shell script was different from \n> the builtin version, and this was the patch that fixed it for me.\n> \n> Maybe it should have been\n> \n> \tif (log_tree_commit(&rev, commit))\n> \t\tprintf(\"\\n\");\n> \n> at the end of print_summary() instead. Can you try if that fixes it for \n> you?\n\nNo, it doesn't, unless you meant \"if (!log_tree_commit(...\". Of course,\nI can't get log_tree_commit to return anything but true anyway (either\nlog_tree_diff shows something, or since we have set always_show_header,\nwe end up calling show_log).\n\nIt would be helpful if you could remember the test case, but perhaps\nthat is not an option at this point.\n\n-Peff\n"},{"id":"61635","messageId":"Pine.LNX.4.64.0712021212490.27959@racer.site","threadId":"10810","inReplyTo":"20071202054030.GA393@coredump.intra.peff.net","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-02T12:13:07Z","receivedAt":"2007-12-02T12:13:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Dec 2007, Jeff King wrote:\n\n> On Sat, Dec 01, 2007 at 10:41:47PM +0000, Johannes Schindelin wrote:\n> \n> > Basically, I ran a test case in which the shell script was different from \n> > the builtin version, and this was the patch that fixed it for me.\n> > \n> > Maybe it should have been\n> > \n> > \tif (log_tree_commit(&rev, commit))\n> > \t\tprintf(\"\\n\");\n> > \n> > at the end of print_summary() instead. Can you try if that fixes it for \n> > you?\n> \n> No, it doesn't, unless you meant \"if (!log_tree_commit(...\". Of course,\n> I can't get log_tree_commit to return anything but true anyway (either\n> log_tree_diff shows something, or since we have set always_show_header,\n> we end up calling show_log).\n> \n> It would be helpful if you could remember the test case, but perhaps\n> that is not an option at this point.\n\nIIRC it was \"git commit -m bla\".\n\nHth,\nDscho\n"},{"id":"61655","messageId":"20071202165409.GA30998@coredump.intra.peff.net","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0712021212490.27959@racer.site","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-02T16:54:09Z","receivedAt":"2007-12-02T16:54:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 02, 2007 at 12:13:07PM +0000, Johannes Schindelin wrote:\n\n> > It would be helpful if you could remember the test case, but perhaps\n> > that is not an option at this point.\n> \n> IIRC it was \"git commit -m bla\".\n\nI have made several attempts to reproduce the problem, looked a bit\nthrough the log-tree code, and checked the results of the t750* series\nof tests; but I have found nothing. I think we are better off reverting,\nwhich fixes every case I have seen; if the problem behavior comes back,\nwe will have figured out what causes it. And if it doesn't come back,\nthen the revert is the right thing. :)\n\n-- >8 --\nRevert \"builtin-commit: Add newline when showing which commit was created\"\n\nThis reverts commit 129fa606365c172d07a5d98bea9345277f221363.\n\nWe end up in most cases with an undesired extra newline. It\nis possible that there is some corner case that requires the\nnewline, but there is no published test case. So let's fix\nthe known problem, and we can deal with the corner case if\nand when there is a bug report.\n---\n builtin-commit.c | 1 -\n 1 files changed, 0 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex f6e8e44..118853d 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -660,7 +660,6 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \tprintf(\"Created %scommit \", initial_commit ? \"initial \" : \"\");\n \n \tlog_tree_commit(&rev, commit);\n-\tprintf(\"\\n\");\n }\n \n int git_commit_config(const char *k, const char *v)\n-- \n1.5.3.6.2094.g3713\n"},{"id":"61661","messageId":"Pine.LNX.4.64.0712021716220.27959@racer.site","threadId":"10810","inReplyTo":"20071202165409.GA30998@coredump.intra.peff.net","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-02T17:18:24Z","receivedAt":"2007-12-02T17:18:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Dec 2007, Jeff King wrote:\n\n> On Sun, Dec 02, 2007 at 12:13:07PM +0000, Johannes Schindelin wrote:\n> \n> > > It would be helpful if you could remember the test case, but perhaps\n> > > that is not an option at this point.\n> > \n> > IIRC it was \"git commit -m bla\".\n> \n> I have made several attempts to reproduce the problem, looked a bit\n> through the log-tree code, and checked the results of the t750* series\n> of tests; but I have found nothing.\n\nI remember again. When I did \"commit -s -m bla\" the empty line between \nthe oneline and the signoff would be missing. But in the meantime, the \nsignoff was dragged into the strbuf and all is well.\n\nACK.\n\nCiao,\nDscho\n"},{"id":"61667","messageId":"7vtzn1x7w6.fsf@gitster.siamese.dyndns.org","threadId":"10810","inReplyTo":"Pine.LNX.4.64.0712021716220.27959@racer.site","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T18:34:49Z","receivedAt":"2007-12-02T18:34:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> I have made several attempts to reproduce the problem, looked a bit\n>> through the log-tree code, and checked the results of the t750* series\n>> of tests; but I have found nothing.\n>\n> I remember again. When I did \"commit -s -m bla\" the empty line between \n> the oneline and the signoff would be missing. But in the meantime, the \n> signoff was dragged into the strbuf and all is well.\n\nSorry, now I am confused. Building the version before that change and\ndoing \"./git-commit -a -s -m bla\", I do not see the extra blank line in\nthe \"Created commit\" response, and I see a blank line before and after\nthe sign-off in the \"git show\" output for the resulting commit.\n\nWas this unnecessary change from the beginning? I am inclined to think\nso...\n"},{"id":"61679","messageId":"Pine.LNX.4.64.0712022107090.27959@racer.site","threadId":"10810","inReplyTo":"7vtzn1x7w6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-02T21:20:09Z","receivedAt":"2007-12-02T21:20:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Dec 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> I have made several attempts to reproduce the problem, looked a bit\n> >> through the log-tree code, and checked the results of the t750* series\n> >> of tests; but I have found nothing.\n> >\n> > I remember again. When I did \"commit -s -m bla\" the empty line between \n> > the oneline and the signoff would be missing. But in the meantime, the \n> > signoff was dragged into the strbuf and all is well.\n> \n> Sorry, now I am confused. Building the version before that change and\n> doing \"./git-commit -a -s -m bla\", I do not see the extra blank line in\n> the \"Created commit\" response, and I see a blank line before and after\n> the sign-off in the \"git show\" output for the resulting commit.\n> \n> Was this unnecessary change from the beginning? I am inclined to think\n> so...\n\nIIRC I did 2150554b0ed60356d8918b610834c04ad2eecdec(builtin-commit --s: \nadd a newline if the last line was no S-O-B) after the commit that \nPeff wants to undo.\n\nSo yes, my mistake.\n\nCiao,\nDscho\n"},{"id":"61743","messageId":"7vzlwsmdvb.fsf@gitster.siamese.dyndns.org","threadId":"10810","inReplyTo":"20071201222106.GA27102@coredump.intra.peff.net","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-03T07:33:28Z","receivedAt":"2007-12-03T07:33:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Nov 11, 2007 at 05:36:52PM +0000, Johannes Schindelin wrote:\n>\n>> The function log_tree_commit() does not break the line, so we have to\n>> do it ourselves.\n>\n> Johannes,\n>\n> Can you explain the rationale for this change in more detail?\n>\n> When I run builtin-commit from the tip of next, I always get an extra\n> newline (as compared to the shell behavior):\n\nAfter reverting this, recording a merge commit seems to have lost the\nnewline. Can be easily reproduced with:\n\n\t$ git merge --no-commit some-branch\n $ git commit -a -m 'foo'\n"},{"id":"61746","messageId":"20071203075357.GB3614@sigill.intra.peff.net","threadId":"10810","inReplyTo":"7vzlwsmdvb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 6/6] builtin-commit: Add newline when showing which commit was created","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-03T07:53:57Z","receivedAt":"2007-12-03T07:53:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 02, 2007 at 11:33:28PM -0800, Junio C Hamano wrote:\n\n> After reverting this, recording a merge commit seems to have lost the\n> newline. Can be easily reproduced with:\n> \n> \t$ git merge --no-commit some-branch\n> $ git commit -a -m 'foo'\n\nSee, I knew reverting would flush it out. ;)\n\nUnfortunately, the fix isn't terribly obvious. log_tree_commit produces\noutput either from log_tree_diff, which appends a newline, or from\nshow_log, which doesn't, and returns the same value in either case.\n\nI tried adding a '%n' to the format specifier (which is \"%h: %s\"),\nbut that has inconsistent results, since another newline is always\nplaced between the diffstat and the log, anyway. The design of\n--pretty=format is a bit inflexible here, since you are stuck with the\n\"$log\\n$diff\" format. But changing it would impact users, and is likely\nto require a lot of surgery in the log machinery.\n\nSo short of munging log_tree_commit to add the newline, I'm not sure how\nto fix it, and I'm a little wary of messing with that function.\n\n-Peff\n"}]}