You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2004/10/24 12:45:58 UTC
Re: svn commit: r11599 - in trunk/subversion: clients/cmdline
include
jpieper@tigris.org writes:
> Author: jpieper
> Date: Sat Oct 23 12:32:03 2004
> New Revision: 11599
>
> Modified:
> trunk/subversion/clients/cmdline/log-cmd.c
> trunk/subversion/include/svn_client.h
> Log:
> Issue #1550: 'svn log' is broken for multiple WC paths.
> --- trunk/subversion/clients/cmdline/log-cmd.c (original)
> +++ trunk/subversion/clients/cmdline/log-cmd.c Sat Oct 23 12:32:03 2004
> @@ -455,6 +455,8 @@
> svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
> apr_array_header_t *targets;
> struct log_receiver_baton lb;
> + const char *target;
> + int i;
>
> SVN_ERR (svn_opt_args_to_target_array (&targets, os,
> opt_state->targets,
> @@ -481,7 +483,7 @@
> }
> else if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
> {
> - const char *target = APR_ARRAY_IDX (targets, 0, const char *);
> + target = APR_ARRAY_IDX (targets, 0, const char *);
>
> /* If the first target is a URL, then we default to HEAD:1.
> Otherwise, the default is BASE:1 since WC@HEAD may not exist. */
> @@ -495,6 +497,27 @@
> opt_state->end_revision.kind = svn_opt_revision_number;
> opt_state->end_revision.value.number = 1; /* oldest commit */
> }
> + }
> +
> + /* Verify that we pass at most one working copy path. */
> + target = APR_ARRAY_IDX (targets, 0, const char *);
There's an identical line inside the else if(), perhaps it should be
moved before the if().
> +
> + if (! svn_path_is_url (target) )
> + {
> + if (targets->nelts > 1)
> + return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("When specifying working copy paths, only "
> + "one target may be given"));
> + } else {
"} else {" is usually written on three lines in the Subversion code.
> + /* Check to make sure there are no other URLs. */
> + for (i = 1; i < targets->nelts; i++) {
"for () {" is usually written on two lines.
> + target = APR_ARRAY_IDX (targets, 1, const char *);
Huh? In that APR_ARRAY_IDX should 1 be i?
> +
> + if (svn_path_is_url (target))
> + return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("Only relative paths can be specified "
> + "after a URL"));
> + }
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r11599 - in trunk/subversion: clients/cmdline include
Posted by Josh Pieper <jj...@pobox.com>.
Thanks for the review Philip. My Subversion style is apparently a
little rusty. Things should be fixed up in r11610.
-Josh
Philip Martin wrote:
> jpieper@tigris.org writes:
[... code ... ]
> > + /* Verify that we pass at most one working copy path. */
> > + target = APR_ARRAY_IDX (targets, 0, const char *);
>
> There's an identical line inside the else if(), perhaps it should be
> moved before the if().
>
> > +
> > + if (! svn_path_is_url (target) )
> > + {
> > + if (targets->nelts > 1)
> > + return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> > + _("When specifying working copy paths, only "
> > + "one target may be given"));
> > + } else {
>
> "} else {" is usually written on three lines in the Subversion code.
>
> > + /* Check to make sure there are no other URLs. */
> > + for (i = 1; i < targets->nelts; i++) {
>
> "for () {" is usually written on two lines.
>
> > + target = APR_ARRAY_IDX (targets, 1, const char *);
>
> Huh? In that APR_ARRAY_IDX should 1 be i?
>
> > +
> > + if (svn_path_is_url (target))
> > + return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> > + _("Only relative paths can be specified "
> > + "after a URL"));
> > + }
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org