[Webkit-unassigned] [Bug 61162] REGRESSION (r86515): svn-apply ignores diffs that omit line count in chunk range

[bugzilla-daemon at webkit.org]

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


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #94252|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #12 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2011-05-24 15:54:55 PST ---
(From update of attachment 94252)
View in context: https://bugs.webkit.org/attachment.cgi?id=94252&action=review

r=me  Thanks Dan!

> Tools/ChangeLog:10
> +        not match a chunk range line that omits a line count. GNU patch(1) will omit the

Change "patch(1)" to "diff(1)" here as discussed in IRC.

> Tools/Scripts/VCSUtils.pm:504
> +    my $chunkRangeRegEx = qr#^\@\@ -(\d+)(,(\d+))? \+(\d+)(,(\d+))? \@\@#; # e.g. "@@ -2,6 +2,18 @@" or "@@ -2,6 +2,18 @@ foo()"

Let's move the "e.g." example text into the header for the method, and document the GNU diff(1) variables (for both the starting and ending ranges).

> Tools/Scripts/VCSUtils.pm:937
> +            $numTextChunks += $isChunkRange;

I would prefer it if you made the boolean-to-integer conversion here more explicit:

    $numTextChunks += 1 if $isChunkRange;

> Tools/Scripts/webkitperl/VCSUtils_unittest/parseChunkRange.pl:44
> +###
> +# Invalid and malformed chunk range
> +##
> +# FIXME: We should make this set of tests more comprehensive.

Is the following a valid range?

@@ -1 +1 @@

We should add a test for this (whether or not it's invalid).  Otherwise, the test coverage looks good.

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