RESOLVED FIXED Bug 31400
Web Inspector: Support Ctrl+P, Ctrl+N, and other Readline keyboard shortcuts in the Console
https://bugs.webkit.org/show_bug.cgi?id=31400
Summary Web Inspector: Support Ctrl+P, Ctrl+N, and other Readline keyboard shortcuts ...
Sam Aaron
Reported 2009-11-12 03:17:44 PST
My muscle memory keeps trying ctrl-p to access history which totally fails to do anything at all. Would it be possible to map ctrl-p and ctrl-n to up and down in the console to mimic the behaviour of Terminal.app?
Attachments
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only (5.19 KB, patch)
2009-11-20 23:01 PST, Joseph Pecoraro
timothy: review-
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only (5.68 KB, patch)
2009-11-21 07:26 PST, Joseph Pecoraro
no flags
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys (10.97 KB, patch)
2009-11-21 09:01 PST, Joseph Pecoraro
no flags
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys (10.96 KB, patch)
2009-11-21 09:16 PST, Joseph Pecoraro
timothy: review-
[PATCH] Ctrl+P, Ctrl+N, and Arrow Keys (9.25 KB, patch)
2009-11-21 19:33 PST, Joseph Pecoraro
timothy: review+
Joseph Pecoraro
Comment 1 2009-11-20 23:00:04 PST
These are well known Readline keyboard shortcuts. Terminal.app Supports a number of them. Here is a general list: http://www.bigsmoke.us/readline/shortcuts keishi mentioned on IRC: > I think it's good for os x but I'm wasn't sure about Windows. > I'm not sure. I was wondering if it had any conflicts when i read it. > all the readline key bindings don't work in Windows. > http://en.wikipedia.org/wiki/Control_key seems like Ctrl+P Ctrl+N > wouldn't have conflicts in the console. I like the readline key bindings > and I want this, but I just wonder if it's natural for Windows users.
Joseph Pecoraro
Comment 2 2009-11-20 23:01:22 PST
Created attachment 43644 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only Also fixes modifier keys (Shift/Alt/Meta/Control) from re-triggering the type ahead.
Brian Weinstein
Comment 3 2009-11-20 23:06:56 PST
I'm not sure the best way to handle this on Windows, maybe the meta key would work (Windows key).
Timothy Hatcher
Comment 4 2009-11-20 23:29:22 PST
Comment on attachment 43644 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only > + // fall through to default > + case "U+004E": // Ctrl+N = Next Are you sure this falls through to the default? I thought it fell through to the next case.
Timothy Hatcher
Comment 5 2009-11-21 00:21:45 PST
Comment on attachment 43644 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only The switch needs fixed. Example: switch(true) { case false: alert('false'); case true: alert('true'); case false: alert('false'); case false: alert('false'); default: alert('default'); }
Joseph Pecoraro
Comment 6 2009-11-21 07:11:52 PST
(In reply to comment #4) > > + // fall through to default > > + case "U+004E": // Ctrl+N = Next > > Are you sure this falls through to the default? I thought it fell through to > the next case. Correct, it falls through to the next case, which then falls through to the next case, and eventually will end up at default. This cases a few extra conditions to be checked, so maybe I should just refactor this anyway.
Joseph Pecoraro
Comment 7 2009-11-21 07:26:13 PST
Created attachment 43648 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only More explicit behavior in the switch. No fall throughs. The only thing I am worried about is the O(n) behavior of deleting nodes in clearBeforeCaret. Even though my tests showed it was immediate for a rather large paste, I still think there should be a better way to delete a range of nodes in 1 operation, rather then n. Any pointers?
Joseph Pecoraro
Comment 8 2009-11-21 09:01:27 PST
Created attachment 43651 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys Included arrow keys for multiline here because bug 30553 seems to have diverted its attention to Snippets.
Joseph Pecoraro
Comment 9 2009-11-21 09:08:30 PST
Video Showing Arrow Traversals so you can get a feel: http://screencast.com/t/MTBlMGZiNjA
Joseph Pecoraro
Comment 10 2009-11-21 09:16:34 PST
Created attachment 43652 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys Removed a "+1" I had added that shouldn't have been there and caused bad behavior.
Timothy Hatcher
Comment 11 2009-11-21 19:10:18 PST
Comment on attachment 43652 [details] [PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys > + this.clearAutoComplete(true); > + if (this.isCaretAtEndOfPrompt()) { > + this.element.removeChildren(); > + this.element.appendChild(document.createElement("br")); > + this.moveCaretToEndOfPrompt(); > + return; > + } > + > + while (focusNode.previousSibling) > + this.element.removeChild(focusNode.previousSibling); > + focusNode.textContent = focusNode.textContent.substring(selection.focusOffset); > + if (this.element.textContent.length === 0) > + this.element.appendChild(document.createElement("br")); > + > + var newRange = document.createRange(); > + newRange.setStart(focusNode, 0); > + newRange.setEnd(focusNode, 0); > + > + selection.removeAllRanges(); > + selection.addRange(newRange); This can be simplified. Just make a DOM range that starts at the beginning and ends at the focusNode and focusOffset. Then call removeContents. This should leave you with a good caret selection too. You might need the <br> still if totally empty. Even better: this.element.execCommand("DeleteToBeginningOfLine") But looking at it, I don't think DeleteToBeginningOfLine is allowed from execCommand! Maybe we can change it to be "supported" not just "supportedFromMenuOrKeyBinding"? http://trac.webkit.org/browser/trunk/WebCore/editing/EditorCommand.cpp#L1313 > + selectionRange.setStart(this.element.firstChild, firstNewlineIndex); > + selectionRange.setEnd(this.element.firstChild, firstNewlineIndex); Is it true the first child is always a text node that contains this index?
Joseph Pecoraro
Comment 12 2009-11-21 19:33:13 PST
Created attachment 43669 [details] [PATCH] Ctrl+P, Ctrl+N, and Arrow Keys Removed Ctrl+U based on chat in IRC, because It should act like Ctrl+K and put the content into the Kill Ring (usable with Ctrl+Y), but we cannot do that and it may be confusing.
Joseph Pecoraro
Comment 13 2009-11-21 19:40:26 PST
Landed in http://trac.webkit.org/changeset/51291 r51291 = 5e12adf30e89cc9f952fb8cd7b39a16731aadb2b I will look into this.element.execCommand("DeleteToBeginningOfLine") when I find time. I'll close this bug.
Note You need to log in before you can comment on or make changes to this bug.