git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Move option parsing code to parse-options.[ch].
@ 2007-09-21 19:01 Kristian Høgsberg
  2007-09-21 19:01 ` [PATCH] Share add_files_to_cache() with builtin-add.c Kristian Høgsberg
  2007-09-21 19:44 ` [PATCH] Move option parsing code to parse-options.[ch] Junio C Hamano
  0 siblings, 2 replies; 4+ messages in thread
From: Kristian Høgsberg @ 2007-09-21 19:01 UTC (permalink / raw)
  To: git; +Cc: Kristian Høgsberg

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

diff --git a/Makefile b/Makefile
index 69ebc7a..2612465 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/builtin-commit.c b/builtin-commit.c
index 3e826ca..90f23de 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -18,6 +18,7 @@
 #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>...]";
@@ -27,90 +28,6 @@ static char *use_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
 static struct lock_file lock_file;
 
-enum option_type {
-    OPTION_NONE,
-    OPTION_STRING,
-    OPTION_INTEGER,
-    OPTION_LAST,
-};
-
-struct option {
-    enum option_type type;
-    const char *long_name;
-    char short_name;
-    void *value;
-};
-
-static int scan_options(const char ***argv, struct option *options)
-{
-	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; options[i].type != OPTION_LAST; i++) {
-		if ((**argv)[1] == '-') {
-			if (!prefixcmp(options[i].long_name, **argv + 2)) {
-				if (options[i].type != OPTION_NONE)
-					value = *++(*argv);
-				goto match;
-			}
-
-			eq = strchr(**argv + 2, '=');
-			if (eq && options[i].type != OPTION_NONE &&
-			    !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_NONE)
-					value = *++(*argv);
-				goto match;
-			}
-
-			if (options[i].type != OPTION_NONE) {
-				value = **argv + 2;
-				goto match;
-			}
-		}
-	}
-
-	usage(builtin_commit_usage);
-
- match:
-	switch (options[i].type) {
-	case OPTION_NONE:
-		*(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;
-}
-
 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;
@@ -120,24 +37,23 @@ 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_NONE, "all", 'a', &all },
-	{ OPTION_STRING, "author", 0, (void *) &force_author },
-	{ OPTION_NONE, "edit", 'e', &edit_flag },
-	{ OPTION_NONE, "include", 'i', &also },
-	{ OPTION_NONE, "interactive", 0, &interactive },
-	{ OPTION_NONE, "only", 'o', &only },
+	{ OPTION_STRING, "file", 'F', &logfile },
+	{ OPTION_BOOLEAN, "all", 'a', &all },
+	{ OPTION_STRING, "author", 0, &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_NONE, "no-verify", 'n', &no_verify },
-	{ OPTION_NONE, "amend", 0, &amend },
+	{ 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_NONE, "signoff", 's', &signoff },
-	{ OPTION_NONE, "quiet", 'q', &quiet },
-	{ OPTION_NONE, "verbose", 'v', &verbose },
-	{ OPTION_NONE, "untracked-files", 0, &untracked_files },
+	{ 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 },
 };
 
 /* FIXME: Taken from builtin-add, should be shared. */
@@ -432,8 +348,9 @@ static void parse_and_validate_options(const char ***argv)
 	int f = 0;
 
 	(*argv)++;
-	while (scan_options(argv, commit_options))
-		;
+	while (parse_options(argv, commit_options, ARRAY_SIZE(commit_options),
+			     builtin_commit_usage))
+	       ;
 
 	if (logfile || message || use_message)
 		no_edit = 1;
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.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] Share add_files_to_cache() with builtin-add.c
  2007-09-21 19:01 [PATCH] Move option parsing code to parse-options.[ch] Kristian Høgsberg
@ 2007-09-21 19:01 ` Kristian Høgsberg
  2007-09-21 19:44 ` [PATCH] Move option parsing code to parse-options.[ch] Junio C Hamano
  1 sibling, 0 replies; 4+ messages in thread
From: Kristian Høgsberg @ 2007-09-21 19:01 UTC (permalink / raw)
  To: git; +Cc: Kristian Høgsberg

This removes the extra copy in builtin-commit.c and uses
the update() function from builtin-add.c.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 builtin-add.c    |    8 +++---
 builtin-commit.c |   65 +++++++++++------------------------------------------
 commit.h         |    2 +
 3 files changed, 20 insertions(+), 55 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/builtin-commit.c b/builtin-commit.c
