We use the regular expressions qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)# and qr#^Index: ([^\r\n]+)# or some form of these expressions in various places throughout VCUtils.pm. We should centralize the parsing of these lines by creating functions that parse the start of Git header and SVN header, respectively.
Created attachment 55494 [details] Patch I did not add any additional unit tests as our existing test coverage should be sufficient.
Created attachment 55495 [details] Patch Based on a Chris's suggestion, stored the regular expressions in global variables instead of encapsulating them in functions.
Comment on attachment 55495 [details] Patch > +++ WebKitTools/ChangeLog (working copy) > + Extract the regular expressions for parsing the start of an SVN and > + Git header into their own respective functions so as to centralize > + these regular expressions as they are used throughout VCSUtils.pm. As we discussed, this needs to be updated. > +++ WebKitTools/Scripts/VCSUtils.pm (working copy) > +my $gitHeaderStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#; > +my $svnHeaderStartRegEx = qr#^Index: ([^\r\n]+)#; Might it be more straightforward to call these gitDiffStartRegEx and svnDiffStartRegEx? It's probably best to re-post after this since the latter involves a bunch of changes.
(In reply to comment #3) > (From update of attachment 55495 [details]) > > +++ WebKitTools/ChangeLog (working copy) > > + Extract the regular expressions for parsing the start of an SVN and > > + Git header into their own respective functions so as to centralize > > + these regular expressions as they are used throughout VCSUtils.pm. > > As we discussed, this needs to be updated. Will change. > > > +++ WebKitTools/Scripts/VCSUtils.pm (working copy) > > +my $gitHeaderStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#; > > +my $svnHeaderStartRegEx = qr#^Index: ([^\r\n]+)#; > > Might it be more straightforward to call these gitDiffStartRegEx and svnDiffStartRegEx? Will change. > > It's probably best to re-post after this since the latter involves a bunch of changes. Will repost with changes.
Created attachment 55498 [details] Patch Updated patch based on Chris's suggestions.
Comment on attachment 55498 [details] Patch Looks good. Thanks for this refactoring!
Comment on attachment 55498 [details] Patch Clearing flags on attachment: 55498 Committed r59051: <http://trac.webkit.org/changeset/59051>
All reviewed patches have been landed. Closing bug.