[PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 Makefile        |    2 +-
 parse-options.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 parse-options.h |   29 +++++++++++++++++++++
 3 files changed, 104 insertions(+), 1 deletions(-)
 create mode 100644 parse-options.c
 create mode 100644 parse-options.h

diff --git a/Makefile b/Makefile
index 62bdac6..d90e959 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ LIB_OBJS = \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
 	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
-	transport.o bundle.o
+	transport.o bundle.o parse-options.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/parse-options.c b/parse-options.c
new file mode 100644
index 0000000..2fb30cd
--- /dev/null
+++ b/parse-options.c
@@ -0,0 +1,74 @@
+#include "git-compat-util.h"
+#include "parse-options.h"
+
+int parse_options(const char ***argv,
+		  struct option *options, int count,
+		  const char *usage_string)
+{
+	const char *value, *eq;
+	int i;
+
+	if (**argv == NULL)
+		return 0;
+	if ((**argv)[0] != '-')
+		return 0;
+	if (!strcmp(**argv, "--"))
+		return 0;
+
+	value = NULL;
+	for (i = 0; i < count; i++) {
+		if ((**argv)[1] == '-') {
+			if (!prefixcmp(options[i].long_name, **argv + 2)) {
+				if (options[i].type != OPTION_BOOLEAN)
+					value = *++(*argv);
+				goto match;
+			}
+
+			eq = strchr(**argv + 2, '=');
+			if (eq && options[i].type != OPTION_BOOLEAN &&
+			    !strncmp(**argv + 2,
+				     options[i].long_name, eq - **argv - 2)) {
+				value = eq + 1;
+				goto match;
+			}
+		}
+
+		if ((**argv)[1] == options[i].short_name) {
+			if ((**argv)[2] == '\0') {
+				if (options[i].type != OPTION_BOOLEAN)
+					value = *++(*argv);
+				goto match;
+			}
+
+			if (options[i].type != OPTION_BOOLEAN) {
+				value = **argv + 2;
+				goto match;
+			}
+		}
+	}
+
+	usage(usage_string);
+
+ match:
+	switch (options[i].type) {
+	case OPTION_BOOLEAN:
+		*(int *)options[i].value = 1;
+		break;
+	case OPTION_STRING:
+		if (value == NULL)
+			die("option %s requires a value.", (*argv)[-1]);
+		*(const char **)options[i].value = value;
+		break;
+	case OPTION_INTEGER:
+		if (value == NULL)
+			die("option %s requires a value.", (*argv)[-1]);
+		*(int *)options[i].value = atoi(value);
+		break;
+	default:
+		assert(0);
+	}
+
+	(*argv)++;
+
+	return 1;
+}
diff --git a/parse-options.h b/parse-options.h
new file mode 100644
index 0000000..39399c3
--- /dev/null
+++ b/parse-options.h
@@ -0,0 +1,29 @@
+#ifndef PARSE_OPTIONS_H
+#define PARSE_OPTIONS_H
+
+enum option_type {
+    OPTION_BOOLEAN,
+    OPTION_STRING,
+    OPTION_INTEGER,
+    OPTION_LAST,
+};
+
+struct option {
+    enum option_type type;
+    const char *long_name;
+    char short_name;
+    void *value;
+};
+
+/* Parse the given options against the list of known options.  The
+ * order of the option structs matters, in that ambiguous
+ * abbreviations (eg, --in could be short for --include or
+ * --interactive) are matched by the first option that share the
+ * prefix.
+ */
+
+extern int parse_options(const char ***argv,
+			 struct option *options, int count,
+			 const char *usage_string);
+
+#endif
-- 
1.5.2.GIT

[PATCH 2/4] This exports the update() function from builtin-add.c as

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 builtin-add.c |    8 ++++----
 commit.h      |    2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 5e30380..966e145 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -107,7 +107,7 @@ static void update_callback(struct diff_queue_struct *q,
 	}
 }
 
-static void update(int verbose, const char *prefix, const char **files)
+void add_files_to_cache(int verbose, const char *prefix, const char **files)
 {
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
@@ -116,8 +116,6 @@ static void update(int verbose, const char *prefix, const char **files)
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &verbose;
-	if (read_cache() < 0)
-		die("index file corrupt");
 	run_diff_files(&rev, 0);
 }
 
@@ -218,7 +216,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	}
 
 	if (take_worktree_changes) {
-		update(verbose, prefix, argv + i);
+		if (read_cache() < 0)
+			die("index file corrupt");
+		add_files_to_cache(verbose, prefix, argv + i);
 		goto finish;
 	}
 
diff --git a/commit.h b/commit.h
index cc8d890..89caa12 100644
--- a/commit.h
+++ b/commit.h
@@ -130,6 +130,8 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
 int in_merge_bases(struct commit *, struct commit **, int);
 
 extern int interactive_add(void);
+extern void add_files_to_cache(int verbose,
+			       const char *prefix, const char **files);
 extern int rerere(void);
 
 #endif /* COMMIT_H */
-- 
1.5.2.GIT

[PATCH 3/4] Implement git commit as a builtin command.

[Kristian Høgsberg]

Move git-commit.sh to contrib/examples.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 Makefile                       |    9 +-
 builtin-commit.c               |  611 +++++++++++++++++++++++++++++++++++++++
 builtin-tag.c                  |    3 +-
 builtin.h                      |    3 +-
 contrib/examples/git-commit.sh |  627 ++++++++++++++++++++++++++++++++++++++++
 git-commit.sh                  |  627 ----------------------------------------
 git.c                          |    3 +-
 strbuf.h                       |    1 +
 t/t3501-revert-cherry-pick.sh  |    4 +-
 t/t3901-i18n-patch.sh          |    8 +-
 t/test-lib.sh                  |    4 +-
 11 files changed, 1255 insertions(+), 645 deletions(-)
 create mode 100644 builtin-commit.c
 create mode 100755 contrib/examples/git-commit.sh
 delete mode 100755 git-commit.sh

diff --git a/Makefile b/Makefile
index d90e959..6172589 100644
--- a/Makefile
+++ b/Makefile
@@ -206,7 +206,7 @@ BASIC_LDFLAGS =
 
 SCRIPT_SH = \
 	git-bisect.sh git-checkout.sh \
-	git-clean.sh git-clone.sh git-commit.sh \
+	git-clean.sh git-clone.sh \
 	git-ls-remote.sh \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh git-rebase--interactive.sh \
@@ -254,7 +254,7 @@ EXTRA_PROGRAMS =
 BUILT_INS = \
 	git-format-patch$X git-show$X git-whatchanged$X git-cherry$X \
 	git-get-tar-commit-id$X git-init$X git-repo-config$X \
-	git-fsck-objects$X git-cherry-pick$X \
+	git-fsck-objects$X git-cherry-pick$X git-status$X\
 	$(patsubst builtin-%.o,git-%$X,$(BUILTIN_OBJS))
 
 # what 'all' will build and 'install' will install, in gitexecdir
@@ -324,6 +324,7 @@ BUILTIN_OBJS = \
 	builtin-check-attr.o \
 	builtin-checkout-index.o \
 	builtin-check-ref-format.o \
+	builtin-commit.o \
 	builtin-commit-tree.o \
 	builtin-count-objects.o \
 	builtin-describe.o \
@@ -362,7 +363,6 @@ BUILTIN_OBJS = \
 	builtin-rev-parse.o \
 	builtin-revert.o \
 	builtin-rm.o \
-	builtin-runstatus.o \
 	builtin-shortlog.o \
 	builtin-show-branch.o \
 	builtin-stripspace.o \
@@ -822,9 +822,6 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
 	chmod +x $@+ && \
 	mv $@+ $@
 
-git-status: git-commit
-	$(QUIET_GEN)cp $< $@+ && mv $@+ $@
-
 gitweb/gitweb.cgi: gitweb/gitweb.perl
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
diff --git a/builtin-commit.c b/builtin-commit.c
new file mode 100644
index 0000000..69e8b19
--- /dev/null
+++ b/builtin-commit.c
@@ -0,0 +1,611 @@
+/*
+ * Builtin "git commit"
+ *
+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
+ */
+
+#include "cache.h"
+#include "cache-tree.h"
+#include "builtin.h"
+#include "diff.h"
+#include "diffcore.h"
+#include "commit.h"
+#include "revision.h"
+#include "wt-status.h"
+#include "run-command.h"
+#include "refs.h"
+#include "log-tree.h"
+#include "strbuf.h"
+#include "utf8.h"
+#include "parse-options.h"
+
+static const char builtin_commit_usage[] =
+	"[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]";
+
+static unsigned char head_sha1[20], merge_head_sha1[20];
+static char *use_message_buffer;
+static const char commit_editmsg[] = "COMMIT_EDITMSG";
+static struct lock_file lock_file;
+
+static char *logfile, *force_author, *message, *template_file;
+static char *edit_message, *use_message;
+static int all, edit_flag, also, interactive, only, no_verify, amend, signoff;
+static int quiet, verbose, untracked_files;
+
+static int no_edit, initial_commit, in_merge;
+const char *only_include_assumed;
+
+static struct option commit_options[] = {
+	{ OPTION_STRING, "file", 'F', (void *) &logfile },
+	{ OPTION_BOOLEAN, "all", 'a', &all },
+	{ OPTION_STRING, "author", 0, (void *) &force_author },
+	{ OPTION_BOOLEAN, "edit", 'e', &edit_flag },
+	{ OPTION_BOOLEAN, "include", 'i', &also },
+	{ OPTION_BOOLEAN, "interactive", 0, &interactive },
+	{ OPTION_BOOLEAN, "only", 'o', &only },
+	{ OPTION_STRING, "message", 'm', &message },
+	{ OPTION_BOOLEAN, "no-verify", 'n', &no_verify },
+	{ OPTION_BOOLEAN, "amend", 0, &amend },
+	{ OPTION_STRING, "reedit-message", 'c', &edit_message },
+	{ OPTION_STRING, "reuse-message", 'C', &use_message },
+	{ OPTION_BOOLEAN, "signoff", 's', &signoff },
+	{ OPTION_BOOLEAN, "quiet", 'q', &quiet },
+	{ OPTION_BOOLEAN, "verbose", 'v', &verbose },
+	{ OPTION_BOOLEAN, "untracked-files", 0, &untracked_files },
+	{ OPTION_STRING, "template", 't', &template_file },
+	{ OPTION_LAST },
+};
+
+static char *
+prepare_index(const char **files, const char *prefix)
+{
+	int fd;
+	struct tree *tree;
+	struct lock_file *next_index_lock;
+
+	fd = hold_locked_index(&lock_file, 1);
+	if (read_cache() < 0)
+		die("index file corrupt");
+
+	if (all) {
+		add_files_to_cache(0, prefix, files);
+		if (write_cache(fd, active_cache, active_nr) || close(fd))
+			die("unable to write new index file");
+		return lock_file.filename;
+	} else if (also) {
+		add_files_to_cache(fd, prefix, files);
+		if (write_cache(fd, active_cache, active_nr) || close(fd))
+			die("unable to write new index file");
+		return lock_file.filename;
+	}
+
+	if (interactive)
+		interactive_add();
+
+	if (*files == NULL) {
+		/* Commit index as-is. */
+		rollback_lock_file(&lock_file);
+		return get_index_file();
+	}
+
+	/*
+	 * FIXME: Warn on unknown files.  Shell script does
+	 *
+	 *   commit_only=`git-ls-files --error-unmatch -- "$@"`
+	 */
+
+	/*
+	 * FIXME: shell script does
+	 *
+	 *   git-read-tree --index-output="$TMP_INDEX" -i -m HEAD
+	 *
+	 * which warns about unmerged files in the index.
+	 */
+
+	/* update the user index file */
+	add_files_to_cache(0, prefix, files);
+	if (write_cache(fd, active_cache, active_nr) || close(fd))
+		die("unable to write new index file");
+
+	if (!initial_commit) {
+		tree = parse_tree_indirect(head_sha1);
+		if (!tree)
+			die("failed to unpack HEAD tree object");
+		if (read_tree(tree, 0, NULL))
+			die("failed to read HEAD tree object");
+	}
+
+	/* Uh oh, abusing lock_file to create a garbage collected file */
+	next_index_lock = xmalloc(sizeof(*next_index_lock));
+	fd = hold_lock_file_for_update(next_index_lock,
+				       git_path("next-index-%d", getpid()), 1);
+	add_files_to_cache(0, prefix, files);
+	if (write_cache(fd, active_cache, active_nr) || close(fd))
+		die("unable to write new index file");
+
+	return next_index_lock->filename;
+}
+
+static int run_status(FILE *fp, const char *index_file)
+{
+	struct wt_status s;
+
+	wt_status_prepare(&s);
+
+	if (amend) {
+		s.amend = 1;
+		s.reference = "HEAD^1";
+	}
+	s.verbose = verbose;
+	s.untracked = untracked_files;
+	s.index_file = index_file;
+	s.fp = fp;
+
+	wt_status_print(&s);
+
+	return s.commitable;
+}
+
+static const char sign_off_header[] = "Signed-off-by: ";
+
+static int prepare_log_message(const char *index_file)
+{
+	struct stat statbuf;
+	int commitable;
+	struct strbuf sb;
+	char *buffer;
+	FILE *fp;
+
+	strbuf_init(&sb, 0);
+	if (message) {
+		strbuf_add(&sb, message, strlen(message));
+	} else if (logfile && !strcmp(logfile, "-")) {
+		if (isatty(0))
+			fprintf(stderr, "(reading log message from standard input)\n");
+		if (strbuf_read(&sb, 0, 0) < 0)
+			die("could not read log from standard input");
+	} else if (logfile) {
+		if (strbuf_read_file(&sb, logfile) < 0)
+			die("could not read log file '%s': %s",
+			    logfile, strerror(errno));
+	} else if (use_message) {
+		buffer = strstr(use_message_buffer, "\n\n");
+		if (!buffer || buffer[2] == '\0')
+			die("commit has empty message");
+		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
+	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
+		if (strbuf_read_file(&sb, git_path("MERGE_MSG")) < 0)
+			die("could not read MERGE_MSG: %s", strerror(errno));
+	} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
+		if (strbuf_read_file(&sb, git_path("SQUASH_MSG")) < 0)
+			die("could not read SQUASH_MSG: %s", strerror(errno));
+	} else if (!stat(template_file, &statbuf)) {
+		if (strbuf_read_file(&sb, template_file) < 0)
+			die("could not read %s: %s",
+			    template_file, strerror(errno));
+	}
+
+	fp = fopen(git_path(commit_editmsg), "w");
+	if (fp == NULL)
+		die("could not open %s\n", git_path(commit_editmsg));
+
+	stripspace(&sb, 0);
+	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
+		die("could not write commit template: %s\n",
+		    strerror(errno));
+
+	if (signoff) {
+		const char *info, *bol;
+
+		info = git_committer_info(1);
+		strbuf_addch(&sb, '\0');
+		bol = strrchr(sb.buf + sb.len - 1, '\n');
+		if (!bol || prefixcmp(bol, sign_off_header))
+			fprintf(fp, "\n");
+		fprintf(fp, "%s%s\n", sign_off_header, git_committer_info(1));
+	}
+
+	strbuf_release(&sb);
+
+	if (in_merge && !no_edit) {
+		fprintf(fp,
+			"#\n"
+			"# It looks like you may be committing a MERGE.\n"
+			"# If this is not correct, please remove the file\n"
+			"#	%s\n"
+			"# and try again.\n"
+			"#\n",
+			git_path("MERGE_HEAD"));
+	}
+
+	fprintf(fp,
+		"\n"
+		"# Please enter the commit message for your changes.\n"
+		"# (Comment lines starting with '#' will not be included)\n");
+	if (only_include_assumed)
+		fprintf(fp, "# %s\n", only_include_assumed);
+
+	commitable = run_status(fp, index_file);
+
+	fclose(fp);
+
+	return commitable;
+}
+
+/* Find out if the message starting at position 'start' in the strbuf
+ * contains only whitespace and Signed-off-by lines. */
+static int message_is_empty(struct strbuf *sb, int start)
+{
+	struct strbuf tmpl;
+	const char *nl;
+	int eol, i;
+
+	/* See if the template is just a prefix of the message. */
+	strbuf_init(&tmpl, 0);
+	if (template_file && strbuf_read_file(&tmpl, template_file) > 0) {
+		stripspace(&tmpl, 1);
+		if (start + tmpl.len <= sb->len &&
+		    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
+			start += tmpl.len;
+	}
+	strbuf_release(&tmpl);
+
+	/* Check if the rest is just whitespace and Signed-of-by's. */
+	for (i = start; i < sb->len; i++) {
+		nl = memchr(sb->buf + i, '\n', sb->len - i);
+		if (nl)
+			eol = nl - sb->buf;
+		else
+			eol = sb->len;
+
+		if (strlen(sign_off_header) <= eol - i &&
+		    !prefixcmp(sb->buf + i, sign_off_header)) {
+			i = eol;
+			continue;
+		}
+		while (i < eol)
+			if (!isspace(sb->buf[i++]))
+				return 0;
+	}
+
+	return 1;
+}
+
+static void determine_author_info(struct strbuf *sb)
+{
+	char *p, *eol;
+	char *name = NULL, *email = NULL;
+
+	if (use_message) {
+		p = strstr(use_message_buffer, "\nauthor");
+		if (!p)
+			die("invalid commit: %s\n", use_message);
+		p++;
+		eol = strchr(p, '\n');
+		if (!eol)
+			die("invalid commit: %s\n", use_message);
+
+		strbuf_add(sb, p, eol + 1 - p);
+	} else if (force_author) {
+		const char *eoname = strstr(force_author, " <");
+		const char *eomail = strchr(force_author, '>');
+
+		if (!eoname || !eomail)
+			die("malformed --author parameter\n");
+		name = xstrndup(force_author, eoname - force_author);
+		email = xstrndup(eoname + 2, eomail - eoname - 2);
+		strbuf_addf(sb, "author %s\n",
+			    fmt_ident(name, email,
+				      getenv("GIT_AUTHOR_DATE"), 1));
+		free(name);
+		free(email);
+	} else {
+		strbuf_addf(sb, "author %s\n", git_author_info(1));
+	}
+}
+
+static void parse_and_validate_options(const char ***argv)
+{
+	int f = 0;
+
+	(*argv)++;
+	while (parse_options(argv, commit_options, ARRAY_SIZE(commit_options),
+			     builtin_commit_usage))
+		;
+
+	if (logfile || message || use_message)
+		no_edit = 1;
+	if (edit_flag)
+		no_edit = 0;
+
+	if (get_sha1("HEAD", head_sha1))
+		initial_commit = 1;
+
+	if (!get_sha1("MERGE_HEAD", merge_head_sha1))
+		in_merge = 1;
+
+	/* Sanity check options */
+	if (amend && initial_commit)
+		die("You have nothing to amend.");
+	if (amend && in_merge)
+		die("You are in the middle of a merger -- cannot amend.");
+
+	if (use_message)
+		f++;
+	if (edit_message)
+		f++;
+	if (logfile)
+		f++;
+	if (f > 1)
+		die("Only one of -c/-C/-F can be used.");
+	if (message && f > 0)
+		die("Option -m cannot be combined with -c/-C/-F.");
+	if (edit_message)
+		use_message = edit_message;
+	if (amend)
+		use_message = "HEAD";
+	if (use_message) {
+		unsigned char sha1[20];
+		static char utf8[] = "UTF-8";
+		const char *out_enc;
+		char *enc, *end;
+		struct commit *commit;
+
+		if (get_sha1(use_message, sha1))
+			die("could not lookup commit %s", use_message);
+		commit = lookup_commit(sha1);
+		if (!commit || parse_commit(commit))
+			die("could not parse commit %s", use_message);
+
+		enc = strstr(commit->buffer, "\nencoding");
+		if (enc) {
+			end = strchr(enc + 10, '\n');
+			enc = xstrndup(enc + 10, end - (enc + 10));
+		} else {
+			enc = utf8;
+		}
+		out_enc = git_commit_encoding ? git_commit_encoding : utf8;
+
+		use_message_buffer =
+			reencode_string(commit->buffer, out_enc, enc);
+		if (enc != utf8)
+			free(enc);
+	}
+
+	if (also && only)
+		die("Only one of --include/--only can be used.");
+	if (!*argv && (also || (only && !amend)))
+		die("No paths with --include/--only does not make sense.");
+	if (!*argv && only && amend)
+		only_include_assumed = "Clever... amending the last one with dirty index.";
+	if (*argv && !also && !only) {
+		only_include_assumed = "Explicit paths specified without -i nor -o; assuming --only paths...";
+		also = 0;
+	}
+
+	if (all && interactive)
+		die("Cannot use -a, --interactive or -i at the same time.");
+	else if (all && **argv)
+		die("Paths with -a does not make sense.");
+	else if (interactive && **argv)
+		die("Paths with --interactive does not make sense.");
+}
+
+int cmd_status(int argc, const char **argv, const char *prefix)
+{
+	const char *index_file;
+	int commitable;
+
+	git_config(git_status_config);
+
+	parse_and_validate_options(&argv);
+
+	index_file = prepare_index(argv, prefix);
+
+	commitable = run_status(stdout, index_file);
+
+	rollback_lock_file(&lock_file);
+
+	return commitable ? 0 : 1;
+}
+
+static int run_hook(const char *index_file, const char *name, const char *arg)
+{
+	struct child_process hook;
+	const char *argv[3], *env[2];
+	char index[PATH_MAX];
+
+	argv[0] = git_path("hooks/%s", name);
+	argv[1] = arg;
+	argv[2] = NULL;
+	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+	env[0] = index;
+	env[1] = NULL;
+
+	if (access(argv[0], X_OK) < 0)
+		return 0;
+
+	memset(&hook, 0, sizeof(hook));
+	hook.argv = argv;
+	hook.no_stdin = 1;
+	hook.stdout_to_stderr = 1;
+	hook.env = env;
+
+	return run_command(&hook);
+}
+
+static void print_summary(const char *prefix, const unsigned char *sha1)
+{
+	struct rev_info rev;
+	struct commit *commit;
+
+	commit = lookup_commit(sha1);
+	if (!commit)
+		die("couldn't look up newly created commit\n");
+	if (!commit || parse_commit(commit))
+		die("could not parse newly created commit");
+
+	init_revisions(&rev, prefix);
+	setup_revisions(0, NULL, &rev, NULL);
+
+	rev.abbrev = 0;
+	rev.diff = 1;
+	rev.diffopt.output_format =
+		DIFF_FORMAT_SHORTSTAT | DIFF_FORMAT_SUMMARY;
+
+	rev.verbose_header = 1;
+	rev.show_root_diff = 1;
+	rev.commit_format = get_commit_format("format:%h: %s");
+	rev.always_show_header = 1;
+
+	printf("Created %scommit ", initial_commit ? "initial " : "");
+
+	log_tree_commit(&rev, commit);
+}
+
+int git_commit_config(const char *k, const char *v)
+{
+	if (!strcmp(k, "commit.template")) {
+		template_file = xstrdup(v);
+		return 0;
+	}
+
+	return git_status_config(k, v);
+}
+
+static const char commit_utf8_warn[] =
+"Warning: commit message does not conform to UTF-8.\n"
+"You may want to amend it after fixing the message, or set the config\n"
+"variable i18n.commitencoding to the encoding your project uses.\n";
+
+int cmd_commit(int argc, const char **argv, const char *prefix)
+{
+	int header_len, parent_count = 0;
+	struct strbuf sb;
+	const char *index_file, *reflog_msg;
+	char *nl, *header_line;
+	unsigned char commit_sha1[20];
+	struct ref_lock *ref_lock;
+
+	git_config(git_commit_config);
+
+	parse_and_validate_options(&argv);
+
+	index_file = prepare_index(argv, prefix);
+
+	if (run_hook(index_file, "pre-commit", NULL))
+		exit(1);
+
+	if (!prepare_log_message(index_file) && !in_merge) {
+		run_status(stdout, index_file);
+		unlink(commit_editmsg);
+		return 1;
+	}
+
+	strbuf_init(&sb, 0);
+
+	/* Start building up the commit header */
+	read_cache_from(index_file);
+	active_cache_tree = cache_tree();
+	if (cache_tree_update(active_cache_tree,
+			      active_cache, active_nr, 0, 0) < 0)
+		die("Error building trees");
+	strbuf_addf(&sb, "tree %s\n",
+		    sha1_to_hex(active_cache_tree->sha1));
+
+	/* Determine parents */
+	if (initial_commit) {
+		reflog_msg = "commit (initial)";
+		parent_count = 0;
+	} else if (amend) {
+		struct commit_list *c;
+		struct commit *commit;
+
+		reflog_msg = "commit (amend)";
+		commit = lookup_commit(head_sha1);
+		if (!commit || parse_commit(commit))
+			die("could not parse HEAD commit");
+
+		for (c = commit->parents; c; c = c->next)
+			strbuf_addf(&sb, "parent %s\n",
+				      sha1_to_hex(c->item->object.sha1));
+	} else if (in_merge) {
+		struct strbuf m;
+		FILE *fp;
+
+		reflog_msg = "commit (merge)";
+		strbuf_addf(&sb, "parent %s\n", sha1_to_hex(head_sha1));
+		strbuf_init(&m, 0);
+		fp = fopen(git_path("MERGE_HEAD"), "r");
+		if (fp == NULL)
+			die("could not open %s for reading: %s",
+			    git_path("MERGE_HEAD"), strerror(errno));
+		while (strbuf_getline(&m, fp, '\n') != EOF)
+			strbuf_addf(&sb, "parent %s\n", m.buf);
+		fclose(fp);
+		strbuf_release(&m);
+	} else {
+		reflog_msg = "commit";
+		strbuf_addf(&sb, "parent %s\n", sha1_to_hex(head_sha1));
+	}
+
+	determine_author_info(&sb);
+	strbuf_addf(&sb, "committer %s\n", git_committer_info(1));
+	if (!is_encoding_utf8(git_commit_encoding))
+		strbuf_addf(&sb, "encoding %s\n", git_commit_encoding);
+	strbuf_addch(&sb, '\n');
+
+	/* Get the commit message and validate it */
+	header_len = sb.len;
+	if (!no_edit) {
+		fprintf(stderr, "launching editor, log %s\n", logfile);
+		launch_editor(git_path(commit_editmsg), &sb);
+	}
+	else if (strbuf_read_file(&sb, git_path(commit_editmsg)) < 0)
+		die("could not read commit message\n");
+	if (run_hook(index_file, "commit-msg", commit_editmsg))
+		exit(1);
+	stripspace(&sb, 1);
+	if (sb.len < header_len ||
+	    message_is_empty(&sb, header_len))
+		die("* no commit message?  aborting commit.");
+	strbuf_addch(&sb, '\0');
+	if (is_encoding_utf8(git_commit_encoding) && !is_utf8(sb.buf))
+		fprintf(stderr, commit_utf8_warn);
+
+	if (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1))
+		die("failed to write commit object");
+
+	ref_lock = lock_any_ref_for_update("HEAD",
+					   initial_commit ? NULL : head_sha1,
+					   0);
+
+	nl = strchr(sb.buf + header_len, '\n');
+	header_line = xstrndup(sb.buf + header_len,
+			       nl - (sb.buf + header_len));
+	strbuf_release(&sb);
+	strbuf_addf(&sb, "%s: %s\n", reflog_msg, header_line);
+	strbuf_addch(&sb, '\0');
+	free(header_line);
+
+	if (!ref_lock)
+		die("cannot lock HEAD ref");
+	if (write_ref_sha1(ref_lock, commit_sha1, sb.buf) < 0)
+		die("cannot update HEAD ref");
+
+	unlink(git_path("MERGE_HEAD"));
+	unlink(git_path("MERGE_MSG"));
+
+	if (lock_file.filename[0] && commit_locked_index(&lock_file))
+		die("failed to write new index");
+
+	rerere();
+
+	run_hook(index_file, "post-commit", NULL);
+
+	if (!quiet)
+		print_summary(prefix, commit_sha1);
+
+	return 0;
+}
diff --git a/builtin-tag.c b/builtin-tag.c
index a1a2cf0..0a36a5d 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -17,12 +17,11 @@ static const char builtin_tag_usage[] =
 
 static char signingkey[1000];
 
-static void launch_editor(const char *path, struct strbuf *buffer)
+void launch_editor(const char *path, struct strbuf *buffer)
 {
 	const char *editor, *terminal;
 	struct child_process child;
 	const char *args[3];
-	int fd;
 
 	editor = getenv("GIT_EDITOR");
 	if (!editor && editor_program)
diff --git a/builtin.h b/builtin.h
index 65cc0fb..470a788 100644
--- a/builtin.h
+++ b/builtin.h
@@ -23,6 +23,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);
 extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
+extern int cmd_commit(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_describe(int argc, const char **argv, const char *prefix);
@@ -67,10 +68,10 @@ extern int cmd_rev_list(int argc, const char **argv, const char *prefix);
 extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
 extern int cmd_revert(int argc, const char **argv, const char *prefix);
 extern int cmd_rm(int argc, const char **argv, const char *prefix);
-extern int cmd_runstatus(int argc, const char **argv, const char *prefix);
 extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
+extern int cmd_status(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_tag(int argc, const char **argv, const char *prefix);
diff --git a/contrib/examples/git-commit.sh b/contrib/examples/git-commit.sh
new file mode 100755
index 0000000..44ccc44
--- /dev/null
+++ b/contrib/examples/git-commit.sh
@@ -0,0 +1,627 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Linus Torvalds
+# Copyright (c) 2006 Junio C Hamano
+
+USAGE='[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]'
+SUBDIRECTORY_OK=Yes
+. git-sh-setup
+require_work_tree
+
+git rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
+
+case "$0" in
+*status)
+	status_only=t
+	;;
+*commit)
+	status_only=
+	;;
+esac
+
+refuse_partial () {
+	echo >&2 "$1"
+	echo >&2 "You might have meant to say 'git commit -i paths...', perhaps?"
+	exit 1
+}
+
+THIS_INDEX="$GIT_DIR/index"
+NEXT_INDEX="$GIT_DIR/next-index$$"
+rm -f "$NEXT_INDEX"
+save_index () {
+	cp -p "$THIS_INDEX" "$NEXT_INDEX"
+}
+
+run_status () {
+	# If TMP_INDEX is defined, that means we are doing
+	# "--only" partial commit, and that index file is used
+	# to build the tree for the commit.  Otherwise, if
+	# NEXT_INDEX exists, that is the index file used to
+	# make the commit.  Otherwise we are using as-is commit
+	# so the regular index file is what we use to compare.
+	if test '' != "$TMP_INDEX"
+	then
+		GIT_INDEX_FILE="$TMP_INDEX"
+		export GIT_INDEX_FILE
+	elif test -f "$NEXT_INDEX"
+	then
+		GIT_INDEX_FILE="$NEXT_INDEX"
+		export GIT_INDEX_FILE
+	fi
+
+	if test "$status_only" = "t" -o "$use_status_color" = "t"; then
+		color=
+	else
+		color=--nocolor
+	fi
+	git runstatus ${color} \
+		${verbose:+--verbose} \
+		${amend:+--amend} \
+		${untracked_files:+--untracked}
+}
+
+trap '
+	test -z "$TMP_INDEX" || {
+		test -f "$TMP_INDEX" && rm -f "$TMP_INDEX"
+	}
+	rm -f "$NEXT_INDEX"
+' 0
+
+################################################################
+# Command line argument parsing and sanity checking
+
+all=
+also=
+interactive=
+only=
+logfile=
+use_commit=
+amend=
+edit_flag=
+no_edit=
+log_given=
+log_message=
+verify=t
+quiet=
+verbose=
+signoff=
+force_author=
+only_include_assumed=
+untracked_files=
+templatefile="`git config commit.template`"
+while test $# != 0
+do
+	case "$1" in
+	-F|--F|-f|--f|--fi|--fil|--file)
+		case "$#" in 1) usage ;; esac
+		shift
+		no_edit=t
+		log_given=t$log_given
+		logfile="$1"
+		;;
+	-F*|-f*)
+		no_edit=t
+		log_given=t$log_given
+		logfile="${1#-[Ff]}"
+		;;
+	--F=*|--f=*|--fi=*|--fil=*|--file=*)
+		no_edit=t
+		log_given=t$log_given
+		logfile="${1#*=}"
+		;;
+	-a|--a|--al|--all)
+		all=t
+		;;
+	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
+		force_author="${1#*=}"
+		;;
+	--au|--aut|--auth|--autho|--author)
+		case "$#" in 1) usage ;; esac
+		shift
+		force_author="$1"
+		;;
+	-e|--e|--ed|--edi|--edit)
+		edit_flag=t
+		;;
+	-i|--i|--in|--inc|--incl|--inclu|--includ|--include)
+		also=t
+		;;
+	--int|--inte|--inter|--intera|--interac|--interact|--interacti|\
+	--interactiv|--interactive)
+		interactive=t
+		;;
+	-o|--o|--on|--onl|--only)
+		only=t
+		;;
+	-m|--m|--me|--mes|--mess|--messa|--messag|--message)
+		case "$#" in 1) usage ;; esac
+		shift
+		log_given=m$log_given
+		log_message="${log_message:+${log_message}
+
+}$1"
+		no_edit=t
+		;;
+	-m*)
+		log_given=m$log_given
+		log_message="${log_message:+${log_message}
+
+}${1#-m}"
+		no_edit=t
+		;;
+	--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
+		log_given=m$log_given
+		log_message="${log_message:+${log_message}
+
+}${1#*=}"
+		no_edit=t
+		;;
+	-n|--n|--no|--no-|--no-v|--no-ve|--no-ver|--no-veri|--no-verif|\
+	--no-verify)
+		verify=
+		;;
+	--a|--am|--ame|--amen|--amend)
+		amend=t
+		use_commit=HEAD
+		;;
+	-c)
+		case "$#" in 1) usage ;; esac
+		shift
+		log_given=t$log_given
+		use_commit="$1"
+		no_edit=
+		;;
+	--ree=*|--reed=*|--reedi=*|--reedit=*|--reedit-=*|--reedit-m=*|\
+	--reedit-me=*|--reedit-mes=*|--reedit-mess=*|--reedit-messa=*|\
+	--reedit-messag=*|--reedit-message=*)
+		log_given=t$log_given
+		use_commit="${1#*=}"
+		no_edit=
+		;;
+	--ree|--reed|--reedi|--reedit|--reedit-|--reedit-m|--reedit-me|\
+	--reedit-mes|--reedit-mess|--reedit-messa|--reedit-messag|\
+	--reedit-message)
+		case "$#" in 1) usage ;; esac
+		shift
+		log_given=t$log_given
+		use_commit="$1"
+		no_edit=
+		;;
+	-C)
+		case "$#" in 1) usage ;; esac
+		shift
+		log_given=t$log_given
+		use_commit="$1"
+		no_edit=t
+		;;
+	--reu=*|--reus=*|--reuse=*|--reuse-=*|--reuse-m=*|--reuse-me=*|\
+	--reuse-mes=*|--reuse-mess=*|--reuse-messa=*|--reuse-messag=*|\
+	--reuse-message=*)
+		log_given=t$log_given
+		use_commit="${1#*=}"
+		no_edit=t
+		;;
+	--reu|--reus|--reuse|--reuse-|--reuse-m|--reuse-me|--reuse-mes|\
+	--reuse-mess|--reuse-messa|--reuse-messag|--reuse-message)
+		case "$#" in 1) usage ;; esac
+		shift
+		log_given=t$log_given
+		use_commit="$1"
+		no_edit=t
+		;;
+	-s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
+		signoff=t
+		;;
+	-t|--t|--te|--tem|--temp|--templ|--templa|--templat|--template)
+		case "$#" in 1) usage ;; esac
+		shift
+		templatefile="$1"
+		no_edit=
+		;;
+	-q|--q|--qu|--qui|--quie|--quiet)
+		quiet=t
+		;;
+	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+		verbose=t
+		;;
+	-u|--u|--un|--unt|--untr|--untra|--untrac|--untrack|--untracke|\
+	--untracked|--untracked-|--untracked-f|--untracked-fi|--untracked-fil|\
+	--untracked-file|--untracked-files)
+		untracked_files=t
+		;;
+	--)
+		shift
+		break
+		;;
+	-*)
+		usage
+		;;
+	*)
+		break
+		;;
+	esac
+	shift
+done
+case "$edit_flag" in t) no_edit= ;; esac
+
+################################################################
+# Sanity check options
+
+case "$amend,$initial_commit" in
+t,t)
+	die "You do not have anything to amend." ;;
+t,)
+	if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
+		die "You are in the middle of a merge -- cannot amend."
+	fi ;;
+esac
+
+case "$log_given" in
+tt*)
+	die "Only one of -c/-C/-F can be used." ;;
+*tm*|*mt*)
+	die "Option -m cannot be combined with -c/-C/-F." ;;
+esac
+
+case "$#,$also,$only,$amend" in
+*,t,t,*)
+	die "Only one of --include/--only can be used." ;;
+0,t,,* | 0,,t,)
+	die "No paths with --include/--only does not make sense." ;;
+0,,t,t)
+	only_include_assumed="# Clever... amending the last one with dirty index." ;;
+0,,,*)
+	;;
+*,,,*)
+	only_include_assumed="# Explicit paths specified without -i nor -o; assuming --only paths..."
+	also=
+	;;
+esac
+unset only
+case "$all,$interactive,$also,$#" in
+*t,*t,*)
+	die "Cannot use -a, --interactive or -i at the same time." ;;
+t,,[1-9]*)
+	die "Paths with -a does not make sense." ;;
+,t,[1-9]*)
+	die "Paths with --interactive does not make sense." ;;
+,,t,0)
+	die "No paths with -i does not make sense." ;;
+esac
+
+if test ! -z "$templatefile" -a -z "$log_given"
+then
+	if test ! -f "$templatefile"
+	then
+		die "Commit template file does not exist."
+	fi
+fi
+
+################################################################
+# Prepare index to have a tree to be committed
+
+case "$all,$also" in
+t,)
+	if test ! -f "$THIS_INDEX"
+	then
+		die 'nothing to commit (use "git add file1 file2" to include for commit)'
+	fi
+	save_index &&
+	(
+		cd_to_toplevel &&
+		GIT_INDEX_FILE="$NEXT_INDEX" &&
+		export GIT_INDEX_FILE &&
+		git diff-files --name-only -z |
+		git update-index --remove -z --stdin
+	) || exit
+	;;
+,t)
+	save_index &&
+	git ls-files --error-unmatch -- "$@" >/dev/null || exit
+
+	git diff-files --name-only -z -- "$@"  |
+	(
+		cd_to_toplevel &&
+		GIT_INDEX_FILE="$NEXT_INDEX" &&
+		export GIT_INDEX_FILE &&
+		git update-index --remove -z --stdin
+	) || exit
+	;;
+,)
+	if test "$interactive" = t; then
+		git add --interactive || exit
+	fi
+	case "$#" in
+	0)
+		;; # commit as-is
+	*)
+		if test -f "$GIT_DIR/MERGE_HEAD"
+		then
+			refuse_partial "Cannot do a partial commit during a merge."
+		fi
+
+		TMP_INDEX="$GIT_DIR/tmp-index$$"
+		W=
+		test -z "$initial_commit" && W=--with-tree=HEAD
+		commit_only=`git ls-files --error-unmatch $W -- "$@"` || exit
+
+		# Build a temporary index and update the real index
+		# the same way.
+		if test -z "$initial_commit"
+		then
+			GIT_INDEX_FILE="$THIS_INDEX" \
+			git read-tree --index-output="$TMP_INDEX" -i -m HEAD
+		else
+			rm -f "$TMP_INDEX"
+		fi || exit
+
+		printf '%s\n' "$commit_only" |
+		GIT_INDEX_FILE="$TMP_INDEX" \
+		git update-index --add --remove --stdin &&
+
+		save_index &&
+		printf '%s\n' "$commit_only" |
+		(
+			GIT_INDEX_FILE="$NEXT_INDEX"
+			export GIT_INDEX_FILE
+			git update-index --add --remove --stdin
+		) || exit
+		;;
+	esac
+	;;
+esac
+
+################################################################
+# If we do as-is commit, the index file will be THIS_INDEX,
+# otherwise NEXT_INDEX after we make this commit.  We leave
+# the index as is if we abort.
+
+if test -f "$NEXT_INDEX"
+then
+	USE_INDEX="$NEXT_INDEX"
+else
+	USE_INDEX="$THIS_INDEX"
+fi
+
+case "$status_only" in
+t)
+	# This will silently fail in a read-only repository, which is
+	# what we want.
+	GIT_INDEX_FILE="$USE_INDEX" git update-index -q --unmerged --refresh
+	run_status
+	exit $?
+	;;
+'')
+	GIT_INDEX_FILE="$USE_INDEX" git update-index -q --refresh || exit
+	;;
+esac
+
+################################################################
+# Grab commit message, write out tree and make commit.
+
+if test t = "$verify" && test -x "$GIT_DIR"/hooks/pre-commit
+then
+    GIT_INDEX_FILE="${TMP_INDEX:-${USE_INDEX}}" "$GIT_DIR"/hooks/pre-commit \
+    || exit
+fi
+
+if test "$log_message" != ''
+then
+	printf '%s\n' "$log_message"
+elif test "$logfile" != ""
+then
+	if test "$logfile" = -
+	then
+		test -t 0 &&
+		echo >&2 "(reading log message from standard input)"
+		cat
+	else
+		cat <"$logfile"
+	fi
+elif test "$use_commit" != ""
+then
+	encoding=$(git config i18n.commitencoding || echo UTF-8)
+	git show -s --pretty=raw --encoding="$encoding" "$use_commit" |
+	sed -e '1,/^$/d' -e 's/^    //'
+elif test -f "$GIT_DIR/MERGE_MSG"
+then
+	cat "$GIT_DIR/MERGE_MSG"
+elif test -f "$GIT_DIR/SQUASH_MSG"
+then
+	cat "$GIT_DIR/SQUASH_MSG"
+elif test "$templatefile" != ""
+then
+	cat "$templatefile"
+fi | git stripspace >"$GIT_DIR"/COMMIT_EDITMSG
+
+case "$signoff" in
+t)
+	sign=$(git-var GIT_COMMITTER_IDENT | sed -e '
+		s/>.*/>/
+		s/^/Signed-off-by: /
+		')
+	blank_before_signoff=
+	tail -n 1 "$GIT_DIR"/COMMIT_EDITMSG |
+	grep 'Signed-off-by:' >/dev/null || blank_before_signoff='
+'
+	tail -n 1 "$GIT_DIR"/COMMIT_EDITMSG |
+	grep "$sign"$ >/dev/null ||
+	printf '%s%s\n' "$blank_before_signoff" "$sign" \
+		>>"$GIT_DIR"/COMMIT_EDITMSG
+	;;
+esac
+
+if test -f "$GIT_DIR/MERGE_HEAD" && test -z "$no_edit"; then
+	echo "#"
+	echo "# It looks like you may be committing a MERGE."
+	echo "# If this is not correct, please remove the file"
+	printf '%s\n' "#	$GIT_DIR/MERGE_HEAD"
+	echo "# and try again"
+	echo "#"
+fi >>"$GIT_DIR"/COMMIT_EDITMSG
+
+# Author
+if test '' != "$use_commit"
+then
+	eval "$(get_author_ident_from_commit "$use_commit")"
+	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
+fi
+if test '' != "$force_author"
+then
+	GIT_AUTHOR_NAME=`expr "z$force_author" : 'z\(.*[^ ]\) *<.*'` &&
+	GIT_AUTHOR_EMAIL=`expr "z$force_author" : '.*\(<.*\)'` &&
+	test '' != "$GIT_AUTHOR_NAME" &&
+	test '' != "$GIT_AUTHOR_EMAIL" ||
+	die "malformed --author parameter"
+	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
+fi
+
+PARENTS="-p HEAD"
+if test -z "$initial_commit"
+then
+	rloga='commit'
+	if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
+		rloga='commit (merge)'
+		PARENTS="-p HEAD "`sed -e 's/^/-p /' "$GIT_DIR/MERGE_HEAD"`
+	elif test -n "$amend"; then
+		rloga='commit (amend)'
+		PARENTS=$(git cat-file commit HEAD |
+			sed -n -e '/^$/q' -e 's/^parent /-p /p')
+	fi
+	current="$(git rev-parse --verify HEAD)"
+else
+	if [ -z "$(git ls-files)" ]; then
+		echo >&2 'nothing to commit (use "git add file1 file2" to include for commit)'
+		exit 1
+	fi
+	PARENTS=""
+	rloga='commit (initial)'
+	current=''
+fi
+set_reflog_action "$rloga"
+
+if test -z "$no_edit"
+then
+	{
+		echo ""
+		echo "# Please enter the commit message for your changes."
+		echo "# (Comment lines starting with '#' will not be included)"
+		test -z "$only_include_assumed" || echo "$only_include_assumed"
+		run_status
+	} >>"$GIT_DIR"/COMMIT_EDITMSG
+else
+	# we need to check if there is anything to commit
+	run_status >/dev/null
+fi
+if [ "$?" != "0" -a ! -f "$GIT_DIR/MERGE_HEAD" ]
+then
+	rm -f "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
+	use_status_color=t
+	run_status
+	exit 1
+fi
+
+case "$no_edit" in
+'')
+	git-var GIT_AUTHOR_IDENT > /dev/null  || die
+	git-var GIT_COMMITTER_IDENT > /dev/null  || die
+	git_editor "$GIT_DIR/COMMIT_EDITMSG"
+	;;
+esac
+
+case "$verify" in
+t)
+	if test -x "$GIT_DIR"/hooks/commit-msg
+	then
+		"$GIT_DIR"/hooks/commit-msg "$GIT_DIR"/COMMIT_EDITMSG || exit
+	fi
+esac
+
+if test -z "$no_edit"
+then
+    sed -e '
+        /^diff --git a\/.*/{
+	    s///
+	    q
+	}
+	/^#/d
+    ' "$GIT_DIR"/COMMIT_EDITMSG
+else
+    cat "$GIT_DIR"/COMMIT_EDITMSG
+fi |
+git stripspace >"$GIT_DIR"/COMMIT_MSG
+
+# Test whether the commit message has any content we didn't supply.
+have_commitmsg=
+grep -v -i '^Signed-off-by' "$GIT_DIR"/COMMIT_MSG |
+	git stripspace > "$GIT_DIR"/COMMIT_BAREMSG
+
+# Is the commit message totally empty?
+if test -s "$GIT_DIR"/COMMIT_BAREMSG
+then
+	if test "$templatefile" != ""
+	then
+		# Test whether this is just the unaltered template.
+		if cnt=`sed -e '/^#/d' < "$templatefile" |
+			git stripspace |
+			diff "$GIT_DIR"/COMMIT_BAREMSG - |
+			wc -l` &&
+		   test 0 -lt $cnt
+		then
+			have_commitmsg=t
+		fi
+	else
+		# No template, so the content in the commit message must
+		# have come from the user.
+		have_commitmsg=t
+	fi
+fi
+
+rm -f "$GIT_DIR"/COMMIT_BAREMSG
+
+if test "$have_commitmsg" = "t"
+then
+	if test -z "$TMP_INDEX"
+	then
+		tree=$(GIT_INDEX_FILE="$USE_INDEX" git write-tree)
+	else
+		tree=$(GIT_INDEX_FILE="$TMP_INDEX" git write-tree) &&
+		rm -f "$TMP_INDEX"
+	fi &&
+	commit=$(git commit-tree $tree $PARENTS <"$GIT_DIR/COMMIT_MSG") &&
+	rlogm=$(sed -e 1q "$GIT_DIR"/COMMIT_MSG) &&
+	git update-ref -m "$GIT_REFLOG_ACTION: $rlogm" HEAD $commit "$current" &&
+	rm -f -- "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/MERGE_MSG" &&
+	if test -f "$NEXT_INDEX"
+	then
+		mv "$NEXT_INDEX" "$THIS_INDEX"
+	else
+		: ;# happy
+	fi
+else
+	echo >&2 "* no commit message?  aborting commit."
+	false
+fi
+ret="$?"
+rm -f "$GIT_DIR/COMMIT_MSG" "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
+
+cd_to_toplevel
+
+git rerere
+
+if test "$ret" = 0
+then
+	git gc --auto
+	if test -x "$GIT_DIR"/hooks/post-commit
+	then
+		"$GIT_DIR"/hooks/post-commit
+	fi
+	if test -z "$quiet"
+	then
+		commit=`git diff-tree --always --shortstat --pretty="format:%h: %s"\
+		       --summary --root HEAD --`
+		echo "Created${initial_commit:+ initial} commit $commit"
+	fi
+fi
+
+exit "$ret"
diff --git a/git-commit.sh b/git-commit.sh
deleted file mode 100755
index 44ccc44..0000000
--- a/git-commit.sh
+++ /dev/null
@@ -1,627 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2005 Linus Torvalds
-# Copyright (c) 2006 Junio C Hamano
-
-USAGE='[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]'
-SUBDIRECTORY_OK=Yes
-. git-sh-setup
-require_work_tree
-
-git rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
-
-case "$0" in
-*status)
-	status_only=t
-	;;
-*commit)
-	status_only=
-	;;
-esac
-
-refuse_partial () {
-	echo >&2 "$1"
-	echo >&2 "You might have meant to say 'git commit -i paths...', perhaps?"
-	exit 1
-}
-
-THIS_INDEX="$GIT_DIR/index"
-NEXT_INDEX="$GIT_DIR/next-index$$"
-rm -f "$NEXT_INDEX"
-save_index () {
-	cp -p "$THIS_INDEX" "$NEXT_INDEX"
-}
-
-run_status () {
-	# If TMP_INDEX is defined, that means we are doing
-	# "--only" partial commit, and that index file is used
-	# to build the tree for the commit.  Otherwise, if
-	# NEXT_INDEX exists, that is the index file used to
-	# make the commit.  Otherwise we are using as-is commit
-	# so the regular index file is what we use to compare.
-	if test '' != "$TMP_INDEX"
-	then
-		GIT_INDEX_FILE="$TMP_INDEX"
-		export GIT_INDEX_FILE
-	elif test -f "$NEXT_INDEX"
-	then
-		GIT_INDEX_FILE="$NEXT_INDEX"
-		export GIT_INDEX_FILE
-	fi
-
-	if test "$status_only" = "t" -o "$use_status_color" = "t"; then
-		color=
-	else
-		color=--nocolor
-	fi
-	git runstatus ${color} \
-		${verbose:+--verbose} \
-		${amend:+--amend} \
-		${untracked_files:+--untracked}
-}
-
-trap '
-	test -z "$TMP_INDEX" || {
-		test -f "$TMP_INDEX" && rm -f "$TMP_INDEX"
-	}
-	rm -f "$NEXT_INDEX"
-' 0
-
-################################################################
-# Command line argument parsing and sanity checking
-
-all=
-also=
-interactive=
-only=
-logfile=
-use_commit=
-amend=
-edit_flag=
-no_edit=
-log_given=
-log_message=
-verify=t
-quiet=
-verbose=
-signoff=
-force_author=
-only_include_assumed=
-untracked_files=
-templatefile="`git config commit.template`"
-while test $# != 0
-do
-	case "$1" in
-	-F|--F|-f|--f|--fi|--fil|--file)
-		case "$#" in 1) usage ;; esac
-		shift
-		no_edit=t
-		log_given=t$log_given
-		logfile="$1"
-		;;
-	-F*|-f*)
-		no_edit=t
-		log_given=t$log_given
-		logfile="${1#-[Ff]}"
-		;;
-	--F=*|--f=*|--fi=*|--fil=*|--file=*)
-		no_edit=t
-		log_given=t$log_given
-		logfile="${1#*=}"
-		;;
-	-a|--a|--al|--all)
-		all=t
-		;;
-	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
-		force_author="${1#*=}"
-		;;
-	--au|--aut|--auth|--autho|--author)
-		case "$#" in 1) usage ;; esac
-		shift
-		force_author="$1"
-		;;
-	-e|--e|--ed|--edi|--edit)
-		edit_flag=t
-		;;
-	-i|--i|--in|--inc|--incl|--inclu|--includ|--include)
-		also=t
-		;;
-	--int|--inte|--inter|--intera|--interac|--interact|--interacti|\
-	--interactiv|--interactive)
-		interactive=t
-		;;
-	-o|--o|--on|--onl|--only)
-		only=t
-		;;
-	-m|--m|--me|--mes|--mess|--messa|--messag|--message)
-		case "$#" in 1) usage ;; esac
-		shift
-		log_given=m$log_given
-		log_message="${log_message:+${log_message}
-
-}$1"
-		no_edit=t
-		;;
-	-m*)
-		log_given=m$log_given
-		log_message="${log_message:+${log_message}
-
-}${1#-m}"
-		no_edit=t
-		;;
-	--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
-		log_given=m$log_given
-		log_message="${log_message:+${log_message}
-
-}${1#*=}"
-		no_edit=t
-		;;
-	-n|--n|--no|--no-|--no-v|--no-ve|--no-ver|--no-veri|--no-verif|\
-	--no-verify)
-		verify=
-		;;
-	--a|--am|--ame|--amen|--amend)
-		amend=t
-		use_commit=HEAD
-		;;
-	-c)
-		case "$#" in 1) usage ;; esac
-		shift
-		log_given=t$log_given
-		use_commit="$1"
-		no_edit=
-		;;
-	--ree=*|--reed=*|--reedi=*|--reedit=*|--reedit-=*|--reedit-m=*|\
-	--reedit-me=*|--reedit-mes=*|--reedit-mess=*|--reedit-messa=*|\
-	--reedit-messag=*|--reedit-message=*)
-		log_given=t$log_given
-		use_commit="${1#*=}"
-		no_edit=
-		;;
-	--ree|--reed|--reedi|--reedit|--reedit-|--reedit-m|--reedit-me|\
-	--reedit-mes|--reedit-mess|--reedit-messa|--reedit-messag|\
-	--reedit-message)
-		case "$#" in 1) usage ;; esac
-		shift
-		log_given=t$log_given
-		use_commit="$1"
-		no_edit=
-		;;
-	-C)
-		case "$#" in 1) usage ;; esac
-		shift
-		log_given=t$log_given
-		use_commit="$1"
-		no_edit=t
-		;;
-	--reu=*|--reus=*|--reuse=*|--reuse-=*|--reuse-m=*|--reuse-me=*|\
-	--reuse-mes=*|--reuse-mess=*|--reuse-messa=*|--reuse-messag=*|\
-	--reuse-message=*)
-		log_given=t$log_given
-		use_commit="${1#*=}"
-		no_edit=t
-		;;
-	--reu|--reus|--reuse|--reuse-|--reuse-m|--reuse-me|--reuse-mes|\
-	--reuse-mess|--reuse-messa|--reuse-messag|--reuse-message)
-		case "$#" in 1) usage ;; esac
-		shift
-		log_given=t$log_given
-		use_commit="$1"
-		no_edit=t
-		;;
-	-s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
-		signoff=t
-		;;
-	-t|--t|--te|--tem|--temp|--templ|--templa|--templat|--template)
-		case "$#" in 1) usage ;; esac
-		shift
-		templatefile="$1"
-		no_edit=
-		;;
-	-q|--q|--qu|--qui|--quie|--quiet)
-		quiet=t
-		;;
-	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
-		verbose=t
-		;;
-	-u|--u|--un|--unt|--untr|--untra|--untrac|--untrack|--untracke|\
-	--untracked|--untracked-|--untracked-f|--untracked-fi|--untracked-fil|\
-	--untracked-file|--untracked-files)
-		untracked_files=t
-		;;
-	--)
-		shift
-		break
-		;;
-	-*)
-		usage
-		;;
-	*)
-		break
-		;;
-	esac
-	shift
-done
-case "$edit_flag" in t) no_edit= ;; esac
-
-################################################################
-# Sanity check options
-
-case "$amend,$initial_commit" in
-t,t)
-	die "You do not have anything to amend." ;;
-t,)
-	if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
-		die "You are in the middle of a merge -- cannot amend."
-	fi ;;
-esac
-
-case "$log_given" in
-tt*)
-	die "Only one of -c/-C/-F can be used." ;;
-*tm*|*mt*)
-	die "Option -m cannot be combined with -c/-C/-F." ;;
-esac
-
-case "$#,$also,$only,$amend" in
-*,t,t,*)
-	die "Only one of --include/--only can be used." ;;
-0,t,,* | 0,,t,)
-	die "No paths with --include/--only does not make sense." ;;
-0,,t,t)
-	only_include_assumed="# Clever... amending the last one with dirty index." ;;
-0,,,*)
-	;;
-*,,,*)
-	only_include_assumed="# Explicit paths specified without -i nor -o; assuming --only paths..."
-	also=
-	;;
-esac
-unset only
-case "$all,$interactive,$also,$#" in
-*t,*t,*)
-	die "Cannot use -a, --interactive or -i at the same time." ;;
-t,,[1-9]*)
-	die "Paths with -a does not make sense." ;;
-,t,[1-9]*)
-	die "Paths with --interactive does not make sense." ;;
-,,t,0)
-	die "No paths with -i does not make sense." ;;
-esac
-
-if test ! -z "$templatefile" -a -z "$log_given"
-then
-	if test ! -f "$templatefile"
-	then
-		die "Commit template file does not exist."
-	fi
-fi
-
-################################################################
-# Prepare index to have a tree to be committed
-
-case "$all,$also" in
-t,)
-	if test ! -f "$THIS_INDEX"
-	then
-		die 'nothing to commit (use "git add file1 file2" to include for commit)'
-	fi
-	save_index &&
-	(
-		cd_to_toplevel &&
-		GIT_INDEX_FILE="$NEXT_INDEX" &&
-		export GIT_INDEX_FILE &&
-		git diff-files --name-only -z |
-		git update-index --remove -z --stdin
-	) || exit
-	;;
-,t)
-	save_index &&
-	git ls-files --error-unmatch -- "$@" >/dev/null || exit
-
-	git diff-files --name-only -z -- "$@"  |
-	(
-		cd_to_toplevel &&
-		GIT_INDEX_FILE="$NEXT_INDEX" &&
-		export GIT_INDEX_FILE &&
-		git update-index --remove -z --stdin
-	) || exit
-	;;
-,)
-	if test "$interactive" = t; then
-		git add --interactive || exit
-	fi
-	case "$#" in
-	0)
-		;; # commit as-is
-	*)
-		if test -f "$GIT_DIR/MERGE_HEAD"
-		then
-			refuse_partial "Cannot do a partial commit during a merge."
-		fi
-
-		TMP_INDEX="$GIT_DIR/tmp-index$$"
-		W=
-		test -z "$initial_commit" && W=--with-tree=HEAD
-		commit_only=`git ls-files --error-unmatch $W -- "$@"` || exit
-
-		# Build a temporary index and update the real index
-		# the same way.
-		if test -z "$initial_commit"
-		then
-			GIT_INDEX_FILE="$THIS_INDEX" \
-			git read-tree --index-output="$TMP_INDEX" -i -m HEAD
-		else
-			rm -f "$TMP_INDEX"
-		fi || exit
-
-		printf '%s\n' "$commit_only" |
-		GIT_INDEX_FILE="$TMP_INDEX" \
-		git update-index --add --remove --stdin &&
-
-		save_index &&
-		printf '%s\n' "$commit_only" |
-		(
-			GIT_INDEX_FILE="$NEXT_INDEX"
-			export GIT_INDEX_FILE
-			git update-index --add --remove --stdin
-		) || exit
-		;;
-	esac
-	;;
-esac
-
-################################################################
-# If we do as-is commit, the index file will be THIS_INDEX,
-# otherwise NEXT_INDEX after we make this commit.  We leave
-# the index as is if we abort.
-
-if test -f "$NEXT_INDEX"
-then
-	USE_INDEX="$NEXT_INDEX"
-else
-	USE_INDEX="$THIS_INDEX"
-fi
-
-case "$status_only" in
-t)
-	# This will silently fail in a read-only repository, which is
-	# what we want.
-	GIT_INDEX_FILE="$USE_INDEX" git update-index -q --unmerged --refresh
-	run_status
-	exit $?
-	;;
-'')
-	GIT_INDEX_FILE="$USE_INDEX" git update-index -q --refresh || exit
-	;;
-esac
-
-################################################################
-# Grab commit message, write out tree and make commit.
-
-if test t = "$verify" && test -x "$GIT_DIR"/hooks/pre-commit
-then
-    GIT_INDEX_FILE="${TMP_INDEX:-${USE_INDEX}}" "$GIT_DIR"/hooks/pre-commit \
-    || exit
-fi
-
-if test "$log_message" != ''
-then
-	printf '%s\n' "$log_message"
-elif test "$logfile" != ""
-then
-	if test "$logfile" = -
-	then
-		test -t 0 &&
-		echo >&2 "(reading log message from standard input)"
-		cat
-	else
-		cat <"$logfile"
-	fi
-elif test "$use_commit" != ""
-then
-	encoding=$(git config i18n.commitencoding || echo UTF-8)
-	git show -s --pretty=raw --encoding="$encoding" "$use_commit" |
-	sed -e '1,/^$/d' -e 's/^    //'
-elif test -f "$GIT_DIR/MERGE_MSG"
-then
-	cat "$GIT_DIR/MERGE_MSG"
-elif test -f "$GIT_DIR/SQUASH_MSG"
-then
-	cat "$GIT_DIR/SQUASH_MSG"
-elif test "$templatefile" != ""
-then
-	cat "$templatefile"
-fi | git stripspace >"$GIT_DIR"/COMMIT_EDITMSG
-
-case "$signoff" in
-t)
-	sign=$(git-var GIT_COMMITTER_IDENT | sed -e '
-		s/>.*/>/
-		s/^/Signed-off-by: /
-		')
-	blank_before_signoff=
-	tail -n 1 "$GIT_DIR"/COMMIT_EDITMSG |
-	grep 'Signed-off-by:' >/dev/null || blank_before_signoff='
-'
-	tail -n 1 "$GIT_DIR"/COMMIT_EDITMSG |
-	grep "$sign"$ >/dev/null ||
-	printf '%s%s\n' "$blank_before_signoff" "$sign" \
-		>>"$GIT_DIR"/COMMIT_EDITMSG
-	;;
-esac
-
-if test -f "$GIT_DIR/MERGE_HEAD" && test -z "$no_edit"; then
-	echo "#"
-	echo "# It looks like you may be committing a MERGE."
-	echo "# If this is not correct, please remove the file"
-	printf '%s\n' "#	$GIT_DIR/MERGE_HEAD"
-	echo "# and try again"
-	echo "#"
-fi >>"$GIT_DIR"/COMMIT_EDITMSG
-
-# Author
-if test '' != "$use_commit"
-then
-	eval "$(get_author_ident_from_commit "$use_commit")"
-	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
-fi
-if test '' != "$force_author"
-then
-	GIT_AUTHOR_NAME=`expr "z$force_author" : 'z\(.*[^ ]\) *<.*'` &&
-	GIT_AUTHOR_EMAIL=`expr "z$force_author" : '.*\(<.*\)'` &&
-	test '' != "$GIT_AUTHOR_NAME" &&
-	test '' != "$GIT_AUTHOR_EMAIL" ||
-	die "malformed --author parameter"
-	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
-fi
-
-PARENTS="-p HEAD"
-if test -z "$initial_commit"
-then
-	rloga='commit'
-	if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
-		rloga='commit (merge)'
-		PARENTS="-p HEAD "`sed -e 's/^/-p /' "$GIT_DIR/MERGE_HEAD"`
-	elif test -n "$amend"; then
-		rloga='commit (amend)'
-		PARENTS=$(git cat-file commit HEAD |
-			sed -n -e '/^$/q' -e 's/^parent /-p /p')
-	fi
-	current="$(git rev-parse --verify HEAD)"
-else
-	if [ -z "$(git ls-files)" ]; then
-		echo >&2 'nothing to commit (use "git add file1 file2" to include for commit)'
-		exit 1
-	fi
-	PARENTS=""
-	rloga='commit (initial)'
-	current=''
-fi
-set_reflog_action "$rloga"
-
-if test -z "$no_edit"
-then
-	{
-		echo ""
-		echo "# Please enter the commit message for your changes."
-		echo "# (Comment lines starting with '#' will not be included)"
-		test -z "$only_include_assumed" || echo "$only_include_assumed"
-		run_status
-	} >>"$GIT_DIR"/COMMIT_EDITMSG
-else
-	# we need to check if there is anything to commit
-	run_status >/dev/null
-fi
-if [ "$?" != "0" -a ! -f "$GIT_DIR/MERGE_HEAD" ]
-then
-	rm -f "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
-	use_status_color=t
-	run_status
-	exit 1
-fi
-
-case "$no_edit" in
-'')
-	git-var GIT_AUTHOR_IDENT > /dev/null  || die
-	git-var GIT_COMMITTER_IDENT > /dev/null  || die
-	git_editor "$GIT_DIR/COMMIT_EDITMSG"
-	;;
-esac
-
-case "$verify" in
-t)
-	if test -x "$GIT_DIR"/hooks/commit-msg
-	then
-		"$GIT_DIR"/hooks/commit-msg "$GIT_DIR"/COMMIT_EDITMSG || exit
-	fi
-esac
-
-if test -z "$no_edit"
-then
-    sed -e '
-        /^diff --git a\/.*/{
-	    s///
-	    q
-	}
-	/^#/d
-    ' "$GIT_DIR"/COMMIT_EDITMSG
-else
-    cat "$GIT_DIR"/COMMIT_EDITMSG
-fi |
-git stripspace >"$GIT_DIR"/COMMIT_MSG
-
-# Test whether the commit message has any content we didn't supply.
-have_commitmsg=
-grep -v -i '^Signed-off-by' "$GIT_DIR"/COMMIT_MSG |
-	git stripspace > "$GIT_DIR"/COMMIT_BAREMSG
-
-# Is the commit message totally empty?
-if test -s "$GIT_DIR"/COMMIT_BAREMSG
-then
-	if test "$templatefile" != ""
-	then
-		# Test whether this is just the unaltered template.
-		if cnt=`sed -e '/^#/d' < "$templatefile" |
-			git stripspace |
-			diff "$GIT_DIR"/COMMIT_BAREMSG - |
-			wc -l` &&
-		   test 0 -lt $cnt
-		then
-			have_commitmsg=t
-		fi
-	else
-		# No template, so the content in the commit message must
-		# have come from the user.
-		have_commitmsg=t
-	fi
-fi
-
-rm -f "$GIT_DIR"/COMMIT_BAREMSG
-
-if test "$have_commitmsg" = "t"
-then
-	if test -z "$TMP_INDEX"
-	then
-		tree=$(GIT_INDEX_FILE="$USE_INDEX" git write-tree)
-	else
-		tree=$(GIT_INDEX_FILE="$TMP_INDEX" git write-tree) &&
-		rm -f "$TMP_INDEX"
-	fi &&
-	commit=$(git commit-tree $tree $PARENTS <"$GIT_DIR/COMMIT_MSG") &&
-	rlogm=$(sed -e 1q "$GIT_DIR"/COMMIT_MSG) &&
-	git update-ref -m "$GIT_REFLOG_ACTION: $rlogm" HEAD $commit "$current" &&
-	rm -f -- "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/MERGE_MSG" &&
-	if test -f "$NEXT_INDEX"
-	then
-		mv "$NEXT_INDEX" "$THIS_INDEX"
-	else
-		: ;# happy
-	fi
-else
-	echo >&2 "* no commit message?  aborting commit."
-	false
-fi
-ret="$?"
-rm -f "$GIT_DIR/COMMIT_MSG" "$GIT_DIR/COMMIT_EDITMSG" "$GIT_DIR/SQUASH_MSG"
-
-cd_to_toplevel
-
-git rerere
-
-if test "$ret" = 0
-then
-	git gc --auto
-	if test -x "$GIT_DIR"/hooks/post-commit
-	then
-		"$GIT_DIR"/hooks/post-commit
-	fi
-	if test -z "$quiet"
-	then
-		commit=`git diff-tree --always --shortstat --pretty="format:%h: %s"\
-		       --summary --root HEAD --`
-		echo "Created${initial_commit:+ initial} commit $commit"
-	fi
-fi
-
-exit "$ret"
diff --git a/git.c b/git.c
index d7c6bca..9565555 100644
--- a/git.c
+++ b/git.c
@@ -320,6 +320,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
@@ -368,10 +369,10 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "rev-parse", cmd_rev_parse, RUN_SETUP },
 		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 		{ "rm", cmd_rm, RUN_SETUP | NEED_WORK_TREE },
