{"thread":{"id":"52208","subject":"[PATCH 2/6] [Outreachy] check-ref-format: parse-options","startedAt":"2019-11-07T08:10:08Z","lastAt":"2019-11-09T07:42:37Z","messageCount":11,"participants":["george espinoza via GitGitGadget","George Espinoza via GitGitGadget","Junio C Hamano","George Espinoza"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"385680","messageId":"e8bca0910e7ba7582a50115eeeb66406d965a52a.1573114201.git.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":"pull.449.git.1573114201.gitgitgadget@gmail.com","subject":"[PATCH 2/6] [Outreachy] check-ref-format: parse-options","fromName":"george espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:09:56Z","receivedAt":"2019-11-07T08:10:08Z","isPatch":true,"sender":{"key":"name:george espinoza","avatar":null},"body":"From: george espinoza \n\nThis command currently handles its own argv so by teaching it to\nuse parse-options instead we can standardize the way commands\nhandle user input across the project.\n\nAs a consequence of using OPT_BOOL data structure on --normalize &\n--refspec-pattern, --no-normalize & --no-refspec-pattern has been\ncan now be used.\n\nNO_PARSEOPT flag was also removed to update git.c with the\nconversion of the structure in this command.\n\nThis is a rough draft and I need some advice if I'm doing this\ncorrectly since its being built but it is failing tests.\n\nHelped by: emily shaffer \nHelped by: johannes schindelin \n\nSigned-off-by: george espinoza \n---\n builtin/check-ref-format.c | 49 +++++++++++++++++++-------------------\n git.c | 2 +-\n 2 files changed, 26 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex bc67d3f0a8..3fe0b5410a 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -6,10 +6,13 @@\n #include \"refs.h\"\n #include \"builtin.h\"\n #include \"strbuf.h\"\n+#include \"parse-options.h\"\n \n-static const char builtin_check_ref_format_usage[] =\n-\"git check-ref-format [--normalize] [] \\n\"\n-\" or: git check-ref-format --branch \";\n+static const char * const builtin_check_ref_format_usage[] = {\n+\tN_(\"git check-ref-format [--normalize] [] \\n\"),\n+\tN_(\" or: git check-ref-format --branch \"),\n+\tNULL,\n+};\n \n /*\n * Return a copy of refname but with leading slashes removed and runs\n@@ -53,31 +56,29 @@ static int check_ref_format_branch(const char *arg)\n \n int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n {\n-\tint i;\n-\tint normalize = 0;\n+\tenum {\n+\t\tCHECK_REF_FORMAT_BRANCH,\n+\t};\n+\n+\tint i = 0;\n+\tint verbose;\n+\tint normalize;\n+\tint allow_onelevel;\n+\tint refspec_pattern;\n \tint flags = 0;\n \tconst char *refname;\n \n-\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n-\t\tusage(builtin_check_ref_format_usage);\n-\n-\tif (argc == 3 && !strcmp(argv[1], \"--branch\"))\n-\t\treturn check_ref_format_branch(argv[2]);\n+\tstruct option options[] = {\n+\t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n+\t\tOPT_GROUP(\"\"),\n+\t\tOPT_CMDMODE( 0 , \"branch\", &check_ref_format_branch, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n+\t\tOPT_BOOL( 0 , \"normalize\", &normalize, N_(\"normalize tracked files\")),\n+\t\tOPT_BOOL( 0 , \"allow-onelevel\", &allow_onelevel, N_(\"allow one level\")),\n+\t\tOPT_BOOL( 0 , \"refspec-pattern\", &refspec_pattern, N_(\"refspec pattern\")),\n+\t\tOPT_END(),\n+\t};\n \n-\tfor (i = 1; i < argc && argv[i][0] == '-'; i++) {\n-\t\tif (!strcmp(argv[i], \"--normalize\") || !strcmp(argv[i], \"--print\"))\n-\t\t\tnormalize = 1;\n-\t\telse if (!strcmp(argv[i], \"--allow-onelevel\"))\n-\t\t\tflags |= REFNAME_ALLOW_ONELEVEL;\n-\t\telse if (!strcmp(argv[i], \"--no-allow-onelevel\"))\n-\t\t\tflags &= ~REFNAME_ALLOW_ONELEVEL;\n-\t\telse if (!strcmp(argv[i], \"--refspec-pattern\"))\n-\t\t\tflags |= REFNAME_REFSPEC_PATTERN;\n-\t\telse\n-\t\t\tusage(builtin_check_ref_format_usage);\n-\t}\n-\tif (! (i == argc - 1))\n-\t\tusage(builtin_check_ref_format_usage);\n+\targc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, PARSE_OPT_KEEP_ARGV0);\n \n \trefname = argv[i];\n \tif (normalize)\ndiff --git a/git.c b/git.c\nindex ce6ab0ece2..13dbfde31c 100644\n--- a/git.c\n+++ b/git.c\n@@ -478,7 +478,7 @@ static struct cmd_struct commands[] = {\n \t{ \"check-attr\", cmd_check_attr, RUN_SETUP },\n \t{ \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"check-mailmap\", cmd_check_mailmap, RUN_SETUP },\n-\t{ \"check-ref-format\", cmd_check_ref_format, NO_PARSEOPT },\n+\t{ \"check-ref-format\", cmd_check_ref_format },\n \t{ \"checkout\", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"checkout-index\", cmd_checkout_index,\n \t\tRUN_SETUP | NEED_WORK_TREE},\n-- \ngitgitgadget\n\n"},{"id":"385681","messageId":"5e3b370b3bf8f34cd975da41fb2a7cc0f633954a.1573114201.git.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":"pull.449.git.1573114201.gitgitgadget@gmail.com","subject":"[PATCH 1/6] [Outreachy] merge-ours: include parse-option.h","fromName":"george espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:09:55Z","receivedAt":"2019-11-07T08:10:08Z","isPatch":true,"sender":{"key":"name:george espinoza","avatar":null},"body":"From: george espinoza \n\nPrepare this command which currently handles its own argv to use\nparse-options instead. A lot of the commands already have\nparse-options and in git.c cmd_struct with the \"NO_PARSEOPT\" are\nthe one's that still need it.\n\nSigned-off-by: george espinoza \n---\n builtin/merge-ours.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/merge-ours.c b/builtin/merge-ours.c\nindex 4594507420..3980f4899a 100644\n--- a/builtin/merge-ours.c\n+++ b/builtin/merge-ours.c\n@@ -11,6 +11,11 @@\n #include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"diff.h\"\n+#include \"parse-options.h\"\n+\n+/* parse-options.h added to initiate replacement of manual option parsing\n+ * with parse-options. \n+ */\n \n static const char builtin_merge_ours_usage[] =\n \t\"git merge-ours ... -- HEAD ...\";\n-- \ngitgitgadget\n\n"},{"id":"385682","messageId":"aeff5d9908e80f8534144529bc5cedd6dd190021.1573114201.git.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":"pull.449.git.1573114201.gitgitgadget@gmail.com","subject":"[PATCH 5/6] [Outreachy] check-ref-format: parse-options","fromName":"george espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:09:59Z","receivedAt":"2019-11-07T08:10:09Z","isPatch":true,"sender":{"key":"name:george espinoza","avatar":null},"body":"From: george espinoza \n\nThis command currently handles its own argv so by teaching it to\nuse parse-options instead we can standardize the way commands\nhandle user input across the project.\n\nBecause OPT_BOOL data structure is being used on --normalize\n--no-normalize can now be utilized.\n\nNO_PARSEOPT flag was also removed to update git.c with the\nconversion of the structure in this command.\n\nI got all tests to pass at the moment. My next commit will be to\ncheck if OPT_NEGBIT isn't needed and moving around OPT_VERBOSE.\n\nHelped by: emily shaffer emilyshaffer@google.com\nHelped by: johannes schindelin johannes.schindelin@gmx.de\n\nSigned-off-by: george espinoza \n---\n builtin/check-ref-format.c | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex c48fb19eda..be3b1bd84d 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -57,10 +57,10 @@ static int check_ref_format_branch(const char *arg)\n int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n+\t\tCHECK_REFNAME_FORMAT = 0,\n \t\tCHECK_REF_FORMAT_BRANCH,\n-\t};\n+\t} mode = CHECK_REFNAME_FORMAT;\n \n-\tint i = 0;\n \tint verbose = 0;\n \tint normalize = 0;\n \tint flags = 0;\n@@ -69,17 +69,19 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n \t\tOPT_GROUP(\"\"),\n-\t\tOPT_CMDMODE( 0 , \"branch\", &check_ref_format_branch, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n-\t\tOPT_BOOL( 0 , \"normalize\", &normalize, N_(\"normalize tracked files\")),\n-\t\tOPT_BIT( 0 , \"allow-onelevel\", &flags, N_(\"allow one level\"), REFNAME_ALLOW_ONELEVEL),\n-\t\tOPT_NEGBIT( 0, \"no-allow-onelevel\", &flags, N_(\"no allow one level\"), REFNAME_ALLOW_ONELEVEL),\n-\t\tOPT_BIT( 0 , \"refspec-pattern\", &flags, N_(\"refspec pattern\"), REFNAME_REFSPEC_PATTERN),\n+\t\tOPT_CMDMODE(0 , \"branch\", &mode, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n+\t\tOPT_BOOL(0 , \"normalize\", &normalize, N_(\"normalize tracked files\")),\n+\t\tOPT_BIT(0 , \"allow-onelevel\", &flags, N_(\"allow one level\"), REFNAME_ALLOW_ONELEVEL),\n+\t\tOPT_NEGBIT(0, \"no-allow-onelevel\", &flags, N_(\"no allow one level\"), REFNAME_ALLOW_ONELEVEL),\n+\t\tOPT_BIT(0 , \"refspec-pattern\", &flags, N_(\"refspec pattern\"), REFNAME_REFSPEC_PATTERN),\n \t\tOPT_END(),\n \t};\n \n \targc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, 0);\n \n-\trefname = argv[i];\n+\trefname = argv[0];\n+\tif (mode)\n+\t\treturn check_ref_format_branch(argv[2]);\n \tif (normalize)\n \t\trefname = collapse_slashes(refname);\n \tif (check_refname_format(refname, flags))\n-- \ngitgitgadget\n\n"},{"id":"385683","messageId":"d224564bc2969c1d95351a5b55b296e399f5b8c8.1573114201.git.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":"pull.449.git.1573114201.gitgitgadget@gmail.com","subject":"[PATCH 6/6] [Outreachy] check-ref-format: parse-options","fromName":"george espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:10:00Z","receivedAt":"2019-11-07T08:10:10Z","isPatch":true,"sender":{"key":"name:george espinoza","avatar":null},"body":"From: george espinoza \n\nThis command currently handles its own argv so by teaching it to\nuse parse-options instead we can standardize the way commands\nhandle user input across the project.\n\nBecause OPT_BOOL data structure is being used on --normalize\n--no-normalize can now be utilized.\n\nNO_PARSEOPT flag was also removed to update git.c with the\nconversion of the structure in this command.\n\nHelped by: emily shaffer emilyshaffer@google.com\nHelped by: johannes schindelin johannes.schindelin@gmx.de\n\nSigned-off-by: george espinoza \n---\n builtin/check-ref-format.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex be3b1bd84d..ea703cfabd 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -61,18 +61,15 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t\tCHECK_REF_FORMAT_BRANCH,\n \t} mode = CHECK_REFNAME_FORMAT;\n \n-\tint verbose = 0;\n \tint normalize = 0;\n \tint flags = 0;\n \tconst char *refname;\n \n \tstruct option options[] = {\n-\t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n \t\tOPT_GROUP(\"\"),\n-\t\tOPT_CMDMODE(0 , \"branch\", &mode, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n+\t\tOPT_CMDMODE(0 , \"branch\", &mode, N_(\"check for valid branch name\"), CHECK_REF_FORMAT_BRANCH),\n \t\tOPT_BOOL(0 , \"normalize\", &normalize, N_(\"normalize tracked files\")),\n \t\tOPT_BIT(0 , \"allow-onelevel\", &flags, N_(\"allow one level\"), REFNAME_ALLOW_ONELEVEL),\n-\t\tOPT_NEGBIT(0, \"no-allow-onelevel\", &flags, N_(\"no allow one level\"), REFNAME_ALLOW_ONELEVEL),\n \t\tOPT_BIT(0 , \"refspec-pattern\", &flags, N_(\"refspec pattern\"), REFNAME_REFSPEC_PATTERN),\n \t\tOPT_END(),\n \t};\n@@ -81,7 +78,7 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \n \trefname = argv[0];\n \tif (mode)\n-\t\treturn check_ref_format_branch(argv[2]);\n+\t\treturn check_ref_format_branch(argv[2]);\n \tif (normalize)\n \t\trefname = collapse_slashes(refname);\n \tif (check_refname_format(refname, flags))\n-- \ngitgitgadget\n"},{"id":"385684","messageId":"078a19e86af6fcf1348d8988dc10cfac2e689ba4.1573114201.git.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":"pull.449.git.1573114201.gitgitgadget@gmail.com","subject":"[PATCH 3/6] This file wasn't supposed to change during my git push for check-ref-format :(","fromName":"george espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:09:57Z","receivedAt":"2019-11-07T08:10:12Z","isPatch":true,"sender":{"key":"name:george espinoza","avatar":null},"body":"From: george espinoza \n\nSigned-off-by: george espinoza \n---\n builtin/merge-ours.c | 5 -----\n 1 file changed, 5 deletions(-)\n\ndiff --git a/builtin/merge-ours.c b/builtin/merge-ours.c\nindex 3980f4899a..4594507420 100644\n--- a/builtin/merge-ours.c\n+++ b/builtin/merge-ours.c\n@@ -11,11 +11,6 @@\n #include \"git-compat-util.h\"\n #include \"builtin.h\"\n #include \"diff.h\"\n-#include \"parse-options.h\"\n-\n-/* parse-options.h added to initiate replacement of manual option parsing\n- * with parse-options. \n- */\n \n static const char builtin_merge_ours_usage[] =\n \t\"git merge-ours ... -- HEAD ...\";\n-- \ngitgitgadget\n\n"},{"id":"385685","messageId":"pull.449.git.1573114201.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":null,"subject":"[PATCH 0/6] [Outreachy] check-ref-format: parse-options","fromName":"George Espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:09:54Z","receivedAt":"2019-11-07T08:10:12Z","isPatch":true,"sender":{"key":"gespinoz2019@gmail.com","avatar":null},"body":"This command currently handles its own argv so by teaching it to use\nparse-options instead we can standardize the way commands handle user input\nacross the project.\n\nBecause OPT_BOOL data structure is being used on --normalize --no-normalize\ncan now be utilized.\n\nNO_PARSEOPT flag was also removed to update git.c with the conversion of the\nstructure in this command.\n\nThis is Micro-Project #3 that states: Teach a command which currently\nhandles its own argv how to use parse-options instead..\n\nHelped by: emily shaffer emilyshaffer@google.com [emilyshaffer@google.com]\nHelped by: johannes schindelin johannes.schindelin@gmx.de\n[johannes.schindelin@gmx.de]\n\nSigned-off-by: george espinoza gespinoz2019@gmail.com\n[gespinoz2019@gmail.com]\n\ngeorge espinoza (6):\n [Outreachy] merge-ours: include parse-option.h\n [Outreachy] check-ref-format: parse-options\n This file wasn't supposed to change during my git push for\n check-ref-format :(\n [Outreachy] check-ref-format: parse options\n [Outreachy] check-ref-format: parse-options\n [Outreachy] check-ref-format: parse-options\n\n builtin/check-ref-format.c | 47 +++++++++++++++++++-------------------\n git.c | 2 +-\n 2 files changed, 24 insertions(+), 25 deletions(-)\n\n\nbase-commit: efd54442381a2792186abc994060b8f7dd8b834b\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-449%2FGeorgecanfly%2Fcheck-ref-format-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-449/Georgecanfly/check-ref-format-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/449\n-- \ngitgitgadget\n"},{"id":"385686","messageId":"0a1c8e2dea25d2e485cbf8350b08953ed33e96b7.1573114201.git.gitgitgadget@gmail.com","threadId":"52208","inReplyTo":"pull.449.git.1573114201.gitgitgadget@gmail.com","subject":"[PATCH 4/6] [Outreachy] check-ref-format: parse options","fromName":"george espinoza via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T08:09:58Z","receivedAt":"2019-11-07T08:10:14Z","isPatch":true,"sender":{"key":"name:george espinoza","avatar":null},"body":"From: george espinoza \n\nThis command currently handles its own argv so by teaching it to\nuse parse-options instead we can standardize the way commands\nhandle user input across the project.\n\nAs a consequence of using OPT_BOOL data structure on --normalize &\n--refspec-pattern, --no-normalize & --no-refspec-pattern can now be\nused.\n\nNO_PARSEOPT flag was also removed to update git.c with the\nconversion of the structure in this command.\n\nThis is a rough draft and I need some advice if I'm doing this\ncorrectly since its being built but it is failing tests.\n\nHelped by: emily shaffer emilyshaffer@google.com\nHelped by: johannes schindelin johannes.schindelin@gmx.de\n\nSigned-off-by: george espinoza \n---\n builtin/check-ref-format.c | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex 3fe0b5410a..c48fb19eda 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -61,10 +61,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t};\n \n \tint i = 0;\n-\tint verbose;\n-\tint normalize;\n-\tint allow_onelevel;\n-\tint refspec_pattern;\n+\tint verbose = 0;\n+\tint normalize = 0;\n \tint flags = 0;\n \tconst char *refname;\n \n@@ -73,12 +71,13 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_CMDMODE( 0 , \"branch\", &check_ref_format_branch, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n \t\tOPT_BOOL( 0 , \"normalize\", &normalize, N_(\"normalize tracked files\")),\n-\t\tOPT_BOOL( 0 , \"allow-onelevel\", &allow_onelevel, N_(\"allow one level\")),\n-\t\tOPT_BOOL( 0 , \"refspec-pattern\", &refspec_pattern, N_(\"refspec pattern\")),\n+\t\tOPT_BIT( 0 , \"allow-onelevel\", &flags, N_(\"allow one level\"), REFNAME_ALLOW_ONELEVEL),\n+\t\tOPT_NEGBIT( 0, \"no-allow-onelevel\", &flags, N_(\"no allow one level\"), REFNAME_ALLOW_ONELEVEL),\n+\t\tOPT_BIT( 0 , \"refspec-pattern\", &flags, N_(\"refspec pattern\"), REFNAME_REFSPEC_PATTERN),\n \t\tOPT_END(),\n \t};\n \n-\targc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, PARSE_OPT_KEEP_ARGV0);\n+\targc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, 0);\n \n \trefname = argv[i];\n \tif (normalize)\n-- \ngitgitgadget\n\n"},{"id":"385689","messageId":"xmqqy2wsovrm.fsf@gitster-ct.c.googlers.com","threadId":"52208","inReplyTo":"5e3b370b3bf8f34cd975da41fb2a7cc0f633954a.1573114201.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/6] [Outreachy] merge-ours: include parse-option.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T09:55:57Z","receivedAt":"2019-11-07T09:56:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"george espinoza via GitGitGadget\" writes:\n\n> From: george espinoza \n>\n> Prepare this command which currently handles its own argv to use\n> parse-options instead. A lot of the commands already have\n> parse-options and in git.c cmd_struct with the \"NO_PARSEOPT\" are\n> the one's that still need it.\n>\n> Signed-off-by: george espinoza \n> ---\n> builtin/merge-ours.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c\n> index 4594507420..3980f4899a 100644\n> --- a/builtin/merge-ours.c\n> +++ b/builtin/merge-ours.c\n> @@ -11,6 +11,11 @@\n> #include \"git-compat-util.h\"\n> #include \"builtin.h\"\n> #include \"diff.h\"\n> +#include \"parse-options.h\"\n> +\n> +/* parse-options.h added to initiate replacement of manual option parsing\n> + * with parse-options. \n> + */\n\nSee Documentation/CodingGuidelines and learn how we write multi-line\ncomments.\n\nSince there is no \"manual option parsing\" to be replaced, there is\nno other change to this file, I guess (so what's the point of this\nstep?).\n\n\n"},{"id":"385691","messageId":"xmqqo8xoouht.fsf@gitster-ct.c.googlers.com","threadId":"52208","inReplyTo":"078a19e86af6fcf1348d8988dc10cfac2e689ba4.1573114201.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/6] This file wasn't supposed to change during my git push for check-ref-format :(","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T10:23:26Z","receivedAt":"2019-11-07T10:23:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"george espinoza via GitGitGadget\" writes:\n\n> From: george espinoza \n>\n> Signed-off-by: george espinoza \n> ---\n\nWhile developing your first patch, it is expected that you make\nmistake in an earlier step (e.g. 2/6) and then later realize it and\nwant to fix, like this one. Instead of going back to the earlier\nmistake and fixing it right there before building more on top,\ntentatively fixing with \"oops that was wrong\" like this one and\nbuilding further (i.e. 4 and later steps) is an expedient way.\n\nBut once you are done, you need to get your act together before\npresenting the resulting series to the general public.\n\nPlease learn to use \"rebase -i\" to clean up the series before\nasking GGG to publish it to the list.\n\nThanks.\n\n> builtin/merge-ours.c | 5 -----\n> 1 file changed, 5 deletions(-)\n>\n> diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c\n> index 3980f4899a..4594507420 100644\n> --- a/builtin/merge-ours.c\n> +++ b/builtin/merge-ours.c\n> @@ -11,11 +11,6 @@\n> #include \"git-compat-util.h\"\n> #include \"builtin.h\"\n> #include \"diff.h\"\n> -#include \"parse-options.h\"\n> -\n> -/* parse-options.h added to initiate replacement of manual option parsing\n> - * with parse-options. \n> - */\n> \n> static const char builtin_merge_ours_usage[] =\n> \t\"git merge-ours ... -- HEAD ...\";\n"},{"id":"385692","messageId":"xmqqmud8ouf2.fsf@gitster-ct.c.googlers.com","threadId":"52208","inReplyTo":"e8bca0910e7ba7582a50115eeeb66406d965a52a.1573114201.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/6] [Outreachy] check-ref-format: parse-options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T10:25:05Z","receivedAt":"2019-11-07T10:25:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"george espinoza via GitGitGadget\" writes:\n\n> From: george espinoza \n>\n> This command currently handles its own argv so by teaching it to\n> use parse-options instead we can standardize the way commands\n> handle user input across the project.\n>\n> As a consequence of using OPT_BOOL data structure on --normalize &\n> --refspec-pattern, --no-normalize & --no-refspec-pattern has been\n> can now be used.\n>\n> NO_PARSEOPT flag was also removed to update git.c with the\n> conversion of the structure in this command.\n>\n> This is a rough draft and I need some advice if I'm doing this\n> correctly since its being built but it is failing tests.\n>\n> Helped by: emily shaffer \n> Helped by: johannes schindelin \n\nI do not think they spell their name like the above. In general,\nmost of us do not spell our names in all lowercase around here. I\nappreciate people with originality, but I'd rather see them to be\noriginal not in how they spell their names but in more productive\nways ;-)\n\n> Signed-off-by: george espinoza \n> ---\n> builtin/check-ref-format.c | 49 +++++++++++++++++++-------------------\n> git.c | 2 +-\n> 2 files changed, 26 insertions(+), 25 deletions(-)\n>\n> diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\n> index bc67d3f0a8..3fe0b5410a 100644\n> --- a/builtin/check-ref-format.c\n> +++ b/builtin/check-ref-format.c\n> @@ -6,10 +6,13 @@\n> #include \"refs.h\"\n> #include \"builtin.h\"\n> #include \"strbuf.h\"\n> +#include \"parse-options.h\"\n> \n> -static const char builtin_check_ref_format_usage[] =\n> -\"git check-ref-format [--normalize] [] \\n\"\n> -\" or: git check-ref-format --branch \";\n> +static const char * const builtin_check_ref_format_usage[] = {\n> +\tN_(\"git check-ref-format [--normalize] [] \\n\"),\n> +\tN_(\" or: git check-ref-format --branch \"),\n> +\tNULL,\n> +};\n\nOK. This is the bog-standard prep for calling usage_with_options().\n\n> @@ -53,31 +56,29 @@ static int check_ref_format_branch(const char *arg)\n> \n> int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n> {\n> -\tint i;\n> -\tint normalize = 0;\n> +\tenum {\n> +\t\tCHECK_REF_FORMAT_BRANCH,\n> +\t};\n> +\n> +\tint i = 0;\n> +\tint verbose;\n> +\tint normalize;\n> +\tint allow_onelevel;\n> +\tint refspec_pattern;\n> \tint flags = 0;\n> \tconst char *refname;\n\nDiscard the blank line before \"int i = 0\" line, and keep the blank\nline you have here between the last declaration and the first\nstatement.\n\n> -\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n> -\t\tusage(builtin_check_ref_format_usage);\n> -\n> -\tif (argc == 3 && !strcmp(argv[1], \"--branch\"))\n> -\t\treturn check_ref_format_branch(argv[2]);\n> +\tstruct option options[] = {\n> +\t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n> +\t\tOPT_GROUP(\"\"),\n> +\t\tOPT_CMDMODE( 0 , \"branch\", &check_ref_format_branch, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n\nThis is an iffy/tricky way to use CMDMODE. The way CMDMODE is\ndesigned to be used is to have multiple ones that sets the same\ntarget variable so that parse_options() can notice conflicting\ncommand line request that gives more than one from the same set.\n\nThe command has two modes (i.e. the \"--branch\" mode and the unnamed\nmode), so it is understandable that there is only one CMDMODE in the\noptions[] array, but I am not sure how well it would work for a\ncommand like this. For example, \"check-ref-format --branch\n--normalize --allow-onelevel $v\" should error out because --branch\nis not compatible with any other options. I do not think a single\nparse_options() call with a single options[] array can express that\nkind of constraints.\n\nBesides, shouldn't the third parameter of OPT_CMDMODE supposed to be\nthe address of the variable that would receive the value in its fifth\nparameter? I do not see a decl for check_ref_format_branch variable\n(isn't that the name of the function???).\n\nAhh, you said it builds but does not pass test. Of course, that is\nbecause this part is completely bogus.\n\nIt appears that to your series the only thing that matters is the\nshape of the tree after applying all of the patches, and individual\nsteps are not ready to be reviewd, so I'd stop here.\n\n"},{"id":"385828","messageId":"CAP_ANin4=ES8PRo=dnPTzyq+BPwqfGGwBkWJzSndfHUk+J1ASg@mail.gmail.com","threadId":"52208","inReplyTo":"xmqqmud8ouf2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/6] [Outreachy] check-ref-format: parse-options","fromName":"George Espinoza","fromEmail":"gespinoz2019@gmail.com","sentAt":"2019-11-09T07:42:21Z","receivedAt":"2019-11-09T07:42:37Z","isPatch":true,"sender":{"key":"gespinoz2019@gmail.com","avatar":null},"body":"On Thu, Nov 7, 2019 at 2:25 AM Junio C Hamano wrote:\n>\n> \"george espinoza via GitGitGadget\" writes:\n>\n> > From: george espinoza \n> >\n> > This command currently handles its own argv so by teaching it to\n> > use parse-options instead we can standardize the way commands\n> > handle user input across the project.\n> >\n> > As a consequence of using OPT_BOOL data structure on --normalize &\n> > --refspec-pattern, --no-normalize & --no-refspec-pattern has been\n> > can now be used.\n> >\n> > NO_PARSEOPT flag was also removed to update git.c with the\n> > conversion of the structure in this command.\n> >\n> > This is a rough draft and I need some advice if I'm doing this\n> > correctly since its being built but it is failing tests.\n> >\n> > Helped by: emily shaffer \n> > Helped by: johannes schindelin \n>\n> I do not think they spell their name like the above. In general,\n> most of us do not spell our names in all lowercase around here. I\n> appreciate people with originality, but I'd rather see them to be\n> original not in how they spell their names but in more productive\n> ways ;-)\n>\n\nAh, I see. I will use capital letters.\n> > Signed-off-by: george espinoza \n> > ---\n> > builtin/check-ref-format.c | 49 +++++++++++++++++++-------------------\n> > git.c | 2 +-\n> > 2 files changed, 26 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\n> > index bc67d3f0a8..3fe0b5410a 100644\n> > --- a/builtin/check-ref-format.c\n> > +++ b/builtin/check-ref-format.c\n> > @@ -6,10 +6,13 @@\n> > #include \"refs.h\"\n> > #include \"builtin.h\"\n> > #include \"strbuf.h\"\n> > +#include \"parse-options.h\"\n> >\n> > -static const char builtin_check_ref_format_usage[] =\n> > -\"git check-ref-format [--normalize] [] \\n\"\n> > -\" or: git check-ref-format --branch \";\n> > +static const char * const builtin_check_ref_format_usage[] = {\n> > + N_(\"git check-ref-format [--normalize] [] \\n\"),\n> > + N_(\" or: git check-ref-format --branch \"),\n> > + NULL,\n> > +};\n>\n> OK. This is the bog-standard prep for calling usage_with_options().\n\nI see, I will look into doing something more here as necessary.\n>\n> > @@ -53,31 +56,29 @@ static int check_ref_format_branch(const char *arg)\n> >\n> > int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n> > {\n> > - int i;\n> > - int normalize = 0;\n> > + enum {\n> > + CHECK_REF_FORMAT_BRANCH,\n> > + };\n> > +\n> > + int i = 0;\n> > + int verbose;\n> > + int normalize;\n> > + int allow_onelevel;\n> > + int refspec_pattern;\n> > int flags = 0;\n> > const char *refname;\n>\n> Discard the blank line before \"int i = 0\" line, and keep the blank\n> line you have here between the last declaration and the first\n> statement.\n\nI meant to squash all my earlier commits but had an issue with git rebase -i.\nIt only says noop. I had an issue when I first started my other branch\nand might of reset it. I'll look into fixing this before submitting my\nnext patch.\n>\n> > - if (argc == 2 && !strcmp(argv[1], \"-h\"))\n> > - usage(builtin_check_ref_format_usage);\n> > -\n> > - if (argc == 3 && !strcmp(argv[1], \"--branch\"))\n> > - return check_ref_format_branch(argv[2]);\n> > + struct option options[] = {\n> > + OPT__VERBOSE(&verbose, N_(\"be verbose\")),\n> > + OPT_GROUP(\"\"),\n> > + OPT_CMDMODE( 0 , \"branch\", &check_ref_format_branch, N_(\"branch\"), CHECK_REF_FORMAT_BRANCH),\n>\n> This is an iffy/tricky way to use CMDMODE. The way CMDMODE is\n> designed to be used is to have multiple ones that sets the same\n> target variable so that parse_options() can notice conflicting\n> command line request that gives more than one from the same set.\n>\n> The command has two modes (i.e. the \"--branch\" mode and the unnamed\n> mode), so it is understandable that there is only one CMDMODE in the\n> options[] array, but I am not sure how well it would work for a\n> command like this. For example, \"check-ref-format --branch\n> --normalize --allow-onelevel $v\" should error out because --branch\n> is not compatible with any other options. I do not think a single\n> parse_options() call with a single options[] array can express that\n> kind of constraints.\n\nOk I will look into another data structure to use for branch.\n>\n> Besides, shouldn't the third parameter of OPT_CMDMODE supposed to be\n> the address of the variable that would receive the value in its fifth\n> parameter? I do not see a decl for check_ref_format_branch variable\n> (isn't that the name of the function???).\n>\n> Ahh, you said it builds but does not pass test. Of course, that is\n> because this part is completely bogus.\n>\n> It appears that to your series the only thing that matters is the\n> shape of the tree after applying all of the patches, and individual\n> steps are not ready to be reviewd, so I'd stop here.\n>\n\nI had only intended /submit to show the last commit I had made that had passed\nall the tests. I will review everything and only submit again once im 100%\nsure everything is in order. Sorry Junio!\n"}]}