Skip to contentSkip to conversation

check-ref-format: parse-options

PATCH In review George Espinoza via GitGitGadget 3 participants 11 messages Nov 7, 2019 – Nov 9, 2019

lore
Share & tools
lore
#0 lore raw

cover check-ref-format: parse-options

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

Because OPT_BOOL data structure is being used on --normalize --no-normalize can now be utilized.

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

This is Micro-Project #3 that states: Teach a command which currently handles its own argv how to use parse-options instead..

Helped by: emily shaffer [email protected] [[email protected]] Helped by: johannes schindelin [email protected] [[email protected]]

george espinoza (6):
[Outreachy] merge-ours: include parse-option.h [Outreachy] check-ref-format: parse-options
This file wasn't supposed to change during my git push for

    check-ref-format :(

[Outreachy] check-ref-format: parse options
[Outreachy] check-ref-format: parse-options
[Outreachy] check-ref-format: parse-options

 builtin/check-ref-format.c | 47 +++++++++++++++++++-------------------
 git.c                      |  2 +-
 2 files changed, 24 insertions(+), 25 deletions(-)

base-commit: efd54442381a2792186abc994060b8f7dd8b834b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-449%2FGeorgecanfly%2Fcheck-ref-format-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-449/Georgecanfly/check-ref-format-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/449

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/6 merge-ours: include parse-option.h

From: george espinoza <[email protected]>

Prepare this command which currently handles its own argv to use parse-options instead. A lot of the commands already have parse-options and in git.c cmd_struct with the "NO_PARSEOPT" are the one's that still need it.

---
 builtin/merge-ours.c | 5 +++++
 1 file changed, 5 insertions(+)
builtin/merge-ours.c +5 -0
index 4594507420..3980f4899a 100644
@@ -11,6 +11,11 @@
 #include "git-compat-util.h"
 #include "builtin.h"
 #include "diff.h"
+#include "parse-options.h"
+
+/* parse-options.h added to initiate replacement of manual option parsing
+ * with parse-options. 
+ */
 
 static const char builtin_merge_ours_usage[] =
 	"git merge-ours <base>... -- HEAD <remote>...";
Signature · 1 line
gitgitgadget
#2 lore raw

Re: [PATCH 1/6] [Outreachy] merge-ours: include parse-option.h

Quoted 17 lines · george espinoza via GitGitGadget

From: george espinoza <[email protected]>

Prepare this command which currently handles its own argv to use parse-options instead. A lot of the commands already have parse-options and in git.c cmd_struct with the "NO_PARSEOPT" are the one's that still need it.

---
 builtin/merge-ours.c | 5 +++++
 1 file changed, 5 insertions(+)
builtin/merge-ours.c +0 -0
index 4594507420..3980f4899a 100644

builtin/merge-ours.c line 18

Quoted 8 lines
builtin/merge-ours.c +5 -0
@@ -11,6 +11,11 @@
 #include "git-compat-util.h"
 #include "builtin.h"
 #include "diff.h"
+#include "parse-options.h"
+
+/* parse-options.h added to initiate replacement of manual option parsing
+ * with parse-options. 
+ */

See Documentation/CodingGuidelines and learn how we write multi-line comments.

Since there is no "manual option parsing" to be replaced, there is no other change to this file, I guess (so what's the point of this step?).

#3 lore raw

PATCH 2/6 check-ref-format: parse-options

In reply to George Espinoza via GitGitGadget

From: george espinoza <[email protected]>

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

As a consequence of using OPT_BOOL data structure on --normalize & --refspec-pattern, --no-normalize & --no-refspec-pattern has been can now be used.

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

This is a rough draft and I need some advice if I'm doing this correctly since its being built but it is failing tests.

Helped by: emily shaffer <[email protected]> Helped by: johannes schindelin <[email protected]>

---
 builtin/check-ref-format.c | 49 +++++++++++++++++++-------------------
 git.c                      |  2 +-
 2 files changed, 26 insertions(+), 25 deletions(-)
builtin/check-ref-format.c +25 -24
index bc67d3f0a8..3fe0b5410a 100644
@@ -6,10 +6,13 @@
 #include "refs.h"
 #include "builtin.h"
 #include "strbuf.h"
+#include "parse-options.h"
 
-static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--normalize] [<options>] <refname>\n"
-"   or: git check-ref-format --branch <branchname-shorthand>";
+static const char * const builtin_check_ref_format_usage[] = {
+	N_("git check-ref-format [--normalize] [<options>] <refname>\n"),
+	N_("   or: git check-ref-format --branch <branchname-shorthand>"),
+	NULL,
+};
 
 /*
  * Return a copy of refname but with leading slashes removed and runs
@@ -53,31 +56,29 @@ static int check_ref_format_branch(const char *arg)
 
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int normalize = 0;
+	enum {
+		CHECK_REF_FORMAT_BRANCH,
+	};
+
+	int i = 0;
+	int verbose;
+	int normalize;
+	int allow_onelevel;
+	int refspec_pattern;
 	int flags = 0;
 	const char *refname;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(builtin_check_ref_format_usage);
-
-	if (argc == 3 && !strcmp(argv[1], "--branch"))
-		return check_ref_format_branch(argv[2]);
+	struct option options[] = {
+		OPT__VERBOSE(&verbose, N_("be verbose")),
+		OPT_GROUP(""),
+		OPT_CMDMODE( 0 , "branch", &check_ref_format_branch, N_("branch"), CHECK_REF_FORMAT_BRANCH),
+		OPT_BOOL( 0 , "normalize", &normalize, N_("normalize tracked files")),
+		OPT_BOOL( 0 , "allow-onelevel", &allow_onelevel, N_("allow one level")),
+		OPT_BOOL( 0 , "refspec-pattern", &refspec_pattern, N_("refspec pattern")),
+		OPT_END(),
+	};
 
-	for (i = 1; i < argc && argv[i][0] == '-'; i++) {
-		if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
-			normalize = 1;
-		else if (!strcmp(argv[i], "--allow-onelevel"))
-			flags |= REFNAME_ALLOW_ONELEVEL;
-		else if (!strcmp(argv[i], "--no-allow-onelevel"))
-			flags &= ~REFNAME_ALLOW_ONELEVEL;
-		else if (!strcmp(argv[i], "--refspec-pattern"))
-			flags |= REFNAME_REFSPEC_PATTERN;
-		else
-			usage(builtin_check_ref_format_usage);
-	}
-	if (! (i == argc - 1))
-		usage(builtin_check_ref_format_usage);
+	argc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, PARSE_OPT_KEEP_ARGV0);
 
 	refname = argv[i];
 	if (normalize)
git.c +1 -1
index ce6ab0ece2..13dbfde31c 100644
@@ -478,7 +478,7 @@ static struct cmd_struct commands[] = {
 	{ "check-attr", cmd_check_attr, RUN_SETUP },
 	{ "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
 	{ "check-mailmap", cmd_check_mailmap, RUN_SETUP },
-	{ "check-ref-format", cmd_check_ref_format, NO_PARSEOPT  },
+	{ "check-ref-format", cmd_check_ref_format },
 	{ "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
 	{ "checkout-index", cmd_checkout_index,
 		RUN_SETUP | NEED_WORK_TREE},
Signature · 1 line
gitgitgadget
#4 lore raw
Quoted 10 lines · george espinoza via GitGitGadget

From: george espinoza <[email protected]>

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

As a consequence of using OPT_BOOL data structure on --normalize & --refspec-pattern, --no-normalize & --no-refspec-pattern has been can now be used.

Quoted 8 lines

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

This is a rough draft and I need some advice if I'm doing this correctly since its being built but it is failing tests.

Helped by: emily shaffer <[email protected]> Helped by: johannes schindelin <[email protected]>

I do not think they spell their name like the above. In general, most of us do not spell our names in all lowercase around here. I appreciate people with originality, but I'd rather see them to be original not in how they spell their names but in more productive ways ;-)

Quoted 14 lines
---
 builtin/check-ref-format.c | 49 +++++++++++++++++++-------------------
 git.c                      |  2 +-
 2 files changed, 26 insertions(+), 25 deletions(-)
builtin/check-ref-format.c +0 -0
index bc67d3f0a8..3fe0b5410a 100644
@@ -6,10 +6,13 @@
 #include "refs.h"
 #include "builtin.h"
 #include "strbuf.h"

builtin/check-ref-format.c line 15

Quoted 10 lines
builtin/check-ref-format.c +6 -3
+#include "parse-options.h"
 
-static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--normalize] [<options>] <refname>\n"
-"   or: git check-ref-format --branch <branchname-shorthand>";
+static const char * const builtin_check_ref_format_usage[] = {
+	N_("git check-ref-format [--normalize] [<options>] <refname>\n"),
+	N_("   or: git check-ref-format --branch <branchname-shorthand>"),
+	NULL,
+};

OK. This is the bog-standard prep for calling usage_with_options().

Quoted 7 lines
builtin/check-ref-format.c +1 -2
@@ -53,31 +56,29 @@ static int check_ref_format_branch(const char *arg)
 
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int normalize = 0;
+	enum {

builtin/check-ref-format.c static int check_ref_format_branch(const char *arg) · line 69

Quoted 10 lines
builtin/check-ref-format.c +8 -0
+		CHECK_REF_FORMAT_BRANCH,
+	};
+
+	int i = 0;
+	int verbose;
+	int normalize;
+	int allow_onelevel;
+	int refspec_pattern;
 	int flags = 0;
 	const char *refname;

Discard the blank line before "int i = 0" line, and keep the blank line you have here between the last declaration and the first statement.

builtin/check-ref-format.c static int check_ref_format_branch(const char *arg) · line 74

Quoted 9 lines
builtin/check-ref-format.c +4 -5
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(builtin_check_ref_format_usage);
-
-	if (argc == 3 && !strcmp(argv[1], "--branch"))
-		return check_ref_format_branch(argv[2]);
+	struct option options[] = {
+		OPT__VERBOSE(&verbose, N_("be verbose")),
+		OPT_GROUP(""),
+		OPT_CMDMODE( 0 , "branch", &check_ref_format_branch, N_("branch"), CHECK_REF_FORMAT_BRANCH),

This is an iffy/tricky way to use CMDMODE. The way CMDMODE is designed to be used is to have multiple ones that sets the same target variable so that parse_options() can notice conflicting command line request that gives more than one from the same set.

The command has two modes (i.e. the "--branch" mode and the unnamed mode), so it is understandable that there is only one CMDMODE in the options[] array, but I am not sure how well it would work for a command like this. For example, "check-ref-format --branch --normalize --allow-onelevel $v" should error out because --branch is not compatible with any other options. I do not think a single parse_options() call with a single options[] array can express that kind of constraints.

Besides, shouldn't the third parameter of OPT_CMDMODE supposed to be the address of the variable that would receive the value in its fifth parameter? I do not see a decl for check_ref_format_branch variable (isn't that the name of the function???).

Ahh, you said it builds but does not pass test. Of course, that is because this part is completely bogus.

It appears that to your series the only thing that matters is the shape of the tree after applying all of the patches, and individual steps are not ready to be reviewd, so I'd stop here.

#5 lore raw
Quoted 19 lines · Junio C Hamano
Quoted 16 lines · george espinoza via GitGitGadget

From: george espinoza <[email protected]>

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

As a consequence of using OPT_BOOL data structure on --normalize & --refspec-pattern, --no-normalize & --no-refspec-pattern has been can now be used.

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

This is a rough draft and I need some advice if I'm doing this correctly since its being built but it is failing tests.

Quoted 9 lines
Quoted 2 lines

Helped by: emily shaffer <[email protected]> Helped by: johannes schindelin <[email protected]>

I do not think they spell their name like the above. In general, most of us do not spell our names in all lowercase around here. I appreciate people with originality, but I'd rather see them to be original not in how they spell their names but in more productive ways ;-)

Ah, I see. I will use capital letters.

Quoted 16 lines
Quoted 16 lines
---
 builtin/check-ref-format.c | 49 +++++++++++++++++++-------------------
 git.c                      |  2 +-
 2 files changed, 26 insertions(+), 25 deletions(-)
builtin/check-ref-format.c +1 -0
index bc67d3f0a8..3fe0b5410a 100644
@@ -6,10 +6,13 @@
 #include "refs.h"
 #include "builtin.h"
 #include "strbuf.h"
+#include "parse-options.h"
 
Quoted 10 lines

builtin/check-ref-format.c line 15

Quoted 8 lines
builtin/check-ref-format.c +5 -3
-static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--normalize] [<options>] <refname>\n"
-"   or: git check-ref-format --branch <branchname-shorthand>";
+static const char * const builtin_check_ref_format_usage[] = {
+     N_("git check-ref-format [--normalize] [<options>] <refname>\n"),
+     N_("   or: git check-ref-format --branch <branchname-shorthand>"),
+     NULL,
+};

OK. This is the bog-standard prep for calling usage_with_options().

I see, I will look into doing something more here as necessary.

Quoted 14 lines
Quoted 14 lines
builtin/check-ref-format.c +7 -2
@@ -53,31 +56,29 @@ static int check_ref_format_branch(const char *arg)
 
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
-     int i;
-     int normalize = 0;
+     enum {
+             CHECK_REF_FORMAT_BRANCH,
+     };
+
+     int i = 0;
+     int verbose;
+     int normalize;
Quoted 8 lines

builtin/check-ref-format.c static int check_ref_format_branch(const char *arg) · line 69

Quoted 4 lines
builtin/check-ref-format.c +2 -0
+     int allow_onelevel;
+     int refspec_pattern;
      int flags = 0;
      const char *refname;

Discard the blank line before "int i = 0" line, and keep the blank line you have here between the last declaration and the first statement.

I meant to squash all my earlier commits but had an issue with git rebase -i. It only says noop. I had an issue when I first started my other branch and might of reset it. I'll look into fixing this before submitting my next patch.

Quoted 16 lines

builtin/check-ref-format.c static int check_ref_format_branch(const char *arg) · line 74

Quoted 10 lines
builtin/check-ref-format.c +4 -5
-     if (argc == 2 && !strcmp(argv[1], "-h"))
-             usage(builtin_check_ref_format_usage);
-
-     if (argc == 3 && !strcmp(argv[1], "--branch"))
-             return check_ref_format_branch(argv[2]);
+     struct option options[] = {
+             OPT__VERBOSE(&verbose, N_("be verbose")),
+             OPT_GROUP(""),
+             OPT_CMDMODE( 0 , "branch", &check_ref_format_branch, N_("branch"), CHECK_REF_FORMAT_BRANCH),

This is an iffy/tricky way to use CMDMODE. The way CMDMODE is designed to be used is to have multiple ones that sets the same target variable so that parse_options() can notice conflicting command line request that gives more than one from the same set.

Quoted 8 lines

The command has two modes (i.e. the "--branch" mode and the unnamed mode), so it is understandable that there is only one CMDMODE in the options[] array, but I am not sure how well it would work for a command like this. For example, "check-ref-format --branch --normalize --allow-onelevel $v" should error out because --branch is not compatible with any other options. I do not think a single parse_options() call with a single options[] array can express that kind of constraints.

Ok I will look into another data structure to use for branch.

Quoted 6 lines

Besides, shouldn't the third parameter of OPT_CMDMODE supposed to be the address of the variable that would receive the value in its fifth parameter? I do not see a decl for check_ref_format_branch variable (isn't that the name of the function???).

Quoted 7 lines

Ahh, you said it builds but does not pass test. Of course, that is because this part is completely bogus.

It appears that to your series the only thing that matters is the shape of the tree after applying all of the patches, and individual steps are not ready to be reviewd, so I'd stop here.

I had only intended /submit to show the last commit I had made that had passed all the tests. I will review everything and only submit again once im 100% sure everything is in order. Sorry Junio!

#6 lore raw

PATCH 3/6 This file wasn't supposed to change during my git push for check-ref-format :(

In reply to George Espinoza via GitGitGadget

From: george espinoza <[email protected]>

---
 builtin/merge-ours.c | 5 -----
 1 file changed, 5 deletions(-)
builtin/merge-ours.c +0 -5
index 3980f4899a..4594507420 100644
@@ -11,11 +11,6 @@
 #include "git-compat-util.h"
 #include "builtin.h"
 #include "diff.h"
-#include "parse-options.h"
-
-/* parse-options.h added to initiate replacement of manual option parsing
- * with parse-options. 
- */
 
 static const char builtin_merge_ours_usage[] =
 	"git merge-ours <base>... -- HEAD <remote>...";
Signature · 1 line
gitgitgadget
#7 lore raw

Re: [PATCH 3/6] This file wasn't supposed to change during my git push for check-ref-format :(

Quoted 4 lines · george espinoza via GitGitGadget

From: george espinoza <[email protected]>

---

While developing your first patch, it is expected that you make mistake in an earlier step (e.g. 2/6) and then later realize it and want to fix, like this one. Instead of going back to the earlier mistake and fixing it right there before building more on top, tentatively fixing with "oops that was wrong" like this one and building further (i.e. 4 and later steps) is an expedient way.

But once you are done, you need to get your act together before presenting the resulting series to the general public.

Please learn to use "rebase -i" to clean up the series before asking GGG to publish it to the list.

Thanks.

Quoted 19 lines
 builtin/merge-ours.c | 5 -----
 1 file changed, 5 deletions(-)
builtin/merge-ours.c +0 -5
index 3980f4899a..4594507420 100644
@@ -11,11 +11,6 @@
 #include "git-compat-util.h"
 #include "builtin.h"
 #include "diff.h"
-#include "parse-options.h"
-
-/* parse-options.h added to initiate replacement of manual option parsing
- * with parse-options. 
- */
 
 static const char builtin_merge_ours_usage[] =
 	"git merge-ours <base>... -- HEAD <remote>...";
#8 lore raw

PATCH 4/6 check-ref-format: parse options

In reply to George Espinoza via GitGitGadget

From: george espinoza <[email protected]>

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

As a consequence of using OPT_BOOL data structure on --normalize & --refspec-pattern, --no-normalize & --no-refspec-pattern can now be used.

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

This is a rough draft and I need some advice if I'm doing this correctly since its being built but it is failing tests.

Helped by: emily shaffer [email protected] Helped by: johannes schindelin [email protected]

---
 builtin/check-ref-format.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
builtin/check-ref-format.c +6 -7
index 3fe0b5410a..c48fb19eda 100644
@@ -61,10 +61,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 	};
 
 	int i = 0;
