Re: [patch] New --targets option

On Fri, Mar 01, 2002 at 06:55:22PM -0500, Ben Collins wrote:
>...
> +/* Convert a newline seperated list of items into an apr_array_header_t */
> +#define IS_NEWLINE(c) (c == '\n' || c == '\r')
> +apr_array_header_t*
> +svn_cl__newlinelist_to_array(svn_stringbuf_t *buffer, apr_pool_t *pool)
> +{
> + apr_array_header_t *array = apr_array_make(pool, DEFAULT_ARRAY_SIZE,
> + sizeof(svn_stringbuf_t *));
>
> + if (buffer != NULL)
> + {
> + apr_size_t start = 0, end = 0;
> + svn_stringbuf_t *item;
> + while (end < buffer->len)
> + {
> + /* Skip blank lines */
> + while (IS_NEWLINE(buffer->data[start]))
> + start++;

Probably want to skip all whitespace, rather than just empty lines.

> +
> + end = start;
> +
> + /* If end of the string, we're done */
> + if (start >= buffer->len)
> + break;
> +
> + while (end < buffer->len && !IS_NEWLINE(buffer->data[end]))
> + end++;

Whitespace here, too, I'd think. See apr_lib.h, apr_isspace().

>...
> +++ ./subversion/clients/cmdline/main.c Fri Mar 1 15:13:56 2002
> @@ -67,6 +67,7 @@
> {"username", svn_cl__auth_username_opt, 1, "specify a username ARG"},
> {"password", svn_cl__auth_password_opt, 1, "specify a password ARG"},
> {"extensions", 'x', 1, "pass \"ARG\" as bundled options to GNU diff"},
> + {"targets", 't', 1, "pass contents of file \"ARG\" as additional args"},

I don't think the option will be used to enough to warrant a short option.
I'd suggest only the long --targets option.

>...

Wow. Looks good but for the '-t' thing. I'm not sure whether people think it
is a good idea or not to have that as a short option.

Thoughts?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org