[Webkit-unassigned] [Bug 39184] Add support for parsing a SVN property

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39184





--- Comment #8 from Chris Jerdonek <cjerdonek at webkit.org>  2010-05-16 22:22:11 PST ---
(In reply to comment #3)
> > I would rewrite this as "...if the first line is not a valid start of an
> > SVN property (e.g. if the value of the property is missing)."
> 
> I am unclear if you are implying we should lump together, for documentation purposes, the cases where we die when the first line is invalid and also when the property value is missing?

No, I didn't mean to lump them together.  You can ignore the comment.  I just didn't understand from what you wrote that you were also checking the validity of the later lines.

> The "svn diff" command does not insert newlines between property values or successive properties. So, these new lines would be part of the property value. This patch does not support property values that end with trailing newlines as it does not have support to disambiguate between these new lines and the new line that precedes the contents of a binary patch. I would suggest adding such support in a future patch. I'll add a FIXME to clarify this.

Okay.  Yes, it might help in case people leave trailing empty lines when setting the svn:ignore value.

> > > +    if ($propertyChangeDelta == -1 * $expectedChangeDelta) {
> > 
> > Why aren't you doing "if ($propertyChangeDelta != $expectedChangeDelta)"?
> 
> Take $propertyChangeDelta := 1, and $expectedChangeDelta := 0. Then we would die with a mismatch error.
> 
> Notice, $expectedChangeDelta is zero when the property type is "Name" or "Modified" since we cannot infer whether this is an addition or a removal of a property. Instead, we must inspect the +/- line(s) that follow this line in the diff.

Okay.  Then I would consider using undef instead of 0 when it cannot be inferred.

> Alternatively, we could write this as "if ($expectedChangeDelta && $propertyChangeDelta != $expectedChangeDelta)".

Yes, this makes it a bit clearer.  And then you can say defined($expectedChangeDelta) instead of $expectedChangeDelta if you switch to undef.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.