[webkit-reviews] review requested: [Bug 33447] Create a function for svn-apply and svn-unapply to parse diff headers : [Attachment 46231] Proposed patch

[bugzilla-daemon at webkit.org]

Chris Jerdonek <chris.jerdonek at gmail.com> has asked  for review:
Bug 33447: Create a function for svn-apply and svn-unapply to parse diff
headers
https://bugs.webkit.org/show_bug.cgi?id=33447

Attachment 46231: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=46231&action=review

------- Additional Comments from Chris Jerdonek <chris.jerdonek at gmail.com>
cq- since parseDiffHeader.pl probably needs the allow-tabs property.

I have a couple questions regarding this patch that maybe you can help me with:


(1) Is the last line of this code ever needed in svn-apply (I have included
similar code again in my patch):

> if ($indexPath) {
>     # Fix paths on diff, ---, and +++ lines to match preceding Index: line.
>     s/\S+$/$indexPath/ if /^diff/;

Note that $_ is Git-formatted by the time this line executes.

(2) In svn-apply, we extract the "version" of a diff by matching with--

> /^--- .+\(revision (\d+)\)$/

For consistency, is there any reason not to also record this version number in
the case of a file move (in which case there is also a "from" clause after the
"revision" portion), e.g.--

> --- WebKitTools/Scripts/webkitpy/move_test.py (revision 53048) (from
WebKitTools/Scripts/webkitpy/initial.py:53048)

(3) Finally, in the case of a file move, can the "revision" number ever be
different from the revision number that appears in the "from" clause portion?