1473923 - Commands prefixed with ":" do not autocomplete

Reporter

Description

•

6 years ago

63.0a1 (2018-07-05) (64-bit) Mac OS X

STR:

1. open console
2. type ":" or ":s" or ":h"

Expected: inline autocomplete suggestion for :help or :screenshot

Actual: no inline autocomplete suggestion

709922234

Comment 1

•

6 years ago

related https://bugzilla.mozilla.org/show_bug.cgi?id=1463674

Nicolas Chevobbe [:nchevobbe]

Assignee

Updated

•

6 years ago

Priority: -- → P2

Nicolas Chevobbe [:nchevobbe]

Assignee

Comment 2

•

6 years ago

Let's block this on Bug 1463674 so we don't have conflicts when one of those 2 land.

Blocks: 1463674

Nicolas Chevobbe [:nchevobbe]

Assignee

Updated

•

6 years ago

Updated

•

6 years ago

Assignee: nobody → nchevobbe

Status: NEW → ASSIGNED

Priority: P2 → P1

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/257532/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257532/

Yulia Startsev [:yulia]

Comment 4

•

6 years ago

mozreview-review

Comment on attachment 8992678 [details]
Bug 1473923 - Autocomplete console commands; .

https://reviewboard.mozilla.org/r/257532/#review264688

Looks really good! I have one small comment that might clean things up a little but thats it. thanks for this!

::: devtools/server/actors/webconsole.js:1095
(Diff revision 1)
> +    const reqText = request.text.substr(0, request.cursor);
>  
> +    if (isCommand(reqText)) {
> +      const commandsCache = this._getWebConsoleCommandsCache();
> +      matchProp = reqText;
> +      matches = validCommands

since validCommands is being used to filter, maybe we can just use that array, rather than using the command cache? I might be missing something here

Attachment #8992678 - Flags: review?(ystartsev) → review+

Nicolas Chevobbe [:nchevobbe]

Assignee

Comment 5

•

6 years ago

> since validCommands is being used to filter, maybe we can just use that array, rather than using the command cache? I might be missing something here

That was my initial thought as well :) After, I was thinking of future commands that might be valid but not registered on a given toolbox for reason X.
I'm not sure this is important for now, but I felt it was more future proof. I don't feel too strong about that, so I can remove it.

Yulia Startsev [:yulia]

Comment 6

•

6 years ago

we probably want to use the validCommands array, as things like `:$$` will throw if its not part of that array. I was thinking that some things are meant to be "javascript-like" and others were only commands. The one that currently spans that gap is `help`. but I am not 100% convinced I am right. Let's keep the `validArray` for now, and then later we can revisit this.

Mozilla Autoland

Comment 7

•

6 years ago

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 0c03f25a17c5e95895d9f6d66edeec2c2e015b50 -d b3e82720e36c: rebasing 473295:0c03f25a17c5 "Bug 1473923 - Autocomplete console commands; r=yulia." (tip)
merging devtools/client/webconsole/components/JSTerm.js
merging devtools/client/webconsole/test/mochitest/browser.ini
merging devtools/server/actors/webconsole.js
warning: conflicts while merging devtools/server/actors/webconsole.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment hidden (mozreview-request)

Comment on attachment 8992678 [details]
Bug 1473923 - Autocomplete console commands; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257532/diff/1-2/

Mozilla Autoland

Comment 9

•

6 years ago

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s f76d71c77d1dd82f2fb6ea1b67378a7b5e6748bf -d 10ca5b14dbdb: rebasing 473301:f76d71c77d1d "Bug 1473923 - Autocomplete console commands; r=yulia." (tip)
merging devtools/client/webconsole/test/mochitest/browser.ini
merging devtools/server/actors/webconsole.js
warning: conflicts while merging devtools/server/actors/webconsole.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment hidden (mozreview-request)

Comment on attachment 8992678 [details]
Bug 1473923 - Autocomplete console commands; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257532/diff/2-3/

Pulsebot

Comment 11

•

6 years ago

Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f8cd8745c66
Autocomplete console commands; r=yulia.

Cosmin Sabou [:CosminS]

Comment 12

•

6 years ago

