Thread (27 messages) flat view 27 messages, 3 authors, 2016-06-15

Re: [PATCH v5 02/12] ref-filter: use strbuf_split_str_omit_term()

From: Jeff King <hidden>
Date: 2016-06-15 23:08:16
Subsystem: the rest · Maintainer: Linus Torvalds

On Tue, Feb 16, 2016 at 02:22:32PM -0500, Jeff King wrote:
On Wed, Feb 17, 2016 at 12:30:05AM +0530, Karthik Nayak wrote:
quoted
Use the newly introduced strbuf_split_str_omit_term() rather than
using strbuf_split_str() and manually removing the ',' terminator.

Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: Karthik Nayak <redacted>
---
 ref-filter.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
Did you consider just using string_list_split for this? AFAICT, you
don't care about the results being strbufs themselves, and it would do
what you want without having to bother with patch 1. The result would
look something like the patch below.
Probably help if I actually included the patch.

---
diff --git a/ref-filter.c b/ref-filter.c
index 9ccfc51..45b3352 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -886,41 +886,34 @@ static void populate_value(struct ref_array_item *ref)
 			continue;
 		} else if (match_atom_name(name, "align", &valp)) {
 			struct align *align = &v->u.align;
-			struct strbuf **s, **to_free;
+			struct string_list params = STRING_LIST_INIT_DUP;
+			int i;
 			int width = -1;
 
 			if (!valp)
 				die(_("expected format: %%(align:<width>,<position>)"));
 
-			/*
-			 * TODO: Implement a function similar to strbuf_split_str()
-			 * which would omit the separator from the end of each value.
-			 */
-			s = to_free = strbuf_split_str(valp, ',', 0);
-
 			align->position = ALIGN_LEFT;
 
-			while (*s) {
-				/*  Strip trailing comma */
-				if (s[1])
-					strbuf_setlen(s[0], s[0]->len - 1);
-				if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)&width))
+			string_list_split(&params, valp, ',', -1);
+			for (i = 0; i < params.nr; i++) {
+				const char *p = params.items[i].string;
+				if (!strtoul_ui(p, 10, (unsigned int *)&width))
 					;
-				else if (!strcmp(s[0]->buf, "left"))
+				else if (!strcmp(p, "left"))
 					align->position = ALIGN_LEFT;
-				else if (!strcmp(s[0]->buf, "right"))
+				else if (!strcmp(p, "right"))
 					align->position = ALIGN_RIGHT;
-				else if (!strcmp(s[0]->buf, "middle"))
+				else if (!strcmp(p, "middle"))
 					align->position = ALIGN_MIDDLE;
 				else
-					die(_("improper format entered align:%s"), s[0]->buf);
-				s++;
+					die(_("improper format entered align:%s"), p);
 			}
 
 			if (width < 0)
 				die(_("positive width expected with the %%(align) atom"));
 			align->width = width;
-			strbuf_list_free(to_free);
+			string_list_clear(&params, 0);
 			v->handler = align_atom_handler;
 			continue;
 		} else if (!strcmp(name, "end")) {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help