# STEPS TO REPRODUCE: 1. inspect any page 2. go to any resource that is editable 3. press tab ↹ => \t inserted instead of " " (4)
Created attachment 376384 [details] Patch
Is this better than having WI make Tab execute `insertSoftTab` and specify `options.tabSize` appropriately?
(In reply to Ross Kirsling from comment #2) > Is this better than having WI make Tab execute `insertSoftTab` and specify `options.tabSize` appropriately? I didn't even know that was a thing! I'll give it a shot =D
Created attachment 376387 [details] Patch
Comment on attachment 376387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376387&action=review It's still a bit sad to me, but I guess it's how we've got to do it: https://github.com/codemirror/CodeMirror/pull/4317 > Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:624 > + CodeMirror.commands.insertTab = function(cm) { Maybe it'd still be better to overwrite `defaultTab`, so that you don't have to store `original`? > Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:625 > + if (WI.settings.indentWithTabs.value) I think this can be `cm.options.indentWithTabs`, given all this: https://github.com/WebKit/webkit/blob/master/Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js#L36-L58
Comment on attachment 376387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=376387&action=review >> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:624 >> + CodeMirror.commands.insertTab = function(cm) { > > Maybe it'd still be better to overwrite `defaultTab`, so that you don't have to store `original`? I'd rather not, as that would require that I duplicate the `if (cm.somethingSelected()) { cm.indentSelection("add") }` logic, which adds complexity and potential confusion each time we update CodeMirror. >> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:625 >> + if (WI.settings.indentWithTabs.value) > > I think this can be `cm.options.indentWithTabs`, given all this: > https://github.com/WebKit/webkit/blob/master/Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js#L36-L58 Good point. I'll switch.
Created attachment 376390 [details] Patch
(In reply to Devin Rousso from comment #6) > I'd rather not, as that would require that I duplicate the `if > (cm.somethingSelected()) { cm.indentSelection("add") }` logic, which adds > complexity and potential confusion each time we update CodeMirror. Even with this solution though, I wonder if a comment would be in order to encourage us to keep an eye on it across CodeMirror updates? (Maybe we could link to the PR as a justification for why we had to go this route.)
Comment on attachment 376390 [details] Patch r=me otherwise
Created attachment 376396 [details] Patch
Comment on attachment 376396 [details] Patch Clearing flags on attachment: 376396 Committed r248739: <https://trac.webkit.org/changeset/248739>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54358187>