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

[Jeff King]

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:
> 
> > Use the newly introduced strbuf_split_str_omit_term() rather than
> > using strbuf_split_str() and manually removing the ',' terminator.
> > 
> > Helped-by: Eric Sunshine <sunsh...@sunshineco.com>
> > Signed-off-by: Karthik Nayak <karthik....@gmail.com>
> > ---
> >  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")) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html