In this patch, I think it would make sense to recast the existing parseDiffHeader() as parseSvnDiffHeader() and then create a new parseDiffHeader() that calls the Git or SVN version based on the first line (whether it begins with "Index:" or "diff --git"). The unit tests for the new parseDiffHeader() can have enough tests just to ensure that it supports carrying over each diffHash key value correctly (since the more thorough tests will be in the tests for the Git and SVN versions). This patch can also delete the gitdiff2svndiff() subroutine.
Created attachment 54958 [details] Proposed patch
Attachment 54958 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:47: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:48: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:57: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:58: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:73: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:74: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:83: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:84: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:99: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:100: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:109: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:110: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:125: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:126: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:135: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:136: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:151: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:152: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:161: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:162: Line contains tab character. [whitespace/tab] [5] Total errors found: 20 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 54959 [details] Proposed patch 2 A minor change to the ChangeLog and a code comment. By the way, expect style errors for tabs in this patch. This is because diffs contain tab characters, and the unit tests contain sample diffs. Also marking cq- since this patch will require setting the svn:allow-tabs property.
Attachment 54959 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:47: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:48: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:57: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:58: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:73: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:74: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:83: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:84: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:99: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:100: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:109: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:110: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:125: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:126: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:135: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:136: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:151: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:152: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:161: Line contains tab character. [whitespace/tab] [5] WebKitTools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:162: Line contains tab character. [whitespace/tab] [5] Total errors found: 20 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 54959 [details] Proposed patch 2 > +# * Neither the name of Apple Computer, Inc. ("Apple") nor the names of > +# its contributors may be used to endorse or promote products derived > +# from this software without specific prior written permission. Just wanted to point out you reference Apple in the copyright here. r=me.
(In reply to comment #5) > (From update of attachment 54959 [details]) > > +# * Neither the name of Apple Computer, Inc. ("Apple") nor the names of > > +# its contributors may be used to endorse or promote products derived > > +# from this software without specific prior written permission. > > Just wanted to point out you reference Apple in the copyright here. > > r=me. Thanks, Dan. I preserved the original copyright and license text since the new file is mostly a copy of the original, though perhaps I can change that text since I'm the sole copyright owner of the file. :)
Comment on attachment 54959 [details] Proposed patch 2 Clearing flags on attachment: 54959 Committed r58739: <http://trac.webkit.org/changeset/58739>
All reviewed patches have been landed. Closing bug.