{"thread":{"id":"9963","subject":"[PATCH] Move option parsing code to parse-options.[ch].","startedAt":"2007-09-21T19:01:17Z","lastAt":"2007-09-24T18:41:02Z","messageCount":4,"participants":["Kristian Høgsberg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"53738","messageId":"1190401278-2869-1-git-send-email-krh@redhat.com","threadId":"9963","inReplyTo":null,"subject":"[PATCH] Move option parsing code to parse-options.[ch].","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-09-21T19:01:17Z","receivedAt":"2007-09-21T19:01:17Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n---\n Makefile         |    2 +-\n builtin-commit.c |  117 ++++++++----------------------------------------------\n parse-options.c  |   74 ++++++++++++++++++++++++++++++++++\n parse-options.h  |   29 +++++++++++++\n 4 files changed, 121 insertions(+), 101 deletions(-)\n create mode 100644 parse-options.c\n create mode 100644 parse-options.h\n\ndiff --git a/Makefile b/Makefile\nindex 69ebc7a..2612465 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -310,7 +310,7 @@ LIB_OBJS = \\\n \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n \tconvert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \\\n-\ttransport.o bundle.o\n+\ttransport.o bundle.o parse-options.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 3e826ca..90f23de 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -18,6 +18,7 @@\n #include \"log-tree.h\"\n #include \"strbuf.h\"\n #include \"utf8.h\"\n+#include \"parse-options.h\"\n \n static const char builtin_commit_usage[] =\n \t\"[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]\";\n@@ -27,90 +28,6 @@ static char *use_message_buffer;\n static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n static struct lock_file lock_file;\n \n-enum option_type {\n-    OPTION_NONE,\n-    OPTION_STRING,\n-    OPTION_INTEGER,\n-    OPTION_LAST,\n-};\n-\n-struct option {\n-    enum option_type type;\n-    const char *long_name;\n-    char short_name;\n-    void *value;\n-};\n-\n-static int scan_options(const char ***argv, struct option *options)\n-{\n-\tconst char *value, *eq;\n-\tint i;\n-\n-\tif (**argv == NULL)\n-\t\treturn 0;\n-\tif ((**argv)[0] != '-')\n-\t\treturn 0;\n-\tif (!strcmp(**argv, \"--\"))\n-\t\treturn 0;\n-\n-\tvalue = NULL;\n-\tfor (i = 0; options[i].type != OPTION_LAST; i++) {\n-\t\tif ((**argv)[1] == '-') {\n-\t\t\tif (!prefixcmp(options[i].long_name, **argv + 2)) {\n-\t\t\t\tif (options[i].type != OPTION_NONE)\n-\t\t\t\t\tvalue = *++(*argv);\n-\t\t\t\tgoto match;\n-\t\t\t}\n-\n-\t\t\teq = strchr(**argv + 2, '=');\n-\t\t\tif (eq && options[i].type != OPTION_NONE &&\n-\t\t\t    !strncmp(**argv + 2,\n-\t\t\t\t     options[i].long_name, eq - **argv - 2)) {\n-\t\t\t\tvalue = eq + 1;\n-\t\t\t\tgoto match;\n-\t\t\t}\n-\t\t}\n-\n-\t\tif ((**argv)[1] == options[i].short_name) {\n-\t\t\tif ((**argv)[2] == '\\0') {\n-\t\t\t\tif (options[i].type != OPTION_NONE)\n-\t\t\t\t\tvalue = *++(*argv);\n-\t\t\t\tgoto match;\n-\t\t\t}\n-\n-\t\t\tif (options[i].type != OPTION_NONE) {\n-\t\t\t\tvalue = **argv + 2;\n-\t\t\t\tgoto match;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\tusage(builtin_commit_usage);\n-\n- match:\n-\tswitch (options[i].type) {\n-\tcase OPTION_NONE:\n-\t\t*(int *)options[i].value = 1;\n-\t\tbreak;\n-\tcase OPTION_STRING:\n-\t\tif (value == NULL)\n-\t\t\tdie(\"option %s requires a value.\", (*argv)[-1]);\n-\t\t*(const char **)options[i].value = value;\n-\t\tbreak;\n-\tcase OPTION_INTEGER:\n-\t\tif (value == NULL)\n-\t\t\tdie(\"option %s requires a value.\", (*argv)[-1]);\n-\t\t*(int *)options[i].value = atoi(value);\n-\t\tbreak;\n-\tdefault:\n-\t\tassert(0);\n-\t}\n-\n-\t(*argv)++;\n-\n-\treturn 1;\n-}\n-\n static char *logfile, *force_author, *message, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, no_verify, amend, signoff;\n@@ -120,24 +37,23 @@ static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n \n static struct option commit_options[] = {\n-\t{ OPTION_STRING, \"file\", 'F', (void *) &logfile },\n-\t{ OPTION_NONE, \"all\", 'a', &all },\n-\t{ OPTION_STRING, \"author\", 0, (void *) &force_author },\n-\t{ OPTION_NONE, \"edit\", 'e', &edit_flag },\n-\t{ OPTION_NONE, \"include\", 'i', &also },\n-\t{ OPTION_NONE, \"interactive\", 0, &interactive },\n-\t{ OPTION_NONE, \"only\", 'o', &only },\n+\t{ OPTION_STRING, \"file\", 'F', &logfile },\n+\t{ OPTION_BOOLEAN, \"all\", 'a', &all },\n+\t{ OPTION_STRING, \"author\", 0, &force_author },\n+\t{ OPTION_BOOLEAN, \"edit\", 'e', &edit_flag },\n+\t{ OPTION_BOOLEAN, \"include\", 'i', &also },\n+\t{ OPTION_BOOLEAN, \"interactive\", 0, &interactive },\n+\t{ OPTION_BOOLEAN, \"only\", 'o', &only },\n \t{ OPTION_STRING, \"message\", 'm', &message },\n-\t{ OPTION_NONE, \"no-verify\", 'n', &no_verify },\n-\t{ OPTION_NONE, \"amend\", 0, &amend },\n+\t{ OPTION_BOOLEAN, \"no-verify\", 'n', &no_verify },\n+\t{ OPTION_BOOLEAN, \"amend\", 0, &amend },\n \t{ OPTION_STRING, \"reedit-message\", 'c', &edit_message },\n \t{ OPTION_STRING, \"reuse-message\", 'C', &use_message },\n-\t{ OPTION_NONE, \"signoff\", 's', &signoff },\n-\t{ OPTION_NONE, \"quiet\", 'q', &quiet },\n-\t{ OPTION_NONE, \"verbose\", 'v', &verbose },\n-\t{ OPTION_NONE, \"untracked-files\", 0, &untracked_files },\n+\t{ OPTION_BOOLEAN, \"signoff\", 's', &signoff },\n+\t{ OPTION_BOOLEAN, \"quiet\", 'q', &quiet },\n+\t{ OPTION_BOOLEAN, \"verbose\", 'v', &verbose },\n+\t{ OPTION_BOOLEAN, \"untracked-files\", 0, &untracked_files },\n \t{ OPTION_STRING, \"template\", 't', &template_file },\n-\t{ OPTION_LAST },\n };\n \n /* FIXME: Taken from builtin-add, should be shared. */\n@@ -432,8 +348,9 @@ static void parse_and_validate_options(const char ***argv)\n \tint f = 0;\n \n \t(*argv)++;\n-\twhile (scan_options(argv, commit_options))\n-\t\t;\n+\twhile (parse_options(argv, commit_options, ARRAY_SIZE(commit_options),\n+\t\t\t     builtin_commit_usage))\n+\t       ;\n \n \tif (logfile || message || use_message)\n \t\tno_edit = 1;\ndiff --git a/parse-options.c b/parse-options.c\nnew file mode 100644\nindex 0000000..2fb30cd\n--- /dev/null\n+++ b/parse-options.c\n@@ -0,0 +1,74 @@\n+#include \"git-compat-util.h\"\n+#include \"parse-options.h\"\n+\n+int parse_options(const char ***argv,\n+\t\t  struct option *options, int count,\n+\t\t  const char *usage_string)\n+{\n+\tconst char *value, *eq;\n+\tint i;\n+\n+\tif (**argv == NULL)\n+\t\treturn 0;\n+\tif ((**argv)[0] != '-')\n+\t\treturn 0;\n+\tif (!strcmp(**argv, \"--\"))\n+\t\treturn 0;\n+\n+\tvalue = NULL;\n+\tfor (i = 0; i < count; i++) {\n+\t\tif ((**argv)[1] == '-') {\n+\t\t\tif (!prefixcmp(options[i].long_name, **argv + 2)) {\n+\t\t\t\tif (options[i].type != OPTION_BOOLEAN)\n+\t\t\t\t\tvalue = *++(*argv);\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\n+\t\t\teq = strchr(**argv + 2, '=');\n+\t\t\tif (eq && options[i].type != OPTION_BOOLEAN &&\n+\t\t\t    !strncmp(**argv + 2,\n+\t\t\t\t     options[i].long_name, eq - **argv - 2)) {\n+\t\t\t\tvalue = eq + 1;\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif ((**argv)[1] == options[i].short_name) {\n+\t\t\tif ((**argv)[2] == '\\0') {\n+\t\t\t\tif (options[i].type != OPTION_BOOLEAN)\n+\t\t\t\t\tvalue = *++(*argv);\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\n+\t\t\tif (options[i].type != OPTION_BOOLEAN) {\n+\t\t\t\tvalue = **argv + 2;\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tusage(usage_string);\n+\n+ match:\n+\tswitch (options[i].type) {\n+\tcase OPTION_BOOLEAN:\n+\t\t*(int *)options[i].value = 1;\n+\t\tbreak;\n+\tcase OPTION_STRING:\n+\t\tif (value == NULL)\n+\t\t\tdie(\"option %s requires a value.\", (*argv)[-1]);\n+\t\t*(const char **)options[i].value = value;\n+\t\tbreak;\n+\tcase OPTION_INTEGER:\n+\t\tif (value == NULL)\n+\t\t\tdie(\"option %s requires a value.\", (*argv)[-1]);\n+\t\t*(int *)options[i].value = atoi(value);\n+\t\tbreak;\n+\tdefault:\n+\t\tassert(0);\n+\t}\n+\n+\t(*argv)++;\n+\n+\treturn 1;\n+}\ndiff --git a/parse-options.h b/parse-options.h\nnew file mode 100644\nindex 0000000..39399c3\n--- /dev/null\n+++ b/parse-options.h\n@@ -0,0 +1,29 @@\n+#ifndef PARSE_OPTIONS_H\n+#define PARSE_OPTIONS_H\n+\n+enum option_type {\n+    OPTION_BOOLEAN,\n+    OPTION_STRING,\n+    OPTION_INTEGER,\n+    OPTION_LAST,\n+};\n+\n+struct option {\n+    enum option_type type;\n+    const char *long_name;\n+    char short_name;\n+    void *value;\n+};\n+\n+/* Parse the given options against the list of known options.  The\n+ * order of the option structs matters, in that ambiguous\n+ * abbreviations (eg, --in could be short for --include or\n+ * --interactive) are matched by the first option that share the\n+ * prefix.\n+ */\n+\n+extern int parse_options(const char ***argv,\n+\t\t\t struct option *options, int count,\n+\t\t\t const char *usage_string);\n+\n+#endif\n-- \n1.5.2.5\n"},{"id":"53739","messageId":"1190401278-2869-2-git-send-email-krh@redhat.com","threadId":"9963","inReplyTo":"1190401278-2869-1-git-send-email-krh@redhat.com","subject":"[PATCH] Share add_files_to_cache() with builtin-add.c","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-09-21T19:01:18Z","receivedAt":"2007-09-21T19:01:18Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"This removes the extra copy in builtin-commit.c and uses\nthe update() function from builtin-add.c.\n\nSigned-off-by: Kristian Høgsberg <krh@redhat.com>\n---\n builtin-add.c    |    8 +++---\n builtin-commit.c |   65 +++++++++++------------------------------------------\n commit.h         |    2 +\n 3 files changed, 20 insertions(+), 55 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 5e30380..966e145 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -107,7 +107,7 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n-static void update(int verbose, const char *prefix, const char **files)\n+void add_files_to_cache(int verbose, const char *prefix, const char **files)\n {\n \tstruct rev_info rev;\n \tinit_revisions(&rev, prefix);\n@@ -116,8 +116,6 @@ static void update(int verbose, const char *prefix, const char **files)\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = update_callback;\n \trev.diffopt.format_callback_data = &verbose;\n-\tif (read_cache() < 0)\n-\t\tdie(\"index file corrupt\");\n \trun_diff_files(&rev, 0);\n }\n \n@@ -218,7 +216,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (take_worktree_changes) {\n-\t\tupdate(verbose, prefix, argv + i);\n+\t\tif (read_cache() < 0)\n+\t\t\tdie(\"index file corrupt\");\n+\t\tadd_files_to_cache(verbose, prefix, argv + i);\n \t\tgoto finish;\n \t}\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 90f23de..3768b53 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -56,53 +56,6 @@ static struct option commit_options[] = {\n \t{ OPTION_STRING, \"template\", 't', &template_file },\n };\n \n-/* FIXME: Taken from builtin-add, should be shared. */\n-\n-static void update_callback(struct diff_queue_struct *q,\n-\t\t\t    struct diff_options *opt, void *cbdata)\n-{\n-\tint i, verbose;\n-\n-\tverbose = *((int *)cbdata);\n-\tfor (i = 0; i < q->nr; i++) {\n-\t\tstruct diff_filepair *p = q->queue[i];\n-\t\tconst char *path = p->one->path;\n-\t\tswitch (p->status) {\n-\t\tdefault:\n-\t\t\tdie(\"unexpacted diff status %c\", p->status);\n-\t\tcase DIFF_STATUS_UNMERGED:\n-\t\tcase DIFF_STATUS_MODIFIED:\n-\t\tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\t\tadd_file_to_cache(path, verbose);\n-\t\t\tbreak;\n-\t\tcase DIFF_STATUS_DELETED:\n-\t\t\tremove_file_from_cache(path);\n-\t\t\tif (verbose)\n-\t\t\t\tprintf(\"remove '%s'\\n\", path);\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-}\n-\n-static void\n-add_files_to_cache(int fd, const char **files, const char *prefix)\n-{\n-\tstruct rev_info rev;\n-\n-\tinit_revisions(&rev, \"\");\n-\tsetup_revisions(0, NULL, &rev, NULL);\n-\trev.prune_data = get_pathspec(prefix, files);\n-\trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n-\trev.diffopt.format_callback = update_callback;\n-\trev.diffopt.format_callback_data = &verbose;\n-\n-\trun_diff_files(&rev, 0);\n-\trefresh_cache(REFRESH_QUIET);\n-\n-\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n-\t\tdie(\"unable to write new index file\");\n-}\n-\n static char *\n prepare_index(const char **files, const char *prefix)\n {\n@@ -115,10 +68,16 @@ prepare_index(const char **files, const char *prefix)\n \t\tdie(\"index file corrupt\");\n \n \tif (all) {\n-\t\tadd_files_to_cache(fd, files, NULL);\n+\t\tadd_files_to_cache(0, prefix, files);\n+\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\t\tdie(\"unable to write new index file\");\n+\n \t\treturn lock_file.filename;\n \t} else if (also) {\n-\t\tadd_files_to_cache(fd, files, prefix);\n+\t\tadd_files_to_cache(0, prefix, files);\n+\t\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\t\tdie(\"unable to write new index file\");\n+\n \t\treturn lock_file.filename;\n \t}\n \n@@ -146,7 +105,9 @@ prepare_index(const char **files, const char *prefix)\n \t */\n \n \t/* update the user index file */\n-\tadd_files_to_cache(fd, files, prefix);\n+\tadd_files_to_cache(0, prefix, files);\n+\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\tdie(\"unable to write new index file\");\n \n \tif (!initial_commit) {\n \t\ttree = parse_tree_indirect(head_sha1);\n@@ -160,7 +121,9 @@ prepare_index(const char **files, const char *prefix)\n \tnext_index_lock = xmalloc(sizeof(*next_index_lock));\n \tfd = hold_lock_file_for_update(next_index_lock,\n \t\t\t\t       git_path(\"next-index-%d\", getpid()), 1);\n-\tadd_files_to_cache(fd, files, prefix);\n+\tadd_files_to_cache(0, prefix, files);\n+\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t\tdie(\"unable to write new index file\");\n \n \treturn next_index_lock->filename;\n }\ndiff --git a/commit.h b/commit.h\nindex cc8d890..89caa12 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -130,6 +130,8 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,\n int in_merge_bases(struct commit *, struct commit **, int);\n \n extern int interactive_add(void);\n+extern void add_files_to_cache(int verbose,\n+\t\t\t       const char *prefix, const char **files);\n extern int rerere(void);\n \n #endif /* COMMIT_H */\n-- \n1.5.2.5\n"},{"id":"53741","messageId":"7vsl57iz1t.fsf@gitster.siamese.dyndns.org","threadId":"9963","inReplyTo":"1190401278-2869-1-git-send-email-krh@redhat.com","subject":"Re: [PATCH] Move option parsing code to parse-options.[ch].","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-21T19:44:14Z","receivedAt":"2007-09-21T19:44:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristian Høgsberg <krh@redhat.com> writes:\n\n> Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n> ---\n>  Makefile         |    2 +-\n>  builtin-commit.c |  117 ++++++++----------------------------------------------\n>  parse-options.c  |   74 ++++++++++++++++++++++++++++++++++\n>  parse-options.h  |   29 +++++++++++++\n>  4 files changed, 121 insertions(+), 101 deletions(-)\n>  create mode 100644 parse-options.c\n>  create mode 100644 parse-options.h\n\nHmmmmmmm. Is it too much to ask to pretend as if the previous\n\"builtin-commit.c\" that had these parts that did not belong to\nit in the first place never happened?\n"},{"id":"53940","messageId":"1190659262.10444.1.camel@hinata.boston.redhat.com","threadId":"9963","inReplyTo":"7vsl57iz1t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Move option parsing code to parse-options.[ch].","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-09-24T18:41:02Z","receivedAt":"2007-09-24T18:41:02Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Fri, 2007-09-21 at 12:44 -0700, Junio C Hamano wrote:\n> Kristian Høgsberg <krh@redhat.com> writes:\n> \n> > Signed-off-by: Kristian Høgsberg <krh@redhat.com>\n> > ---\n> >  Makefile         |    2 +-\n> >  builtin-commit.c |  117 ++++++++----------------------------------------------\n> >  parse-options.c  |   74 ++++++++++++++++++++++++++++++++++\n> >  parse-options.h  |   29 +++++++++++++\n> >  4 files changed, 121 insertions(+), 101 deletions(-)\n> >  create mode 100644 parse-options.c\n> >  create mode 100644 parse-options.h\n> \n> Hmmmmmmm. Is it too much to ask to pretend as if the previous\n> \"builtin-commit.c\" that had these parts that did not belong to\n> it in the first place never happened?\n\nNo problem, I wasn't sure whether to update the patches or to just send\nfollow-up patches now that it was in pu.  I'll resend 7/7 as 3 new\npatches that exports add_files_to_cache(), adds option parsing, and\nfinally builtin-commit.\n\nKristian\n"}]}