RESOLVED FIXED 147918
Web Inspector: Pressing Command-Enter should re-evaluate selected console user command
https://bugs.webkit.org/show_bug.cgi?id=147918
Summary Web Inspector: Pressing Command-Enter should re-evaluate selected console use...
Nikita Vasilyev
Reported 2015-08-11 19:19:17 PDT
Created attachment 258792 [details] [Animated GIF] Option-Click to re-run See the attached GIF. Note that we don't attach Math.random() row on every click because it matches the last user command. However, in the following example: > Math.random() < 0.4242424242 > document.location.pathname < "/" clicking Math.random() should attach it again, e.g.: > Math.random() < 0.4242424242 > document.location.pathname < "/" > Math.random() > 0.2354678908 Also, Command+Enter, Option-Enter, or both should re-evaluate selected console message.
Attachments
[Animated GIF] Option-Click to re-run (72.44 KB, image/gif)
2015-08-11 19:19 PDT, Nikita Vasilyev
no flags
Patch (8.04 KB, patch)
2015-08-16 00:32 PDT, Nikita Vasilyev
no flags
[Animated GIF] Re-submit on click (275.91 KB, image/gif)
2015-08-16 00:53 PDT, Nikita Vasilyev
no flags
[Animated GIF] With the patch applied (82.90 KB, image/gif)
2015-08-16 01:01 PDT, Nikita Vasilyev
no flags
Patch (7.99 KB, patch)
2015-08-17 21:19 PDT, Nikita Vasilyev
nvasilyev: review-
nvasilyev: commit-queue-
Patch (2.12 KB, patch)
2015-08-18 23:39 PDT, Nikita Vasilyev
no flags
Timothy Hatcher
Comment 1 2015-08-11 20:52:39 PDT
I like! I agree Command or Option-Enter on the selected row should do this too.
Timothy Hatcher
Comment 2 2015-08-11 20:53:09 PDT
Or maybe just Enter for selected row?
Timothy Hatcher
Comment 3 2015-08-11 22:26:58 PDT
Lets also make the double arrow that appears more like the existing grey arrow. So same size/shape but double and blue.
Radar WebKit Bug Importer
Comment 4 2015-08-16 00:20:56 PDT
Nikita Vasilyev
Comment 5 2015-08-16 00:32:04 PDT
Nikita Vasilyev
Comment 6 2015-08-16 00:53:34 PDT
Created attachment 259116 [details] [Animated GIF] Re-submit on click I implemented re-submit on click and run it for a day. It turned out not as useful as I have thought because: 1. Console messages shift up every time a new one is added — I need to move mouse cursor after every click. 2. Console always scrolls to the bottom when the new user command result is added, as it should be, but it often leaves the clicked user command out of the view port. In the attached patch I only implemented re-submit on Enter. I want to give Option-Click to re-run a bit more thought and not include it in this patch.
Nikita Vasilyev
Comment 7 2015-08-16 01:01:50 PDT
Created attachment 259117 [details] [Animated GIF] With the patch applied
Devin Rousso
Comment 8 2015-08-17 16:49:46 PDT
Comment on attachment 259115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259115&action=review > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:828 > + var message = this._selectedMessages[0]; Let instead of var. > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:839 > + if (consoleItemElement.__commandView) { We should change this from "__" to a symbol (can be a different patch, but should be done). > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:861 > + WebInspector.logManager.dispatchEventToListeners(WebInspector.LogManager.Event.MessageAdded, {message: commandResultMessage}); Won't this event be triggered once the new command has been executed and added to the frontend, or is it necessary to fire MessageAdded before the message is actually added?
Nikita Vasilyev
Comment 9 2015-08-17 17:09:51 PDT
Comment on attachment 259115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259115&action=review >> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:861 >> + WebInspector.logManager.dispatchEventToListeners(WebInspector.LogManager.Event.MessageAdded, {message: commandResultMessage}); > > Won't this event be triggered once the new command has been executed and added to the frontend, or is it necessary to fire MessageAdded before the message is actually added? Removing this line would prevent the result from being added to the console. It looks like I could use this._logViewController.appendConsoleMessage instead.
Nikita Vasilyev
Comment 10 2015-08-17 21:19:17 PDT
Nikita Vasilyev
Comment 11 2015-08-18 00:27:44 PDT
Comment on attachment 259237 [details] Patch Since https://bugs.webkit.org/show_bug.cgi?id=148121, pressing Enter when a console message is selected behaves the same way as pressing Enter in the console prompt. I think this is right. We should be consistent with "Web Inspector: Option-Enter should commit console command without erasing the prompt" https://bugs.webkit.org/show_bug.cgi?id=148123 and use the same shortcut. I used Option–Enter there but I'm open to suggestions.
Nikita Vasilyev
Comment 12 2015-08-18 23:39:01 PDT
Created attachment 259363 [details] Patch Rebaselined and significantly simplified.
WebKit Commit Bot
Comment 13 2015-08-19 02:35:44 PDT
Comment on attachment 259363 [details] Patch Clearing flags on attachment: 259363 Committed r188637: <http://trac.webkit.org/changeset/188637>
WebKit Commit Bot
Comment 14 2015-08-19 02:35:50 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.