-		{ "runstatus", cmd_runstatus, RUN_SETUP | NEED_WORK_TREE },
 		{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
 		{ "show-branch", cmd_show_branch, RUN_SETUP },
 		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
+		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 		{ "stripspace", cmd_stripspace },
 		{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 		{ "tag", cmd_tag, RUN_SETUP },
diff --git a/strbuf.h b/strbuf.h
index 5657e3d..eef4e6d 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -113,5 +113,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path);
 extern int strbuf_getline(struct strbuf *, FILE *, int);
 
 extern void stripspace(struct strbuf *buf, int skip_comments);
+extern void launch_editor(const char *path, struct strbuf *buffer);
 
 #endif /* STRBUF_H */
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 552af1c..2dbe04f 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -44,7 +44,7 @@ test_expect_success setup '
 test_expect_success 'cherry-pick after renaming branch' '
 
 	git checkout rename2 &&
-	EDITOR=: VISUAL=: git cherry-pick added &&
+	git cherry-pick added &&
 	test -f opos &&
 	grep "Add extra line at the end" opos
 
@@ -53,7 +53,7 @@ test_expect_success 'cherry-pick after renaming branch' '
 test_expect_success 'revert after renaming branch' '
 
 	git checkout rename1 &&
-	EDITOR=: VISUAL=: git revert added &&
+	git revert added &&
 	test -f spoo &&
 	! grep "Add extra line at the end" spoo
 
diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh
index 28e9e37..235f372 100755
--- a/t/t3901-i18n-patch.sh
+++ b/t/t3901-i18n-patch.sh
@@ -154,7 +154,7 @@ test_expect_success 'cherry-pick(U/U)' '
 	git reset --hard master &&
 	git cherry-pick side^ &&
 	git cherry-pick side &&
-	EDITOR=: VISUAL=: git revert HEAD &&
+	git revert HEAD &&
 
 	check_encoding 3
 '
@@ -169,7 +169,7 @@ test_expect_success 'cherry-pick(L/L)' '
 	git reset --hard master &&
 	git cherry-pick side^ &&
 	git cherry-pick side &&
-	EDITOR=: VISUAL=: git revert HEAD &&
+	git revert HEAD &&
 
 	check_encoding 3 8859
 '
@@ -184,7 +184,7 @@ test_expect_success 'cherry-pick(U/L)' '
 	git reset --hard master &&
 	git cherry-pick side^ &&
 	git cherry-pick side &&
-	EDITOR=: VISUAL=: git revert HEAD &&
+	git revert HEAD &&
 
 	check_encoding 3
 '
@@ -200,7 +200,7 @@ test_expect_success 'cherry-pick(L/U)' '
 	git reset --hard master &&
 	git cherry-pick side^ &&
 	git cherry-pick side &&
-	EDITOR=: VISUAL=: git revert HEAD &&
+	git revert HEAD &&
 
 	check_encoding 3 8859
 '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index cc1253c..a232bd6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -9,8 +9,8 @@ LC_ALL=C
 PAGER=cat
 TZ=UTC
 export LANG LC_ALL PAGER TZ
-EDITOR=:
-VISUAL=:
+EDITOR=/bin/true
+VISUAL=/bin/true
 unset GIT_EDITOR
 unset AUTHOR_DATE
 unset AUTHOR_EMAIL
-- 
1.5.2.GIT

[PATCH 4/4] Move launch_editor() and stripspace() to new file editor.c.

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 Makefile             |    2 +-
 builtin-commit.c     |    1 +
 builtin-stripspace.c |   68 +------------------------------
 builtin-tag.c        |   40 +------------------
 editor.c             |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 editor.h             |    9 ++++
 strbuf.h             |    3 -
 7 files changed, 122 insertions(+), 110 deletions(-)
 create mode 100644 editor.c
 create mode 100644 editor.h

diff --git a/Makefile b/Makefile
index 6172589..f6d991e 100644
--- a/Makefile
+++ b/Makefile
@@ -310,7 +310,7 @@ LIB_OBJS = \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
 	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
-	transport.o bundle.o parse-options.o
+	transport.o bundle.o parse-options.o editor.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/builtin-commit.c b/builtin-commit.c
index 69e8b19..7d87812 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -19,6 +19,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "editor.h"
 
 static const char builtin_commit_usage[] =
 	"[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]";
diff --git a/builtin-stripspace.c b/builtin-stripspace.c
index c0b2130..4e5bb07 100644
--- a/builtin-stripspace.c
+++ b/builtin-stripspace.c
@@ -1,72 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
-
-/*
- * Returns the length of a line, without trailing spaces.
- *
- * If the line ends with newline, it will be removed too.
- */
-static size_t cleanup(char *line, size_t len)
-{
-	while (len) {
-		unsigned char c = line[len - 1];
-		if (!isspace(c))
-			break;
-		len--;
-	}
-
-	return len;
-}
-
-/*
- * Remove empty lines from the beginning and end
- * and also trailing spaces from every line.
- *
- * Note that the buffer will not be NUL-terminated.
- *
- * Turn multiple consecutive empty lines between paragraphs
- * into just one empty line.
- *
- * If the input has only empty lines and spaces,
- * no output will be produced.
- *
- * If last line does not have a newline at the end, one is added.
- *
- * Enable skip_comments to skip every line starting with "#".
- */
-void stripspace(struct strbuf *sb, int skip_comments)
-{
-	int empties = 0;
-	size_t i, j, len, newlen;
-	char *eol;
-
-	/* We may have to add a newline. */
-	strbuf_grow(sb, 1);
-
-	for (i = j = 0; i < sb->len; i += len, j += newlen) {
-		eol = memchr(sb->buf + i, '\n', sb->len - i);
-		len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
-
-		if (skip_comments && len && sb->buf[i] == '#') {
-			newlen = 0;
-			continue;
-		}
-		newlen = cleanup(sb->buf + i, len);
-
-		/* Not just an empty line? */
-		if (newlen) {
-			if (empties > 0 && j > 0)
-				sb->buf[j++] = '\n';
-			empties = 0;
-			memmove(sb->buf + j, sb->buf + i, newlen);
-			sb->buf[newlen + j++] = '\n';
-		} else {
-			empties++;
-		}
-	}
-
-	strbuf_setlen(sb, j);
-}
+#include "editor.h"
 
 int cmd_stripspace(int argc, const char **argv, const char *prefix)
 {
diff --git a/builtin-tag.c b/builtin-tag.c
index 0a36a5d..c9a0497 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -10,6 +10,7 @@
 #include "builtin.h"
 #include "refs.h"
 #include "tag.h"
+#include "editor.h"
 #include "run-command.h"
 
 static const char builtin_tag_usage[] =
@@ -17,45 +18,6 @@ static const char builtin_tag_usage[] =
 
 static char signingkey[1000];
 
-void launch_editor(const char *path, struct strbuf *buffer)
-{
-	const char *editor, *terminal;
-	struct child_process child;
-	const char *args[3];
-
-	editor = getenv("GIT_EDITOR");
-	if (!editor && editor_program)
-		editor = editor_program;
-	if (!editor)
-		editor = getenv("VISUAL");
-	if (!editor)
-		editor = getenv("EDITOR");
-
-	terminal = getenv("TERM");
-	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
-		fprintf(stderr,
-		"Terminal is dumb but no VISUAL nor EDITOR defined.\n"
-		"Please supply the message using either -m or -F option.\n");
-		exit(1);
-	}
-
-	if (!editor)
-		editor = "vi";
-
-	memset(&child, 0, sizeof(child));
-	child.argv = args;
-	args[0] = editor;
-	args[1] = path;
-	args[2] = NULL;
-
-	if (run_command(&child))
-		die("There was a problem with the editor %s.", editor);
-
-	if (strbuf_read_file(buffer, path) < 0)
-		die("could not read message file '%s': %s",
-		    path, strerror(errno));
-}
-
 struct tag_filter {
 	const char *pattern;
 	int lines;
diff --git a/editor.c b/editor.c
new file mode 100644
index 0000000..6bc3033
--- /dev/null
+++ b/editor.c
@@ -0,0 +1,109 @@
+#include "git-compat-util.h"
+#include "editor.h"
+#include "run-command.h"
+
+void launch_editor(const char *path, struct strbuf *buffer)
+{
+	const char *editor, *terminal;
+	struct child_process child;
+	const char *args[3];
+
+	editor = getenv("GIT_EDITOR");
+	if (!editor && editor_program)
+		editor = editor_program;
+	if (!editor)
+		editor = getenv("VISUAL");
+	if (!editor)
+		editor = getenv("EDITOR");
+
+	terminal = getenv("TERM");
+	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
+		fprintf(stderr,
+		"Terminal is dumb but no VISUAL nor EDITOR defined.\n"
+		"Please supply the message using either -m or -F option.\n");
+		exit(1);
+	}
+
+	if (!editor)
+		editor = "vi";
+
+	memset(&child, 0, sizeof(child));
+	child.argv = args;
+	args[0] = editor;
+	args[1] = path;
+	args[2] = NULL;
+
+	if (run_command(&child))
+		die("There was a problem with the editor %s.", editor);
+
+	if (strbuf_read_file(buffer, path) < 0)
+		die("could not read message file '%s': %s",
+		    path, strerror(errno));
+}
+
+/*
+ * Returns the length of a line, without trailing spaces.
+ *
+ * If the line ends with newline, it will be removed too.
+ */
+static size_t cleanup(char *line, size_t len)
+{
+	while (len) {
+		unsigned char c = line[len - 1];
+		if (!isspace(c))
+			break;
+		len--;
+	}
+
+	return len;
+}
+
+/*
+ * Remove empty lines from the beginning and end
+ * and also trailing spaces from every line.
+ *
+ * Note that the buffer will not be NUL-terminated.
+ *
+ * Turn multiple consecutive empty lines between paragraphs
+ * into just one empty line.
+ *
+ * If the input has only empty lines and spaces,
+ * no output will be produced.
+ *
+ * If last line does not have a newline at the end, one is added.
+ *
+ * Enable skip_comments to skip every line starting with "#".
+ */
+void stripspace(struct strbuf *sb, int skip_comments)
+{
+	int empties = 0;
+	size_t i, j, len, newlen;
+	char *eol;
+
+	/* We may have to add a newline. */
+	strbuf_grow(sb, 1);
+
+	for (i = j = 0; i < sb->len; i += len, j += newlen) {
+		eol = memchr(sb->buf + i, '\n', sb->len - i);
+		len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
+
+		if (skip_comments && len && sb->buf[i] == '#') {
+			newlen = 0;
+			continue;
+		}
+		newlen = cleanup(sb->buf + i, len);
+
+		/* Not just an empty line? */
+		if (newlen) {
+			if (empties > 0 && j > 0)
+				sb->buf[j++] = '\n';
+			empties = 0;
+			memmove(sb->buf + j, sb->buf + i, newlen);
+			sb->buf[newlen + j++] = '\n';
+		} else {
+			empties++;
+		}
+	}
+
+	strbuf_setlen(sb, j);
+}
diff --git a/editor.h b/editor.h
new file mode 100644
index 0000000..9dc39f5
--- /dev/null
+++ b/editor.h
@@ -0,0 +1,9 @@
+#ifndef EDITOR_H
+#define EDITOR_H
+
+#include "cache.h"
+
+extern void launch_editor(const char *path, struct strbuf *buffer);
+extern void stripspace(struct strbuf *sb, int skip_comments);
+
+#endif
diff --git a/strbuf.h b/strbuf.h
index eef4e6d..d4d9e56 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -112,7 +112,4 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path);
 
 extern int strbuf_getline(struct strbuf *, FILE *, int);
 
