{"thread":{"id":"10593","subject":"[PATCH 1/4] Add testcase for ammending and fixing author in git commit.","startedAt":"2007-11-02T15:33:06Z","lastAt":"2007-11-08T16:01:48Z","messageCount":24,"participants":["Kristian Høgsberg","Andreas Ericsson","Junio C Hamano","Johannes Schindelin","Björn Steinbrink","Pierre Habouzit","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"57996","messageId":"1194017589-4669-1-git-send-email-krh@redhat.com","threadId":"10593","inReplyTo":null,"subject":"[PATCH 1/4] Add testcase for ammending and fixing author in git commit.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-02T15:33:06Z","receivedAt":"2007-11-02T15:33:06Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n---\n t/t7501-commit.sh |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex b151b51..3f2112a 100644\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -163,4 +163,14 @@ test_expect_success 'partial commit that involves removal (3)' '\n \n '\n \n+author=\"The Real Author <someguy@his.email.org>\"\n+test_expect_success 'amend commit to fix author' '\n+\n+\tgit reset --hard\n+\tgit cat-file -p HEAD | sed -e \"s/author.*>/author $author/\" > expected &&\n+\tgit commit --amend --author=\"$author\" &&\n+\tgit cat-file -p HEAD > current &&\n+\tdiff expected current\n+\t\n+'\n test_done\n-- \n1.5.3.4.206.g58ba4\n"},{"id":"57997","messageId":"1194017589-4669-2-git-send-email-krh@redhat.com","threadId":"10593","inReplyTo":"1194017589-4669-1-git-send-email-krh@redhat.com","subject":"[PATCH 2/4] Remove unecessary hard-coding of EDITOR=':' VISUAL=':' in some test suites.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-02T15:33:07Z","receivedAt":"2007-11-02T15:33:07Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n---\n t/t3501-revert-cherry-pick.sh |    4 ++--\n t/t3901-i18n-patch.sh         |    8 ++++----\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh\nindex 552af1c..2dbe04f 100755\n--- a/t/t3501-revert-cherry-pick.sh\n+++ b/t/t3501-revert-cherry-pick.sh\n@@ -44,7 +44,7 @@ test_expect_success setup '\n test_expect_success 'cherry-pick after renaming branch' '\n \n \tgit checkout rename2 &&\n-\tEDITOR=: VISUAL=: git cherry-pick added &&\n+\tgit cherry-pick added &&\n \ttest -f opos &&\n \tgrep \"Add extra line at the end\" opos\n \n@@ -53,7 +53,7 @@ test_expect_success 'cherry-pick after renaming branch' '\n test_expect_success 'revert after renaming branch' '\n \n \tgit checkout rename1 &&\n-\tEDITOR=: VISUAL=: git revert added &&\n+\tgit revert added &&\n \ttest -f spoo &&\n \t! grep \"Add extra line at the end\" spoo\n \ndiff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh\nindex 28e9e37..235f372 100755\n--- a/t/t3901-i18n-patch.sh\n+++ b/t/t3901-i18n-patch.sh\n@@ -154,7 +154,7 @@ test_expect_success 'cherry-pick(U/U)' '\n \tgit reset --hard master &&\n \tgit cherry-pick side^ &&\n \tgit cherry-pick side &&\n-\tEDITOR=: VISUAL=: git revert HEAD &&\n+\tgit revert HEAD &&\n \n \tcheck_encoding 3\n '\n@@ -169,7 +169,7 @@ test_expect_success 'cherry-pick(L/L)' '\n \tgit reset --hard master &&\n \tgit cherry-pick side^ &&\n \tgit cherry-pick side &&\n-\tEDITOR=: VISUAL=: git revert HEAD &&\n+\tgit revert HEAD &&\n \n \tcheck_encoding 3 8859\n '\n@@ -184,7 +184,7 @@ test_expect_success 'cherry-pick(U/L)' '\n \tgit reset --hard master &&\n \tgit cherry-pick side^ &&\n \tgit cherry-pick side &&\n-\tEDITOR=: VISUAL=: git revert HEAD &&\n+\tgit revert HEAD &&\n \n \tcheck_encoding 3\n '\n@@ -200,7 +200,7 @@ test_expect_success 'cherry-pick(L/U)' '\n \tgit reset --hard master &&\n \tgit cherry-pick side^ &&\n \tgit cherry-pick side &&\n-\tEDITOR=: VISUAL=: git revert HEAD &&\n+\tgit revert HEAD &&\n \n \tcheck_encoding 3 8859\n '\n-- \n1.5.3.4.206.g58ba4\n"},{"id":"57998","messageId":"1194017589-4669-3-git-send-email-krh@redhat.com","threadId":"10593","inReplyTo":"1194017589-4669-2-git-send-email-krh@redhat.com","subject":"[PATCH 3/4] Export launch_editor() and make it accept ':' as a no-op editor.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-02T15:33:08Z","receivedAt":"2007-11-02T15:33:08Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n---\n builtin-tag.c |    5 ++++-\n strbuf.h      |    1 +\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 66e5a58..bcb3a7a 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -17,7 +17,7 @@ static const char builtin_tag_usage[] =\n \n static char signingkey[1000];\n \n-static void launch_editor(const char *path, struct strbuf *buffer)\n+void launch_editor(const char *path, struct strbuf *buffer)\n {\n \tconst char *editor, *terminal;\n \tstruct child_process child;\n@@ -42,6 +42,9 @@ static void launch_editor(const char *path, struct strbuf *buffer)\n \tif (!editor)\n \t\teditor = \"vi\";\n \n+\tif (!strcmp(editor, \":\"))\n+\t\treturn;\n+\n \tmemset(&child, 0, sizeof(child));\n \tchild.argv = args;\n \targs[0] = editor;\ndiff --git a/strbuf.h b/strbuf.h\nindex 9b9e861..9720826 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -113,5 +113,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n+extern void launch_editor(const char *path, struct strbuf *buffer);\n \n #endif /* STRBUF_H */\n-- \n1.5.3.4.206.g58ba4\n"},{"id":"57999","messageId":"1194017589-4669-4-git-send-email-krh@redhat.com","threadId":"10593","inReplyTo":"1194017589-4669-3-git-send-email-krh@redhat.com","subject":"[PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-02T15:33:09Z","receivedAt":"2007-11-02T15:33:09Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Move git-commit.sh to contrib/examples.  This also removes the git-runstatus\nhelper, which was mostly just a git-status.sh implementation detail.\n\nSigned-off-by: Kristian Høgsberg <krh@redhat.com>\n---\n Makefile                                        |    9 +-\n builtin-commit.c                                |  614 +++++++++++++++++++++++\n builtin.h                                       |    3 +-\n git-commit.sh => contrib/examples/git-commit.sh |    0 \n git.c                                           |    3 +-\n 5 files changed, 621 insertions(+), 8 deletions(-)\n create mode 100644 builtin-commit.c\n rename git-commit.sh => contrib/examples/git-commit.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex 042f79e..689eb33 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -209,7 +209,7 @@ BASIC_LDFLAGS =\n \n SCRIPT_SH = \\\n \tgit-bisect.sh git-checkout.sh \\\n-\tgit-clean.sh git-clone.sh git-commit.sh \\\n+\tgit-clean.sh git-clone.sh \\\n \tgit-ls-remote.sh \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n \tgit-pull.sh git-rebase.sh git-rebase--interactive.sh \\\n@@ -256,7 +256,7 @@ EXTRA_PROGRAMS =\n BUILT_INS = \\\n \tgit-format-patch$X git-show$X git-whatchanged$X git-cherry$X \\\n \tgit-get-tar-commit-id$X git-init$X git-repo-config$X \\\n-\tgit-fsck-objects$X git-cherry-pick$X \\\n+\tgit-fsck-objects$X git-cherry-pick$X git-status$X\\\n \t$(patsubst builtin-%.o,git-%$X,$(BUILTIN_OBJS))\n \n # what 'all' will build and 'install' will install, in gitexecdir\n@@ -326,6 +326,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-check-attr.o \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\n+\tbuiltin-commit.o \\\n \tbuiltin-commit-tree.o \\\n \tbuiltin-count-objects.o \\\n \tbuiltin-describe.o \\\n@@ -364,7 +365,6 @@ BUILTIN_OBJS = \\\n \tbuiltin-rev-parse.o \\\n \tbuiltin-revert.o \\\n \tbuiltin-rm.o \\\n-\tbuiltin-runstatus.o \\\n \tbuiltin-shortlog.o \\\n \tbuiltin-show-branch.o \\\n \tbuiltin-stripspace.o \\\n@@ -830,9 +830,6 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-git-status: git-commit\n-\t$(QUIET_GEN)cp $< $@+ && mv $@+ $@\n-\n gitweb/gitweb.cgi: gitweb/gitweb.perl\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\ndiff --git a/builtin-commit.c b/builtin-commit.c\nnew file mode 100644\nindex 0000000..a5a9e3e\n--- /dev/null\n+++ b/builtin-commit.c\n@@ -0,0 +1,614 @@\n+/*\n+ * Builtin \"git commit\"\n+ *\n+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>\n+ * Based on git-commit.sh by Junio C Hamano and Linus Torvalds\n+ */\n+\n+#include \"git-compat-util.h\"\n+\n+#include \"cache.h\"\n+#include \"cache-tree.h\"\n+#include \"builtin.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+#include \"commit.h\"\n+#include \"revision.h\"\n+#include \"wt-status.h\"\n+#include \"run-command.h\"\n+#include \"refs.h\"\n+#include \"log-tree.h\"\n+#include \"strbuf.h\"\n+#include \"utf8.h\"\n+#include \"parse-options.h\"\n+\n+static const char * const builtin_commit_usage[] = {\n+\t\"git-commit [options] [--] <filepattern>...\",\n+\tNULL\n+};\n+\n+static unsigned char head_sha1[20], merge_head_sha1[20];\n+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 *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+\n+static struct option builtin_commit_options[] = {\n+\tOPT__QUIET(&quiet),\n+\tOPT__VERBOSE(&verbose),\n+\tOPT_GROUP(\"Commit message 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_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+\tOPT_STRING('t', \"template\", &template_file, \"FILE\", \"use specified template file\"),\n+\tOPT_BOOLEAN('e', \"edit\", &edit_flag, \"force edit of commit\"),\n+\n+\tOPT_GROUP(\"Commit contents options\"),\n+\tOPT_BOOLEAN('a', \"all\", &all, \"commit all changed files\"),\n+\tOPT_BOOLEAN('i', \"include\", &also, \"add specified files to index for commit\"),\n+\tOPT_BOOLEAN(0, \"interactive\", &interactive, \"interactively add files\"),\n+\tOPT_BOOLEAN('o', \"only\", &only, \"\"),\n+\tOPT_BOOLEAN('n', \"no-verify\", &no_verify, \"bypass pre-commit hook\"),\n+\tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n+\tOPT_BOOLEAN(0, \"untracked-files\", &untracked_files, \"show all untracked files\"),\n+\n+\tOPT_END()\n+};\n+\n+static char *\n+prepare_index(const char **files, const char *prefix)\n+{\n+\tint fd;\n+\tstruct tree *tree;\n+\tstruct lock_file *next_index_lock;\n+\n+\tfd = hold_locked_index(&lock_file, 1);\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\n+\tif (all) {\n+\t\tadd_files_to_cache(verbose, NULL, files);\n+\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\t\tdie(\"unable to write new_index file\");\n+\t\treturn lock_file.filename;\n+\t} else if (also) {\n+\t\tadd_files_to_cache(verbose, prefix, files);\n+\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\t\tdie(\"unable to write new_index file\");\n+\t\treturn lock_file.filename;\n+\t}\n+\n+\tif (interactive)\n+\t\tinteractive_add();\n+\n+\tif (*files == NULL) {\n+\t\t/* Commit index as-is. */\n+\t\trollback_lock_file(&lock_file);\n+\t\treturn get_index_file();\n+\t}\n+\n+\t/* update the user index file */\n+\tadd_files_to_cache(verbose, prefix, files);\n+\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\tdie(\"unable to write new_index file\");\n+\n+\tif (!initial_commit) {\n+\t\ttree = parse_tree_indirect(head_sha1);\n+\t\tif (!tree)\n+\t\t\tdie(\"failed to unpack HEAD tree object\");\n+\t\tif (read_tree(tree, 0, NULL))\n+\t\t\tdie(\"failed to read HEAD tree object\");\n+\t}\n+\n+\t/* Uh oh, abusing lock_file to create a garbage collected file */\n+\tnext_index_lock = xmalloc(sizeof(*next_index_lock));\n+\tfd = hold_lock_file_for_update(next_index_lock,\n+\t\t\t\t       git_path(\"next-index-%d\", getpid()), 1);\n+\tadd_files_to_cache(verbose, prefix, files);\n+\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\tdie(\"unable to write new_index file\");\n+\n+\treturn next_index_lock->filename;\n+}\n+\n+static int run_status(FILE *fp, const char *index_file)\n+{\n+\tstruct wt_status s;\n+\n+\twt_status_prepare(&s);\n+\n+\tif (amend) {\n+\t\ts.amend = 1;\n+\t\ts.reference = \"HEAD^1\";\n+\t}\n+\ts.verbose = verbose;\n+\ts.untracked = untracked_files;\n+\ts.index_file = index_file;\n+\ts.fp = fp;\n+\n+\twt_status_print(&s);\n+\n+\treturn s.commitable;\n+}\n+\n+static const char sign_off_header[] = \"Signed-off-by: \";\n+\n+static int prepare_log_message(const char *index_file)\n+{\n+\tstruct stat statbuf;\n+\tint commitable;\n+\tstruct strbuf sb;\n+\tchar *buffer;\n+\tFILE *fp;\n+\n+\tstrbuf_init(&sb, 0);\n+\tif (message) {\n+\t\tstrbuf_add(&sb, message, strlen(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+\t\tif (strbuf_read(&sb, 0, 0) < 0)\n+\t\t\tdie(\"could not read log from standard input\");\n+\t} else if (logfile) {\n+\t\tif (strbuf_read_file(&sb, logfile, 0) < 0)\n+\t\t\tdie(\"could not read log file '%s': %s\",\n+\t\t\t    logfile, strerror(errno));\n+\t} else if (use_message) {\n+\t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n+\t\tif (!buffer || buffer[2] == '\\0')\n+\t\t\tdie(\"commit has empty message\");\n+\t\tstrbuf_add(&sb, buffer + 2, strlen(buffer + 2));\n+\t} else if (!stat(git_path(\"MERGE_MSG\"), &statbuf)) {\n+\t\tif (strbuf_read_file(&sb, git_path(\"MERGE_MSG\"), 0) < 0)\n+\t\t\tdie(\"could not read MERGE_MSG: %s\", strerror(errno));\n+\t} else if (!stat(git_path(\"SQUASH_MSG\"), &statbuf)) {\n+\t\tif (strbuf_read_file(&sb, git_path(\"SQUASH_MSG\"), 0) < 0)\n+\t\t\tdie(\"could not read SQUASH_MSG: %s\", strerror(errno));\n+\t} else if (!stat(template_file, &statbuf)) {\n+\t\tif (strbuf_read_file(&sb, template_file, 0) < 0)\n+\t\t\tdie(\"could not read %s: %s\",\n+\t\t\t    template_file, strerror(errno));\n+\t}\n+\n+\tfp = fopen(git_path(commit_editmsg), \"w\");\n+\tif (fp == NULL)\n+\t\tdie(\"could not open %s\\n\", git_path(commit_editmsg));\n+\t\t\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}\n+\n+\tstrbuf_release(&sb);\n+\n+\tif (in_merge && !no_edit) {\n+\t\tfprintf(fp,\n+\t\t\t\"#\\n\"\n+\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n+\t\t\t\"# If this is not correct, please remove the file\\n\"\n+\t\t\t\"#\t%s\\n\"\n+\t\t\t\"# and try again.\\n\"\n+\t\t\t\"#\\n\",\n+\t\t\tgit_path(\"MERGE_HEAD\"));\n+\t}\n+\n+\tfprintf(fp,\n+\t\t\"\\n\"\n+\t\t\"# Please enter the commit message for your changes.\\n\"\n+\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\tif (only_include_assumed)\n+\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n+\n+\tcommitable = run_status(fp, index_file);\n+\n+\tfclose(fp);\n+\n+\treturn commitable;\n+}\n+\n+/* Find out if the message starting at position 'start' in the strbuf\n+ * contains only whitespace and Signed-off-by lines. */\n+static int message_is_empty(struct strbuf *sb, int start)\n+{\n+\tstruct strbuf tmpl;\n+\tconst char *nl;\n+\tint eol, i;\n+\n+\t/* See if the template is just a prefix of the message. */\n+\tstrbuf_init(&tmpl, 0);\n+\tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n+\t\tstripspace(&tmpl, 1);\n+\t\tif (start + tmpl.len <= sb->len &&\n+\t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n+\t\t\tstart += tmpl.len;\n+\t}\n+\tstrbuf_release(&tmpl);\n+\n+\t/* Check if the rest is just whitespace and Signed-of-by's. */\n+\tfor (i = start; i < sb->len; i++) {\n+\t\tnl = memchr(sb->buf + i, '\\n', sb->len - i);\n+\t\tif (nl)\n+\t\t\teol = nl - sb->buf;\n+\t\telse\n+\t\t\teol = sb->len;\n+\n+\t\tif (strlen(sign_off_header) <= eol - i &&\n+\t\t    !prefixcmp(sb->buf + i, sign_off_header)) {\n+\t\t\ti = eol;\n+\t\t\tcontinue;\n+\t\t}\n+\t\twhile (i < eol)\n+\t\t\tif (!isspace(sb->buf[i++]))\n+\t\t\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n+static void determine_author_info(struct strbuf *sb)\n+{\n+\tchar *p, *eol;\n+\tchar *name = NULL, *email = NULL;\n+\n+\tif (force_author) {\n+\t\tconst char *eoname = strstr(force_author, \" <\");\n+\t\tconst char *eomail = strchr(force_author, '>');\n+\n+\t\tif (!eoname || !eomail)\n+\t\t\tdie(\"malformed --author parameter\\n\");\n+\t\tname = xstrndup(force_author, eoname - force_author);\n+\t\temail = xstrndup(eoname + 2, eomail - eoname - 2);\n+\t\tstrbuf_addf(sb, \"author %s\\n\",\n+\t\t\t    fmt_ident(name, email, \n+\t\t\t\t      getenv(\"GIT_AUTHOR_DATE\"), 1));\n+\t\tfree(name);\n+\t\tfree(email);\n+\t} else if (use_message) {\n+\t\tp = strstr(use_message_buffer, \"\\nauthor\");\n+\t\tif (!p)\n+\t\t\tdie(\"invalid commit: %s\\n\", use_message);\n+\t\tp++;\n+\t\teol = strchr(p, '\\n');\n+\t\tif (!eol)\n+\t\t\tdie(\"invalid commit: %s\\n\", use_message);\n+\n+\t\tstrbuf_add(sb, p, eol + 1 - p);\n+\t} else {\n+\t\tstrbuf_addf(sb, \"author %s\\n\", git_author_info(1));\n+\t}\n+}\n+\n+static int parse_and_validate_options(int argc, const char *argv[])\n+{\n+\tint f = 0;\n+\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+\t\tno_edit = 1;\n+\tif (edit_flag)\n+\t\tno_edit = 0;\n+\n+\tif (get_sha1(\"HEAD\", head_sha1))\n+\t\tinitial_commit = 1;\n+\n+\tif (!get_sha1(\"MERGE_HEAD\", merge_head_sha1))\n+\t\tin_merge = 1;\n+\n+\t/* Sanity check options */\n+\tif (amend && initial_commit)\n+\t\tdie(\"You have nothing to amend.\");\n+\tif (amend && in_merge)\n+\t\tdie(\"You are in the middle of a merger -- cannot amend.\");\n+\n+\tif (use_message)\n+\t\tf++;\n+\tif (edit_message)\n+\t\tf++;\n+\tif (logfile)\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+\t\tdie(\"Option -m cannot be combined with -c/-C/-F.\");\n+\tif (edit_message)\n+\t\tuse_message = edit_message;\n+\tif (amend)\n+\t\tuse_message = \"HEAD\";\n+\tif (use_message) {\n+\t\tunsigned char sha1[20];\n+\t\tstatic char utf8[] = \"UTF-8\";\n+\t\tconst char *out_enc;\n+\t\tchar *enc, *end;\n+\t\tstruct commit *commit;\n+\n+\t\tif (get_sha1(use_message, sha1))\n+\t\t\tdie(\"could not lookup commit %s\", use_message);\n+\t\tcommit = lookup_commit(sha1);\n+\t\tif (!commit || parse_commit(commit))\n+\t\t\tdie(\"could not parse commit %s\", use_message);\n+\n+\t\tenc = strstr(commit->buffer, \"\\nencoding\");\n+\t\tif (enc) {\n+\t\t\tend = strchr(enc + 10, '\\n');\n+\t\t\tenc = xstrndup(enc + 10, end - (enc + 10));\n+\t\t} else {\n+\t\t\tenc = utf8;\n+\t\t}\n+\t\tout_enc = git_commit_encoding ? git_commit_encoding : utf8;\n+\n+\t\tif (strcmp(out_enc, enc))\n+\t\t\tuse_message_buffer =\n+\t\t\t\treencode_string(commit->buffer, out_enc, enc);\n+\n+\t\t/* If we failed to reencode the buffer, just copy it\n+\t\t * byte for byte so the user can try to fix it up.\n+\t\t * This also handles the case where input and output\n+\t\t * encodings are identical. */\n+\t\tif (use_message_buffer == NULL)\n+\t\t\tuse_message_buffer = xstrdup(commit->buffer);\n+\t\tif (enc != utf8)\n+\t\t\tfree(enc);\n+\t}\n+\n+\tif (also && only)\n+\t\tdie(\"Only one of --include/--only can be used.\");\n+\tif (!*argv && (also || (only && !amend)))\n+\t\tdie(\"No paths with --include/--only does not make sense.\");\n+\tif (!*argv && only && amend)\n+\t\tonly_include_assumed = \"Clever... amending the last one with dirty index.\";\n+\tif (*argv && !also && !only) {\n+\t\tonly_include_assumed = \"Explicit paths specified without -i nor -o; assuming --only paths...\";\n+\t\talso = 0;\n+\t}\n+\n+\tif (all && interactive)\n+\t\tdie(\"Cannot use -a, --interactive or -i at the same time.\");\n+\telse if (all && argc > 0)\n+\t\tdie(\"Paths with -a does not make sense.\");\n+\telse if (interactive && argc > 0)\n+\t\tdie(\"Paths with --interactive does not make sense.\");\n+\n+\treturn argc;\n+}\n+\n+int cmd_status(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *index_file;\n+\tint commitable;\n+\n+\tgit_config(git_status_config);\n+\n+\targc = parse_and_validate_options(argc, argv);\n+\n+\tindex_file = prepare_index(argv, prefix);\n+\n+\tcommitable = run_status(stdout, index_file);\n+\n+\trollback_lock_file(&lock_file);\n+\n+\treturn commitable ? 0 : 1;\n+}\n+\n+static int run_hook(const char *index_file, const char *name, const char *arg)\n+{\n+\tstruct child_process hook;\n+\tconst char *argv[3], *env[2];\n+\tchar index[PATH_MAX];\n+\n+\targv[0] = git_path(\"hooks/%s\", name);\n+\targv[1] = arg;\n+\targv[2] = NULL;\n+\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n+\tenv[0] = index;\n+\tenv[1] = NULL;\n+\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn 0;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.no_stdin = 1;\n+\thook.stdout_to_stderr = 1;\n+\thook.env = env;\n+\n+\treturn run_command(&hook);\n+}\n+\n+static void print_summary(const char *prefix, const unsigned char *sha1)\n+{\n+\tstruct rev_info rev;\n+\tstruct commit *commit;\n+\n+\tcommit = lookup_commit(sha1);\n+\tif (!commit)\n+\t\tdie(\"couldn't look up newly created commit\\n\");\n+\tif (!commit || parse_commit(commit))\n+\t\tdie(\"could not parse newly created commit\");\n+\n+\tinit_revisions(&rev, prefix);\n+\tsetup_revisions(0, NULL, &rev, NULL);\n+\n+\trev.abbrev = 0;\n+\trev.diff = 1;\n+\trev.diffopt.output_format =\n+\t\tDIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;\n+\n+\trev.verbose_header = 1;\n+\trev.show_root_diff = 1;\n+\trev.commit_format = get_commit_format(\"format:%h: %s\");\n+\trev.always_show_header = 1;\n+\t\t\n+\tprintf(\"Created %scommit \", initial_commit ? \"initial \" : \"\");\n+\n+\tlog_tree_commit(&rev, commit);\n+}\n+\n+int git_commit_config(const char *k, const char *v)\n+{\n+\tif (!strcmp(k, \"commit.template\")) {\n+\t\ttemplate_file = xstrdup(v);\n+\t\treturn 0;\n+\t}\n+\n+\treturn git_status_config(k, v);\n+}\n+\n+static const char commit_utf8_warn[] =\n+\"Warning: commit message does not conform to UTF-8.\\n\"\n+\"You may want to amend it after fixing the message, or set the config\\n\"\n+\"variable i18n.commitencoding to the encoding your project uses.\\n\";\n+\n+int cmd_commit(int argc, const char **argv, const char *prefix)\n+{\n+\tint header_len, parent_count = 0;\n+\tstruct strbuf sb;\n+\tconst char *index_file, *reflog_msg;\n+\tchar *nl, *header_line;\n+\tunsigned char commit_sha1[20];\n+\tstruct ref_lock *ref_lock;\n+\n+\tgit_config(git_commit_config);\n+\n+\targc = parse_and_validate_options(argc, argv);\n+\n+\tindex_file = prepare_index(argv, prefix);\n+\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+\t\tunlink(commit_editmsg);\n+\t\treturn 1;\n+\t}\n+\n+\tstrbuf_init(&sb, 0);\n+\n+\t/* Start building up the commit header */\n+\tread_cache_from(index_file);\n+\tactive_cache_tree = cache_tree();\n+\tif (cache_tree_update(active_cache_tree,\n+\t\t\t      active_cache, active_nr, 0, 0) < 0)\n+\t\tdie(\"Error building trees\");\n+\tstrbuf_addf(&sb, \"tree %s\\n\",\n+\t\t    sha1_to_hex(active_cache_tree->sha1));\n+\n+\t/* Determine parents */\n+\tif (initial_commit) {\n+\t\treflog_msg = \"commit (initial)\"; \n+\t\tparent_count = 0;\n+\t} else if (amend) {\n+\t\tstruct commit_list *c;\n+\t\tstruct commit *commit;\n+\n+\t\treflog_msg = \"commit (amend)\";\n+\t\tcommit = lookup_commit(head_sha1);\n+\t\tif (!commit || parse_commit(commit))\n+\t\t\tdie(\"could not parse HEAD commit\");\n+\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+\t} else if (in_merge) {\n+\t\tstruct strbuf m;\n+\t\tFILE *fp;\n+\n+\t\treflog_msg = \"commit (merge)\";\n+\t\tstrbuf_addf(&sb, \"parent %s\\n\", sha1_to_hex(head_sha1));\n+\t\tstrbuf_init(&m, 0);\n+\t\tfp = fopen(git_path(\"MERGE_HEAD\"), \"r\");\n+\t\tif (fp == NULL)\n+\t\t\tdie(\"could not open %s for reading: %s\",\n+\t\t\t    git_path(\"MERGE_HEAD\"), strerror(errno));\n+\t\twhile (strbuf_getline(&m, fp, '\\n') != EOF)\n+\t\t\tstrbuf_addf(&sb, \"parent %s\\n\", m.buf);\n+\t\tfclose(fp);\n+\t\tstrbuf_release(&m);\n+\t} else {\n+\t\treflog_msg = \"commit\";\n+\t\tstrbuf_addf(&sb, \"parent %s\\n\", sha1_to_hex(head_sha1));\n+\t}\n+\n+\tdetermine_author_info(&sb);\n+\tstrbuf_addf(&sb, \"committer %s\\n\", git_committer_info(1));\n+\tif (!is_encoding_utf8(git_commit_encoding))\n+\t\tstrbuf_addf(&sb, \"encoding %s\\n\", git_commit_encoding);\n+\tstrbuf_addch(&sb, '\\n');\n+\n+\t/* Get the commit message and validate it */\n+\theader_len = sb.len;\n+\tif (!no_edit) {\n+\t\tfprintf(stderr, \"launching editor, log %s\\n\", logfile);\n+\t\tlaunch_editor(git_path(commit_editmsg), &sb);\n+\t}\n+\telse if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0)\n+\t\tdie(\"could not read commit message\\n\");\n+\tif (run_hook(index_file, \"commit-msg\", commit_editmsg))\n+\t\texit(1);\n+\tstripspace(&sb, 1);\n+\tif (sb.len < header_len ||\n+\t    message_is_empty(&sb, header_len))\n+\t\tdie(\"* no commit message?  aborting commit.\");\n+\tstrbuf_addch(&sb, '\\0');\n+\tif (is_encoding_utf8(git_commit_encoding) && !is_utf8(sb.buf))\n+\t\tfprintf(stderr, commit_utf8_warn);\n+\n+\tif (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1))\n+\t\tdie(\"failed to write commit object\");\n+\t\t       \n+\tref_lock = lock_any_ref_for_update(\"HEAD\",\n+\t\t\t\t\t   initial_commit ? NULL : head_sha1,\n+\t\t\t\t\t   0);\n+\n+\tnl = strchr(sb.buf + header_len, '\\n');\n+\theader_line = xstrndup(sb.buf + header_len,\n+\t\t\t       nl - (sb.buf + header_len));\n+\tstrbuf_release(&sb);\n+\tstrbuf_addf(&sb, \"%s: %s\\n\", reflog_msg, header_line);\n+\tstrbuf_addch(&sb, '\\0');\n+\tfree(header_line);\n+\n+\tif (!ref_lock)\n+\t\tdie(\"cannot lock HEAD ref\");\n+\tif (write_ref_sha1(ref_lock, commit_sha1, sb.buf) < 0)\n+\t\tdie(\"cannot update HEAD ref\");\n+\n+\tunlink(git_path(\"MERGE_HEAD\"));\n+\tunlink(git_path(\"MERGE_MSG\"));\n+\n+\tif (lock_file.filename[0] && commit_locked_index(&lock_file))\n+\t\tdie(\"failed to write new index\");\n+\n+\trerere();\n+\n+\trun_hook(index_file, \"post-commit\", NULL);\n+\n+\tif (!quiet)\n+\t\tprint_summary(prefix, commit_sha1);\n+\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex 9a6213a..0f8456c 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -24,6 +24,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n+extern int cmd_commit(int argc, const char **argv, const char *prefix);\n extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_count_objects(int argc, const char **argv, const char *prefix);\n extern int cmd_describe(int argc, const char **argv, const char *prefix);\n@@ -68,10 +69,10 @@ extern int cmd_rev_list(int argc, const char **argv, const char *prefix);\n extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);\n extern int cmd_revert(int argc, const char **argv, const char *prefix);\n extern int cmd_rm(int argc, const char **argv, const char *prefix);\n-extern int cmd_runstatus(int argc, const char **argv, const char *prefix);\n extern int cmd_shortlog(int argc, const char **argv, const char *prefix);\n extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n+extern int cmd_status(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);\n extern int cmd_tag(int argc, const char **argv, const char *prefix);\ndiff --git a/git-commit.sh b/contrib/examples/git-commit.sh\nsimilarity index 100%\nrename from git-commit.sh\nrename to contrib/examples/git-commit.sh\ndiff --git a/git.c b/git.c\nindex 4e10581..8522095 100644\n--- a/git.c\n+++ b/git.c\n@@ -298,6 +298,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"cherry\", cmd_cherry, RUN_SETUP },\n \t\t{ \"cherry-pick\", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"commit\", cmd_commit, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n \t\t{ \"config\", cmd_config },\n \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n@@ -346,10 +347,10 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"rev-parse\", cmd_rev_parse, RUN_SETUP },\n \t\t{ \"revert\", cmd_revert, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"rm\", cmd_rm, RUN_SETUP | NEED_WORK_TREE },\n-\t\t{ \"runstatus\", cmd_runstatus, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"shortlog\", cmd_shortlog, RUN_SETUP | USE_PAGER },\n \t\t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t\t{ \"show\", cmd_show, RUN_SETUP | USE_PAGER },\n+\t\t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"stripspace\", cmd_stripspace },\n \t\t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n \t\t{ \"tag\", cmd_tag, RUN_SETUP },\n-- \n1.5.3.4.206.g58ba4\n"},{"id":"58001","messageId":"472B466E.8070805@op5.se","threadId":"10593","inReplyTo":"1194017589-4669-3-git-send-email-krh@redhat.com","subject":"Re: [PATCH 3/4] Export launch_editor() and make it accept ':' as a no-op editor.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-02T15:46:54Z","receivedAt":"2007-11-02T15:46:54Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Kristian Høgsberg wrote:\n>  \n> +\tif (!strcmp(editor, \":\"))\n> +\t\treturn;\n> +\n\nDoesn't this make the change in 2/4 superfluous?\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"58006","messageId":"1194020172.25288.14.camel@hinata.boston.redhat.com","threadId":"10593","inReplyTo":"472B466E.8070805@op5.se","subject":"Re: [PATCH 3/4] Export launch_editor() and make it accept ':' as a no-op editor.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-02T16:16:12Z","receivedAt":"2007-11-02T16:16:12Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Fri, 2007-11-02 at 16:46 +0100, Andreas Ericsson wrote:\n> Kristian Høgsberg wrote:\n> >  \n> > +\tif (!strcmp(editor, \":\"))\n> > +\t\treturn;\n> > +\n> \n> Doesn't this make the change in 2/4 superfluous?\n\nYeah, but it's still a nice cleanup, I think.\n\nKristian\n"},{"id":"58029","messageId":"7vy7dg1kdg.fsf@gitster.siamese.dyndns.org","threadId":"10593","inReplyTo":"1194017589-4669-1-git-send-email-krh@redhat.com","subject":"Re: [PATCH 1/4] Add testcase for ammending and fixing author in git commit.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-02T20:07:55Z","receivedAt":"2007-11-02T20:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristian Høgsberg <krh@redhat.com> writes:\n\n> Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n> ---\n>  t/t7501-commit.sh |   10 ++++++++++\n>  1 files changed, 10 insertions(+), 0 deletions(-)\n>\n> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\n> index b151b51..3f2112a 100644\n> --- a/t/t7501-commit.sh\n> +++ b/t/t7501-commit.sh\n> @@ -163,4 +163,14 @@ test_expect_success 'partial commit that involves removal (3)' '\n>  \n>  '\n>  \n> +author=\"The Real Author <someguy@his.email.org>\"\n> +test_expect_success 'amend commit to fix author' '\n> +\n> +\tgit reset --hard\n> +\tgit cat-file -p HEAD | sed -e \"s/author.*>/author $author/\" > expected &&\n> +\tgit commit --amend --author=\"$author\" &&\n> +\tgit cat-file -p HEAD > current &&\n> +\tdiff expected current\n> +\t\n> +'\n>  test_done\n\nThis can't be right.  How are you ignoring the differences in\ncommitter dates?\n\nBy the way, I _think_ git-commit.sh allows fixing author name/email\nwithout molesting the author timestamp (i.e. takes it from the\namended commit).  That should probably be checked with the test\nas well.\n"},{"id":"58030","messageId":"7vtzo41ka3.fsf@gitster.siamese.dyndns.org","threadId":"10593","inReplyTo":"1194017589-4669-2-git-send-email-krh@redhat.com","subject":"Re: [PATCH 2/4] Remove unecessary hard-coding of EDITOR=':' VISUAL=':' in some test suites.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-02T20:09:56Z","receivedAt":"2007-11-02T20:09:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristian Høgsberg <krh@redhat.com> writes:\n\n> Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n> ---\n>  t/t3501-revert-cherry-pick.sh |    4 ++--\n>  t/t3901-i18n-patch.sh         |    8 ++++----\n>  2 files changed, 6 insertions(+), 6 deletions(-)\n\nThis is a good patch, but a real commit message that says why\nthey are unnecessary is lacking.  Something like...\n\n        Sourcing of ./test-lib.sh at the very beginning of test scripts\n        already define and export them.\n"},{"id":"58040","messageId":"1194038030.25288.20.camel@hinata.boston.redhat.com","threadId":"10593","inReplyTo":"7vy7dg1kdg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/4] Add testcase for ammending and fixing author in git commit.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-02T21:13:50Z","receivedAt":"2007-11-02T21:13:50Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Fri, 2007-11-02 at 13:07 -0700, Junio C Hamano wrote:\n> Kristian Høgsberg <krh@redhat.com> writes:\n> \n> > Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n> > ---\n> >  t/t7501-commit.sh |   10 ++++++++++\n> >  1 files changed, 10 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\n> > index b151b51..3f2112a 100644\n> > --- a/t/t7501-commit.sh\n> > +++ b/t/t7501-commit.sh\n> > @@ -163,4 +163,14 @@ test_expect_success 'partial commit that involves removal (3)' '\n> >  \n> >  '\n> >  \n> > +author=\"The Real Author <someguy@his.email.org>\"\n> > +test_expect_success 'amend commit to fix author' '\n> > +\n> > +\tgit reset --hard\n> > +\tgit cat-file -p HEAD | sed -e \"s/author.*>/author $author/\" > expected &&\n> > +\tgit commit --amend --author=\"$author\" &&\n> > +\tgit cat-file -p HEAD > current &&\n> > +\tdiff expected current\n> > +\t\n> > +'\n> >  test_done\n> \n> This can't be right.  How are you ignoring the differences in\n> committer dates?\n\nt/test-lib.sh fixes GIT_COMMITTER_DATE so all commits have the date\n\ncommitter C O Mitter <committer@example.com> 1112911993 -0700\n\nunless you use test_tick.\n\n> By the way, I _think_ git-commit.sh allows fixing author name/email\n> without molesting the author timestamp (i.e. takes it from the\n> amended commit).  That should probably be checked with the test\n> as well.\n\nYou're right, I need to pick GIT_AUTHOR_DATE from the ammended commit.\nOk, I'll need to rewrite determine_author_info() a little bit.  I might\nget an update patch out this weekend.\n\ncheers,\nKristian\n"},{"id":"58050","messageId":"7vk5p0z39l.fsf@gitster.siamese.dyndns.org","threadId":"10593","inReplyTo":"1194038030.25288.20.camel@hinata.boston.redhat.com","subject":"Re: [PATCH 1/4] Add testcase for ammending and fixing author in git commit.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-02T22:33:26Z","receivedAt":"2007-11-02T22:33:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristian Høgsberg <krh@redhat.com> writes:\n\n>> This can't be right.  How are you ignoring the differences in\n>> committer dates?\n>\n> t/test-lib.sh fixes GIT_COMMITTER_DATE so all commits have the date\n>\n> committer C O Mitter <committer@example.com> 1112911993 -0700\n>\n> unless you use test_tick.\n\nHeh, that was actually what I was getting at.  By not doing\ntest_tick, you are not making sure that the commit gets the\ntimestamp of the time --amend is run.\n\n>> By the way, I _think_ git-commit.sh allows fixing author name/email\n>> without molesting the author timestamp (i.e. takes it from the\n>> amended commit).  That should probably be checked with the test\n>> as well.\n>\n> You're right, I need to pick GIT_AUTHOR_DATE from the ammended commit.\n> Ok, I'll need to rewrite determine_author_info() a little bit.  I might\n> get an update patch out this weekend.\n"},{"id":"58134","messageId":"Pine.LNX.4.64.0711031328500.4362@racer.site","threadId":"10593","inReplyTo":"1194017589-4669-4-git-send-email-krh@redhat.com","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-03T13:56:24Z","receivedAt":"2007-11-03T13:56:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 Nov 2007, Kristian Høgsberg wrote:\n\n> +static char *\n> +prepare_index(const char **files, const char *prefix)\n> +{\n> +\tint fd;\n> +\tstruct tree *tree;\n> +\tstruct lock_file *next_index_lock;\n> +\n> +\tfd = hold_locked_index(&lock_file, 1);\n> +\tif (read_cache() < 0)\n> +\t\tdie(\"index file corrupt\");\n> +\n> +\tif (all) {\n> +\t\tadd_files_to_cache(verbose, NULL, files);\n> +\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n> +\t\t\tdie(\"unable to write new_index file\");\n> +\t\treturn lock_file.filename;\n> +\t} else if (also) {\n> +\t\tadd_files_to_cache(verbose, prefix, files);\n> +\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n> +\t\t\tdie(\"unable to write new_index file\");\n> +\t\treturn lock_file.filename;\n> +\t}\n\nUnless something slips by my mind, this could be written as\n\n\tif (all || also) {\n\t\tadd_files_to_cache(verbose, also ? prefix : NULL, files);\n\t\t...\n\t}\n\n> +\n> +\tif (interactive)\n> +\t\tinteractive_add();\n> +\n> +\tif (*files == NULL) {\n> +\t\t/* Commit index as-is. */\n> +\t\trollback_lock_file(&lock_file);\n> +\t\treturn get_index_file();\n> +\t}\n\nWould an interactive add not conflict with this rollback?  And indeed with \nthe locked index to begin with?\n\n> +\t/* update the user index file */\n> +\tadd_files_to_cache(verbose, prefix, files);\n> +\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n> +\t\tdie(\"unable to write new_index file\");\n\nDoes that mean that the index is _always_ written?  Even when not \nspecifying and paths on the command line?\n\n> +\t/* Uh oh, abusing lock_file to create a garbage collected file */\n\nIt's not that bad.  But I would mention that it is a temporary index which \nyou are building.\n\n> +static const char sign_off_header[] = \"Signed-off-by: \";\n\nFunny, I thought it was a footer ;-)\n\n> +\t} else if (!stat(template_file, &statbuf)) {\n\nShould this not test \"if (template_file && \" first?\n\n> +/* Find out if the message starting at position 'start' in the strbuf\n> + * contains only whitespace and Signed-off-by lines. */\n> +static int message_is_empty(struct strbuf *sb, int start)\n> +{\n> +\tstruct strbuf tmpl;\n> +\tconst char *nl;\n> +\tint eol, i;\n> +\n> +\t/* See if the template is just a prefix of the message. */\n> +\tstrbuf_init(&tmpl, 0);\n> +\tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n> +\t\tstripspace(&tmpl, 1);\n> +\t\tif (start + tmpl.len <= sb->len &&\n> +\t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n> +\t\t\tstart += tmpl.len;\n> +\t}\n> +\tstrbuf_release(&tmpl);\n\nThe release could go inside the if block, no?\n\n> +static int run_hook(const char *index_file, const char *name, const char *arg)\n\nWould this function not prefer to live in run-command.c?\n\n> +{\n> +\tstruct child_process hook;\n> +\tconst char *argv[3], *env[2];\n> +\tchar index[PATH_MAX];\n> +\n> +\targv[0] = git_path(\"hooks/%s\", name);\n> +\targv[1] = arg;\n> +\targv[2] = NULL;\n> +\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n> +\tenv[0] = index;\n> +\tenv[1] = NULL;\n> +\n> +\tif (access(argv[0], X_OK) < 0)\n> +\t\treturn 0;\n\nThat check logically belongs 6 lines higher...\n\n> +\trev.abbrev = 0;\n> +\trev.diff = 1;\n> +\trev.diffopt.output_format =\n> +\t\tDIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;\n> +\n> +\trev.verbose_header = 1;\n> +\trev.show_root_diff = 1;\n> +\trev.commit_format = get_commit_format(\"format:%h: %s\");\n\nThat's interesting.  Wouldn't have thought of that.  Reusing the log_tree \nmachinery to output the summary.  Cute.\n\nNote that one relatively low-hanging fruit will be to teach builtin-commit \nto use a cheaper \"no changes?\" check when no_edit = 1.\n\nThanks,\nDscho\n"},{"id":"58144","messageId":"20071103150637.GA11172@atjola.homenet","threadId":"10593","inReplyTo":"1194017589-4669-4-git-send-email-krh@redhat.com","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-03T15:06:37Z","receivedAt":"2007-11-03T15:06:37Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.02 11:33:09 -0400, Kristian Høgsberg wrote:\n> +\tif (all && interactive)\n> +\t\tdie(\"Cannot use -a, --interactive or -i at the same time.\");\n\nShouldn't that be \"if (all && (interactive || also))\"?\n\nBjörn\n"},{"id":"58398","messageId":"1194289073.13968.16.camel@hinata.boston.redhat.com","threadId":"10593","inReplyTo":"20071103150637.GA11172@atjola.homenet","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-05T18:57:53Z","receivedAt":"2007-11-05T18:57:53Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Sat, 2007-11-03 at 16:06 +0100, Björn Steinbrink wrote:\n> On 2007.11.02 11:33:09 -0400, Kristian Høgsberg wrote:\n> > +\tif (all && interactive)\n> > +\t\tdie(\"Cannot use -a, --interactive or -i at the same time.\");\n> \n> Shouldn't that be \"if (all && (interactive || also))\"?\n\nThe shell script just has\n\ncase \"$all,$interactive,$also,$#\" in\n*t,*t,*)\n        die \"Cannot use -a, --interactive or -i at the same time.\" ;;\n\nwhich doesn't seem to care about the value of $also.  As far as I\nunderstand git commit, it doesn't make sense to pass any of -a, -i, -o\nor --interactive at the same time so I guess I could join the checks\n\n        if (also && only)\n                die(\"Only one of --include/--only can be used.\");\n        if (all && interactive)\n                die(\"Cannot use -a, --interactive or -i at the same\ntime.\");\n\ninto something like\n\n\tif (also + only + all + interactive > 1)              \n\t\tdie(\"Only one of --include/--only/--all/--interactive can be used.\");\n\nDoes that sound right?\n\nKristian\n"},{"id":"58403","messageId":"20071105192347.GA29997@atjola.homenet","threadId":"10593","inReplyTo":"1194289073.13968.16.camel@hinata.boston.redhat.com","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-05T19:23:47Z","receivedAt":"2007-11-05T19:23:47Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.05 13:57:53 -0500, Kristian Høgsberg wrote:\n> On Sat, 2007-11-03 at 16:06 +0100, Björn Steinbrink wrote:\n> > On 2007.11.02 11:33:09 -0400, Kristian Høgsberg wrote:\n> > > +\tif (all && interactive)\n> > > +\t\tdie(\"Cannot use -a, --interactive or -i at the same time.\");\n> > \n> > Shouldn't that be \"if (all && (interactive || also))\"?\n\nAh, damn. I (obviously) misread the error message as:\n\tfoo and (bar or bar2)\n\nIOW \"and\" instead of the first comma.\n\nActually it should be:\n\tif ((all && (interactive || also)) || (interactive && also))\n\n(or whatever more simple version you come up with)\n\n> The shell script just has\n> \n> case \"$all,$interactive,$also,$#\" in\n> *t,*t,*)\n>         die \"Cannot use -a, --interactive or -i at the same time.\" ;;\n> \n> which doesn't seem to care about the value of $also.  As far as I\n> understand git commit, it doesn't make sense to pass any of -a, -i, -o\n> or --interactive at the same time so I guess I could join the checks\n\nNote that there are only two commas. The asterisks catch everything and\n$# won't be \"t\", so that catches anything with at least two t's.\n\nAlso note, that the two checks after that one in git-commit.sh as of the\ncurrent master (140dd77a5cb2e61dcb942e245a2474fae95e42a5) are broken\n(I'll send a patch in a separate mail).\n\n>         if (also && only)\n>                 die(\"Only one of --include/--only can be used.\");\n>         if (all && interactive)\n>                 die(\"Cannot use -a, --interactive or -i at the same\n> time.\");\n> \n> into something like\n> \n> \tif (also + only + all + interactive > 1)              \n> \t\tdie(\"Only one of --include/--only/--all/--interactive can be used.\");\n> \n> Does that sound right?\n\n--include is not in the manpage, and I only glanced over git-commit.sh,\nbut yeah, at least to me, that sounds right.\n\nBjörn\n"},{"id":"58448","messageId":"Pine.LNX.4.64.0711052317170.4362@racer.site","threadId":"10593","inReplyTo":"20071105192347.GA29997@atjola.homenet","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-05T23:18:36Z","receivedAt":"2007-11-05T23:18:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 5 Nov 2007, Bj?rn Steinbrink wrote:\n\n> On 2007.11.05 13:57:53 -0500, Kristian H?gsberg wrote:\n>\n> > The shell script just has\n> > \n> > case \"$all,$interactive,$also,$#\" in\n> > *t,*t,*)\n> >         die \"Cannot use -a, --interactive or -i at the same time.\" ;;\n> > \n> > which doesn't seem to care about the value of $also.  As far as I \n> > understand git commit, it doesn't make sense to pass any of -a, -i, -o \n> > or --interactive at the same time so I guess I could join the checks\n> \n> Note that there are only two commas. The asterisks catch everything and\n> $# won't be \"t\", so that catches anything with at least two t's.\n\nSo shouldn't it be\n\n\tif (!!all + !!interactive + !!also > 1)\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"58494","messageId":"20071106065941.GA6423@atjola.homenet","threadId":"10593","inReplyTo":"Pine.LNX.4.64.0711052317170.4362@racer.site","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-06T06:59:41Z","receivedAt":"2007-11-06T06:59:41Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.05 23:18:36 +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 5 Nov 2007, Bj?rn Steinbrink wrote:\n> \n> > On 2007.11.05 13:57:53 -0500, Kristian H?gsberg wrote:\n> >\n> > > The shell script just has\n> > > \n> > > case \"$all,$interactive,$also,$#\" in\n> > > *t,*t,*)\n> > >         die \"Cannot use -a, --interactive or -i at the same time.\" ;;\n> > > \n> > > which doesn't seem to care about the value of $also.  As far as I \n> > > understand git commit, it doesn't make sense to pass any of -a, -i, -o \n> > > or --interactive at the same time so I guess I could join the checks\n> > \n> > Note that there are only two commas. The asterisks catch everything and\n> > $# won't be \"t\", so that catches anything with at least two t's.\n> \n> So shouldn't it be\n> \n> \tif (!!all + !!interactive + !!also > 1)\n> \n> Hmm?\n\nAh, yeah, that's the short and sweet version, I always forget about the\nconversion to bool giving you 0/1 values... ;-)\n\nNote though, that Kristian had a similar check at the end of his email,\nthat included \"only\" (but lacked the bool conversion). The original\nreason why I thought that it would be better was that for example\n\"git commit --all --only foo\" didn't care about \"only\" at all. But that\nactually was because the --all + paths usage check was broken. So the\nfixed version actually refuses to use accept that, but with a (IMHO) not\nso good error message:\n\n$ git commit -a -o file\nPaths with -a does not make sense.\n\nGiven that some people are used to just pass -a all the time, they might\njust automatically pass it together with -o. And I think that we\nactually want to tell them that -a + -o makes no sense instead. Just\nlike we do for -a + -i, which is kind of the complementary usage error.\n\nSo I'd go for a correct version of Kristian's suggestion:\n\nif (!!also + !!only + !!all + !!interactive > 1)\n\tdie(\"Only one of --include/--only/--all/--interactive can be used.\");\n\nBjörn\n"},{"id":"58512","messageId":"20071106091222.GE4435@artemis.corp","threadId":"10593","inReplyTo":"Pine.LNX.4.64.0711052317170.4362@racer.site","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-06T09:12:22Z","receivedAt":"2007-11-06T09:12:22Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Nov 05, 2007 at 11:18:36PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 5 Nov 2007, Bj?rn Steinbrink wrote:\n> \n> > On 2007.11.05 13:57:53 -0500, Kristian H?gsberg wrote:\n> >\n> > > The shell script just has\n> > > \n> > > case \"$all,$interactive,$also,$#\" in\n> > > *t,*t,*)\n> > >         die \"Cannot use -a, --interactive or -i at the same time.\" ;;\n> > > \n> > > which doesn't seem to care about the value of $also.  As far as I \n> > > understand git commit, it doesn't make sense to pass any of -a, -i, -o \n> > > or --interactive at the same time so I guess I could join the checks\n> > \n> > Note that there are only two commas. The asterisks catch everything and\n> > $# won't be \"t\", so that catches anything with at least two t's.\n> \n> So shouldn't it be\n> \n> \tif (!!all + !!interactive + !!also > 1)\n\nBtw, I'm starting to work slowly on the diff_opt_parse conversion to the\nmacro we discussed, and the need for new option parsing callbacks\narised, and I've created a:\n\n  parse_opt_mask_{or,and,xor} commands that you declare this way:\n\n    OPT_MASK_OR('a', \"all\",         &mode, \"...\", MASK_ALL),\n    OPT_MASK_OR('i', \"interactive\", &mode, \"...\", MASK_INTERACTIVE),\n    ...\n\nAnd if you chose MASK_ALL/INTERACTIVE/.. to be single bits,\n\n    if (!!all + !!interactive ... > 1)\n\nbecomes[0]:\n\n    if (mode & (mode - 1)) {\n\n    }\n\nI've not read your patch thoroughly, but if you feel such a thing would\nhelp you, I could send a preliminar set of patches to enable this\nfeature for people that may need it. Though you can look at my WIP on my\ngit public repository[1].\n\nFor those who care, this need arised because parse_diff_opts have a\n_lot_ of single bit options, and that expansing it on many many full\nblown integers looked like a regression, so I took the option to have a\n`flags` member with explicit masks, and those masks will be used from\nthe parse-option callbacks[2]\n\n\n  [0] for those who don't already know it, (i & (i - 1)) == 0\n      iff i is 0 or a power of 2.\n\n  [1] the patch[3]:\n      http://git.madism.org/?p=git.git;a=commitdiff;h=9d75b0a00915fa81657934f36318c1c0f5bac96b\n      example of use:\n      http://git.madism.org/?p=git.git;a=commitdiff;h=74aacc487579d0cbd638adf883e743caeaee0f76\n      http://git.madism.org/?p=git.git;a=commitdiff;h=af15793dde94119faa1577c9eec7e839ae628011\n\n  [2] http://git.madism.org/?p=git.git;a=commitdiff;h=86032204f1bdf5da2fee555ec917709b3e6f662c#patch10\n\n  [3] oh boy gitweb urls are really way too long :/\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"58513","messageId":"4730314A.9010403@viscovery.net","threadId":"10593","inReplyTo":"20071106091222.GE4435@artemis.corp","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-11-06T09:18:02Z","receivedAt":"2007-11-06T09:18:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Pierre Habouzit schrieb:\n> Btw, I'm starting to work slowly on the diff_opt_parse conversion to the\n> macro we discussed, and the need for new option parsing callbacks\n> arised, and I've created a:\n> \n>   parse_opt_mask_{or,and,xor} commands that you declare this way:\n> \n>     OPT_MASK_OR('a', \"all\",         &mode, \"...\", MASK_ALL),\n>     OPT_MASK_OR('i', \"interactive\", &mode, \"...\", MASK_INTERACTIVE),\n>     ...\n> \n> And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,\n> \n>     if (!!all + !!interactive ... > 1)\n> \n> becomes[0]:\n> \n>     if (mode & (mode - 1)) {\n> \n>     }\n\nThis goes too far, IMHO. That's unnecessary cleverness/microoptimization at \nthe expense of readability.\n\n-- Hannes\n"},{"id":"58514","messageId":"20071106092648.GG4435@artemis.corp","threadId":"10593","inReplyTo":"4730314A.9010403@viscovery.net","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-06T09:26:48Z","receivedAt":"2007-11-06T09:26:48Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Nov 06, 2007 at 09:18:02AM +0000, Johannes Sixt wrote:\n> Pierre Habouzit schrieb:\n> >Btw, I'm starting to work slowly on the diff_opt_parse conversion to the\n> >macro we discussed, and the need for new option parsing callbacks\n> >arised, and I've created a:\n> >  parse_opt_mask_{or,and,xor} commands that you declare this way:\n> >    OPT_MASK_OR('a', \"all\",         &mode, \"...\", MASK_ALL),\n> >    OPT_MASK_OR('i', \"interactive\", &mode, \"...\", MASK_INTERACTIVE),\n> >    ...\n> >And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,\n> >    if (!!all + !!interactive ... > 1)\n> >becomes[0]:\n> >    if (mode & (mode - 1)) {\n> >    }\n> \n> This goes too far, IMHO. That's unnecessary cleverness/microoptimization \n> at the expense of readability.\n\nThe reason why I did that is not to be able to do mode & (mode - 1).\nHave a look at diff.c, imagine the insane amount of intermediate\nvariables we would need, and now understand why I introduced that :)\n\nI'm not sure it's useful for git-commit, I was mentioning it just in case.\n\nNote: the fact that an OPT_BOOLEAN when you repeat it increments the\n      value isn't always a good thing, and is the reason why you need\n      the (quite ugly) double bangs.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"58517","messageId":"47303828.3060106@op5.se","threadId":"10593","inReplyTo":"20071106092648.GG4435@artemis.corp","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-06T09:47:20Z","receivedAt":"2007-11-06T09:47:20Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Pierre Habouzit wrote:\n> On Tue, Nov 06, 2007 at 09:18:02AM +0000, Johannes Sixt wrote:\n>> Pierre Habouzit schrieb:\n>>> Btw, I'm starting to work slowly on the diff_opt_parse conversion to the\n>>> macro we discussed, and the need for new option parsing callbacks\n>>> arised, and I've created a:\n>>>  parse_opt_mask_{or,and,xor} commands that you declare this way:\n>>>    OPT_MASK_OR('a', \"all\",         &mode, \"...\", MASK_ALL),\n>>>    OPT_MASK_OR('i', \"interactive\", &mode, \"...\", MASK_INTERACTIVE),\n>>>    ...\n>>> And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,\n>>>    if (!!all + !!interactive ... > 1)\n>>> becomes[0]:\n>>>    if (mode & (mode - 1)) {\n>>>    }\n>> This goes too far, IMHO. That's unnecessary cleverness/microoptimization \n>> at the expense of readability.\n> \n> The reason why I did that is not to be able to do mode & (mode - 1).\n\nWith a macro along the lines of\n\n  #define IS_MULTI_FLAGGED(x) (x & (x - 1))\n\nat least this particular construct becomes a lot more readable.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"58549","messageId":"1194367366.20020.5.camel@hinata.boston.redhat.com","threadId":"10593","inReplyTo":"20071106091222.GE4435@artemis.corp","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-06T16:42:46Z","receivedAt":"2007-11-06T16:42:46Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Tue, 2007-11-06 at 10:12 +0100, Pierre Habouzit wrote:\n> On Mon, Nov 05, 2007 at 11:18:36PM +0000, Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Mon, 5 Nov 2007, Bj?rn Steinbrink wrote:\n> > \n> > > On 2007.11.05 13:57:53 -0500, Kristian H?gsberg wrote:\n> > >\n> > > > The shell script just has\n> > > > \n> > > > case \"$all,$interactive,$also,$#\" in\n> > > > *t,*t,*)\n> > > >         die \"Cannot use -a, --interactive or -i at the same time.\" ;;\n> > > > \n> > > > which doesn't seem to care about the value of $also.  As far as I \n> > > > understand git commit, it doesn't make sense to pass any of -a, -i, -o \n> > > > or --interactive at the same time so I guess I could join the checks\n> > > \n> > > Note that there are only two commas. The asterisks catch everything and\n> > > $# won't be \"t\", so that catches anything with at least two t's.\n> > \n> > So shouldn't it be\n> > \n> > \tif (!!all + !!interactive + !!also > 1)\n> \n> Btw, I'm starting to work slowly on the diff_opt_parse conversion to the\n> macro we discussed, and the need for new option parsing callbacks\n> arised, and I've created a:\n> \n>   parse_opt_mask_{or,and,xor} commands that you declare this way:\n> \n>     OPT_MASK_OR('a', \"all\",         &mode, \"...\", MASK_ALL),\n>     OPT_MASK_OR('i', \"interactive\", &mode, \"...\", MASK_INTERACTIVE),\n>     ...\n\nThat's useful here, and it should be useful in a lot of other places\nusing parse_options() where we end up or'ing flags into a mask based on\nboolean options.\n\ncheers,\nKristian\n"},{"id":"58550","messageId":"1194367619.20020.9.camel@hinata.boston.redhat.com","threadId":"10593","inReplyTo":"20071106065941.GA6423@atjola.homenet","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-06T16:46:59Z","receivedAt":"2007-11-06T16:46:59Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Tue, 2007-11-06 at 07:59 +0100, Björn Steinbrink wrote:\n...\n> Note though, that Kristian had a similar check at the end of his email,\n> that included \"only\" (but lacked the bool conversion). The original\n> reason why I thought that it would be better was that for example\n> \"git commit --all --only foo\" didn't care about \"only\" at all. But that\n> actually was because the --all + paths usage check was broken. So the\n> fixed version actually refuses to use accept that, but with a (IMHO) not\n> so good error message:\n> \n> $ git commit -a -o file\n> Paths with -a does not make sense.\n> \n> Given that some people are used to just pass -a all the time, they might\n> just automatically pass it together with -o. And I think that we\n> actually want to tell them that -a + -o makes no sense instead. Just\n> like we do for -a + -i, which is kind of the complementary usage error.\n> \n> So I'd go for a correct version of Kristian's suggestion:\n> \n> if (!!also + !!only + !!all + !!interactive > 1)\n> \tdie(\"Only one of --include/--only/--all/--interactive can be used.\");\n\nGood points, I will use that in the next version of the patch.  Just a\nnote about the !! idiom (which I can't stand, fwiw): my version just\nadded the variables, which were all integers, initialized to zero and\nincremented by the option parser when it sees the corresponding option.\nSo what I had would work too, with the extra check that:\n\n  $ git commit -a -a\n\netc would give the error\n\n  Only one of --include/--only/--all/--interactive can be used.\n\nwhich is acutally accurate.\n\ncheers,\nKristian\n"},{"id":"58553","messageId":"20071106170813.GA27100@atjola.homenet","threadId":"10593","inReplyTo":"1194367619.20020.9.camel@hinata.boston.redhat.com","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-06T17:08:13Z","receivedAt":"2007-11-06T17:08:13Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.06 11:46:59 -0500, Kristian Høgsberg wrote:\n> On Tue, 2007-11-06 at 07:59 +0100, Björn Steinbrink wrote:\n> ...\n> > Note though, that Kristian had a similar check at the end of his email,\n> > that included \"only\" (but lacked the bool conversion). The original\n> > reason why I thought that it would be better was that for example\n> > \"git commit --all --only foo\" didn't care about \"only\" at all. But that\n> > actually was because the --all + paths usage check was broken. So the\n> > fixed version actually refuses to use accept that, but with a (IMHO) not\n> > so good error message:\n> > \n> > $ git commit -a -o file\n> > Paths with -a does not make sense.\n> > \n> > Given that some people are used to just pass -a all the time, they might\n> > just automatically pass it together with -o. And I think that we\n> > actually want to tell them that -a + -o makes no sense instead. Just\n> > like we do for -a + -i, which is kind of the complementary usage error.\n> > \n> > So I'd go for a correct version of Kristian's suggestion:\n> > \n> > if (!!also + !!only + !!all + !!interactive > 1)\n> > \tdie(\"Only one of --include/--only/--all/--interactive can be used.\");\n> \n> Good points, I will use that in the next version of the patch.  Just a\n> note about the !! idiom (which I can't stand, fwiw): my version just\n> added the variables, which were all integers, initialized to zero and\n> incremented by the option parser when it sees the corresponding option.\n> So what I had would work too, with the extra check that:\n> \n>   $ git commit -a -a\n> \n> etc would give the error\n> \n>   Only one of --include/--only/--all/--interactive can be used.\n> \n> which is acutally accurate.\n\nHm, why? The user used only one of them. The error message does not say\nthat you cannot pass the same one multiple times. And I don't think that\npassing the same boolean flag twice should be treated as an usage error\neither. There's no contradiction in wanting all files and, well, all\nfiles to be committed.\n\nSidenote: The \"or mask\" stuff in the option parser would probably\nprevent you from catching \"-a -a\" anyway, because the flag becomes truly\nboolean ;-)\n\nBjörn\n"},{"id":"58914","messageId":"1194537708.27305.6.camel@hinata.boston.redhat.com","threadId":"10593","inReplyTo":"Pine.LNX.4.64.0711031328500.4362@racer.site","subject":"Re: [PATCH 4/4] Implement git commit and status as a builtin commands.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-11-08T16:01:48Z","receivedAt":"2007-11-08T16:01:48Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Sat, 2007-11-03 at 13:56 +0000, Johannes Schindelin wrote: \n> Hi,\n\nHi, wanted to send this out earlier, but I punted it and a couple of\ndays went by...\n\n> On Fri, 2 Nov 2007, Kristian Høgsberg wrote:\n> \n> > +static char *\n> > +prepare_index(const char **files, const char *prefix)\n> > +{\n> > +\tint fd;\n> > +\tstruct tree *tree;\n> > +\tstruct lock_file *next_index_lock;\n> > +\n> > +\tfd = hold_locked_index(&lock_file, 1);\n> > +\tif (read_cache() < 0)\n> > +\t\tdie(\"index file corrupt\");\n> > +\n> > +\tif (all) {\n> > +\t\tadd_files_to_cache(verbose, NULL, files);\n> > +\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n> > +\t\t\tdie(\"unable to write new_index file\");\n> > +\t\treturn lock_file.filename;\n> > +\t} else if (also) {\n> > +\t\tadd_files_to_cache(verbose, prefix, files);\n> > +\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n> > +\t\t\tdie(\"unable to write new_index file\");\n> > +\t\treturn lock_file.filename;\n> > +\t}\n> \n> Unless something slips by my mind, this could be written as\n> \n> \tif (all || also) {\n> \t\tadd_files_to_cache(verbose, also ? prefix : NULL, files);\n> \t\t...\n> \t}\n\nYup, that looks right.\n\n> > +\n> > +\tif (interactive)\n> > +\t\tinteractive_add();\n> > +\n> > +\tif (*files == NULL) {\n> > +\t\t/* Commit index as-is. */\n> > +\t\trollback_lock_file(&lock_file);\n> > +\t\treturn get_index_file();\n> > +\t}\n> \n> Would an interactive add not conflict with this rollback?  And indeed with \n> the locked index to begin with?\n\nYes, I've moved the interactive case up to the beginning of\nprepare_index() and made it return get_index_file() after running\ninteractive_add().\n\n> > +\t/* update the user index file */\n> > +\tadd_files_to_cache(verbose, prefix, files);\n> > +\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n> > +\t\tdie(\"unable to write new_index file\");\n> \n> Does that mean that the index is _always_ written?  Even when not \n> specifying and paths on the command line?\n\nDo you mean \"not specifying any options and paths...\"?  In that case we\nadd the files to the index and create a temporary index from HEAD and\nadd the files there too, which we then commit.  So we have to write the\nindex in that case.  If there are no files on the command line, we roll\nback the lock and bail out just above the part you quoted.\n\n> > +\t/* Uh oh, abusing lock_file to create a garbage collected file */\n> \n> It's not that bad.  But I would mention that it is a temporary index which \n> you are building.\n\nHeh, yeah, I toned it down a bit :)\n\n> > +static const char sign_off_header[] = \"Signed-off-by: \";\n> \n> Funny, I thought it was a footer ;-)\n> \n> > +\t} else if (!stat(template_file, &statbuf)) {\n> \n> Should this not test \"if (template_file && \" first?\n\nYup, added.\n\n> > +/* Find out if the message starting at position 'start' in the strbuf\n> > + * contains only whitespace and Signed-off-by lines. */\n> > +static int message_is_empty(struct strbuf *sb, int start)\n> > +{\n> > +\tstruct strbuf tmpl;\n> > +\tconst char *nl;\n> > +\tint eol, i;\n> > +\n> > +\t/* See if the template is just a prefix of the message. */\n> > +\tstrbuf_init(&tmpl, 0);\n> > +\tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n> > +\t\tstripspace(&tmpl, 1);\n> > +\t\tif (start + tmpl.len <= sb->len &&\n> > +\t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n> > +\t\t\tstart += tmpl.len;\n> > +\t}\n> > +\tstrbuf_release(&tmpl);\n> \n> The release could go inside the if block, no?\n\nSure, not a big deal, though.\n\n> > +static int run_hook(const char *index_file, const char *name, const char *arg)\n> \n> Would this function not prefer to live in run-command.c?\n\nYeah, we can move it later, though.\n\n> > +{\n> > +\tstruct child_process hook;\n> > +\tconst char *argv[3], *env[2];\n> > +\tchar index[PATH_MAX];\n> > +\n> > +\targv[0] = git_path(\"hooks/%s\", name);\n> > +\targv[1] = arg;\n> > +\targv[2] = NULL;\n> > +\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n> > +\tenv[0] = index;\n> > +\tenv[1] = NULL;\n> > +\n> > +\tif (access(argv[0], X_OK) < 0)\n> > +\t\treturn 0;\n> \n> That check logically belongs 6 lines higher...\n\nYou mean you want to do it right after the argv[0] assignment?  I'd like\nto keep the block of assignments together so it's easy to see how the\narray is set up.  Don't tell me you worry about performance here... ;)\n\n> > +\trev.abbrev = 0;\n> > +\trev.diff = 1;\n> > +\trev.diffopt.output_format =\n> > +\t\tDIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;\n> > +\n> > +\trev.verbose_header = 1;\n> > +\trev.show_root_diff = 1;\n> > +\trev.commit_format = get_commit_format(\"format:%h: %s\");\n> \n> That's interesting.  Wouldn't have thought of that.  Reusing the log_tree \n> machinery to output the summary.  Cute.\n\nHeh, I just did what the shell script did.  It uses git diff-tree for\nthis, and the above is just the relevant bits from builtin-diff-tree.c.\n\ncheers,\nKristian\n"}]}