[webkit-reviews] review granted: [Bug 27323] Better support for non-Cygwin SVN on Windows : [Attachment 34584] More line-ending stripping for svn-create-patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Peter Kasting <pkasting at google.com>'s
request for review:
Bug 27323: Better support for non-Cygwin SVN on Windows
https://bugs.webkit.org/show_bug.cgi?id=27323

Attachment 34584: More line-ending stripping for svn-create-patch
https://bugs.webkit.org/attachment.cgi?id=34584&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -	   if (/^URL: (.+)/) {
> +	   if (/^URL: (.*\S+)\s*$/) {

A simpler way to write this is:

    (.+?)\s*$

But I guess it's OK as is.

I think stripping trailing whitespace is an unnecessarily subtle way to allow
for arbitrary line endings, and is the kind of thing that requires a comment or
function name to make it clear.

I haven't seen it before, although I assume we've been doing this elsewhere,
but with changes like this one we're making these scripts more subtle that I
don't think future maintainers will have a way of understanding what they need
to preserve or change. There's nothing in this regular expression that says
"line endings" to me.

r=me