-	int verbose;
-	int normalize;
-	int allow_onelevel;
-	int refspec_pattern;
+	int verbose = 0;
+	int normalize = 0;
 	int flags = 0;
 	const char *refname;
 
@@ -73,12 +71,13 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_CMDMODE( 0 , "branch", &check_ref_format_branch, N_("branch"), CHECK_REF_FORMAT_BRANCH),
 		OPT_BOOL( 0 , "normalize", &normalize, N_("normalize tracked files")),
-		OPT_BOOL( 0 , "allow-onelevel", &allow_onelevel, N_("allow one level")),
-		OPT_BOOL( 0 , "refspec-pattern", &refspec_pattern, N_("refspec pattern")),
+		OPT_BIT( 0 , "allow-onelevel", &flags, N_("allow one level"), REFNAME_ALLOW_ONELEVEL),
+		OPT_NEGBIT( 0, "no-allow-onelevel", &flags, N_("no allow one level"), REFNAME_ALLOW_ONELEVEL),
+		OPT_BIT( 0 , "refspec-pattern", &flags, N_("refspec pattern"), REFNAME_REFSPEC_PATTERN),
 		OPT_END(),
 	};
 
-	argc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, PARSE_OPT_KEEP_ARGV0);
+	argc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, 0);
 
 	refname = argv[i];
 	if (normalize)
