[PATCH v5 00/12] ref-filter: use parsing functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This series cleans up populate_value() in ref-filter, by moving out
the parsing part of atoms to separate parsing functions. This ensures
that parsing is only done once and also improves the modularity of the
code.

v1: http://thread.gmane.org/gmane.comp.version-control.git/281180
v2: http://thread.gmane.org/gmane.comp.version-control.git/282563
v3: http://thread.gmane.org/gmane.comp.version-control.git/283350
v4: http://thread.gmane.org/gmane.comp.version-control.git/285158

Changes:
* Fixed small errors in multiple 'die(..)' messages.
* Removed unecessary braces.
* In parse_align_position() use 'v', 's' to denote the vector of arguments
and individual argument respectively rather than re-using 'arg'.
* Fix error in parse_ref_filter_atom() where length of current atom wasn't
accurately calculated.
* Small code and indentation fixes

Thanks to Eric, Junio, Ramsay and Andreas for their comments on the previous
version. And everyone else who helped review the previous patch series.

Karthik Nayak (12):
  strbuf: introduce strbuf_split_str_omit_term()
  ref-filter: use strbuf_split_str_omit_term()
  ref-filter: bump 'used_atom' and related code to the top
  ref-filter: introduce struct used_atom
  ref-filter: introduce parsing functions for each valid atom
  ref-filter: introduce color_atom_parser()
  ref-filter: introduce parse_align_position()
  ref-filter: introduce align_atom_parser()
  ref-filter: align: introduce long-form syntax
  ref-filter: introduce remote_ref_atom_parser()
  ref-filter: introduce contents_atom_parser()
  ref-filter: introduce objectname_atom_parser()

 Documentation/git-for-each-ref.txt |  20 +-
 ref-filter.c                       | 434 +++++++++++++++++++++----------------
 strbuf.c                           |  14 +-
 strbuf.h                           |  25 ++-
 t/t6302-for-each-ref-filter.sh     |  42 ++++
 5 files changed, 329 insertions(+), 206 deletions(-)

Interdiff:

diff --git a/ref-filter.c b/ref-filter.c
index d48e2a3..21f4937 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -54,14 +54,14 @@ static void color_atom_parser(struct used_atom *atom, const char *color_value)
 	if (!color_value)
 		die(_("expected format: %%(color:<color>)"));
 	if (color_parse(color_value, atom->u.color) < 0)
-		die(_("invalid color value: %s"), atom->u.color);
+		die(_("unrecognized color: %%(color:%s)"), color_value);
 }
 
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
-	if (!arg) {
+	if (!arg)
 		atom->u.remote_ref = RR_NORMAL;
-	} else if (!strcmp(arg, "short"))
+	else if (!strcmp(arg, "short"))
 		atom->u.remote_ref = RR_SHORTEN;
 	else if (!strcmp(arg, "track"))
 		atom->u.remote_ref = RR_TRACK;
@@ -74,14 +74,14 @@ static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 static void body_atom_parser(struct used_atom *atom, const char *arg)
 {
 	if (arg)
-		die("%%(body) atom does not take arguments");
+		die("%%(body) does not take arguments");
 	atom->u.contents.option = C_BODY_DEP;
 }
 
 static void subject_atom_parser(struct used_atom *atom, const char *arg)
 {
 	if (arg)
-		die("%%(subject) atom does not take arguments");
+		die("%%(subject) does not take arguments");
 	atom->u.contents.option = C_SUB;
 }
 
@@ -127,34 +127,34 @@ static align_type parse_align_position(const char *s)
 static void align_atom_parser(struct used_atom *atom, const char *arg)
 {
 	struct align *align = &atom->u.align;
-	struct strbuf **s, **to_free;
+	struct strbuf **v, **to_free;
 	unsigned int width = ~0U;
 
 	if (!arg)
 		die(_("expected format: %%(align:<width>,<position>)"));
-	s = to_free = strbuf_split_str_omit_term(arg, ',', 0);
+	v = to_free = strbuf_split_str_omit_term(arg, ',', 0);
 
 	align->position = ALIGN_LEFT;
 
-	while (*s) {
+	while (*v) {
 		int position;
-		arg = s[0]->buf;
+		const char *s = v[0]->buf;
 
-		if (skip_prefix(arg, "position=", &arg)) {
-			position = parse_align_position(arg);
+		if (skip_prefix(s, "position=", &s)) {
+			position = parse_align_position(s);
 			if (position < 0)
-				die(_("unrecognized position:%s"), arg);
+				die(_("unrecognized position:%s"), s);
 			align->position = position;
-		} else if (skip_prefix(arg, "width=", &arg)) {
-			if (strtoul_ui(arg, 10, &width))
-				die(_("unrecognized width:%s"), arg);
-		} else if (!strtoul_ui(arg, 10, &width))
+		} else if (skip_prefix(s, "width=", &s)) {
+			if (strtoul_ui(s, 10, &width))
+				die(_("unrecognized width:%s"), s);
+		} else if (!strtoul_ui(s, 10, &width))
 			;
-		else if ((position = parse_align_position(arg)) >= 0)
+		else if ((position = parse_align_position(s)) >= 0)
 			align->position = position;
 		else
-			die(_("unrecognized %%(align) argument: %s"), arg);
-		s++;
+			die(_("unrecognized %%(align) argument: %s"), s);
+		v++;
 	}
 
 	if (width == ~0U)
@@ -253,6 +253,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
 	/* Is the atom a valid one? */
 	for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
 		int len = strlen(valid_atom[i].name);
+
 		/*
 		 * If the atom name has a colon, strip it and everything after
 		 * it off - it specifies the format for this entry, and
@@ -260,7 +261,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
 		 * table.
 		 */
 		arg = memchr(sp, ':', ep - sp);
-		if ((!arg || len == arg - sp) &&
+		if (len == (arg ? arg : ep) - sp &&
 		    !memcmp(valid_atom[i].name, sp, len))
 			break;
 	}
@@ -775,8 +776,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 	unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
 		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index f79355d..bcf472b 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -149,8 +149,8 @@ test_align_permutations() {
 	while read -r option
 	do
 		test_expect_success "align:$option" '
-		git for-each-ref --format="|%(align:$option)refname is %(refname)%(end)|%(refname)" >actual &&
-		test_cmp expect actual
+			git for-each-ref --format="|%(align:$option)refname is %(refname)%(end)|%(refname)" >actual &&
+			test_cmp expect actual
 		'
 	done
 }

-- 
2.7.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]