Bug 27973

Summary:

REGRESSION(r46700): bugzilla-tool land-diff double-spaces ChangeLogs

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ddkilzer, levin, vestbo

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Patch v1	abarth: review+

Description Eric Seidel (no email) 2009-08-04 01:29:29 PDT

REGRESSION: bugzilla-tool land-diff double-spaces ChangeLogs

An example of the badness:
http://trac.webkit.org/changeset/46750

I have no clue what broke this.  It must have been very recent, since I use land-diff to land all my changes...

Comment 1 Eric Seidel (no email) 2009-08-04 01:31:27 PDT

I wonder if it could be this change:
http://trac.webkit.org/changeset/46700/trunk/WebKitTools/Scripts/bugzilla-tool

I kinda doubt it though.

Comment 2 Eric Seidel (no email) 2009-08-04 01:43:01 PDT

It seems to be in the reviewer update code (not un-expected):

bugzilla-tool --dry-run land-diff --no-build --reviewer="Bozo" 

reproduces the bug.

Comment 3 Eric Seidel (no email) 2009-08-04 01:49:54 PDT

Ha!  http://trac.webkit.org/changeset/46700/trunk/WebKitTools/Scripts/bugzilla-tool is the regression!  Can you find it? ;)

Comment 4 Eric Seidel (no email) 2009-08-04 01:52:13 PDT

Created attachment 34047 [details]
Patch v1

Comment 5 Adam Barth 2009-08-04 01:56:23 PDT

Comment on attachment 34047 [details]
Patch v1

Yay.  Now if only we had better unit testing...

Comment 6 Eric Seidel (no email) 2009-08-04 01:59:38 PDT

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
Committed r46754
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
r46754 = 3d81d05269c879e0aac096ceee849d47735726dd (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46754

Comment 7 David Kilzer (:ddkilzer) 2009-08-04 07:42:39 PDT

Comment on attachment 34047 [details]
Patch v1

>+        The trailing comma (suppresses newlines) was lost in r46700.

Heh...I thought the trailing comma was a typo.  That's one of the weirdest language behaviors I've ever seen.

Comment 8 Eric Seidel (no email) 2009-08-04 08:43:41 PDT

The comma is the tuple operator in python.  So I assume print detects this by detecting if it's passed a list/tuple instead of a string.

http://mail.python.org/pipermail/python-list/2005-July/332230.html
talks more about this strange behavior.