Signature · 1 line
gitgitgadget
#9 lore raw

PATCH 5/6 check-ref-format: parse-options

In reply to George Espinoza via GitGitGadget

From: george espinoza <[email protected]>

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

Because OPT_BOOL data structure is being used on --normalize --no-normalize can now be utilized.

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

I got all tests to pass at the moment. My next commit will be to check if OPT_NEGBIT isn't needed and moving around OPT_VERBOSE.

Helped by: emily shaffer [email protected] Helped by: johannes schindelin [email protected]

---
 builtin/check-ref-format.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
builtin/check-ref-format.c +10 -8
index c48fb19eda..be3b1bd84d 100644
@@ -57,10 +57,10 @@ static int check_ref_format_branch(const char *arg)
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
 	enum {
+		CHECK_REFNAME_FORMAT = 0,
 		CHECK_REF_FORMAT_BRANCH,
-	};
+	} mode = CHECK_REFNAME_FORMAT;
 
-	int i = 0;
 	int verbose = 0;
 	int normalize = 0;
 	int flags = 0;
@@ -69,17 +69,19 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT__VERBOSE(&verbose, N_("be verbose")),
 		OPT_GROUP(""),
-		OPT_CMDMODE( 0 , "branch", &check_ref_format_branch, N_("branch"), CHECK_REF_FORMAT_BRANCH),
-		OPT_BOOL( 0 , "normalize", &normalize, N_("normalize tracked files")),
-		OPT_BIT( 0 , "allow-onelevel", &flags, N_("allow one level"), REFNAME_ALLOW_ONELEVEL),
-		OPT_NEGBIT( 0, "no-allow-onelevel", &flags, N_("no allow one level"), REFNAME_ALLOW_ONELEVEL),
-		OPT_BIT( 0 , "refspec-pattern", &flags, N_("refspec pattern"), REFNAME_REFSPEC_PATTERN),
+		OPT_CMDMODE(0 , "branch", &mode, N_("branch"), CHECK_REF_FORMAT_BRANCH),
+		OPT_BOOL(0 , "normalize", &normalize, N_("normalize tracked files")),
+		OPT_BIT(0 , "allow-onelevel", &flags, N_("allow one level"), REFNAME_ALLOW_ONELEVEL),
+		OPT_NEGBIT(0, "no-allow-onelevel", &flags, N_("no allow one level"), REFNAME_ALLOW_ONELEVEL),
+		OPT_BIT(0 , "refspec-pattern", &flags, N_("refspec pattern"), REFNAME_REFSPEC_PATTERN),
 		OPT_END(),
 	};
 
 	argc = parse_options(argc, argv, prefix, options, builtin_check_ref_format_usage, 0);
 
