Ivan Zhakov wrote: > On 4 February 2015 at 20:45, Julian Foad wrote: >> So we're on the third version of the code and third version of the test, >> for a >> tiny edge-case feature. Clearly it's fragile. I had a bad feeling about >> writing it >> this way in the first place. > > I had the same feelings when were reviewing backport nomination in 1.8.x. > > I wonder why you duplicated code that parses svn:mergeinfo instead of > using svn_mergeinfo_parse() ? I mean: > 1. Parse svn:mergeinfo using svn_mergeinfo_parse()
The parser contains checks including a check that the start rev is not zero. I would have to split the low-level parsing from the validation feature of the current parser. I started doing this... but didn't know when to stop. (There are lots of things the current parser currently checks for, and canonicalizes, and so lots of possibilities for separating low-level parsing from higher-level processing.) I then thought writing the stripping-r0 code as a text manipulation would be "simpler" in the sense of a smaller change. > 2. For every path remove zero revision using svn_rangelist_remove() > 3. Build new svn:mergeinfo using svn_mergeinfo_to_string() So, yes, this is how it *should* be done, I agree. Maybe I will do it. - Julian