-extern void stripspace(struct strbuf *buf, int skip_comments);
-extern void launch_editor(const char *path, struct strbuf *buffer);
-
 #endif /* STRBUF_H */
-- 
1.5.2.GIT

Re: [PATCH 2/4] This exports the update() function from builtin-add.c as

[Junio C Hamano]

Kristian Høgsberg <krh@redhat.com> writes:

> Signed-off-by: Kristian Høgsberg <krh@redhat.com>

Huh?  -EPARSE_TITLE_STRING

[PATCH 1/4] Add a simple option parser for use by builtin-commit.c.
[PATCH 2/4] This exports the update() function from builtin-add.c as
[PATCH 3/4] Implement git commit as a builtin command.
[PATCH 4/4] Move launch_editor() and stripspace() to new file editor.c.

Let's step back a bit.  The whole series organization is very
screwy.  Especially I do not think 4/4 should be at the end.

Reviewing your old series...

 * Enable wt-status output to a given FILE pointer.
 * Enable wt-status to run against non-standard index file.

Let's have the above two from the previous series in 'next'.

Now the following five that have been in 'pu' are from the older
series:

 * Introduce entry point for launching add--interactive.
 * Clean up stripspace a bit, use strbuf even more.
 * Add strbuf_read_file().
 * Export rerere() and launch_editor().
 * Implement git commit as a builtin command.

