In addition to Philip's review:
On Thu, 10 Mar 2005 fitz@tigris.org wrote:
> Author: fitz
> Date: Thu Mar 10 15:52:46 2005
> New Revision: 13359
>
> Modified: branches/locking/subversion/clients/cmdline/lock-cmd.c
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/clients/cmdline/lock-cmd.c?view=diff&rev=13359&p1=branches/locking/subversion/clients/cmdline/lock-cmd.c&r1=13358&p2=branches/locking/subversion/clients/cmdline/lock-cmd.c&r2=13359
> ==============================================================================
> --- branches/locking/subversion/clients/cmdline/lock-cmd.c (original)
> +++ branches/locking/subversion/clients/cmdline/lock-cmd.c Thu Mar 10 15:52:46 2005
> @@ -107,8 +107,11 @@
> svn_error_t *err;
> struct lock_baton *lb = baton;
>
> - err = svn_cmdline_printf (lb->pool, _("'%s' locked by user '%s'.\n"),
> - path, lock->owner);
> + if (ra_err)
> + err = svn_cmdline_printf (lb->pool, _("Error: %s\n"), ra_err->message);
Why don't you use the standard error handling routine here? This wil just
print the first error message in the chain, and, more seriously, there is
nothing guaranteeing that ra_err->message is non-NULL. Kaboom!
> Modified: branches/locking/subversion/clients/cmdline/unlock-cmd.c
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/clients/cmdline/unlock-cmd.c?view=diff&rev=13359&p1=branches/locking/subversion/clients/cmdline/unlock-cmd.c&r1=13358&p2=branches/locking/subversion/clients/cmdline/unlock-cmd.c&r2=13359
> ==============================================================================
> --- branches/locking/subversion/clients/cmdline/unlock-cmd.c (original)
> +++ branches/locking/subversion/clients/cmdline/unlock-cmd.c Thu Mar 10 15:52:46 2005
> @@ -54,7 +54,10 @@
> svn_error_t *err;
> struct lock_baton *lb = baton;
>
> - err = svn_cmdline_printf (lb->pool, _("Unlocked '%s'.\n"), path);
> + if (ra_err)
> + err = svn_cmdline_printf (lb->pool, _("Error: %s\n"), ra_err->message);
> + else
> + err = svn_cmdline_printf (lb->pool, _("Unlocked '%s'.\n"), path);
>
Dito. Also, should we put the path a the beginning of the line to be
consistentwith the lock command? (I know who wrote that linte in the first
place, just came to think of it now that I reread it:-)
> Modified: branches/locking/subversion/include/svn_error.h
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/include/svn_error.h?view=diff&rev=13359&p1=branches/locking/subversion/include/svn_error.h&r1=13358&p2=branches/locking/subversion/include/svn_error.h&r2=13359
> ==============================================================================
> --- branches/locking/subversion/include/svn_error.h (original)
> +++ branches/locking/subversion/include/svn_error.h Thu Mar 10 15:52:46 2005
> @@ -218,6 +218,21 @@
>
> /** @} */
>
> +/*
> + * @since New in 1.2.
> + *
> + * Return TRUE if @a err is an error specifically related to locking a
> + * path, FALSE otherwise. */
> +svn_boolean_t svn_error_is_lock_error (svn_error_t *err);
> +
I am not sure if I like putting this in the generic error file. Maybe we'd
better have an svn_lock.h file where we put stuff related to locking. As
it stands, we have the svn_lock_t and the callback in svn_types.h and
these utility functions in svn_error.h. Also, we need to be more careful
in disambiguiting this kind of locks with the locks in the WC for the
admin area. I think this docstring needs to be more clear.
> +/*
> + * @since New in 1.2.
> + *
> + * Return TRUE if @a err is an error specifically related to unlocking
> + * a path, FALSE otherwise. */
> +svn_boolean_t svn_error_is_unlock_error (svn_error_t *err);
> +
> +
Samma sak här. ;)
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
>
> Modified: branches/locking/subversion/libsvn_client/locking_commands.c
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/libsvn_client/locking_commands.c?view=diff&rev=13359&p1=branches/locking/subversion/libsvn_client/locking_commands.c&r1=13358&p2=branches/locking/subversion/libsvn_client/locking_commands.c&r2=13359
> ==============================================================================
> --- branches/locking/subversion/libsvn_client/locking_commands.c (original)
> +++ branches/locking/subversion/libsvn_client/locking_commands.c Thu Mar 10 15:52:46 2005
> @@ -104,18 +104,24 @@
> struct lock_baton *lb = baton;
> svn_wc_adm_access_t *adm_access;
> const char *abs_path;
> -
> +
I think I am guilty for these whitespace only lines. Does anyone know how
to get Emacs to not insert those?
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 11 08:40:36 2005