Bug 38812 - Clean up: Make regular expressions for parsing the start of an SVN and Git header global variables
Summary: Clean up: Make regular expressions for parsing the start of an SVN and Git he...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-09 01:12 PDT by Daniel Bates
Modified: 2010-05-09 02:50 PDT (History)
2 users (show)

See Also:


Attachments
Patch (5.41 KB, patch)
2010-05-09 01:15 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (3.90 KB, patch)
2010-05-09 02:28 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (4.35 KB, patch)
2010-05-09 02:45 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-05-09 01:12:19 PDT
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.
Comment 1 Daniel Bates 2010-05-09 01:15:12 PDT
Created attachment 55494 [details]
Patch

I did not add any additional unit tests as our existing test coverage should be sufficient.
Comment 2 Daniel Bates 2010-05-09 02:28:37 PDT
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 3 Chris Jerdonek 2010-05-09 02:38:29 PDT
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.
Comment 4 Daniel Bates 2010-05-09 02:41:05 PDT
(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.
Comment 5 Daniel Bates 2010-05-09 02:45:05 PDT
Created attachment 55498 [details]
Patch

Updated patch based on Chris's suggestions.
Comment 6 Chris Jerdonek 2010-05-09 02:47:34 PDT
Comment on attachment 55498 [details]
Patch

Looks good.  Thanks for this refactoring!
Comment 7 Daniel Bates 2010-05-09 02:50:39 PDT
Comment on attachment 55498 [details]
Patch

Clearing flags on attachment: 55498

Committed r59051: <http://trac.webkit.org/changeset/59051>
Comment 8 Daniel Bates 2010-05-09 02:50:47 PDT
All reviewed patches have been landed.  Closing bug.