The changes to stripspace and strbuf_read_file() are unrelated
to making the commit command a builtin.  I have extended the
strbuf topic with these two and merged the result to 'next'.

I think the right organization for the "builtin-commit" series
should be:

 * merge strbuf topic in kh/commit topic, in order to get the
   stripspace updates and strbuf_read_file();

 * add--interactive entry point change (respin the one from the
   old series);

 * rename update() to add_files_to_cache() and export (respin
   this [2/4] with a better commit message);

 * create a separate rerere() function and export (respin part
   of old series, with proper refactoring);

   I am not happy with builtin-foo.c calling into something from
   builtin-bar.c, though.  We probably would want to move
   rerere() and add_files_to_cache() somewhere else.

 * move launch_editor() and stripspace() to create editor.c (new
   [4/4]);

 * add option parser in parse-options.[ch] (new [1/4]);

 * finally, create builtin-commit that uses the groundwork laid
   out above (new [3/4]).

I ended up doing the above up to the rerere() one myself, but
haven't done the rest.

I probably would start more aggressively asking the original
author to clean up and resubmit from now on.  I haven't managed
to scrape enough time for myself to code anything meaningful for
git recently, and instead spent too much time fixing up other
peoples code.

Re: [PATCH 2/4] This exports the update() function from builtin-add.c as

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 235 bytes --]

