{"thread":{"id":"15923","subject":"[PATCH] parse-opt: migrate builtin-checkout-index.","startedAt":"2008-10-15T22:55:43Z","lastAt":"2008-10-19T21:45:28Z","messageCount":9,"participants":["Miklos Vajna","Pierre Habouzit","Junio C Hamano","Raphael Zimmerer"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"93154","messageId":"1224111343-17433-1-git-send-email-vmiklos@frugalware.org","threadId":"15923","inReplyTo":null,"subject":"[PATCH] parse-opt: migrate builtin-checkout-index.","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-10-15T22:55:43Z","receivedAt":"2008-10-15T22:55:43Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nNOTE: I introduced the force/quiet/not_new helper variables because they\nare originally bitfields, so passing their address is not possible. One\ncould say that introducing helper functions for those as well would be\nnicer, but I think that would just make the code even longer with no\ngood reason.\n\n builtin-checkout-index.c |  151 +++++++++++++++++++++++++---------------------\n 1 files changed, 82 insertions(+), 69 deletions(-)\n\ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex 4ba2702..e241cd1 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -40,6 +40,7 @@\n #include \"cache.h\"\n #include \"quote.h\"\n #include \"cache-tree.h\"\n+#include \"parse-options.h\"\n \n #define CHECKOUT_ALL 4\n static int line_termination = '\\n';\n@@ -153,11 +154,55 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\texit(128);\n }\n \n-static const char checkout_cache_usage[] =\n-\"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...\";\n+static const char * const builtin_checkout_index_usage[] = {\n+\t\"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...\",\n+\tNULL\n+};\n \n static struct lock_file lock_file;\n \n+static int option_parse_u(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tint *newfd = opt->value;\n+\n+\tstate.refresh_cache = 1;\n+\tif (*newfd < 0)\n+\t\t*newfd = hold_locked_index(&lock_file, 1);\n+\treturn 0;\n+}\n+\n+static int option_parse_z(const struct option *opt,\n+\t\t\t  const char *arg, int unset)\n+{\n+\tline_termination = unset;\n+\treturn 0;\n+}\n+\n+static int option_parse_prefix(const struct option *opt,\n+\t\t\t       const char *arg, int unset)\n+{\n+\tstate.base_dir = arg;\n+\tstate.base_dir_len = strlen(arg);\n+\treturn 0;\n+}\n+\n+static int option_parse_stage(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tif (!strcmp(arg, \"all\")) {\n+\t\tto_tempfile = 1;\n+\t\tcheckout_stage = CHECKOUT_ALL;\n+\t} else {\n+\t\tint ch = arg[0];\n+\t\tif ('1' <= ch && ch <= '3')\n+\t\t\tcheckout_stage = arg[0] - '0';\n+\t\telse\n+\t\t\tdie(\"stage should be between 1 and 3 or all\");\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -165,6 +210,33 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \tint all = 0;\n \tint read_from_stdin = 0;\n \tint prefix_length;\n+\tint force = 0, quiet = 0, not_new = 0;\n+\tstruct option builtin_checkout_index_options[] = {\n+\t\tOPT_BOOLEAN('a', \"all\", &all,\n+\t\t\t\"checks out all files in the index\"),\n+\t\tOPT_BOOLEAN('f', \"force\", &force,\n+\t\t\t\"forces overwrite of existing files\"),\n+\t\tOPT__QUIET(&quiet),\n+\t\tOPT_BOOLEAN('n', \"no-create\", &not_new,\n+\t\t\t\"don't checkout new files\"),\n+\t\t{ OPTION_CALLBACK, 'u', \"index\", &newfd, NULL,\n+\t\t\t\"update stat information in the index file\",\n+\t\t\tPARSE_OPT_NOARG, option_parse_u },\n+\t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n+\t\t\t\"paths are separated with NUL character\",\n+\t\t\tPARSE_OPT_NOARG, option_parse_z },\n+\t\tOPT_BOOLEAN(0, \"stdin\", &read_from_stdin,\n+\t\t\t\"read list of paths from the standard input\"),\n+\t\tOPT_BOOLEAN(0, \"temp\", &to_tempfile,\n+\t\t\t\"write the content to temporary files\"),\n+\t\tOPT_CALLBACK(0, \"prefix\", NULL, \"string\",\n+\t\t\t\"when creating files, prepend <string>\",\n+\t\t\toption_parse_prefix),\n+\t\tOPT_CALLBACK(0, \"stage\", NULL, NULL,\n+\t\t\t\"copy out the files from named stage\",\n+\t\t\toption_parse_stage),\n+\t\tOPT_END()\n+\t};\n \n \tgit_config(git_default_config, NULL);\n \tstate.base_dir = \"\";\n@@ -174,72 +246,13 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\tdie(\"invalid cache\");\n \t}\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\n-\t\tif (!strcmp(arg, \"--\")) {\n-\t\t\ti++;\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-a\") || !strcmp(arg, \"--all\")) {\n-\t\t\tall = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-f\") || !strcmp(arg, \"--force\")) {\n-\t\t\tstate.force = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-q\") || !strcmp(arg, \"--quiet\")) {\n-\t\t\tstate.quiet = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-n\") || !strcmp(arg, \"--no-create\")) {\n-\t\t\tstate.not_new = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-u\") || !strcmp(arg, \"--index\")) {\n-\t\t\tstate.refresh_cache = 1;\n-\t\t\tif (newfd < 0)\n-\t\t\t\tnewfd = hold_locked_index(&lock_file, 1);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-z\")) {\n-\t\t\tline_termination = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--stdin\")) {\n-\t\t\tif (i != argc - 1)\n-\t\t\t\tdie(\"--stdin must be at the end\");\n-\t\t\tread_from_stdin = 1;\n-\t\t\ti++; /* do not consider arg as a file name */\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--temp\")) {\n-\t\t\tto_tempfile = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--prefix=\")) {\n-\t\t\tstate.base_dir = arg+9;\n-\t\t\tstate.base_dir_len = strlen(state.base_dir);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--stage=\")) {\n-\t\t\tif (!strcmp(arg + 8, \"all\")) {\n-\t\t\t\tto_tempfile = 1;\n-\t\t\t\tcheckout_stage = CHECKOUT_ALL;\n-\t\t\t} else {\n-\t\t\t\tint ch = arg[8];\n-\t\t\t\tif ('1' <= ch && ch <= '3')\n-\t\t\t\t\tcheckout_stage = arg[8] - '0';\n-\t\t\t\telse\n-\t\t\t\t\tdie(\"stage should be between 1 and 3 or all\");\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (arg[0] == '-')\n-\t\t\tusage(checkout_cache_usage);\n-\t\tbreak;\n-\t}\n+\targc = parse_options(argc, argv, builtin_checkout_index_options,\n+\t\t\tbuiltin_checkout_index_usage, 0);\n+\tstate.force = force;\n+\tstate.quiet = quiet;\n+\tstate.not_new = not_new;\n+\tif (argc && read_from_stdin)\n+\t\tdie(\"--stdin must be at the end\");\n \n \tif (state.base_dir_len || to_tempfile) {\n \t\t/* when --prefix is specified we do not\n@@ -253,7 +266,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Check out named files first */\n-\tfor ( ; i < argc; i++) {\n+\tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tconst char *p;\n \n-- \n1.6.0.2\n"},{"id":"93170","messageId":"20081016082340.GB15266@artemis.corp","threadId":"15923","inReplyTo":"1224111343-17433-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] parse-opt: migrate builtin-checkout-index.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-10-16T08:23:40Z","receivedAt":"2008-10-16T08:23:40Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Oct 15, 2008 at 10:55:43PM +0000, Miklos Vajna wrote:\n> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n> ---\n> \n> NOTE: I introduced the force/quiet/not_new helper variables because they\n> are originally bitfields, so passing their address is not possible. One\n> could say that introducing helper functions for those as well would be\n> nicer, but I think that would just make the code even longer with no\n> good reason.\n\nWell the alternative is to replace the bitfields with an enum, but it's\nnot always that nice as a result. C bit-fields sucks when it comes to\naddress them, it's silly not to be able to have the same as offsetof in\n\"bits\" for a structure, but oh well.\n\n> diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\n> index 4ba2702..e241cd1 100644\n> --- a/builtin-checkout-index.c\n> +++ b/builtin-checkout-index.c\n> @@ -40,6 +40,7 @@\n>  #include \"cache.h\"\n>  #include \"quote.h\"\n>  #include \"cache-tree.h\"\n> +#include \"parse-options.h\"\n>  \n>  #define CHECKOUT_ALL 4\n>  static int line_termination = '\\n';\n> @@ -153,11 +154,55 @@ static void checkout_all(const char *prefix, int prefix_length)\n>  \t\texit(128);\n>  }\n>  \n> -static const char checkout_cache_usage[] =\n> -\"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...\";\n> +static const char * const builtin_checkout_index_usage[] = {\n> +\t\"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...\",\n> +\tNULL\n> +};\n\nSince git checkout-index -h will show you all the options, I usually\nprefer to use \"[options] [--] <file>...\", it's 10x as readable, and the\nuser will have the [options] detail just below.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"93197","messageId":"20081016132810.GG536@genesis.frugalware.org","threadId":"15923","inReplyTo":"20081016082340.GB15266@artemis.corp","subject":"[PATCH v2] parse-opt: migrate builtin-checkout-index.","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-10-16T13:28:10Z","receivedAt":"2008-10-16T13:28:10Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Thu, Oct 16, 2008 at 10:23:40AM +0200, Pierre Habouzit <madcoder@debian.org> wrote:\n> Since git checkout-index -h will show you all the options, I usually\n> prefer to use \"[options] [--] <file>...\", it's 10x as readable, and the\n> user will have the [options] detail just below.\n\nOK, changed.\n\n builtin-checkout-index.c |  151 +++++++++++++++++++++++++---------------------\n 1 files changed, 82 insertions(+), 69 deletions(-)\n\ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex 4ba2702..bc91f94 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -40,6 +40,7 @@\n #include \"cache.h\"\n #include \"quote.h\"\n #include \"cache-tree.h\"\n+#include \"parse-options.h\"\n \n #define CHECKOUT_ALL 4\n static int line_termination = '\\n';\n@@ -153,11 +154,55 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\texit(128);\n }\n \n-static const char checkout_cache_usage[] =\n-\"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...\";\n+static const char * const builtin_checkout_index_usage[] = {\n+\t\"git checkout-index [options] [--] <file>...\",\n+\tNULL\n+};\n \n static struct lock_file lock_file;\n \n+static int option_parse_u(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tint *newfd = opt->value;\n+\n+\tstate.refresh_cache = 1;\n+\tif (*newfd < 0)\n+\t\t*newfd = hold_locked_index(&lock_file, 1);\n+\treturn 0;\n+}\n+\n+static int option_parse_z(const struct option *opt,\n+\t\t\t  const char *arg, int unset)\n+{\n+\tline_termination = unset;\n+\treturn 0;\n+}\n+\n+static int option_parse_prefix(const struct option *opt,\n+\t\t\t       const char *arg, int unset)\n+{\n+\tstate.base_dir = arg;\n+\tstate.base_dir_len = strlen(arg);\n+\treturn 0;\n+}\n+\n+static int option_parse_stage(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tif (!strcmp(arg, \"all\")) {\n+\t\tto_tempfile = 1;\n+\t\tcheckout_stage = CHECKOUT_ALL;\n+\t} else {\n+\t\tint ch = arg[0];\n+\t\tif ('1' <= ch && ch <= '3')\n+\t\t\tcheckout_stage = arg[0] - '0';\n+\t\telse\n+\t\t\tdie(\"stage should be between 1 and 3 or all\");\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -165,6 +210,33 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \tint all = 0;\n \tint read_from_stdin = 0;\n \tint prefix_length;\n+\tint force = 0, quiet = 0, not_new = 0;\n+\tstruct option builtin_checkout_index_options[] = {\n+\t\tOPT_BOOLEAN('a', \"all\", &all,\n+\t\t\t\"checks out all files in the index\"),\n+\t\tOPT_BOOLEAN('f', \"force\", &force,\n+\t\t\t\"forces overwrite of existing files\"),\n+\t\tOPT__QUIET(&quiet),\n+\t\tOPT_BOOLEAN('n', \"no-create\", &not_new,\n+\t\t\t\"don't checkout new files\"),\n+\t\t{ OPTION_CALLBACK, 'u', \"index\", &newfd, NULL,\n+\t\t\t\"update stat information in the index file\",\n+\t\t\tPARSE_OPT_NOARG, option_parse_u },\n+\t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n+\t\t\t\"paths are separated with NUL character\",\n+\t\t\tPARSE_OPT_NOARG, option_parse_z },\n+\t\tOPT_BOOLEAN(0, \"stdin\", &read_from_stdin,\n+\t\t\t\"read list of paths from the standard input\"),\n+\t\tOPT_BOOLEAN(0, \"temp\", &to_tempfile,\n+\t\t\t\"write the content to temporary files\"),\n+\t\tOPT_CALLBACK(0, \"prefix\", NULL, \"string\",\n+\t\t\t\"when creating files, prepend <string>\",\n+\t\t\toption_parse_prefix),\n+\t\tOPT_CALLBACK(0, \"stage\", NULL, NULL,\n+\t\t\t\"copy out the files from named stage\",\n+\t\t\toption_parse_stage),\n+\t\tOPT_END()\n+\t};\n \n \tgit_config(git_default_config, NULL);\n \tstate.base_dir = \"\";\n@@ -174,72 +246,13 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\tdie(\"invalid cache\");\n \t}\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\n-\t\tif (!strcmp(arg, \"--\")) {\n-\t\t\ti++;\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-a\") || !strcmp(arg, \"--all\")) {\n-\t\t\tall = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-f\") || !strcmp(arg, \"--force\")) {\n-\t\t\tstate.force = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-q\") || !strcmp(arg, \"--quiet\")) {\n-\t\t\tstate.quiet = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-n\") || !strcmp(arg, \"--no-create\")) {\n-\t\t\tstate.not_new = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-u\") || !strcmp(arg, \"--index\")) {\n-\t\t\tstate.refresh_cache = 1;\n-\t\t\tif (newfd < 0)\n-\t\t\t\tnewfd = hold_locked_index(&lock_file, 1);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-z\")) {\n-\t\t\tline_termination = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--stdin\")) {\n-\t\t\tif (i != argc - 1)\n-\t\t\t\tdie(\"--stdin must be at the end\");\n-\t\t\tread_from_stdin = 1;\n-\t\t\ti++; /* do not consider arg as a file name */\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--temp\")) {\n-\t\t\tto_tempfile = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--prefix=\")) {\n-\t\t\tstate.base_dir = arg+9;\n-\t\t\tstate.base_dir_len = strlen(state.base_dir);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--stage=\")) {\n-\t\t\tif (!strcmp(arg + 8, \"all\")) {\n-\t\t\t\tto_tempfile = 1;\n-\t\t\t\tcheckout_stage = CHECKOUT_ALL;\n-\t\t\t} else {\n-\t\t\t\tint ch = arg[8];\n-\t\t\t\tif ('1' <= ch && ch <= '3')\n-\t\t\t\t\tcheckout_stage = arg[8] - '0';\n-\t\t\t\telse\n-\t\t\t\t\tdie(\"stage should be between 1 and 3 or all\");\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (arg[0] == '-')\n-\t\t\tusage(checkout_cache_usage);\n-\t\tbreak;\n-\t}\n+\targc = parse_options(argc, argv, builtin_checkout_index_options,\n+\t\t\tbuiltin_checkout_index_usage, 0);\n+\tstate.force = force;\n+\tstate.quiet = quiet;\n+\tstate.not_new = not_new;\n+\tif (argc && read_from_stdin)\n+\t\tdie(\"--stdin must be at the end\");\n \n \tif (state.base_dir_len || to_tempfile) {\n \t\t/* when --prefix is specified we do not\n@@ -253,7 +266,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Check out named files first */\n-\tfor ( ; i < argc; i++) {\n+\tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tconst char *p;\n \n-- \n1.6.0.2\n"},{"id":"93323","messageId":"7v63nqg4f4.fsf@gitster.siamese.dyndns.org","threadId":"15923","inReplyTo":"20081016132810.GG536@genesis.frugalware.org","subject":"Re: [PATCH v2] parse-opt: migrate builtin-checkout-index.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-17T23:58:23Z","receivedAt":"2008-10-17T23:58:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> +static int option_parse_z(const struct option *opt,\n> +\t\t\t  const char *arg, int unset)\n> +{\n> +\tline_termination = unset;\n> +\treturn 0;\n> +}\n> ...\n> +\t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n> +\t\t\t\"paths are separated with NUL character\",\n> +\t\t\tPARSE_OPT_NOARG, option_parse_z },\n\nThis adds a new feature to say --no-z from the command line, doesn't it?\nAnd I suspect the feature is broken ;-).\n\n> +\t\tOPT_BOOLEAN(0, \"stdin\", &read_from_stdin,\n> +\t\t\t\"read list of paths from the standard input\"),\n> ...\n> +\targc = parse_options(argc, argv, builtin_checkout_index_options,\n> +\t\t\tbuiltin_checkout_index_usage, 0);\n> +\tstate.force = force;\n> +\tstate.quiet = quiet;\n> +\tstate.not_new = not_new;\n> +\tif (argc && read_from_stdin)\n> +\t\tdie(\"--stdin must be at the end\");\n\nIs this comment still correct?  Do the original and your version act the\nsame way when the user says \"checkout --stdin -f\", for example?  I suspect\nthe original refused it and yours take it (and do much more sensible\nthing), which would be an improvement, but then the error message should\nbe reworded perhaps?\n"},{"id":"93329","messageId":"1224292643-28704-1-git-send-email-vmiklos@frugalware.org","threadId":"15923","inReplyTo":"7v63nqg4f4.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] parse-opt: migrate builtin-checkout-index.","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-10-18T01:17:23Z","receivedAt":"2008-10-18T01:17:23Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-checkout-index.c |  152 +++++++++++++++++++++++++---------------------\n 1 files changed, 83 insertions(+), 69 deletions(-)\n\nOn Fri, Oct 17, 2008 at 04:58:23PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> > +           { OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n> > +                   \"paths are separated with NUL character\",\n> > +                   PARSE_OPT_NOARG, option_parse_z },\n>\n> This adds a new feature to say --no-z from the command line, doesn't\n> it?\n> And I suspect the feature is broken ;-).\n\nRight, I fixed this in option_parse_z(). --no-z should set\nline_termination to \\n instead of 1.\n\n> > +           OPT_BOOLEAN(0, \"stdin\", &read_from_stdin,\n> > +                   \"read list of paths from the standard input\"),\n> > ...\n> > +   argc = parse_options(argc, argv, builtin_checkout_index_options,\n> > +                   builtin_checkout_index_usage, 0);\n> > +   state.force = force;\n> > +   state.quiet = quiet;\n> > +   state.not_new = not_new;\n> > +   if (argc && read_from_stdin)\n> > +           die(\"--stdin must be at the end\");\n>\n> Is this comment still correct?  Do the original and your version act\n> the\n> same way when the user says \"checkout --stdin -f\", for example?  I\n> suspect\n> the original refused it and yours take it (and do much more sensible\n> thing), which would be an improvement, but then the error message\n> should\n> be reworded perhaps?\n\nUnless I missed something, that was a limitation of the option parser.\ncheckout-index --stdin -f works fine for me after removing those two\nlines, so I left them out from the updated patch.\n\ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex 4ba2702..0d534bc 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -40,6 +40,7 @@\n #include \"cache.h\"\n #include \"quote.h\"\n #include \"cache-tree.h\"\n+#include \"parse-options.h\"\n \n #define CHECKOUT_ALL 4\n static int line_termination = '\\n';\n@@ -153,11 +154,58 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\texit(128);\n }\n \n-static const char checkout_cache_usage[] =\n-\"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...\";\n+static const char * const builtin_checkout_index_usage[] = {\n+\t\"git checkout-index [options] [--] <file>...\",\n+\tNULL\n+};\n \n static struct lock_file lock_file;\n \n+static int option_parse_u(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tint *newfd = opt->value;\n+\n+\tstate.refresh_cache = 1;\n+\tif (*newfd < 0)\n+\t\t*newfd = hold_locked_index(&lock_file, 1);\n+\treturn 0;\n+}\n+\n+static int option_parse_z(const struct option *opt,\n+\t\t\t  const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\tline_termination = '\\n';\n+\telse\n+\t\tline_termination = 0;\n+\treturn 0;\n+}\n+\n+static int option_parse_prefix(const struct option *opt,\n+\t\t\t       const char *arg, int unset)\n+{\n+\tstate.base_dir = arg;\n+\tstate.base_dir_len = strlen(arg);\n+\treturn 0;\n+}\n+\n+static int option_parse_stage(const struct option *opt,\n+\t\t\t      const char *arg, int unset)\n+{\n+\tif (!strcmp(arg, \"all\")) {\n+\t\tto_tempfile = 1;\n+\t\tcheckout_stage = CHECKOUT_ALL;\n+\t} else {\n+\t\tint ch = arg[0];\n+\t\tif ('1' <= ch && ch <= '3')\n+\t\t\tcheckout_stage = arg[0] - '0';\n+\t\telse\n+\t\t\tdie(\"stage should be between 1 and 3 or all\");\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -165,6 +213,33 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \tint all = 0;\n \tint read_from_stdin = 0;\n \tint prefix_length;\n+\tint force = 0, quiet = 0, not_new = 0;\n+\tstruct option builtin_checkout_index_options[] = {\n+\t\tOPT_BOOLEAN('a', \"all\", &all,\n+\t\t\t\"checks out all files in the index\"),\n+\t\tOPT_BOOLEAN('f', \"force\", &force,\n+\t\t\t\"forces overwrite of existing files\"),\n+\t\tOPT__QUIET(&quiet),\n+\t\tOPT_BOOLEAN('n', \"no-create\", &not_new,\n+\t\t\t\"don't checkout new files\"),\n+\t\t{ OPTION_CALLBACK, 'u', \"index\", &newfd, NULL,\n+\t\t\t\"update stat information in the index file\",\n+\t\t\tPARSE_OPT_NOARG, option_parse_u },\n+\t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n+\t\t\t\"paths are separated with NUL character\",\n+\t\t\tPARSE_OPT_NOARG, option_parse_z },\n+\t\tOPT_BOOLEAN(0, \"stdin\", &read_from_stdin,\n+\t\t\t\"read list of paths from the standard input\"),\n+\t\tOPT_BOOLEAN(0, \"temp\", &to_tempfile,\n+\t\t\t\"write the content to temporary files\"),\n+\t\tOPT_CALLBACK(0, \"prefix\", NULL, \"string\",\n+\t\t\t\"when creating files, prepend <string>\",\n+\t\t\toption_parse_prefix),\n+\t\tOPT_CALLBACK(0, \"stage\", NULL, NULL,\n+\t\t\t\"copy out the files from named stage\",\n+\t\t\toption_parse_stage),\n+\t\tOPT_END()\n+\t};\n \n \tgit_config(git_default_config, NULL);\n \tstate.base_dir = \"\";\n@@ -174,72 +249,11 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\tdie(\"invalid cache\");\n \t}\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\n-\t\tif (!strcmp(arg, \"--\")) {\n-\t\t\ti++;\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-a\") || !strcmp(arg, \"--all\")) {\n-\t\t\tall = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-f\") || !strcmp(arg, \"--force\")) {\n-\t\t\tstate.force = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-q\") || !strcmp(arg, \"--quiet\")) {\n-\t\t\tstate.quiet = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-n\") || !strcmp(arg, \"--no-create\")) {\n-\t\t\tstate.not_new = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-u\") || !strcmp(arg, \"--index\")) {\n-\t\t\tstate.refresh_cache = 1;\n-\t\t\tif (newfd < 0)\n-\t\t\t\tnewfd = hold_locked_index(&lock_file, 1);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-z\")) {\n-\t\t\tline_termination = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--stdin\")) {\n-\t\t\tif (i != argc - 1)\n-\t\t\t\tdie(\"--stdin must be at the end\");\n-\t\t\tread_from_stdin = 1;\n-\t\t\ti++; /* do not consider arg as a file name */\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--temp\")) {\n-\t\t\tto_tempfile = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--prefix=\")) {\n-\t\t\tstate.base_dir = arg+9;\n-\t\t\tstate.base_dir_len = strlen(state.base_dir);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(arg, \"--stage=\")) {\n-\t\t\tif (!strcmp(arg + 8, \"all\")) {\n-\t\t\t\tto_tempfile = 1;\n-\t\t\t\tcheckout_stage = CHECKOUT_ALL;\n-\t\t\t} else {\n-\t\t\t\tint ch = arg[8];\n-\t\t\t\tif ('1' <= ch && ch <= '3')\n-\t\t\t\t\tcheckout_stage = arg[8] - '0';\n-\t\t\t\telse\n-\t\t\t\t\tdie(\"stage should be between 1 and 3 or all\");\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (arg[0] == '-')\n-\t\t\tusage(checkout_cache_usage);\n-\t\tbreak;\n-\t}\n+\targc = parse_options(argc, argv, builtin_checkout_index_options,\n+\t\t\tbuiltin_checkout_index_usage, 0);\n+\tstate.force = force;\n+\tstate.quiet = quiet;\n+\tstate.not_new = not_new;\n \n \tif (state.base_dir_len || to_tempfile) {\n \t\t/* when --prefix is specified we do not\n@@ -253,7 +267,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* Check out named files first */\n-\tfor ( ; i < argc; i++) {\n+\tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tconst char *p;\n \n-- \n1.6.0.2\n"},{"id":"93373","messageId":"20081018155436.GA4803@artemis","threadId":"15923","inReplyTo":"7v63nqg4f4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] parse-opt: migrate builtin-checkout-index.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-10-18T15:54:37Z","receivedAt":"2008-10-18T15:54:37Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Oct 17, 2008 at 11:58:23PM +0000, Junio C Hamano wrote:\n> Miklos Vajna <vmiklos@frugalware.org> writes:\n> \n> > +static int option_parse_z(const struct option *opt,\n> > +\t\t\t  const char *arg, int unset)\n> > +{\n> > +\tline_termination = unset;\n> > +\treturn 0;\n> > +}\n> > ...\n> > +\t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n> > +\t\t\t\"paths are separated with NUL character\",\n> > +\t\t\tPARSE_OPT_NOARG, option_parse_z },\n> \n> This adds a new feature to say --no-z from the command line, doesn't it?\n> And I suspect the feature is broken ;-).\n\nNo it doesn't, --no-foo is only active for long options.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"93465","messageId":"20081019192906.GA26073@rdrz.de","threadId":"15923","inReplyTo":"1224292643-28704-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] parse-opt: migrate builtin-checkout-index.","fromName":"Raphael Zimmerer","fromEmail":"killekulla@rdrz.de","sentAt":"2008-10-19T19:29:08Z","receivedAt":"2008-10-19T19:29:08Z","isPatch":true,"sender":{"key":"killekulla@rdrz.de","avatar":"https://avatars.githubusercontent.com/u/35472983?v=4"},"body":"On Sat, Oct 18, 2008 at 03:17:23AM +0200, Miklos Vajna wrote:\n> Right, I fixed this in option_parse_z(). --no-z should set\n> line_termination to \\n instead of 1.\n\nHow about \"--no-null\"?\n\nThe long option name for \"-z\" is \"--null\", as used in git-config and\ngit-grep. So I suggest to use that as the long option name for \"-z\",\nas the enhanced option parser automatically will recongnize\n\"--no-null\", when used. That helps avoid further confusion with git\noption names.\n\n - Raphael\n"},{"id":"93464","messageId":"20081019193427.GK16610@artemis.corp","threadId":"15923","inReplyTo":"20081019192906.GA26073@rdrz.de","subject":"Re: [PATCH] parse-opt: migrate builtin-checkout-index.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-10-19T19:34:27Z","receivedAt":"2008-10-19T19:34:27Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Oct 19, 2008 at 07:29:08PM +0000, Raphael Zimmerer wrote:\n> On Sat, Oct 18, 2008 at 03:17:23AM +0200, Miklos Vajna wrote:\n> > Right, I fixed this in option_parse_z(). --no-z should set\n> > line_termination to \\n instead of 1.\n> \n> How about \"--no-null\"?\n> \n> The long option name for \"-z\" is \"--null\", as used in git-config and\n> git-grep. So I suggest to use that as the long option name for \"-z\",\n> as the enhanced option parser automatically will recongnize\n> \"--no-null\", when used. That helps avoid further confusion with git\n> option names.\n\ngit checkout-index has no --null option. If you make --null be the long\nform for -z then --no-null will be \"autogenerated\".\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"93451","messageId":"7vr66c5kef.fsf@gitster.siamese.dyndns.org","threadId":"15923","inReplyTo":"1224292643-28704-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] parse-opt: migrate builtin-checkout-index.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-19T21:45:28Z","receivedAt":"2008-10-19T21:45:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n>> > +   if (argc && read_from_stdin)\n>> > +           die(\"--stdin must be at the end\");\n>>\n>> Is this comment still correct?  Do the original and your version act\n>> the\n>> same way when the user says \"checkout --stdin -f\", for example?  I\n>> suspect\n>> the original refused it and yours take it (and do much more sensible\n>> thing), which would be an improvement, but then the error message\n>> should\n>> be reworded perhaps?\n>\n> Unless I missed something, that was a limitation of the option parser.\n> checkout-index --stdin -f works fine for me after removing those two\n> lines, so I left them out from the updated patch.\n\nThanks.  I think you got what I meant and dropping the part is right.\n\n\"--stdin -f\" was rejected by the original code, and you improved to take\nit with the new parser.  In fact, the above quoted if() statement should\nnot trigger when \"--stdin -f\" is given, due to the way the new option\nparser is structured.  The original had an explicit \"break\" in the loop\nwhen it saw \"--stdin\".  The above would still trigger if \"--stdin foo\" is\ngiven, but there is a code to catch that already, so it is not necessary.\n"}]}