[webkit-reviews] review granted: [Bug 33415] VCSUtils::gitdiff2svndiff() should accept strings ending in a newline : [Attachment 46191] Patch 2

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Chris Jerdonek
<chris.jerdonek at gmail.com>'s request for review:
Bug 33415: VCSUtils::gitdiff2svndiff() should accept strings ending in a
newline
https://bugs.webkit.org/show_bug.cgi?id=33415

Attachment 46191: Patch 2
https://bugs.webkit.org/attachment.cgi?id=46191&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
> Index: WebKitTools/Scripts/VCSUtils.pm
> +# Convert a line of a git-formatted patch to SVN format, while
> +# preserving any end-of-line characters.
>  sub gitdiff2svndiff($)
>  {
>      $_ = shift @_;
> -    if (m#^diff --git \w/(.+) \w/(.+)#) {
> -	   return "Index: $1";
> +    
> +    # \V is any character that is not vertical white space
> +    if (m#^diff --git \w/(.+) \w/(\V+)#) {
> +	   return "Index: $1$'";

I find that $' is pretty obscure.  Might be nice to "use English;" then use
$POSTMATCH instead of $'.

>      } elsif (m#^index [0-9a-f]{7}\.\.[0-9a-f]{7} [0-9]{6}#) {
> -	   return
"===================================================================";
> -    } elsif (m#^--- \w/(.+)#) {
> -	   return "--- $1";
> -    } elsif (m#^\+\+\+ \w/(.+)#) {
> -	   return "+++ $1";
> +	   return
"===================================================================$'";
> +    } elsif (m#^--- \w/(\V+)#) {
> +	   return "--- $1$'";
> +    } elsif (m#^\+\+\+ \w/(\V+)#) {
> +	   return "+++ $1$'";
>      }
>      return $_;
>  }

Technically we could just use if() statements instead of a large elsif() block
since there are return statements, but it's okay as-is.

r=me

You can decide if you want to set cq+ or if you want to address either of the
above issues.  :)