Hi,

On Thu, 27 Sep 2007, Kristian Høgsberg wrote:

>  builtin-add.c |    8 ++++----
>  commit.h      |    2 ++

Maybe move it to read-cache.c, just after "add_file_to_index()"?  And 
expose it via cache.h, not commit.h?

Ciao,
Dscho

Re: [PATCH 2/4] This exports the update() function from builtin-add.c as

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Thu, 27 Sep 2007, Kristian Høgsberg wrote:
>
>>  builtin-add.c |    8 ++++----
>>  commit.h      |    2 ++
>
> Maybe move it to read-cache.c, just after "add_file_to_index()"?  And 
> expose it via cache.h, not commit.h?

Hmmmmmmmm.  read-cache.c has been one of the lowest level files
that define atomic operations, similar to sha1_file.c, and this
function is much more porcelain-ish molecule operation.  I'd
rather not.

What other useful molecules would we have and/or need to have by
splitting existing standalone commands?  rerere() is another,
and probably we would need to rip the --with-tree bit from
ls-files out to make it usable from builtin-commit.

I do not think we would want to go to the "one file per
function" extreme, either.  Can we have a new file that hold
these helper functions for porcelains?

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Jonas Fonseca]

Hello Kristian,

I have some comments on your patch. Some of the "improvement" might have
to wait until after your builtin-commit changes hits git.git. However,
if we could agree on some of the general changes, I could start porting
other of the main porcelain commands to use the option parser without
depending on the state of the remaining builtin-commit series.

Kristian Høgsberg <krh@redhat.com> wrote Thu, Sep 27, 2007:
> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> ---
>  Makefile        |    2 +-
>  parse-options.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  parse-options.h |   29 +++++++++++++++++++++
>  3 files changed, 104 insertions(+), 1 deletions(-)
>  create mode 100644 parse-options.c
>  create mode 100644 parse-options.h
> 
> diff --git a/Makefile b/Makefile
> index 62bdac6..d90e959 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -310,7 +310,7 @@ LIB_OBJS = \
>  	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
>  	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
>  	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
> -	transport.o bundle.o
> +	transport.o bundle.o parse-options.o
>  
>  BUILTIN_OBJS = \
>  	builtin-add.o \
> diff --git a/parse-options.c b/parse-options.c
> new file mode 100644
> index 0000000..2fb30cd
> --- /dev/null
> +++ b/parse-options.c
> @@ -0,0 +1,74 @@
> +#include "git-compat-util.h"
> +#include "parse-options.h"
> +
> +int parse_options(const char ***argv,
> +		  struct option *options, int count,
> +		  const char *usage_string)
> +{
> +	const char *value, *eq;
> +	int i;
> +
> +	if (**argv == NULL)
> +		return 0;
> +	if ((**argv)[0] != '-')
> +		return 0;
> +	if (!strcmp(**argv, "--"))
> +		return 0;

I don't know if this is a bug, but you do not remove "--" from argv,
which is later (in the patch that adds builtin-commit.c) passed to
add_files_to_cache and then get_pathspec where it is not removed or
detected either.

> +
> +	value = NULL;
> +	for (i = 0; i < count; i++) {
> +		if ((**argv)[1] == '-') {
> +			if (!prefixcmp(options[i].long_name, **argv + 2)) {
> +				if (options[i].type != OPTION_BOOLEAN)
> +					value = *++(*argv);
> +				goto match;
> +			}
> +
> +			eq = strchr(**argv + 2, '=');
> +			if (eq && options[i].type != OPTION_BOOLEAN &&
> +			    !strncmp(**argv + 2,
> +				     options[i].long_name, eq - **argv - 2)) {
> +				value = eq + 1;
> +				goto match;
> +			}
> +		}
> +
> +		if ((**argv)[1] == options[i].short_name) {
> +			if ((**argv)[2] == '\0') {
> +				if (options[i].type != OPTION_BOOLEAN)
> +					value = *++(*argv);
> +				goto match;
> +			}
> +
> +			if (options[i].type != OPTION_BOOLEAN) {
> +				value = **argv + 2;
> +				goto match;
> +			}
> +		}
> +	}
> +
> +	usage(usage_string);
> +
> + match:

I think the goto can be avoided by simply breaking out of the above loop
when an option has been found and add ...

> +	switch (options[i].type) {
	case OPTION_LAST
		usage(usage_string);
		break;

> +	case OPTION_BOOLEAN:
> +		*(int *)options[i].value = 1;
> +		break;

I've been looking at builtin-blame.c which IMO has some of the most
obscure option parsing and maybe this can be changed to increment in
order to support stuff like changing the meaning by passing the same arg
multiple times (e.g. "-C -C -C") better.

Blame option parsing also sports (enum) flags being masked together,
this can of course be rewritten to a boolean option followed by
masking when parse_options is done (to keep it sane).

> +	case OPTION_STRING:
> +		if (value == NULL)
> +			die("option %s requires a value.", (*argv)[-1]);

Maybe change this ...

> +		*(const char **)options[i].value = value;
> +		break;
> +	case OPTION_INTEGER:
> +		if (value == NULL)
> +			die("option %s requires a value.", (*argv)[-1]);

... and this to:

		if (!value) {
			error("option %s requires a value.", (*argv)[-1]);
			usage(usage_string);
		}

> +		*(int *)options[i].value = atoi(value);
> +		break;
> +	default:
> +		assert(0);
> +	}
> +
> +	(*argv)++;
> +
> +	return 1;
> +}
> diff --git a/parse-options.h b/parse-options.h
> new file mode 100644
> index 0000000..39399c3
> --- /dev/null
> +++ b/parse-options.h
> @@ -0,0 +1,29 @@
> +#ifndef PARSE_OPTIONS_H
> +#define PARSE_OPTIONS_H
> +
> +enum option_type {
> +    OPTION_BOOLEAN,
> +    OPTION_STRING,
> +    OPTION_INTEGER,
> +    OPTION_LAST,
> +};
> +
> +struct option {
> +    enum option_type type;
> +    const char *long_name;
> +    char short_name;
> +    void *value;
> +};

Space vs tab indentation.

One of the last things I miss from Cogito is the nice abbreviated help
messages that was available via '-h'. I don't know if it would be
acceptable (at least for the main porcelain commands) to put this
functionality into the option parser by adding a "description" member to
struct option and have parse_options print a nice:

	<error message if any>
	<usage string>
	<option summary>

on failure, or, if that is regarded as too verbose, simply when -h is
detected.

> +
> +/* Parse the given options against the list of known options.  The
> + * order of the option structs matters, in that ambiguous
> + * abbreviations (eg, --in could be short for --include or
> + * --interactive) are matched by the first option that share the
> + * prefix.
> + */

This prefix aware option parsing has not been ported over to the other
builtins when they were lifted from shell code. It might be nice to have
of course. Is it really needed?

> +
> +extern int parse_options(const char ***argv,
> +			 struct option *options, int count,
> +			 const char *usage_string);

I think the interface could be improved a bit. For example, it doesn't
need to count argument since the last entry in the options array is
OPTION_LAST and thus the size can be detected that way.

Also, I think for this to be more usable for other built-in programs it
shouldn't modify argv, but instead take both argc and argv (so we don't
need to have code like "*++(*argv)" ;), parse _all_ options in one go,
and return the index (of argv) for any remaining options.

Then the following:

	while (parse_options(argv, commit_options, ARRAY_SIZE(commit_options),
		builtin_commit_usage))
		;

becomes:

	int i;
	...
	i = parse_options(argc, argv, commit_options, builtin_commit_usage);

This fits better with how option parsing is currently done. Take
builtin-add for example:

	for (i = 1 ; i < argc ; i++) {
		const char *arg = argv[i];
		/* ... */
	}
	if (argc <= i)
		usage(builtin_rm_usage);

[ BTW, blame option parsing actually wants to know if "--" has been seen,
  but I think that can be worked around by simply checking argv[i - 1]
  after calling the option parser. ]

> +
> +#endif

OK, I will stop these ramblings here. I hope the fact that I read your
patch both back and forth and added comments in the process didn't make
it too confusing.

-- 
Jonas Fonseca

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Johannes Schindelin]

Hi,

On Sun, 30 Sep 2007, Jonas Fonseca wrote:

> Also, I think for this to be more usable for other built-in programs it 
> shouldn't modify argv, but instead take both argc and argv (so we don't 
> need to have code like "*++(*argv)" ;), parse _all_ options in one go, 
> and return the index (of argv) for any remaining options.

We _have_ to modify argv.  For example, "git log master -p" is perfectly 
valid.

Ciao,
Dscho

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Jonas Fonseca]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote Mon, Oct 01, 2007:
> Hi,
> 
> On Sun, 30 Sep 2007, Jonas Fonseca wrote:
> 
> > Also, I think for this to be more usable for other built-in programs it 
> > shouldn't modify argv, but instead take both argc and argv (so we don't 
> > need to have code like "*++(*argv)" ;), parse _all_ options in one go, 
> > and return the index (of argv) for any remaining options.
> 
> We _have_ to modify argv.  For example, "git log master -p" is perfectly 
> valid.

Ah, yes this could be nice to also finally have (more universally) in
git. But for this to be possible I don't see any reason for it to modify
the pointer to argv. Instead, it can just reshuffle entries in argv.

-- 
Jonas Fonseca

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Johannes Schindelin]

Hi,

On Mon, 1 Oct 2007, Jonas Fonseca wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote Mon, Oct 01, 2007:
> 
> > On Sun, 30 Sep 2007, Jonas Fonseca wrote:
> > 
> > > Also, I think for this to be more usable for other built-in programs 
> > > it shouldn't modify argv, but instead take both argc and argv (so we 
> > > don't need to have code like "*++(*argv)" ;), parse _all_ options in 
> > > one go, and return the index (of argv) for any remaining options.
> > 
> > We _have_ to modify argv.  For example, "git log master -p" is 
> > perfectly valid.
> 
> Ah, yes this could be nice to also finally have (more universally) in 
> git. But for this to be possible I don't see any reason for it to modify 
> the pointer to argv. Instead, it can just reshuffle entries in argv.

In that case, I misunderstood you.  Indeed, I'd only reshuffle the 
entries of argv.

Ciao,
Dscho

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Jeff King]

On Mon, Oct 01, 2007 at 11:14:48AM +0100, Johannes Schindelin wrote:

> We _have_ to modify argv.  For example, "git log master -p" is perfectly 
> valid.

We're not going to support POSIXLY_CORRECT!? ;)

-Peff

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Kristian Høgsberg]

On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
> Hello Kristian,
> 
> I have some comments on your patch. Some of the "improvement" might have
> to wait until after your builtin-commit changes hits git.git. However,
> if we could agree on some of the general changes, I could start porting
> other of the main porcelain commands to use the option parser without
> depending on the state of the remaining builtin-commit series.

Hi Jonas,

That's sounds like a good plan.  In fact, in you want to update the
patch with your changes (they all sound good) and start porting over
some of the other builtins feel free.  I don't have much time follow up
on these comments right now, but I will get to it eventually - unless
you beat me to it of course ;)  I will update builtin-commit.c to work
with whatever changes you introduce once I get around to updating that
patch.

