[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


Chris Jerdonek <cjerdonek at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56212|review?                     |review+
               Flag|                            |




--- Comment #9 from Chris Jerdonek <cjerdonek at webkit.org>  2010-05-16 22:36:31 PST ---
(From update of attachment 56212)
Feel free to consider these nits and minor comments below before landing. r=me.


> +#     value: the last property value.  For instance, suppose the property is "Modified".
> +#            Then it has both a '-' and '+' property value in that order.  Therefore,
> +#            the value of this key is the value of the '+' property by ordering (since
> +#            it is the last-most value).

Nit: last instead of last-most.

> +    # FIXME: We do not support property values that contain tailing new line characters

trailing, newline (instead of new line)

> +    #        as it is difficult to disambiguate these trailing new lines from the empty

newline

> +    #        line that preceeds the contents of a binary patch.

precedes


> +    my $propertyValue;
> +    my $propertyValueType;
> +    while (defined($_) && /$svnPropertyValueStartRegEx/) {
> +        # Note, a '-' property may be followed by '+' property in the case of a "Modified"

a '+' property

> +    my $propertyChangeDelta;
> +    if ($propertyValueType eq '+') {
> +        $propertyChangeDelta = 1;
> +    } elsif ($propertyValueType eq '-') {
> +        $propertyChangeDelta = -1;
> +    } else {
> +        # Not reached.
> +        die;

Would die("Not reached.") be better?  Just in case. :)

> +    # New test
> +    diffName => "single-line '+' with trailing new line, followed by empty line and start of binary patch",

Did you do any tests for property value followed by start of next diff?

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