On Thu, Apr 05, 2012 at 01:12:51PM -0700, Junio C Hamano wrote:
> Neil Horman <nhorman@xxxxxxxxxxxxx> writes:
>
> > As a convienience, it would be nice if we could pop entries off the argv_array
> > structs so that if they had multiple uses in a function, we wouldn't have to
> > clear them and repopulate common entries. This patch adds the argv_array_pop
> > function to do just that. Common entries can be added to an argv_array first,
> > then useage specific ones can be added on the end and removed later on.
> >
> > Signed-off-by: Neil Horman <nhorman@xxxxxxxxxxxxx>
>
>
> > CC: Jeff King <peff@xxxxxxxx>
> > CC: Phil Hord <phil.hord@xxxxxxxxx>
> > CC: Junio C Hamano <gitster@xxxxxxxxx>
> > ---
>
> Please don't do "Cc:" here; they belong to your e-mail header.
>
You mean place them below the snip line? I can do that.
> > diff --git a/argv-array.c b/argv-array.c
> > index a4e0420..ce24a48 100644
> > --- a/argv-array.c
> > +++ b/argv-array.c
> > @@ -39,6 +39,18 @@ void argv_array_pushf(struct argv_array *array, const char *fmt, ...)
> > argv_array_push_nodup(array, strbuf_detach(&v, NULL));
> > }
> >
> > +int argv_array_pop(struct argv_array *array, unsigned int num)
> > +{
> > + if (num > array->argc)
> > + return -1;
>
> If your use case is "After using an argv_array for the first invocation,
> truncate it while keeping the common ones that appear early, so that ones
> that are specific to the second invocation can be pushed", it strikes me
> somewhat odd why you would want to specify "how many to pop".
>
Why? It seems perfectly logical to me to be able to, as a convienience, specify
how many items to pop, and the api call seems pleasantly symmetric to the
push[f] calls.
> Wouldn't argv_array_truncate() or argv_array_setlen() make more sense?
>
No, truncate is vague about its meaning. Truncate to zero would be useless, as
its equivalent to the clear call at that point, and truncating to a specific
value is exactly the same as what I have currently, minus the symmetry to the
push[f] calls. Likewise setlen doesn't seem to fit in properly, plus theres the
possibility there of believing the api call might be able to set a length longer
than what exists in the array already, which, while silly, requires additional
error checking. Popping a fixed number of elements off an argv_array that have
previously been pushed makes perfectly good sense to me.
>
> > + for(num--; num>0; num--) {
>
> Gaah.
>
Eeek :). If you want something else equally....equal here, please ask for it.
I prefer for loops, but if you would rather have a while loop here, I'm fine
with that.
Regards
Neil
--
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