31248 2009-11-08 19:11:52 -0800 Tab width for javascript source is 8, should be 4 2010-02-05 04:47:25 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 aparajita webkit-unassigned bweinstein commit-queue joepeck keishi pfeldman pmuellr rik timothy webkit.review.bot oldest_to_newest 161477 0 aparajita 2009-11-08 19:11:52 -0800 Currently the tab width for the javascript source display is 8 spaces. These days most people use 4 spaces in their editors, so perhaps it should be 4 in the source display. 168802 1 timothy 2009-12-04 09:37:37 -0800 I agree 168837 2 pmuellr 2009-12-04 10:41:54 -0800 I briefly looked into this at one point. I didn't see that we supported any kind of tab expansion setting in WebKit's CSS. There were a couple of proposals to in CSS spec-land to set tab expansion values, but they all seemed kind of dead-ish when I remember looking at them. I suppose we could expand the tabs ourselves. No fun, but not very hard either. It would be nice to do something, as tabs == 8 spaces is so 1970's ... 187372 3 aparajita 2010-02-03 08:17:48 -0800 I should be noted that FireBug uses 4 spaces for tabs. 187963 4 48176 aparajita 2010-02-04 15:26:39 -0800 Created attachment 48176 Patch to convert tabs to 4 spaces 187964 5 aparajita 2010-02-04 15:27:27 -0800 It was an easy fix, patch is attached. Hope this makes it into the source. 187986 6 timothy 2010-02-04 15:57:09 -0800 You should make a ChangeLog entry so we can land this. Looks good though! http://webkit.org/coding/contributing.html 187991 7 48176 pfeldman 2010-02-04 16:05:27 -0800 Comment on attachment 48176 Patch to convert tabs to 4 spaces Text editor does not support tabs yet. I'd like to keep it (and its model) operating real characters with no tab substitutions for now. There are good reasons for your change though, so if you are to make this replace, please do it in the SourceFrame.js (setContent method). Please also follow the patch submit guidelines (your change is missing ChangeLog entry). 188038 8 48187 aparajita 2010-02-04 18:03:29 -0800 Created attachment 48187 Proposed patch 188039 9 48188 aparajita 2010-02-04 18:04:30 -0800 Created attachment 48188 Proposed patch 188040 10 aparajita 2010-02-04 18:06:08 -0800 Okay, here is a complete patch with ChangeLog, and the mod was moved to SourceFrame#setContent 188042 11 webkit.review.bot 2010-02-04 18:08:42 -0800 Attachment 48188 did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 If any of these errors are false positives, please file a bug against check-webkit-style. 188072 12 pmuellr 2010-02-04 22:19:57 -0800 The patch is doing a blind replacement of \t with 4 space characters; but that's not how tabs actually work; they add a variable # of spaces to get the following characters to align on the tab-width boundaries. For example, if the original text looks is this, where - is the tab character ---------------- -a-b-c -d -e -f ---------------- then the text will be displayed like this: ---------------- a b c d e f ---------------- The following original text, again, with - as the tab character, yields the same display as above; note there is a space before each tab character: ---------------- -a-b-c -d -e -f ---------------- However, if we use the "\t == 4 spaces" replacement, the expanded text becomes this: ---------------- a b c d e f ---------------- In practice, if people consistently use tabs just at the beginnings of lines, and never accidently inject spaces, then the expansion will work fine. In my practice, when I used to use tabs in source files, I found that I frequently accidently mixed spaces with tabs; everything LOOKED ok in the editor. But using the "\t == 4" spaces replacement would have ended up with the unaligned output as above. 188101 13 48208 pfeldman 2010-02-05 00:01:18 -0800 Created attachment 48208 [PATCH] Proposed change. 188103 14 pfeldman 2010-02-05 00:04:43 -0800 (In reply to comment #10) > Okay, here is a complete patch with ChangeLog, and the mod was moved to > SourceFrame#setContent I added a setting into the editor model and made a correct tab substitution based on the character offset instead. @aparajita sorry for stealing your bug / patch. We've got lots of stuff to do though, so if you have spare cycles / are interested in doing something, jump into #webkit-inspector IRC channel and talk to us! 188126 15 48208 commit-queue 2010-02-05 02:50:48 -0800 Comment on attachment 48208 [PATCH] Proposed change. Clearing flags on attachment: 48208 Committed r54415: <http://trac.webkit.org/changeset/54415> 188127 16 commit-queue 2010-02-05 02:50:59 -0800 All reviewed patches have been landed. Closing bug. 188150 17 aparajita 2010-02-05 04:47:25 -0800 @pavel don't be sorry, I'm happy I pushed you guys to fix this. ;-) 48176 2010-02-04 15:26:39 -0800 2010-02-04 18:03:29 -0800 Patch to convert tabs to 4 spaces tabfix.diff text/plain 659 aparajita SW5kZXg6IFRleHRFZGl0b3JNb2RlbC5qcwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUZXh0RWRpdG9yTW9kZWwu anMJKHJldmlzaW9uIDU0MzQxKQorKysgVGV4dEVkaXRvck1vZGVsLmpzCSh3b3JraW5nIGNvcHkp CkBAIC0xMDMsNyArMTAzLDcgQEAKICAgICAgICAgaWYgKHRleHQgPT09ICIiKQogICAgICAgICAg ICAgcmV0dXJuIG5ldyBXZWJJbnNwZWN0b3IuVGV4dFJhbmdlKHJhbmdlLnN0YXJ0TGluZSwgcmFu Z2Uuc3RhcnRDb2x1bW4sIHJhbmdlLnN0YXJ0TGluZSwgcmFuZ2Uuc3RhcnRDb2x1bW4pOwogCi0g ICAgICAgIHZhciBuZXdMaW5lcyA9IHRleHQuc3BsaXQoIlxuIik7CisgICAgICAgIHZhciBuZXdM aW5lcyA9IHRleHQucmVwbGFjZSgvXHQvZywgIiAgICAiKS5zcGxpdCgiXG4iKTsKICAgICAgICAg dmFyIHByZWZpeCA9IHRoaXMuX2xpbmVzW3JhbmdlLnN0YXJ0TGluZV0uc3Vic3RyaW5nKDAsIHJh bmdlLnN0YXJ0Q29sdW1uKTsKICAgICAgICAgdmFyIHByZWZpeEFyZ3VtZW50cyA9IHRoaXMuX2Fy Z3VtZW50cwogICAgICAgICB2YXIgc3VmZml4ID0gdGhpcy5fbGluZXNbcmFuZ2Uuc3RhcnRMaW5l XS5zdWJzdHJpbmcocmFuZ2Uuc3RhcnRDb2x1bW4pOwo= 48187 2010-02-04 18:03:29 -0800 2010-02-04 18:04:30 -0800 Proposed patch tabfix.diff text/plain 1268 aparajita SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NDM5MikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDItMDQgIEFwYXJhaml0YSBGaXNobWFuICA8YXBhcmFqaXRh QGFwYXJhaml0YS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgQnVnIDMxMjQ4IC0gVGFiIHdpZHRoIGZvciBqYXZhc2NyaXB0IHNvdXJjZSBpcyA4 LCBzaG91bGQgYmUgNAorCQlodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MzEyNDgKKworICAgICAgICBObyBuZXcgdGVzdHMsIGV4aXN0aW5nIGluc3BlY3RvciB0ZXN0IHBh c3Nlcy4KKworICAgICAgICAqIGluc3BlY3Rvci9mcm9udC1lbmQvU291cmNlRnJhbWUuanM6Cisg ICAgICAgIChXZWJJbnNwZWN0b3IuU291cmNlRnJhbWUucHJvdG90eXBlLnNldENvbnRlbnQpOgor CiAyMDEwLTAyLTA0ICBQZXRlciBLYXN0aW5nICA8cGthc3RpbmdAZ29vZ2xlLmNvbT4KIAogICAg ICAgICBOb3QgcmV2aWV3ZWQsIHJvbGxiYWNrLgpJbmRleDogV2ViQ29yZS9pbnNwZWN0b3IvZnJv bnQtZW5kL1NvdXJjZUZyYW1lLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvaW5zcGVjdG9yL2Zy b250LWVuZC9Tb3VyY2VGcmFtZS5qcwkocmV2aXNpb24gNTQzNDEpCisrKyBXZWJDb3JlL2luc3Bl Y3Rvci9mcm9udC1lbmQvU291cmNlRnJhbWUuanMJKHdvcmtpbmcgY29weSkKQEAgLTEyNiw3ICsx MjYsNyBAQCBXZWJJbnNwZWN0b3IuU291cmNlRnJhbWUucHJvdG90eXBlID0gewogICAgIHNldENv bnRlbnQ6IGZ1bmN0aW9uKG1pbWVUeXBlLCBjb250ZW50LCB1cmwpCiAgICAgewogICAgICAgICB0 aGlzLl9sb2FkZWQgPSB0cnVlOwotICAgICAgICB0aGlzLl90ZXh0TW9kZWwuc2V0VGV4dChudWxs LCBjb250ZW50KTsKKyAgICAgICAgdGhpcy5fdGV4dE1vZGVsLnNldFRleHQobnVsbCwgY29udGVu dC5yZXBsYWNlKC9cdC9nLCAnICAgICcpKTsKICAgICAgICAgdGhpcy5fbWltZVR5cGUgPSBtaW1l VHlwZTsKICAgICAgICAgdGhpcy5fdXJsID0gdXJsOwogICAgICAgICB0aGlzLl9jcmVhdGVFZGl0 b3JJZk5lZWRlZCgpOwo= 48188 2010-02-04 18:04:30 -0800 2010-02-05 00:01:18 -0800 Proposed patch tabfix.diff text/plain 1268 aparajita SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NDM5MikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDItMDQgIEFwYXJhaml0YSBGaXNobWFuICA8YXBhcmFqaXRh QGFwYXJhaml0YS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgQnVnIDMxMjQ4IC0gVGFiIHdpZHRoIGZvciBqYXZhc2NyaXB0IHNvdXJjZSBpcyA4 LCBzaG91bGQgYmUgNAorCQlodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MzEyNDgKKworICAgICAgICBObyBuZXcgdGVzdHMsIGV4aXN0aW5nIGluc3BlY3RvciB0ZXN0IHBh c3Nlcy4KKworICAgICAgICAqIGluc3BlY3Rvci9mcm9udC1lbmQvU291cmNlRnJhbWUuanM6Cisg ICAgICAgIChXZWJJbnNwZWN0b3IuU291cmNlRnJhbWUucHJvdG90eXBlLnNldENvbnRlbnQpOgor CiAyMDEwLTAyLTA0ICBQZXRlciBLYXN0aW5nICA8cGthc3RpbmdAZ29vZ2xlLmNvbT4KIAogICAg ICAgICBOb3QgcmV2aWV3ZWQsIHJvbGxiYWNrLgpJbmRleDogV2ViQ29yZS9pbnNwZWN0b3IvZnJv bnQtZW5kL1NvdXJjZUZyYW1lLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvaW5zcGVjdG9yL2Zy b250LWVuZC9Tb3VyY2VGcmFtZS5qcwkocmV2aXNpb24gNTQzNDEpCisrKyBXZWJDb3JlL2luc3Bl Y3Rvci9mcm9udC1lbmQvU291cmNlRnJhbWUuanMJKHdvcmtpbmcgY29weSkKQEAgLTEyNiw3ICsx MjYsNyBAQCBXZWJJbnNwZWN0b3IuU291cmNlRnJhbWUucHJvdG90eXBlID0gewogICAgIHNldENv bnRlbnQ6IGZ1bmN0aW9uKG1pbWVUeXBlLCBjb250ZW50LCB1cmwpCiAgICAgewogICAgICAgICB0 aGlzLl9sb2FkZWQgPSB0cnVlOwotICAgICAgICB0aGlzLl90ZXh0TW9kZWwuc2V0VGV4dChudWxs LCBjb250ZW50KTsKKyAgICAgICAgdGhpcy5fdGV4dE1vZGVsLnNldFRleHQobnVsbCwgY29udGVu dC5yZXBsYWNlKC9cdC9nLCAnICAgICcpKTsKICAgICAgICAgdGhpcy5fbWltZVR5cGUgPSBtaW1l VHlwZTsKICAgICAgICAgdGhpcy5fdXJsID0gdXJsOwogICAgICAgICB0aGlzLl9jcmVhdGVFZGl0 b3JJZk5lZWRlZCgpOwo= 48208 2010-02-05 00:01:18 -0800 2010-02-05 02:50:47 -0800 [PATCH] Proposed change. tab_size text/plain 3174 pfeldman ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg Y2Q1YTFkMC4uY2JiZGUxMiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMC0wMi0wNCAgUGF2ZWwgRmVsZG1h biAgPHBmZWxkbWFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBXZWIgSW5zcGVjdG9yOiBUYWIgd2lkdGggZm9yIGphdmFzY3Jp cHQgc291cmNlIGlzIDgsIHNob3VsZCBiZSA0CisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTMxMjQ4CisKKyAgICAgICAgKiBpbnNwZWN0b3IvZnJvbnQt ZW5kL1NvdXJjZUZyYW1lLmpzOgorICAgICAgICAoV2ViSW5zcGVjdG9yLlNvdXJjZUZyYW1lKToK KyAgICAgICAgKiBpbnNwZWN0b3IvZnJvbnQtZW5kL1RleHRFZGl0b3JNb2RlbC5qczoKKyAgICAg ICAgKFdlYkluc3BlY3Rvci5UZXh0RWRpdG9yTW9kZWwucHJvdG90eXBlLnNldCByZXBsYWNlVGFi c1dpdGhTcGFjZXMpOgorICAgICAgICAoV2ViSW5zcGVjdG9yLlRleHRFZGl0b3JNb2RlbC5wcm90 b3R5cGUuX2lubmVyU2V0VGV4dCk6CisgICAgICAgIChXZWJJbnNwZWN0b3IuVGV4dEVkaXRvck1v ZGVsLnByb3RvdHlwZS5fcmVwbGFjZVRhYnNJZk5lZWRlZCk6CisKIDIwMTAtMDItMDQgIEVucmlj YSBDYXN1Y2NpICA8ZW5yaWNhQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBPbGl2 ZXIgSHVudC4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9Tb3VyY2VG cmFtZS5qcyBiL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9Tb3VyY2VGcmFtZS5qcwppbmRl eCBlMzBkYmRiLi5iYzkxMjJiIDEwMDY0NAotLS0gYS9XZWJDb3JlL2luc3BlY3Rvci9mcm9udC1l bmQvU291cmNlRnJhbWUuanMKKysrIGIvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL1NvdXJj ZUZyYW1lLmpzCkBAIC0zMyw2ICszMyw3IEBAIFdlYkluc3BlY3Rvci5Tb3VyY2VGcmFtZSA9IGZ1 bmN0aW9uKHBhcmVudEVsZW1lbnQsIGFkZEJyZWFrcG9pbnREZWxlZ2F0ZSkKICAgICB0aGlzLl9w YXJlbnRFbGVtZW50ID0gcGFyZW50RWxlbWVudDsKIAogICAgIHRoaXMuX3RleHRNb2RlbCA9IG5l dyBXZWJJbnNwZWN0b3IuVGV4dEVkaXRvck1vZGVsKCk7CisgICAgdGhpcy5fdGV4dE1vZGVsLnJl cGxhY2VUYWJzV2l0aFNwYWNlcyA9IHRydWU7CiAKICAgICB0aGlzLl9tZXNzYWdlcyA9IFtdOwog ICAgIHRoaXMuX3Jvd01lc3NhZ2VzID0ge307CmRpZmYgLS1naXQgYS9XZWJDb3JlL2luc3BlY3Rv ci9mcm9udC1lbmQvVGV4dEVkaXRvck1vZGVsLmpzIGIvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQt ZW5kL1RleHRFZGl0b3JNb2RlbC5qcwppbmRleCBmYzU2MDI2Li5lNTZjMjY5IDEwMDY0NAotLS0g YS9XZWJDb3JlL2luc3BlY3Rvci9mcm9udC1lbmQvVGV4dEVkaXRvck1vZGVsLmpzCisrKyBiL1dl YkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9UZXh0RWRpdG9yTW9kZWwuanMKQEAgLTk3LDYgKzk3 LDExIEBAIFdlYkluc3BlY3Rvci5UZXh0RWRpdG9yTW9kZWwucHJvdG90eXBlID0gewogICAgICAg ICByZXR1cm4gbmV3UmFuZ2U7CiAgICAgfSwKIAorICAgIHNldCByZXBsYWNlVGFic1dpdGhTcGFj ZXMocmVwbGFjZVRhYnNXaXRoU3BhY2VzKQorICAgIHsKKyAgICAgICAgdGhpcy5fcmVwbGFjZVRh YnNXaXRoU3BhY2VzID0gcmVwbGFjZVRhYnNXaXRoU3BhY2VzOworICAgIH0sCisKICAgICBfaW5u ZXJTZXRUZXh0OiBmdW5jdGlvbihyYW5nZSwgdGV4dCkKICAgICB7CiAgICAgICAgIHRoaXMuX2Vy YXNlUmFuZ2UocmFuZ2UpOwpAQCAtMTA0LDYgKzEwOSw4IEBAIFdlYkluc3BlY3Rvci5UZXh0RWRp dG9yTW9kZWwucHJvdG90eXBlID0gewogICAgICAgICAgICAgcmV0dXJuIG5ldyBXZWJJbnNwZWN0 b3IuVGV4dFJhbmdlKHJhbmdlLnN0YXJ0TGluZSwgcmFuZ2Uuc3RhcnRDb2x1bW4sIHJhbmdlLnN0 YXJ0TGluZSwgcmFuZ2Uuc3RhcnRDb2x1bW4pOwogCiAgICAgICAgIHZhciBuZXdMaW5lcyA9IHRl eHQuc3BsaXQoIlxuIik7CisgICAgICAgIHRoaXMuX3JlcGxhY2VUYWJzSWZOZWVkZWQobmV3TGlu ZXMpOworCiAgICAgICAgIHZhciBwcmVmaXggPSB0aGlzLl9saW5lc1tyYW5nZS5zdGFydExpbmVd LnN1YnN0cmluZygwLCByYW5nZS5zdGFydENvbHVtbik7CiAgICAgICAgIHZhciBwcmVmaXhBcmd1 bWVudHMgPSB0aGlzLl9hcmd1bWVudHMKICAgICAgICAgdmFyIHN1ZmZpeCA9IHRoaXMuX2xpbmVz W3JhbmdlLnN0YXJ0TGluZV0uc3Vic3RyaW5nKHJhbmdlLnN0YXJ0Q29sdW1uKTsKQEAgLTEyNCw2 ICsxMzEsMjIgQEAgV2ViSW5zcGVjdG9yLlRleHRFZGl0b3JNb2RlbC5wcm90b3R5cGUgPSB7CiAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICByYW5nZS5zdGFydExpbmUg KyBuZXdMaW5lcy5sZW5ndGggLSAxLCBwb3N0Q2FyZXQpOwogICAgIH0sCiAKKyAgICBfcmVwbGFj ZVRhYnNJZk5lZWRlZDogZnVuY3Rpb24obGluZXMpCisgICAgeworICAgICAgICBpZiAoIXRoaXMu X3JlcGxhY2VUYWJzV2l0aFNwYWNlcykKKyAgICAgICAgICAgIHJldHVybjsKKyAgICAgICAgdmFy IHNwYWNlcyA9IFsgIiAgICAiLCAiICAgIiwgIiAgIiwgIiAiXTsKKyAgICAgICAgZm9yICh2YXIg aSA9IDA7IGkgPCBsaW5lcy5sZW5ndGg7ICsraSkgeworICAgICAgICAgICAgdmFyIGxpbmUgPSBs aW5lc1tpXTsKKyAgICAgICAgICAgIHZhciBpbmRleCA9IGxpbmUuaW5kZXhPZigiXHQiKTsKKyAg ICAgICAgICAgIHdoaWxlIChpbmRleCAhPT0gLTEpIHsKKyAgICAgICAgICAgICAgICBsaW5lID0g bGluZS5zdWJzdHJpbmcoMCwgaW5kZXgpICsgc3BhY2VzW2luZGV4ICUgNF0gKyBsaW5lLnN1YnN0 cmluZyhpbmRleCArIDEpOworICAgICAgICAgICAgICAgIGluZGV4ID0gbGluZS5pbmRleE9mKCJc dCIsIGluZGV4ICsgMSk7CisgICAgICAgICAgICB9CisgICAgICAgICAgICBsaW5lc1tpXSA9IGxp bmU7CisgICAgICAgIH0KKyAgICB9LAorCiAgICAgX2VyYXNlUmFuZ2U6IGZ1bmN0aW9uKHJhbmdl KQogICAgIHsKICAgICAgICAgaWYgKHJhbmdlLmlzRW1wdHkoKSkK