1024913 - Add bash-like reverse search for command history in webconsole

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[hemanth]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.114 Safari/537.36

Steps to reproduce:

Something similar to ctrl+r in Bash.


Actual results:

Nothing.


Expected results:

Reverse lookup?

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Since we can't use Ctrl-R, what other shortcut would you suggest and would it make this feature discoverable?

Comment 2

[hemanth]

Ctrl-X ?

Comment 3

[:Gijs (he/him)]

(In reply to hemanth from comment #2)
> Ctrl-X ?

That's in use for "cut", isn't it? :-\

Comment 4

[hemanth]

Heh Heh I was so much in Bash, well it depends on the OS what Ctrl-X means...hmm how about `Ctrl-Q` ?

Comment 5

[:Gijs (he/him)]

(In reply to hemanth from comment #4)
> Heh Heh I was so much in Bash, well it depends on the OS what Ctrl-X
> means...hmm how about `Ctrl-Q` ?

Closes the browser on Linux, as does the OS X equivalent cmd-q on OS X.

Comment 6

[hemanth]

Hmm..interesting, now were are left with combinations: 

> Cmd + Opt + K

Comment 7

[Matthew N. [:MattN]]

(In reply to Panos Astithas [:past] from comment #1)
> Since we can't use Ctrl-R, what other shortcut would you suggest and would
> it make this feature discoverable?

Can we use Ctrl-R on OS X?

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Ctrl-R currently clears the console output, but that's not terribly important (Ctrl-L does it, too). Inconsistency among supported platforms would be the main problem, depending on which shortcut we would pick on Windows and Linux.

Comment 9

[Brian Grinstead [:bgrins]]

Looks like Windows uses F8: https://en.wikipedia.org/wiki/Table_of_keyboard_shortcuts#Command_line_shortcuts.  But that is already used for web IDE and would be inconvenient for OSX.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

WebIDE uses Shift-F8, but the point about OS X still stands.

Comment 11

[:Gijs (he/him)]

Could do ctrl-r on OS X, F8 elsewhere? Sad that we can't use the same shortcut everywhere, but it'd work, from the sounds of it...

Comment 12

[Adrian Aichner [:anaran]]

How about Shift+UP ARROW for reverse search, provided that UP ARROW already navigates backward in history?

Would that work?

Comment 13

[Panos Astithas (he/him) [:past] (please ni?)]

Shift + up arrow has the problem of discoverability. The best path so far seems to be Ctrl-R/Mac, F8/Win, F8/Linux, where only the latter would be unintuitive.

Comment 14

[Adrian Aichner [:anaran]]

(In reply to Panos Astithas [:past] from comment #13)
> Shift + up arrow has the problem of discoverability. The best path so far
> seems to be Ctrl-R/Mac, F8/Win, F8/Linux, where only the latter would be
> unintuitive.

In what sense would Ctrl+R be intuitive?

I think documentation is a more reliable than intuition.

Speaking of which - I was only able to find keybindings help for Scrachpad (via Scratchpad->Help)

How does one access keybindings help for other devtools easily?

I guess a link in devtools settings, e.g. to MDN documentation (TBD) would be practical.

Would Ctrl+SPACE work for Linux and Mac? Already used for symbol completion in Scratchpad.

Comment 15

[Brian Grinstead [:bgrins]]

(In reply to adrian.aichner from comment #14)
> How does one access keybindings help for other devtools easily?
> 
> I guess a link in devtools settings, e.g. to MDN documentation (TBD) would
> be practical.

The documentation for keyboard shortcuts is here: https://developer.mozilla.org/en-US/docs/Tools/Keyboard_shortcuts.  I agree that it would be great to make these (or at least a link to them) visible from the options panel - see Bug 1122699.

Comment 16

[Matthew N. [:MattN]]

(In reply to adrian.aichner from comment #14)
> In what sense would Ctrl+R be intuitive?

Because it's what Bash uses.

Comment 17

[Walter S.]

Why can't Ctrl+R be used? Is this a limitation of the way short-cuts are captured? I would think that there should be a way to check the current "context" and then have Ctrl+R do something else if the cursor is inside the console. But I don't have a clue of how short-cuts (and other things) work in FF.

P.S. In bash you can also use "Ctrl+Shift+R".

Comment 18

[Brian Grinstead [:bgrins]]

(In reply to SchnWalter from comment #17)
> Why can't Ctrl+R be used? Is this a limitation of the way short-cuts are
> captured? I would think that there should be a way to check the current
> "context" and then have Ctrl+R do something else if the cursor is inside the
> console. But I don't have a clue of how short-cuts (and other things) work
> in FF.

That could be done, but it's annoying when the toolbox takes over really common shortcuts (see also Bug 1162848).  I think the current plan is Ctrl+R on OSX (to match batch) and F8 for Windows and Linux (to not override the 'reload' shortcut).

Comment 21

[Nicolas Chevobbe [:nchevobbe]]

We should also think of how we navigate in the history once we're in this mode.
In most terminal, you hit Ctrl + R to go to older history entries, and you can also do Ctrl + Shift + R to go to "younger" entries.

i.e., for the following history entries (all the same days): 

08:00 - `x = 1`
09:00 - `x = 2`
10:00 - `x = 3`

If the user do:
Ctrl+R, then hit "x" -> `x = 3`
Ctrl+R -> `x = 2`
Ctrl+R -> `x = 1`
Ctrl+R -> `x = 1` //maybe indicate that it's the last matching item
Ctrl+Shift+R -> `x = 2`
Ctrl+Shift+R -> `x = 3`
Ctrl+Shift+R -> `x = 3` //maybe indicate that it's the first matching item

I don't know if Shift + F8 would be acceptable for non OSX platform. Should we allow to navigate through matching history entries with Up/Down arrow keys ?

Comment 22

[Nicolas Chevobbe [:nchevobbe]]

We should also think how you exit the reverse search.

<kbd>Esc</kbd> would close the reverse search UI, but should it clear the input content ? I tend to think no, because you might trigger the UI by accident, and we shouldn't erase what the user entered.

<kbd>Enter</kbd> would close the reverse search UI and evaluate what's in the input

<kbd>Tab</kbd> would close the reverse search UI and keep the input content to what it is

<kbd>Shift + Tab</kbd>: should it move the focus away (and thus close the reverse search) ? or do nothing ? I guess should be consistent with what we have in the input itself

Navigation keys (ArrowUp/Down/Left/Right, Home, End, …): Not sure ? Maybe have them behave like in a regular text input ?

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

In addition with the keyboard shortcut to enable this, maybe we could have a button to open it as well ? We could even use as a toggle so user can open/close the UI.
We also need to think at how this will play with the button to toggle multiline mode.

Comment 24

[Nicolas Chevobbe [:nchevobbe]]

I think it would be nice to only show unique history items, so with the given history: 

08:00 - `   x = 1  `
09:00 - `x = 2`
10:00 - `x = 1`

<kbd>Ctrl+R</kbd> then "x" -> `x = 1`
<kbd>Ctrl+R</kbd> -> `x = 2`
<kbd>Ctrl+R</kbd> -> `x = 2` // maybe indicate that it's the last matching item

I don't know if there would be any reason to *not* do that ?

Comment 25

[Walter S.]

I believe that you should make sure to reset the "history position" when you press enter, and continue further back in history if you "backspace" some characters and then trigger the shortcut again. -- I believe this is the default behavior that people would expect. 

You can search the BASH man pages for the behavior, here's is one page with available commands/shortcuts for manipulating the history:

https://www.gnu.org/software/bash/manual/bashref.html#Commands-For-History

Comment 26

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1024913 - Enable reverse search in jsterm; r=bgrins.

Comment 27

[Nicolas Chevobbe [:nchevobbe]]

In comment 26 there's a patch you can play with to see how it feels.

There's still work to do:
 - Make it looks nice on dark mode
 - Ensure we don't mess with the history position
 - Save the current content of the input ?
 - Add tests

Comment 28

[Brian Grinstead [:bgrins]]

I pulled this down and played with it - I really like it!

A few notes:
- I kept on trying to click on the help bubble and accidentally closing the search. I realized after a bit that it was just tooltip text on hover, but still kept clicking on accident. At least preventDefault? Ditto on the magnifying glass.
- In bash you can exit with ctrl+c. Might be good to add in addition to ESC (although on Windows that would be overloaded with 'copy' so might make that non-win only).
- A 'close' icon could make sense for less keyboard-inclined users or for someone who accidentally opens it.
- Would be a nice-to-have to somehow highlight matches for the searched string in the CM instance.

Comment 29

[Nicolas Chevobbe [:nchevobbe]]

So about that icon, it might be a bad idea.
I had an iteration where I was displaying the next and previous shortcut directly next to the input.
This might work, and we'd put the close shortcut (esc and ctrl-c on osx) as title of the close button.

I did not think of the magnifying glass, I may put it as a background of the input, so clicking on it won't blur the input.

Comment 30

[Nicolas Chevobbe [:nchevobbe]]

oh, I forgot something: we could then have a proper help icon like in the debugger that would open a modal with all the existing shortcuts in the console. This way we are consistent across panel and we solve this "clicking" issue.

Comment 31

[Nicolas Chevobbe [:nchevobbe]]

Attachment: reverse-search-mouse-friendly

So here's what it could look like after Brian's suggestions.
Instead of only relying on keyboard, there's now button to navigate to previous/next search, and one to close the search input.

The buttons indicate the keyboard shortcut so people can then learn to be more efficient with the keyboard.

The UI is basically the same as the debugger's "Search in file" so we are consistent.

The input closes when jsterm.focus() is called.

The input sticks to the very bottom so you can hit the navigation multiple time in a row. This couldn't be the case if it was just under the input as the input height might vary when cycling through results.

What do you think of it people ?

Comment 32

[Nicolas Chevobbe [:nchevobbe]]

pushed the new version to phab if people want to try it live: https://phabricator.services.mozilla.com/D3114

Also, I was thinking of a way to "cancel" the search and get back to the input value before triggering the UI.
The reasoning being that if someone is typing a long snippet, then hit Ctrl + R inadvertently and keep on typing, they probably lost what they typed.

So we could have something to get back this "stashed text", but I don't what interaction makes the most sense:
 - if we get it back when the user hit Escape (or the close button), then actual users of the reverse search might be confused, because they were looking for a given input to modify, and they're back to something else. The close button could have a "Cancel" tooltip, which might make it more obvious what's going to happen in this case. We could do it on Ctrl+C since it's the native behavior (sort of, the input gets cleared and reverse search hidden), but it will be enabled only on OSX, and people might not know about it.
 - a dedicated icon ? But I'm not sure about that one. Note that this could be something we also have for Bug 1463681
 - something else ?

Comment 33

[Brian Grinstead [:bgrins]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #32)
> pushed the new version to phab if people want to try it live:
> https://phabricator.services.mozilla.com/D3114
> 
> Also, I was thinking of a way to "cancel" the search and get back to the
> input value before triggering the UI.
> The reasoning being that if someone is typing a long snippet, then hit Ctrl
> + R inadvertently and keep on typing, they probably lost what they typed.

FWIW: On bash if you have input, then when you press ctrl+r it won't go away until you start typing. If you press ESC without having typed anything into the input field then the original text is restored. If you start typing then the original input goes away and can't be restored AFAICT.

We have more flexibility with our UI so we could probably do more, but I'd suggest at least that if you press ctrl+r then ESC immediately the original text is restored.

Comment 34

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1024913 - Add a preference to enable the reverse search UI; r=bgrins.

Retrieve this preference in the console to pass ot as a prop to the root
App component.

Comment 35

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1024913 - Add SVG images for the Reverse Search input; r=bgrins.

This adds 2 arrowhead images to navigate to the next and previous result
in the reverse search UI.
The search.svg icon is modified to accept fill through CSS.

Depends on D5149

Comment 36

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1024913 - Move jsterm auto-focus handling into App.js; r=bgrins.

The App component takes the whole document space, so we can move the
code that listen for every click in the console UI to decide if the
jsterm should be focused or not.
This feels more like the React way, and will also provide us better
access to the store and the props when we need them in this focus
handling (like this will be the case for hiding the reverse search UI).

Depends on D5150

Comment 37

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1024913 - Add tests for Reverse search; r=bgrins.

Depends on D3114

Comment 38

[Brian Grinstead [:bgrins]]

Comment on attachment 9006921 [details]
Bug 1024913 - Move jsterm auto-focus handling into App.js; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.

Comment 39

[Brian Grinstead [:bgrins]]

Comment on attachment 9006917 [details]
Bug 1024913 - Add a preference to enable the reverse search UI; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.

Comment 40

[Francesco Lodolo [:flod]]

Comment on attachment 8999212 [details]
Bug 1024913 - Enable reverse search in jsterm; r=bgrins.

Francesco Lodolo [:flod] has approved the revision.

Comment 41

[Brian Grinstead [:bgrins]]

Comment on attachment 9006918 [details]
Bug 1024913 - Add SVG images for the Reverse Search input; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.

Comment 42

[Nicolas Chevobbe [:nchevobbe]]

TRY push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f36f2738aeaba4f9349bc951dd196e805a2da734

Comment 43

[Brian Grinstead [:bgrins]]

Comment on attachment 9006926 [details]
Bug 1024913 - Add tests for Reverse search; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.

Comment 44

[Brian Grinstead [:bgrins]]

Comment on attachment 8999212 [details]
Bug 1024913 - Enable reverse search in jsterm; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.

Comment 45

[Nicolas Chevobbe [:nchevobbe]]

So, I'm getting TRY failures on an old debugger test:  https://searchfox.org/mozilla-central/rev/6201a9e0067cf6af118c6a99ae9314b8ceb2c4d5/devtools/client/debugger/test/mochitest/browser_dbg_split-console-keypress.js

Basically, this test ensure that Stepping key shortcut works when the debugger is opened with the split console focused.
One of the shortcut, Resume/Pause, is F8, which in our case, clashes with the Reverse Search opening on Windows/Linux.

By the way, the "event forwarding" between split console and debugger only works with the old debugger not the new one. I suspect that we might want that to work in the new debugger as well but I'm not sure.

So we're back to the beginning, what shortcut should we have on Windows/Linux ? Ctrl+Shift+R ? Could we override Ctrl+R since Windows/Linux also have easy access to F5 to refresh the page ?

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1613377c1b43
Add a preference to enable the reverse search UI; r=bgrins.

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ccd6afc2a739
Add SVG images for the Reverse Search input; r=bgrins.

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f269add6e54
Move jsterm auto-focus handling into App.js; r=bgrins.

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4e5a7ff8e53
Enable reverse search in jsterm; r=bgrins,flod.

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c18e0422ffa
Add tests for Reverse search; r=bgrins.

Comment 51

[Eliza Balazs [:ebalazs_]]

Backed out 5 changesets (Bug 1024913) for causing perma damp failures. CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c4e5a7ff8e53cc3d8ad7d8eec573f6678de66ee8&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&filter-searchStr=damp&selectedJob=198359596

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=198359596&repo=autoland&lineNumber=1077

Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bda5c062ad28d29ff2be86c3dbd9f4e1af449dcb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&filter-searchStr=damp

Comment 52

[Nicolas Chevobbe [:nchevobbe]]

Because of a change made in setInputValue (moved lastInputValue to the top of the function to avoid triggering autocompletion when browsing through reverse history search results), the autocompletion popup does not appear anymore in the autocompletion damp test.
I made a change to make it work like the old jsterm (i.e. call updateCompletion in the damp test to make the popup appear).
I pushed to DAMP to make sure we don't alter the results too much: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=84a6312aa7787bf9d4390c71e13335b84420a992&newProject=try&newRevision=2434774f6307d2cec7d5686a2850fa429ec1654a&framework=12

Comment 53

[Nicolas Chevobbe [:nchevobbe]]

okay, the fix seems to be good, and does not impact the test duration (https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=84a6312aa7787bf9d4390c71e13335b84420a992&newProject=try&newRevision=2434774f6307d2cec7d5686a2850fa429ec1654a&originalSignature=121f8de11f76e03cd20d8df97192d3af2ec23ab5&newSignature=121f8de11f76e03cd20d8df97192d3af2ec23ab5&filter=autocomplete&framework=12)

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c73046f4daa3
Add a preference to enable the reverse search UI; r=bgrins.

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab579e9e03e7
Add SVG images for the Reverse Search input; r=bgrins.

Comment 56

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28becf26d84b
Move jsterm auto-focus handling into App.js; r=bgrins.

Comment 57

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3025bb790d39
Enable reverse search in jsterm; r=bgrins,flod.

Comment 58

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e3364f777f0
Add tests for Reverse search; r=bgrins.

Comment 59

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/c73046f4daa3
https://hg.mozilla.org/mozilla-central/rev/ab579e9e03e7
https://hg.mozilla.org/mozilla-central/rev/28becf26d84b
https://hg.mozilla.org/mozilla-central/rev/3025bb790d39
https://hg.mozilla.org/mozilla-central/rev/8e3364f777f0

Comment 60

[:Harald Kirschner :digitarald]

Irene, while we need this documented for the Console docs; the other important aspect is to have the shortcut added to the keyboard shortcut page. Can you help, please?

Comment 61

[Irene Smith]

(In reply to :Harald Kirschner :digitarald from comment #60)

Irene, while we need this documented for the Console docs; the other important aspect is to have the shortcut added to the keyboard shortcut page. Can you help, please?

Sure. I have documenting the reverse search on my list of tasks for this sprint. Thank you for reminding me about the shortcut. I will add it to: https://developer.mozilla.org/en-US/docs/Tools/Keyboard_shortcuts

Comment 62

[Irene Smith]

I have:

1. updated the command history section on the console page
2. Added the command shortcut to the Command line interpreter table
3. Made sure that the feature is mentioned in the Firefox 65 release notes

Comment 63

[Nicolas Chevobbe [:nchevobbe]]

Thanks Irene, that looks good to me