Backed out for devtools failures on browser_jsterm_autocomplete_commands

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=pending&filter-resultStatus=running&filter-searchStr=devtools&fromchange=7f8cd8745c66b7f7358549f0cefc5f2385d96a37&tochange=491f090adec46abb9337153a2fee24efcad11a11&selectedJob=188810543

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

Backout link: https://hg.mozilla.org/integration/autoland/rev/491f090adec46abb9337153a2fee24efcad11a11

19:35:05     INFO -  276 INFO TEST-START | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js
19:35:05     INFO -  TEST-INFO | started process screenshot
19:35:05     INFO -  TEST-INFO | screenshot: exit 0
19:35:05     INFO -  Buffered messages logged at 19:35:04
19:35:05     INFO -  277 INFO Entering test bound
19:35:05     INFO -  278 INFO Adding a new tab with URL: data:text/html;charset=utf-8,Test command autocomplete
19:35:05     INFO -  Buffered messages logged at 19:35:05
19:35:05     INFO -  279 INFO Tab added and finished loading
19:35:05     INFO -  280 INFO Opening the toolbox
19:35:05     INFO -  281 INFO Toolbox opened and focused
19:35:05     INFO -  282 INFO Enter ":"
19:35:05     INFO -  Buffered messages finished
19:35:05    ERROR -  283 INFO TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js | popup contains expected commands - Got :help
19:35:05     INFO -  :screenshot, expected :screenshot
19:35:05     INFO -  :help
19:35:05     INFO -  Stack trace:
19:35:05     INFO -  chrome://mochikit/content/browser-test.js:test_is:1305
19:35:05     INFO -  chrome://mochitests/content/browser/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js:performTests:33
19:35:05     INFO -  chrome://mochitests/content/browser/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js:null:12
19:35:05     INFO -  chrome://mochikit/content/browser-test.js:Tester_execTest/<:1103
19:35:05     INFO -  chrome://mochikit/content/browser-test.js:Tester_execTest:1094
19:35:05     INFO -  chrome://mochikit/content/browser-test.js:nextTest/<:996
19:35:05     INFO -  chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
19:35:05     INFO -  284 INFO Expects "  creenshot", is "  creenshot"
19:35:05     INFO -  285 INFO TEST-PASS | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js | completion node has expected :screenshot value -
19:35:05     INFO -  286 INFO TEST-PASS | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js | Tab key correctly completed :screenshot -
19:35:05     INFO -  287 INFO TEST-PASS | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js | popup is closed after Tab -
19:35:05     INFO -  288 INFO Test :hel completion
19:35:05     INFO -  289 INFO Expects "    p", is "    p"
19:35:05     INFO -  290 INFO TEST-PASS | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js | completion node has expected :help value -
19:35:05     INFO -  291 INFO TEST-PASS | devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_commands.js | Tab key correctly completes :help -
19:35:05     INFO -  292 INFO Adding a new tab with URL: data:text/html;charset=utf-8,Test command autocomplete
19:35:05     INFO -  293 INFO Tab added and finished loading
19:35:05     INFO -  294 INFO Opening the toolbox
19:35:06     INFO -  295 INFO Toolbox opened and focused
19:35:06     INFO -  296 INFO Enter ":"
19:35:06     INFO -  Not taking screenshot here: see the one that was previously logged

Flags: needinfo?(nchevobbe)

Nicolas Chevobbe [:nchevobbe]

Assignee

Comment 13

•

6 years ago

Bug 1475165 landed before this one, and thus made the test fail (the autocompletion results are flipped).
A new patch will be updated in the next minutes.

Blocks: 1458831

Flags: needinfo?(nchevobbe)

Whiteboard: [boogaloo-mvp]

Comment hidden (mozreview-request)

Comment on attachment 8992678 [details]
Bug 1473923 - Autocomplete console commands; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257532/diff/3-4/

Pulsebot

Comment 15

•

6 years ago

Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5d6d122c7127
Autocomplete console commands; r=yulia.

Mozilla Autoland

Comment 16

•

6 years ago

We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.

Natalia Csoregi [:nataliaCs]

Comment 17

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/5d6d122c7127

Status: ASSIGNED → RESOLVED

Closed: 6 years ago

status-firefox63: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 63