39208 2010-05-17 01:37:25 -0700 webkit-patch needs --verbose flag to enable DEBUG logging 2010-05-24 01:32:39 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 eric webkit-unassigned abarth cjerdonek ojan oldest_to_newest 226681 0 eric 2010-05-17 01:37:25 -0700 webkit-patch needs --verbose flag to enable DEBUG logging 226683 1 56222 eric 2010-05-17 01:40:13 -0700 Created attachment 56222 Patch 226699 2 56222 cjerdonek 2010-05-17 02:09:02 -0700 Comment on attachment 56222 Patch Some thoughts to consider prior to landing. r=me > + is_verbose = "-v" in sys.argv or "--verbose" in sys.argv I wonder if there is a nice way to avoid writing sys.argv twice -- e.g. Python set intersection? > + def _command_for_printing(self, args): > + """Used for producing a print-ready string representing command args. > + The string should be copy/paste ready for execution in a shell.""" PEP8 says the doc string for functions should begin with an imperative word, e.g. "Return a print-ready string...." PEP8 also has pretty specific rules about the formatting of doc strings. This one would look something like this-- def _command_for_printing(self, args): """Return a print-ready string that represents command args. The string should be copy/paste ready for execution in a shell. """ > + if isinstance(arg, unicode): > + # Escape any non-ascii characters for easy copy/paste > + arg = arg.encode("unicode_escape") > + # quotes? What does the "quotes?" comment mean -- is that a FIXME? > > + _log.debug("\"%s\" took %.2fs" % (self._command_for_printing(args), time.time() - start_time)) I would use single quotes to avoid escaping the double quotes. 229649 3 eric 2010-05-24 01:27:41 -0700 Addressed Chris's comments. landing. 229651 4 eric 2010-05-24 01:32:39 -0700 Committed r60066: <http://trac.webkit.org/changeset/60066> 56222 2010-05-17 01:40:13 -0700 2010-05-17 02:09:02 -0700 Patch bug-39208-20100517014012.patch text/plain 4976 eric ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA0MGJkMTZlMDdhNGJjMGNhOTJmYjZjYmFjYzYxMWNkNTczODY2MGZkLi5lYTY0ZDg5 MzRhYzU2OWExNzg2Y2ZjYTEyZjkyM2E0OTQxMmE4MmI3IDEwMDY0NAotLS0gYS9XZWJLaXRUb29s cy9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAK KzIwMTAtMDUtMTcgIEVyaWMgU2VpZGVsICA8ZXJpY0B3ZWJraXQub3JnPgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIHdlYmtpdC1wYXRjaCBuZWVkcyAt LXZlcmJvc2UgZmxhZyB0byBlbmFibGUgREVCVUcgbG9nZ2luZworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzkyMDgKKworICAgICAgICBJIGFsc28gYWRk ZWQgc29tZSBjb2RlIHRvIHByaW50IG91dCBob3cgbG9uZyBjb21tYW5kcyB0YWtlIHRvIHJ1bi4K KworICAgICAgICAqIFNjcmlwdHMvd2Via2l0LXBhdGNoOgorICAgICAgICAgLSBBZGQgaGFja2lz aCAtdi8tLXZlcmJvc2UgcGFyc2luZyAoc2ltaWxhciB0byBjaGVjay13ZWJraXQtc3R5bGUpCisg ICAgICAgICogU2NyaXB0cy93ZWJraXRweS9jb21tb24vc3lzdGVtL2V4ZWN1dGl2ZS5weToKKyAg ICAgICAgIC0gTG9nIGhvdyBsb25nIGNvbW1hbmRzIHRha2UgdG8gcnVuLgorICAgICAgICAqIFNj cmlwdHMvd2Via2l0cHkvdG9vbC9tYWluLnB5OgorICAgICAgICAgLSBBZGQgLXYvLS12ZXJib3Nl IG9wdGlvbiB0byBnbG9iYWwgb3B0aW9ucy4KKwogMjAxMC0wNS0xNiAgUm9iZXJ0IEhvZ2FuICA8 cm9iZXJ0QHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgS2VubmV0aCBSb2hkZSBD aHJpc3RpYW5zZW4uCmRpZmYgLS1naXQgYS9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdC1wYXRj aCBiL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0LXBhdGNoCmluZGV4IGUwMTcwZWRhMGQ2NGVj YTM5OGI0OWJiNGQyM2ZkODk4MGM1ZDc0ZDYuLmM3ZGEyNjY4Yzk5YWE4M2Q5Y2QxODhiMDBiZTE0 MTc0YjE4NDE5NWMgMTAwNzU1Ci0tLSBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0LXBhdGNo CisrKyBiL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0LXBhdGNoCkBAIC0zMSw2ICszMSw3IEBA CiAjCiAjIEEgdG9vbCBmb3IgYXV0b21hdGluZyBkZWFsaW5nIHdpdGggYnVnemlsbGEsIHBvc3Rp bmcgcGF0Y2hlcywgY29tbWl0dGluZyBwYXRjaGVzLCBldGMuCiAKK2ltcG9ydCBsb2dnaW5nCiBp bXBvcnQgb3MKIGltcG9ydCBzeXMKIApAQCAtMzksNyArNDAsMTUgQEAgaW1wb3J0IHdlYmtpdHB5 LnB5dGhvbjI0LnZlcnNpb25pbmcgYXMgdmVyc2lvbmluZwogCiAKIGRlZiBtYWluKCk6Ci0gICAg Y29uZmlndXJlX2xvZ2dpbmcoKQorICAgICMgVGhpcyBpcyBhIGhhY2sgdG8gbGV0IHVzIGVuYWJs ZSBERUJVRyBsb2dnaW5nIGFzIGVhcmx5IGFzIHBvc3NpYmxlLgorICAgIGlzX3ZlcmJvc2UgPSAi LXYiIGluIHN5cy5hcmd2IG9yICItLXZlcmJvc2UiIGluIHN5cy5hcmd2CisgICAgIyBOb3RlIHRo aXMgY2FuJ3QgYmUgdGVybmFyeSBhcyB2ZXJzaW9uaW5nLmNoZWNrX3ZlcnNpb24oKQorICAgICMg aGFzbid0IHJ1biB5ZXQgYW5kIHRoaXMgcHl0aG9uIG1pZ2h0IGJlIG9sZGVyIHRoYW4gMi41Lgor ICAgIGlmIGlzX3ZlcmJvc2U6CisgICAgICAgIGxvZ2dpbmdfbGV2ZWwgPSBsb2dnaW5nLkRFQlVH CisgICAgZWxzZToKKyAgICAgICAgbG9nZ2luZ19sZXZlbCA9IGxvZ2dpbmcuSU5GTworICAgIGNv bmZpZ3VyZV9sb2dnaW5nKGxvZ2dpbmdfbGV2ZWw9bG9nZ2luZ19sZXZlbCkKIAogICAgIHZlcnNp b25pbmcuY2hlY2tfdmVyc2lvbigpCiAKZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL1NjcmlwdHMv d2Via2l0cHkvY29tbW9uL3N5c3RlbS9leGVjdXRpdmUucHkgYi9XZWJLaXRUb29scy9TY3JpcHRz L3dlYmtpdHB5L2NvbW1vbi9zeXN0ZW0vZXhlY3V0aXZlLnB5CmluZGV4IGM3YTdhZWMyNGNkMTA1 YTE4ODBkMzYxMjQ3MGE3NTYzYjkyN2JmMTQuLjFiMjNjYzNmM2I5ZGUxMWIwZjBkZjdkMTU4MGYz MjFlNzMyNDIxNTcgMTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29t bW9uL3N5c3RlbS9leGVjdXRpdmUucHkKKysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRw eS9jb21tb24vc3lzdGVtL2V4ZWN1dGl2ZS5weQpAQCAtNDEsNiArNDEsNyBAQCBpbXBvcnQgU3Ry aW5nSU8KIGltcG9ydCBzaWduYWwKIGltcG9ydCBzdWJwcm9jZXNzCiBpbXBvcnQgc3lzCitpbXBv cnQgdGltZQogCiBmcm9tIHdlYmtpdHB5LmNvbW1vbi5zeXN0ZW0uZGVwcmVjYXRlZF9sb2dnaW5n IGltcG9ydCB0ZWUKIApAQCAtMjU1LDYgKzI1NiwxOCBAQCBjbGFzcyBFeGVjdXRpdmUob2JqZWN0 KToKICAgICAgICAgICAgIGlucHV0ID0gaW5wdXQuZW5jb2RlKCJ1dGYtOCIpCiAgICAgICAgIHJl dHVybiAoc3VicHJvY2Vzcy5QSVBFLCBpbnB1dCkKIAorICAgIGRlZiBfY29tbWFuZF9mb3JfcHJp bnRpbmcoc2VsZiwgYXJncyk6CisgICAgICAgICIiIlVzZWQgZm9yIHByb2R1Y2luZyBhIHByaW50 LXJlYWR5IHN0cmluZyByZXByZXNlbnRpbmcgY29tbWFuZCBhcmdzLgorICAgICAgICBUaGUgc3Ry aW5nIHNob3VsZCBiZSBjb3B5L3Bhc3RlIHJlYWR5IGZvciBleGVjdXRpb24gaW4gYSBzaGVsbC4i IiIKKyAgICAgICAgZXNjYXBlZF9hcmdzID0gW10KKyAgICAgICAgZm9yIGFyZyBpbiBhcmdzOgor ICAgICAgICAgICAgaWYgaXNpbnN0YW5jZShhcmcsIHVuaWNvZGUpOgorICAgICAgICAgICAgICAg ICMgRXNjYXBlIGFueSBub24tYXNjaWkgY2hhcmFjdGVycyBmb3IgZWFzeSBjb3B5L3Bhc3RlCisg ICAgICAgICAgICAgICAgYXJnID0gYXJnLmVuY29kZSgidW5pY29kZV9lc2NhcGUiKQorICAgICAg ICAgICAgIyBxdW90ZXM/CisgICAgICAgICAgICBlc2NhcGVkX2FyZ3MuYXBwZW5kKGFyZykKKyAg ICAgICAgcmV0dXJuICIgIi5qb2luKGVzY2FwZWRfYXJncykKKwogICAgICMgRklYTUU6IHJ1bl9h bmRfdGhyb3dfaWZfZmFpbCBzaG91bGQgYmUgbWVyZ2VkIGludG8gdGhpcyBtZXRob2QuCiAgICAg ZGVmIHJ1bl9jb21tYW5kKHNlbGYsCiAgICAgICAgICAgICAgICAgICAgIGFyZ3MsCkBAIC0yNjUs NiArMjc4LDcgQEAgY2xhc3MgRXhlY3V0aXZlKG9iamVjdCk6CiAgICAgICAgICAgICAgICAgICAg IHJldHVybl9zdGRlcnI9VHJ1ZSwKICAgICAgICAgICAgICAgICAgICAgZGVjb2RlX291dHB1dD1U cnVlKToKICAgICAgICAgIiIiUG9wZW4gd3JhcHBlciBmb3IgY29udmVuaWVuY2UgYW5kIHRvIHdv cmsgYXJvdW5kIHB5dGhvbiBidWdzLiIiIgorICAgICAgICBzdGFydF90aW1lID0gdGltZS50aW1l KCkKICAgICAgICAgYXJncyA9IG1hcCh1bmljb2RlLCBhcmdzKSAgIyBQb3BlbiB3aWxsIHRocm93 IGFuIGV4Y2VwdGlvbiBpZiBhcmdzIGFyZSBub24tc3RyaW5ncyAobGlrZSBpbnQoKSkKICAgICAg ICAgc3RkaW4sIHN0cmluZ190b19jb21tdW5pY2F0ZSA9IHNlbGYuX2NvbXB1dGVfc3RkaW4oaW5w dXQpCiAgICAgICAgIHN0ZGVyciA9IHN1YnByb2Nlc3MuU1RET1VUIGlmIHJldHVybl9zdGRlcnIg ZWxzZSBOb25lCkBAIC0yODMsNiArMjk3LDggQEAgY2xhc3MgRXhlY3V0aXZlKG9iamVjdCk6CiAg ICAgICAgICMgaHR0cDovL2J1Z3MucHl0aG9uLm9yZy9pc3N1ZTE3MzE3MTcKICAgICAgICAgZXhp dF9jb2RlID0gcHJvY2Vzcy53YWl0KCkKIAorICAgICAgICBfbG9nLmRlYnVnKCJcIiVzXCIgdG9v ayAlLjJmcyIgJSAoc2VsZi5fY29tbWFuZF9mb3JfcHJpbnRpbmcoYXJncyksIHRpbWUudGltZSgp IC0gc3RhcnRfdGltZSkpCisKICAgICAgICAgaWYgcmV0dXJuX2V4aXRfY29kZToKICAgICAgICAg ICAgIHJldHVybiBleGl0X2NvZGUKIApkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvU2NyaXB0cy93 ZWJraXRweS90b29sL21haW4ucHkgYi9XZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L3Rvb2wv bWFpbi5weQppbmRleCAyZGMxNzdkOWQ0NzQzYjRmYTJkMDIyN2ZlMjMzZjQ2MzY5ZWJjYzFlLi4x ZjQzMTQ1NGU4ZGZlNjI5MGE4YmU1OGJhZmNjOGJkOTYyYjJlYTgyIDEwMDc1NQotLS0gYS9XZWJL aXRUb29scy9TY3JpcHRzL3dlYmtpdHB5L3Rvb2wvbWFpbi5weQorKysgYi9XZWJLaXRUb29scy9T Y3JpcHRzL3dlYmtpdHB5L3Rvb2wvbWFpbi5weQpAQCAtNTYsNiArNTYsNyBAQCBmcm9tIHdlYmtp dHB5LmNvbW1vbi5zeXN0ZW0uZGVwcmVjYXRlZF9sb2dnaW5nIGltcG9ydCBsb2cKIAogY2xhc3Mg V2ViS2l0UGF0Y2goTXVsdGlDb21tYW5kVG9vbCk6CiAgICAgZ2xvYmFsX29wdGlvbnMgPSBbCisg ICAgICAgIG1ha2Vfb3B0aW9uKCItdiIsICItLXZlcmJvc2UiLCBhY3Rpb249InN0b3JlX3RydWUi LCBkZXN0PSJ2ZXJib3NlIiwgZGVmYXVsdD1GYWxzZSwgaGVscD0iZW5hYmxlIGFsbCBsb2dnaW5n IiksCiAgICAgICAgIG1ha2Vfb3B0aW9uKCItLWRyeS1ydW4iLCBhY3Rpb249InN0b3JlX3RydWUi LCBkZXN0PSJkcnlfcnVuIiwgZGVmYXVsdD1GYWxzZSwgaGVscD0iZG8gbm90IHRvdWNoIHJlbW90 ZSBzZXJ2ZXJzIiksCiAgICAgICAgIG1ha2Vfb3B0aW9uKCItLXN0YXR1cy1ob3N0IiwgYWN0aW9u PSJzdG9yZSIsIGRlc3Q9InN0YXR1c19ob3N0IiwgdHlwZT0ic3RyaW5nIiwgbmFyZ3M9MSwgaGVs cD0iSG9zdG5hbWUgKGUuZy4gbG9jYWxob3N0IG9yIGNvbW1pdC53ZWJraXQub3JnKSB3aGVyZSBz dGF0dXMgdXBkYXRlcyBzaG91bGQgYmUgcG9zdGVkLiIpLAogICAgICAgICBtYWtlX29wdGlvbigi LS1pcmMtcGFzc3dvcmQiLCBhY3Rpb249InN0b3JlIiwgZGVzdD0iaXJjX3Bhc3N3b3JkIiwgdHlw ZT0ic3RyaW5nIiwgbmFyZ3M9MSwgaGVscD0iUGFzc3dvcmQgdG8gdXNlIHdoZW4gY29tbXVuaWNh dGluZyB2aWEgSVJDLiIpLAo=