-	refname = argv[i];
+	refname = argv[0];
+	if (mode)
+		return  check_ref_format_branch(argv[2]);
 	if (normalize)
 		refname = collapse_slashes(refname);
 	if (check_refname_format(refname, flags))
Signature · 1 line
gitgitgadget
#10 lore raw

PATCH 6/6 check-ref-format: parse-options

In reply to George Espinoza via GitGitGadget

From: george espinoza <[email protected]>

This command currently handles its own argv so by teaching it to use parse-options instead we can standardize the way commands handle user input across the project.

Because OPT_BOOL data structure is being used on --normalize --no-normalize can now be utilized.

NO_PARSEOPT flag was also removed to update git.c with the conversion of the structure in this command.

Helped by: emily shaffer [email protected] Helped by: johannes schindelin [email protected]

---
 builtin/check-ref-format.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
builtin/check-ref-format.c +2 -5
index be3b1bd84d..ea703cfabd 100644
@@ -61,18 +61,15 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 		CHECK_REF_FORMAT_BRANCH,
 	} mode = CHECK_REFNAME_FORMAT;
 
-	int verbose = 0;
 	int normalize = 0;
 	int flags = 0;
 	const char *refname;
 
 	struct option options[] = {
-		OPT__VERBOSE(&verbose, N_("be verbose")),
 		OPT_GROUP(""),
-		OPT_CMDMODE(0 , "branch", &mode, N_("branch"), CHECK_REF_FORMAT_BRANCH),
+		OPT_CMDMODE(0 , "branch", &mode, N_("check for valid branch name"), CHECK_REF_FORMAT_BRANCH),
 		OPT_BOOL(0 , "normalize", &normalize, N_("normalize tracked files")),
 		OPT_BIT(0 , "allow-onelevel", &flags, N_("allow one level"), REFNAME_ALLOW_ONELEVEL),
-		OPT_NEGBIT(0, "no-allow-onelevel", &flags, N_("no allow one level"), REFNAME_ALLOW_ONELEVEL),
 		OPT_BIT(0 , "refspec-pattern", &flags, N_("refspec pattern"), REFNAME_REFSPEC_PATTERN),
 		OPT_END(),
 	};
@@ -81,7 +78,7 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 
 	refname = argv[0];
 	if (mode)
-		return  check_ref_format_branch(argv[2]);
+		return check_ref_format_branch(argv[2]);
 	if (normalize)
 		refname = collapse_slashes(refname);
 	if (check_refname_format(refname, flags))
Signature · 1 line
gitgitgadget