> Kristian Høgsberg <krh@redhat.com> wrote Thu, Sep 27, 2007:
> > Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> > ---
> >  Makefile        |    2 +-
> >  parse-options.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  parse-options.h |   29 +++++++++++++++++++++
> >  3 files changed, 104 insertions(+), 1 deletions(-)
> >  create mode 100644 parse-options.c
> >  create mode 100644 parse-options.h
> > 
> > diff --git a/Makefile b/Makefile
> > index 62bdac6..d90e959 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -310,7 +310,7 @@ LIB_OBJS = \
> >  	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
> >  	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
> >  	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
> > -	transport.o bundle.o
> > +	transport.o bundle.o parse-options.o
> >  
> >  BUILTIN_OBJS = \
> >  	builtin-add.o \
> > diff --git a/parse-options.c b/parse-options.c
> > new file mode 100644
> > index 0000000..2fb30cd
> > --- /dev/null
> > +++ b/parse-options.c
> > @@ -0,0 +1,74 @@
> > +#include "git-compat-util.h"
> > +#include "parse-options.h"
> > +
> > +int parse_options(const char ***argv,
> > +		  struct option *options, int count,
> > +		  const char *usage_string)
> > +{
> > +	const char *value, *eq;
> > +	int i;
> > +
> > +	if (**argv == NULL)
> > +		return 0;
> > +	if ((**argv)[0] != '-')
> > +		return 0;
> > +	if (!strcmp(**argv, "--"))
> > +		return 0;
> 
> I don't know if this is a bug, but you do not remove "--" from argv,
> which is later (in the patch that adds builtin-commit.c) passed to
> add_files_to_cache and then get_pathspec where it is not removed or
> detected either.

That's an oversight, good catch.

> > +
> > +	value = NULL;
> > +	for (i = 0; i < count; i++) {
> > +		if ((**argv)[1] == '-') {
> > +			if (!prefixcmp(options[i].long_name, **argv + 2)) {
> > +				if (options[i].type != OPTION_BOOLEAN)
> > +					value = *++(*argv);
> > +				goto match;
> > +			}
> > +
> > +			eq = strchr(**argv + 2, '=');
> > +			if (eq && options[i].type != OPTION_BOOLEAN &&
> > +			    !strncmp(**argv + 2,
> > +				     options[i].long_name, eq - **argv - 2)) {
> > +				value = eq + 1;
> > +				goto match;
> > +			}
> > +		}
> > +
> > +		if ((**argv)[1] == options[i].short_name) {
> > +			if ((**argv)[2] == '\0') {
> > +				if (options[i].type != OPTION_BOOLEAN)
> > +					value = *++(*argv);
> > +				goto match;
> > +			}
> > +
> > +			if (options[i].type != OPTION_BOOLEAN) {
> > +				value = **argv + 2;
> > +				goto match;
> > +			}
> > +		}
> > +	}
> > +
> > +	usage(usage_string);
> > +
> > + match:
> 
> I think the goto can be avoided by simply breaking out of the above loop
> when an option has been found and add ...
> 
> > +	switch (options[i].type) {
> 	case OPTION_LAST
> 		usage(usage_string);
> 		break;
> 
> > +	case OPTION_BOOLEAN:
> > +		*(int *)options[i].value = 1;
> > +		break;

Yeah, that looks nicer.  I think the goto structure is a leftover from
when there was more logic between the loop and the switch.  It's good to
get some fresh eyes on this code.  Junio didn't like the OPTION_LAST
terminator, so I changed the interface to take a count.  We can do

	if (i == count)
		usage();
	else switch (options[i].type) {
		...
	}

of course.

> I've been looking at builtin-blame.c which IMO has some of the most
> obscure option parsing and maybe this can be changed to increment in
> order to support stuff like changing the meaning by passing the same arg
> multiple times (e.g. "-C -C -C") better.

That would be fine, yes.

> Blame option parsing also sports (enum) flags being masked together,
> this can of course be rewritten to a boolean option followed by
> masking when parse_options is done (to keep it sane).

Yup.

> > +	case OPTION_STRING:
> > +		if (value == NULL)
> > +			die("option %s requires a value.", (*argv)[-1]);
> 
> Maybe change this ...
> 
> > +		*(const char **)options[i].value = value;
> > +		break;
> > +	case OPTION_INTEGER:
> > +		if (value == NULL)
> > +			die("option %s requires a value.", (*argv)[-1]);
> 
> ... and this to:
> 
> 		if (!value) {
> 			error("option %s requires a value.", (*argv)[-1]);
> 			usage(usage_string);
> 		}

Sure, that's friendlier.

> > +		*(int *)options[i].value = atoi(value);
> > +		break;
> > +	default:
> > +		assert(0);
> > +	}
> > +
> > +	(*argv)++;
> > +
> > +	return 1;
> > +}
> > diff --git a/parse-options.h b/parse-options.h
> > new file mode 100644
> > index 0000000..39399c3
> > --- /dev/null
> > +++ b/parse-options.h
> > @@ -0,0 +1,29 @@
> > +#ifndef PARSE_OPTIONS_H
> > +#define PARSE_OPTIONS_H
> > +
> > +enum option_type {
> > +    OPTION_BOOLEAN,
> > +    OPTION_STRING,
> > +    OPTION_INTEGER,
> > +    OPTION_LAST,
> > +};
> > +
> > +struct option {
> > +    enum option_type type;
> > +    const char *long_name;
> > +    char short_name;
> > +    void *value;
> > +};
> 
> Space vs tab indentation.
> 
> One of the last things I miss from Cogito is the nice abbreviated help
> messages that was available via '-h'. I don't know if it would be
> acceptable (at least for the main porcelain commands) to put this
> functionality into the option parser by adding a "description" member to
> struct option and have parse_options print a nice:
> 
> 	<error message if any>
> 	<usage string>
> 	<option summary>
> 
> on failure, or, if that is regarded as too verbose, simply when -h is
> detected.

Yeah, that might be nice.  We can add it in a follow-on patch, if the
list agrees that it's a good thing, I guess.

> > +
> > +/* Parse the given options against the list of known options.  The
> > + * order of the option structs matters, in that ambiguous
> > + * abbreviations (eg, --in could be short for --include or
> > + * --interactive) are matched by the first option that share the
> > + * prefix.
> > + */
> 
> This prefix aware option parsing has not been ported over to the other
> builtins when they were lifted from shell code. It might be nice to have
> of course. Is it really needed?

I don't ever use it myself and I think it's more confusing than helpful.
I only added it to avoid introducing behavior changes in the port.  I
don't have strong feelings either way.

> > +
> > +extern int parse_options(const char ***argv,
> > +			 struct option *options, int count,
> > +			 const char *usage_string);
> 
> I think the interface could be improved a bit. For example, it doesn't
> need to count argument since the last entry in the options array is
> OPTION_LAST and thus the size can be detected that way.

Hehe, yeah, that's how I did it first.  I don't have a strong preference
for terminator elements vs. ARRAY_SIZE(), but Junio prefers the
ARRAY_SIZE() approach, I guess.  At this point I'm just trying the get
the patches upstream...

> Also, I think for this to be more usable for other built-in programs it
> shouldn't modify argv, but instead take both argc and argv (so we don't
> need to have code like "*++(*argv)" ;), parse _all_ options in one go,
> and return the index (of argv) for any remaining options.
> 
> Then the following:
> 
> 	while (parse_options(argv, commit_options, ARRAY_SIZE(commit_options),
> 		builtin_commit_usage))
> 		;
> 
> becomes:
> 
> 	int i;
> 	...
> 	i = parse_options(argc, argv, commit_options, builtin_commit_usage);
> 
> This fits better with how option parsing is currently done. Take
> builtin-add for example:
> 
> 	for (i = 1 ; i < argc ; i++) {
> 		const char *arg = argv[i];
> 		/* ... */
> 	}
> 	if (argc <= i)
> 		usage(builtin_rm_usage);

No objections, I think that looks better too.

> [ BTW, blame option parsing actually wants to know if "--" has been seen,
>   but I think that can be worked around by simply checking argv[i - 1]
>   after calling the option parser. ]
> 
> > +
> > +#endif
> 
> OK, I will stop these ramblings here. I hope the fact that I read your
> patch both back and forth and added comments in the process didn't make
> it too confusing.

Heh, that's what I do myself :)

thanks for the comments,
Kristian

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Johannes Schindelin]

Hi,

On Mon, 1 Oct 2007, Kristian H?gsberg wrote:

> On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
>
> > One of the last things I miss from Cogito is the nice abbreviated help 
> > messages that was available via '-h'. I don't know if it would be 
> > acceptable (at least for the main porcelain commands) to put this 
> > functionality into the option parser by adding a "description" member 
> > to struct option and have parse_options print a nice:
> > 
> > 	<error message if any>
> > 	<usage string>
> > 	<option summary>
> > 
> > on failure, or, if that is regarded as too verbose, simply when -h is 
> > detected.
> 
> Yeah, that might be nice.  We can add it in a follow-on patch, if the 
> list agrees that it's a good thing, I guess.

That's a good idea; I would put the usage string there, too.

> > > +
> > > +/* Parse the given options against the list of known options.  The
> > > + * order of the option structs matters, in that ambiguous
> > > + * abbreviations (eg, --in could be short for --include or
> > > + * --interactive) are matched by the first option that share the
> > > + * prefix.
> > > + */
> > 
> > This prefix aware option parsing has not been ported over to the other 
> > builtins when they were lifted from shell code. It might be nice to 
> > have of course. Is it really needed?
> 
> I don't ever use it myself and I think it's more confusing than helpful. 
> I only added it to avoid introducing behavior changes in the port.  I 
> don't have strong feelings either way.

It might be convenient, but I think that it is really more confusing than 
helpful, especially with options that share a prefix.  Besides, we have 
good completion for bash now (and I hear that this "zsh" thing also has 
quite good completion), I recommend <TAB> over prefix DWIMery.

> > > +
> > > +extern int parse_options(const char ***argv,
> > > +			 struct option *options, int count,
> > > +			 const char *usage_string);
> > 
> > I think the interface could be improved a bit. For example, it doesn't 
> > need to count argument since the last entry in the options array is 
> > OPTION_LAST and thus the size can be detected that way.
> 
> Hehe, yeah, that's how I did it first.  I don't have a strong preference 
> for terminator elements vs. ARRAY_SIZE(), but Junio prefers the 
> ARRAY_SIZE() approach, I guess.  At this point I'm just trying the get 
> the patches upstream...

FWIW I like the ARRAY_SIZE() approach better, too, since it is less error 
prone.

Ciao,
Dscho

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Jonas Fonseca]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote Mon, Oct 01, 2007:
> On Mon, 1 Oct 2007, Kristian H?gsberg wrote:
> > On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
> > > > +
> > > > +extern int parse_options(const char ***argv,
> > > > +			 struct option *options, int count,
> > > > +			 const char *usage_string);
> > > 
> > > I think the interface could be improved a bit. For example, it doesn't 
> > > need to count argument since the last entry in the options array is 
> > > OPTION_LAST and thus the size can be detected that way.
> > 
> > Hehe, yeah, that's how I did it first.  I don't have a strong preference 
> > for terminator elements vs. ARRAY_SIZE(), but Junio prefers the 
> > ARRAY_SIZE() approach, I guess.  At this point I'm just trying the get 
> > the patches upstream...
> 
> FWIW I like the ARRAY_SIZE() approach better, too, since it is less error 
> prone.

OK, I must have missed that comment. Good point.

Thanks for the comments both of you. It's great to have something to
work from. However, I also fear it will also require that some extra
flags or information is added to the option information to make it more
generally usable. But I guess that is easier to discuss in the context
of a patch.

-- 
Jonas Fonseca

Re: [PATCH 1/4] Add a simple option parser for use by builtin-commit.c.

[Kristian Høgsberg]

On Wed, 2007-10-03 at 22:11 +0200, Jonas Fonseca wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote Mon, Oct 01, 2007:
> > On Mon, 1 Oct 2007, Kristian H?gsberg wrote:
> > > On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
> > > > > +
> > > > > +extern int parse_options(const char ***argv,
> > > > > +			 struct option *options, int count,
> > > > > +			 const char *usage_string);
> > > > 
> > > > I think the interface could be improved a bit. For example, it doesn't 
> > > > need to count argument since the last entry in the options array is 
> > > > OPTION_LAST and thus the size can be detected that way.
> > > 
> > > Hehe, yeah, that's how I did it first.  I don't have a strong preference 
> > > for terminator elements vs. ARRAY_SIZE(), but Junio prefers the 
> > > ARRAY_SIZE() approach, I guess.  At this point I'm just trying the get 
> > > the patches upstream...
> > 
> > FWIW I like the ARRAY_SIZE() approach better, too, since it is less error 
> > prone.
> 
> OK, I must have missed that comment. Good point.
> 
> Thanks for the comments both of you. It's great to have something to
> work from. However, I also fear it will also require that some extra
> flags or information is added to the option information to make it more
> generally usable. But I guess that is easier to discuss in the context
> of a patch.

I just sent an updated option parser patch that incorporates your
suggestions along with a patch that ports builtin-add.c to use it.  I
looked briefly into porting over a few other builtins, but you're right,
we need a couple of extra features for this to be really worthwile:

      * OPTION_SET_FLAG: sets the bit (we need to add a bit value that
        the option parser can or in)
      * OPTION_CLEAR_FLAG: clear the bit
      * OPTION_ADD: adds the value to the destination integer
      * OPTION_CALLBACK: calls the given function when the option is
        matched.  We'll need this for builtin-grep that has positional
        args such as --and etc.

Also, the option parser should probably verify that a string option
isn't passed more than once.  Bundling of single letter options would be
nice to add.  But the patch I just sent out should be a good start, and
it lets us move forward with builtin-commit.c.

cheers,
Kristian

Re: [PATCH 2/4] This exports the update() function from builtin-add.c as

[Kristian Høgsberg]

On Thu, 2007-09-27 at 02:05 -0700, Junio C Hamano wrote:
...
> I think the right organization for the "builtin-commit" series
> should be:
> 
>  * merge strbuf topic in kh/commit topic, in order to get the
>    stripspace updates and strbuf_read_file();
> 
>  * add--interactive entry point change (respin the one from the
>    old series);
> 
>  * rename update() to add_files_to_cache() and export (respin
>    this [2/4] with a better commit message);
> 
>  * create a separate rerere() function and export (respin part
>    of old series, with proper refactoring);
> 
>    I am not happy with builtin-foo.c calling into something from
>    builtin-bar.c, though.  We probably would want to move
>    rerere() and add_files_to_cache() somewhere else.
> 
>  * move launch_editor() and stripspace() to create editor.c (new
>    [4/4]);
> 
>  * add option parser in parse-options.[ch] (new [1/4]);
> 
>  * finally, create builtin-commit that uses the groundwork laid
>    out above (new [3/4]).
> 
> I ended up doing the above up to the rerere() one myself, but
> haven't done the rest.

>From what I see in next today, it looks like we're just missing the
parse-options patch and builtin-commit patches.  I resent a better
version of parse-options and a patch that ports builtin-add.c to the
option parser.  To use the option parser in more places, we'll probably
have to extend it a bit, but the patch is a good start.  Let's get that
in shape and into next and then I'll send an updated builtin-commit
patch.

thanks,
Kristian