Bug 33414 - Let webkit-patch work with options in $EDITOR
Summary: Let webkit-patch work with options in $EDITOR
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-08 18:12 PST by Adam Barth
Modified: 2010-01-10 21:11 PST (History)
2 users (show)

See Also:

Attachments
Patch (1.16 KB, patch)
2010-01-08 18:13 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (1.17 KB, patch)
2010-01-08 18:23 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (1.27 KB, patch)
2010-01-09 10:01 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-01-08 18:12:34 PST 
Let webkit-patch work with options in $EDITOR
Comment 1 Adam Barth 2010-01-08 18:13:33 PST 
Created attachment 46188 [details]
Patch
Comment 2 WebKit Review Bot 2010-01-08 18:18:33 PST 
Attachment 46188 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
WebKitTools/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2
Comment 3 Adam Barth 2010-01-08 18:23:14 PST 
Created attachment 46189 [details]
Patch
Comment 4 Adam Barth 2010-01-08 18:23:50 PST 
Interesting failure mode.  We're checking style before editing the changelog.  :)
Comment 5 Mark Rowe (bdash) 2010-01-09 03:32:47 PST 
This naive approach will do the wrong thing if the path to your editor’s binary happens to contain a space. <http://subversion.tigris.org/faq.html#svn-editor> talks about how SVN handles this.  Python’s shlex module may be useful to address this.
Comment 6 Adam Barth 2010-01-09 09:58:23 PST 
Comment on attachment 46189 [details]
Patch

Good point.  I looked around for a python module to help, but I couldn't find one.  shlex looks perfect.
Comment 7 Adam Barth 2010-01-09 10:01:41 PST 
Created attachment 46211 [details]
Patch
Comment 8 Eric Seidel (no email) 2010-01-09 15:48:17 PST 
Comment on attachment 46211 [details]
Patch

Sweet!  Seems we need a test case though.  One should be very easy to write, no?
Comment 9 Adam Barth 2010-01-09 16:44:50 PST 
It's hard to write a test case because this code is calling system APIs directly.  We could introduce another abstraction layer, but the point of this code is to be the abstraction layer around these APIs...
Comment 10 Darin Adler 2010-01-10 20:49:23 PST 
Comment on attachment 46211 [details]
Patch

rs=me
Comment 11 WebKit Commit Bot 2010-01-10 21:11:28 PST 
Comment on attachment 46211 [details]
Patch

Clearing flags on attachment: 46211

Committed r53063: <http://trac.webkit.org/changeset/53063>
Comment 12 WebKit Commit Bot 2010-01-10 21:11:35 PST 
All reviewed patches have been landed.  Closing bug.