index 90f23de..3768b53 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -56,53 +56,6 @@ static struct option commit_options[] = {
 	{ OPTION_STRING, "template", 't', &template_file },
 };
 
-/* FIXME: Taken from builtin-add, should be shared. */
-
-static void update_callback(struct diff_queue_struct *q,
-			    struct diff_options *opt, void *cbdata)
-{
-	int i, verbose;
-
-	verbose = *((int *)cbdata);
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		const char *path = p->one->path;
-		switch (p->status) {
-		default:
-			die("unexpacted diff status %c", p->status);
-		case DIFF_STATUS_UNMERGED:
-		case DIFF_STATUS_MODIFIED:
-		case DIFF_STATUS_TYPE_CHANGED:
-			add_file_to_cache(path, verbose);
-			break;
-		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (verbose)
-				printf("remove '%s'\n", path);
-			break;
-		}
-	}
-}
-
-static void
-add_files_to_cache(int fd, const char **files, const char *prefix)
-{
-	struct rev_info rev;
-
-	init_revisions(&rev, "");
-	setup_revisions(0, NULL, &rev, NULL);
-	rev.prune_data = get_pathspec(prefix, files);
-	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
-	rev.diffopt.format_callback = update_callback;
-	rev.diffopt.format_callback_data = &verbose;
-
-	run_diff_files(&rev, 0);
-	refresh_cache(REFRESH_QUIET);
-
-	if (write_cache(fd, active_cache, active_nr) || close(fd))
-		die("unable to write new index file");
-}
-
 static char *
 prepare_index(const char **files, const char *prefix)
 {
@@ -115,10 +68,16 @@ prepare_index(const char **files, const char *prefix)
 		die("index file corrupt");
 
 	if (all) {
-		add_files_to_cache(fd, files, NULL);
+		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, files, prefix);
+		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;
 	}
 
@@ -146,7 +105,9 @@ prepare_index(const char **files, const char *prefix)
 	 */
 
 	/* update the user index file */
-	add_files_to_cache(fd, files, prefix);
+	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);
@@ -160,7 +121,9 @@ prepare_index(const char **files, const char *prefix)
 	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(fd, files, prefix);
+	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;
 }
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.5

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Move option parsing code to parse-options.[ch].
  2007-09-21 19:01 [PATCH] Move option parsing code to parse-options.[ch] Kristian Høgsberg
  2007-09-21 19:01 ` [PATCH] Share add_files_to_cache() with builtin-add.c Kristian Høgsberg
@ 2007-09-21 19:44 ` Junio C Hamano
  2007-09-24 18:41   ` Kristian Høgsberg
  1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2007-09-21 19:44 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git

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

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

Hmmmmmmm. Is it too much to ask to pretend as if the previous
"builtin-commit.c" that had these parts that did not belong to
it in the first place never happened?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Move option parsing code to parse-options.[ch].
  2007-09-21 19:44 ` [PATCH] Move option parsing code to parse-options.[ch] Junio C Hamano
@ 2007-09-24 18:41   ` Kristian Høgsberg
  0 siblings, 0 replies; 4+ messages in thread
From: Kristian Høgsberg @ 2007-09-24 18:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Fri, 2007-09-21 at 12:44 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> > ---
> >  Makefile         |    2 +-
> >  builtin-commit.c |  117 ++++++++----------------------------------------------
> >  parse-options.c  |   74 ++++++++++++++++++++++++++++++++++
> >  parse-options.h  |   29 +++++++++++++
> >  4 files changed, 121 insertions(+), 101 deletions(-)
> >  create mode 100644 parse-options.c
> >  create mode 100644 parse-options.h
> 
> Hmmmmmmm. Is it too much to ask to pretend as if the previous
> "builtin-commit.c" that had these parts that did not belong to
> it in the first place never happened?

No problem, I wasn't sure whether to update the patches or to just send
follow-up patches now that it was in pu.  I'll resend 7/7 as 3 new
patches that exports add_files_to_cache(), adds option parsing, and
finally builtin-commit.

Kristian

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-09-24 18:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-09-21 19:01 [PATCH] Move option parsing code to parse-options.[ch] Kristian Høgsberg
2007-09-21 19:01 ` [PATCH] Share add_files_to_cache() with builtin-add.c Kristian Høgsberg
2007-09-21 19:44 ` [PATCH] Move option parsing code to parse-options.[ch] Junio C Hamano
2007-09-24 18:41   ` Kristian Høgsberg

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).