Add bash-like reverse search for command history in webconsole

RESOLVED FIXED in Firefox 64

Status

enhancement
P1
normal
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: hemanth.hm, Assigned: nchevobbe)

Tracking

(Depends on 1 bug, Blocks 3 bugs, {dev-doc-complete})

Trunk
Firefox 64
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

(Whiteboard: [boogaloo-mvp])

Attachments

(6 attachments)

Reporter

Description

5 years ago
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?

Updated

5 years ago
Component: Untriaged → Developer Tools: Console
Since we can't use Ctrl-R, what other shortcut would you suggest and would it make this feature discoverable?
Reporter

Comment 2

5 years ago
Ctrl-X ?

Comment 3

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

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

Comment 4

5 years ago
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

5 years ago
(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.
Reporter

Comment 6

5 years ago
Hmm..interesting, now were are left with combinations: 

> Cmd + Opt + K
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: FR : (reverse-i-search) on the console. → Add bash-like reverse search for command history in webconsole
(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?
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.
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.
WebIDE uses Shift-F8, but the point about OS X still stands.

Comment 11

4 years ago
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...
How about Shift+UP ARROW for reverse search, provided that UP ARROW already navigates backward in history?

Would that work?
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 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.
(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.
(In reply to adrian.aichner from comment #14)
> In what sense would Ctrl+R be intuitive?

Because it's what Bash uses.
OS: Windows 7 → All
Hardware: x86 → All
Version: 33 Branch → Trunk

Comment 17

4 years ago
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".
(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).

Updated

4 years ago
Duplicate of this bug: 1197500

Updated

4 years ago
Severity: normal → enhancement

Updated

2 years ago
Duplicate of this bug: 1267061
Priority: -- → P2
Whiteboard: [boogaloo-mvp]

Updated

a year ago
Product: Firefox → DevTools
Assignee

Comment 21

11 months ago
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 ?
Assignee

Comment 22

11 months ago
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 ?
Assignee

Comment 23

11 months ago
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.
Assignee

Comment 24

11 months ago
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 ?
Assignee

Updated

10 months ago
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1

Comment 25

10 months ago
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
Assignee

Comment 27

10 months ago
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
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.
Assignee

Comment 29

9 months ago
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.
Assignee

Comment 30

9 months ago
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.
Assignee

Comment 31

9 months ago
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 ?
Assignee

Comment 32

9 months ago
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 ?
(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.
Assignee

Comment 34

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

Comment 35

9 months ago
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
Assignee

Comment 36

9 months ago
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 on attachment 9006921 [details]
Bug 1024913 - Move jsterm auto-focus handling into App.js; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.
Attachment #9006921 - Flags: review+
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.
Attachment #9006917 - Flags: review+
Comment on attachment 8999212 [details]
Bug 1024913 - Enable reverse search in jsterm; r=bgrins.

Francesco Lodolo [:flod] has approved the revision.
Attachment #8999212 - Flags: review+
Comment on attachment 9006918 [details]
Bug 1024913 - Add SVG images for the Reverse Search input; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.
Attachment #9006918 - Flags: review+
Comment on attachment 9006926 [details]
Bug 1024913 - Add tests for Reverse search; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.
Attachment #9006926 - Flags: review+
Comment on attachment 8999212 [details]
Bug 1024913 - Enable reverse search in jsterm; r=bgrins.

Brian Grinstead [:bgrins] has approved the revision.
Attachment #8999212 - Flags: review+
Assignee

Comment 45

9 months ago
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 ?
Assignee

Updated

9 months ago
Blocks: 1489489
Assignee

Updated

9 months ago
Blocks: 1489491

Comment 46

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1613377c1b43
Add a preference to enable the reverse search UI; r=bgrins.

Comment 47

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ccd6afc2a739
Add SVG images for the Reverse Search input; r=bgrins.

Comment 48

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f269add6e54
Move jsterm auto-focus handling into App.js; r=bgrins.

Comment 49

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c4e5a7ff8e53
Enable reverse search in jsterm; r=bgrins,flod.

Comment 50

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c18e0422ffa
Add tests for Reverse search; r=bgrins.
Assignee

Comment 52

9 months ago
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
Flags: needinfo?(nchevobbe)

Comment 54

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c73046f4daa3
Add a preference to enable the reverse search UI; r=bgrins.

Comment 55

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab579e9e03e7
Add SVG images for the Reverse Search input; r=bgrins.

Comment 56

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/28becf26d84b
Move jsterm auto-focus handling into App.js; r=bgrins.

Comment 57

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3025bb790d39
Enable reverse search in jsterm; r=bgrins,flod.

Comment 58

9 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e3364f777f0
Add tests for Reverse search; r=bgrins.

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?

Flags: needinfo?(ismith)

Comment 61

4 months ago

(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

Flags: needinfo?(ismith)
Depends on: 1520844

Comment 62

4 months ago

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

Updated

4 months ago
Flags: needinfo?(nchevobbe)
Assignee

Comment 63

4 months ago

Thanks Irene, that looks good to me

Flags: needinfo?(nchevobbe)
You need to log in before you can comment on or make changes to this bug.