> -----Original Message-----
> From: Mark Phippard [mailto:markphip@gmail.com]
> Sent: Wednesday, November 07, 2007 11:51 AM
> To: Paul Burba
> Cc: dev@subversion.tigris.org
> Subject: Re: [RFC] QA of manually set svn:mergeinfo
>
> On 11/7/07, Paul Burba <pburba@collab.net> wrote:
> > In the case of improperly ordered ranges I think we can safely say
> > there is only one correct interpretation ("/trunk:6,3" ==
> "/trunk:3,6"
> > no matter how much you look at it :-).
> >
> > With overlapping changes with the same inheritability there is only
> > one correct meaning as well ("/trunk:5-6,6-7" == "/trunk:5-7").
> >
> > *But* in both cases the fact that a user specified something odd
> > raises a red flag for me that maybe they meant something
> else entirely
> > or simply don't understand what they are doing. I'm quite
> comfortable
> > being ultra-strict when someone is using propset to set
> svn:mergeinfo.
>
> That was exactly my point. Your first example above is
> perfect. Was the user just sloppy and ordered the value
> wrong ... or is the ",3"
> supposed to be ",13"? There is no way to know. The fact
> that the format is not right is a clue that something might
> be wrong, so maybe we should just fail it.
Not entirely sure now that unordered ranges should result in an error.
The 'Note' in the doc string for svn_mergeinfo_parse() implies that the
input string need not have revision ranges in order:
/** Parse the mergeinfo from @a input into @a *mergeinfo, mapping from
* paths to @c apr_array_header_t *'s of @c svn_merge_range_t *
* elements. If no mergeinfo is available, return an empty hash
* (never @c NULL). Perform temporary allocations in @a pool.
*
* Note: @a *mergeinfo will contain rangelists that are guaranteed to
* be sorted (ordered by smallest revision ranges to largest).
* @since New in 1.5.
*/
svn_error_t *
svn_mergeinfo_parse(apr_hash_t **mergeinfo, const char *input,
apr_pool_t *pool);
Keeping the explicit guarantee is fine, but should we accept unordered
ranges in INPUT and correctly order them or return an error? IIUC the
only time that a user would see this error is when manually setting
svn:mergeinfo with propset and thus far we seem to agree that an error
is ok in that case. We make heavy use of svn_mergeinfo_parse()
elsewhere, but in those cases we are parsing svn:mergeinfo that has
already been set (and *should* be ordered).
> If we are going to fail it, the error message has to help.
> They could have thousands of revisions listed. If they put
> two of them out of order we have to be able to tell them that
> or users are going to hate us.
>
> Think of the difference between saying an XML file is invalid
> and telling you the line that is invalid. We need to try to
> help if we are going to fail the command.
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 8 17:34:19 2007