WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U for Macs Only
(5.68 KB, patch)
2009-11-21 07:26 PST
,
Joseph Pecoraro
no flags
Details
Formatted Diff
Diff
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys
(10.97 KB, patch)
2009-11-21 09:01 PST
,
Joseph Pecoraro
no flags
Details
Formatted Diff
Diff
[PATCH] Ctrl+P, Ctrl+N, Ctrl+U, Arrow Keys
(10.96 KB, patch)
2009-11-21 09:16 PST
,
Joseph Pecoraro
timothy
: review-
Details
Formatted Diff
Diff
[PATCH] Ctrl+P, Ctrl+N, and Arrow Keys
(9.25 KB, patch)
2009-11-21 19:33 PST
,
Joseph Pecoraro
timothy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug