Bug 38812 – Clean up: Make regular expressions for parsing the start of an SVN and Git header global variables

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Daniel Bates]

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.

[Daniel Bates]

Created attachment 55494 [details]
Patch

I did not add any additional unit tests as our existing test coverage should be sufficient.

[Daniel Bates]

Created attachment 55495 [details]
Patch

Based on a Chris's suggestion, stored the regular expressions in global variables instead of encapsulating them in functions.

[Chris Jerdonek]

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.

[Daniel Bates]

(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.

[Daniel Bates]

Created attachment 55498 [details]
Patch

Updated patch based on Chris's suggestions.

[Chris Jerdonek]

Comment on attachment 55498 [details]
Patch

Looks good.  Thanks for this refactoring!

[Daniel Bates]

Comment on attachment 55498 [details]
Patch

Clearing flags on attachment: 55498

Committed r59051: <http://trac.webkit.org/changeset/59051>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.