Re: svn commit: r1352935 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/deprecated.c libsvn_client/mergeinfo.c svn/main.c svn/mergeinfo-cmd.c

On 06/23/2012 12:44 AM, Greg Stein wrote:
>> @@ -1662,6 +1663,23 @@ svn_client_mergeinfo_log(svn_boolean_t f
>> SVN_ERR_UNSUPPORTED_FEATURE, NULL,
>> _("Only depths 'infinity' and 'empty' are currently supported"));
>>
>> + /* Validate and sanitize the incoming source operative revision range. */
>> + if (!((source_start_revision->kind == svn_opt_revision_unspecified) ||
>> + (source_start_revision->kind == svn_opt_revision_number) ||
>> + (source_start_revision->kind == svn_opt_revision_date) ||
>> + (source_start_revision->kind == svn_opt_revision_head)))
>> + return svn_error_create
>> + (SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
>> + if (!((source_end_revision->kind == svn_opt_revision_unspecified) ||
>> + (source_end_revision->kind == svn_opt_revision_number) ||
>> + (source_end_revision->kind == svn_opt_revision_date) ||
>> + (source_end_revision->kind == svn_opt_revision_head)))
>> + return svn_error_create
>> + (SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
>
> Style: white space before paren, and it would be better to lead (not trail)
> the lines with ||.

Agreed on the whitespace. (Silly anyway ... the whole statement fits in 80
columns.)

We're a bit less consistent on the operator placement in the codebase. With
mixed-operator conditions, I agree with (and employ) the "trailing operator"
style. But in this situation, I think readability is greatly enhanced by
having having conditions vertically align the way they appear above.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development