Skip to content

Commit

Permalink
Bug 1604411 - Add expression variables in autocomplete popup. r=Honza.
Browse files Browse the repository at this point in the history
Variables are retrieved from CodeMirror state and sent to the
webconsole actor, where the filtering is done.

Differential Revision: https://phabricator.services.mozilla.com/D57427
  • Loading branch information
nchevobbe committed Dec 18, 2019
1 parent a8e8224 commit c1dd141
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 10 deletions.
9 changes: 7 additions & 2 deletions devtools/client/webconsole/actions/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ const {
* from the cache).
* @param {Array<String>} getterPath: Array representing the getter access (i.e.
* `a.b.c.d.` is described as ['a', 'b', 'c', 'd'] ).
* @param {Array<String>} expressionVars: Array of the variables defined in the expression.
*/
function autocompleteUpdate(force, getterPath) {
function autocompleteUpdate(force, getterPath, expressionVars) {
return async ({ dispatch, getState, webConsoleUI, hud }) => {
if (hud.inputHasSelection()) {
return dispatch(autocompleteClear());
Expand Down Expand Up @@ -82,6 +83,7 @@ function autocompleteUpdate(force, getterPath) {
webConsoleFront,
authorizedEvaluations,
force,
expressionVars,
})
);
};
Expand Down Expand Up @@ -133,6 +135,7 @@ function autocompleteDataFetch({
force,
webConsoleFront,
authorizedEvaluations,
expressionVars,
}) {
return ({ dispatch, webConsoleUI }) => {
const selectedNodeActor = webConsoleUI.getSelectedNodeActor();
Expand All @@ -144,7 +147,8 @@ function autocompleteDataFetch({
undefined,
frameActorId,
selectedNodeActor,
authorizedEvaluations
authorizedEvaluations,
expressionVars
)
.then(data => {
dispatch(
Expand All @@ -155,6 +159,7 @@ function autocompleteDataFetch({
frameActorId,
data,
authorizedEvaluations,
expressionVars,
})
);
})
Expand Down
46 changes: 41 additions & 5 deletions devtools/client/webconsole/components/Input/JSTerm.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@ class JSTerm extends Component {
autoCloseBrackets: false,
lineNumbers: this.props.editorMode,
lineWrapping: true,
mode: Editor.modes.js,
mode: {
name: "javascript",
globalVars: true,
},
styleActiveLine: false,
tabIndex: "0",
viewportMargin: Infinity,
Expand Down Expand Up @@ -464,7 +467,11 @@ class JSTerm extends Component {

"Ctrl-Space": () => {
if (!this.autocompletePopup.isOpen) {
this.props.autocompleteUpdate(true);
this.props.autocompleteUpdate(
true,
null,
this._getExpressionVariables()
);
return null;
}

Expand Down Expand Up @@ -770,6 +777,35 @@ class JSTerm extends Component {
}
}

/**
* Retrieve variable declared in the expression from the CodeMirror state, in order
* to display them in the autocomplete popup.
*/
_getExpressionVariables() {
const cm = this.editor.codeMirror;
const { state } = cm.getTokenAt(cm.getCursor());
const variables = [];

if (state.context) {
for (let c = state.context; c; c = c.prev) {
for (let v = c.vars; v; v = v.next) {
variables.push(v.name);
}
}
}

const keys = ["localVars", "globalVars"];
for (const key of keys) {
if (state[key]) {
for (let v = state[key]; v; v = v.next) {
variables.push(v.name);
}
}
}

return variables;
}

/**
* The editor "changes" event handler.
*/
Expand All @@ -787,7 +823,7 @@ class JSTerm extends Component {
!isJsTermChangeOnly &&
(this.props.autocomplete || this.hasAutocompletionSuggestion())
) {
this.autocompleteUpdate();
this.autocompleteUpdate(false, null, this._getExpressionVariables());
}
this.lastInputValue = value;
this.terminalInputChanged(value);
Expand Down Expand Up @@ -1281,8 +1317,8 @@ function mapDispatchToProps(dispatch) {
return {
updateHistoryPosition: (direction, expression) =>
dispatch(actions.updateHistoryPosition(direction, expression)),
autocompleteUpdate: (force, getterPath) =>
dispatch(actions.autocompleteUpdate(force, getterPath)),
autocompleteUpdate: (force, getterPath, expressionVars) =>
dispatch(actions.autocompleteUpdate(force, getterPath, expressionVars)),
autocompleteClear: () => dispatch(actions.autocompleteClear()),
evaluateExpression: expression =>
dispatch(actions.evaluateExpression(expression)),
Expand Down
1 change: 1 addition & 0 deletions devtools/client/webconsole/test/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ skip-if = verify
[browser_jsterm_autocomplete_crossdomain_iframe.js]
[browser_jsterm_autocomplete_disabled.js]
[browser_jsterm_autocomplete_escape_key.js]
[browser_jsterm_autocomplete_expression_variables.js]
[browser_jsterm_autocomplete_extraneous_closing_brackets.js]
[browser_jsterm_autocomplete_getters_cache.js]
[browser_jsterm_autocomplete_getters_cancel.js]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */

// Tests that variable created in the expression are displayed in the autocomplete.

"use strict";

const TEST_URI = `data:text/html;charset=utf8,Test autocompletion for expression variables<script>
var testGlobal;
</script>`;

add_task(async function() {
const hud = await openNewTabAndConsole(TEST_URI);
const { jsterm } = hud;
const { autocompletePopup } = jsterm;

await setInputValueForAutocompletion(
hud,
`
var testVar;
let testLet;
const testConst;
class testClass {}
function testFunc(testParam1, testParam2, ...testParamRest) {
var [testParamRestFirst] = testParamRest;
let {testDeconstruct1,testDeconstruct2, ...testDeconstructRest} = testParam1;
test`
);

is(
getPopupLabels(autocompletePopup).join("\n"),
[
"testClass",
"testConst",
"testDeconstruct1",
"testDeconstruct2",
"testDeconstructRest",
"testFunc",
"testGlobal",
"testLet",
"testParam1",
"testParam2",
"testParamRest",
"testParamRestFirst",
"testVar",
].join("\n"),
"Autocomplete popup displays both global and local variables"
);
});

function getPopupLabels(popup) {
return popup.getItems().map(item => item.label);
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ add_task(async function() {
"Hitting Enter quickly after a letter that should close the popup evaluates the expression"
);
onPopupOpened = autocompletePopup.once("popup-opened");
await setInputValueForAutocompletion(hud, "var docx = 1; doc");
await setInputValueForAutocompletion(hud, "doc");
await onPopupOpened;
checkInputCompletionValue(hud, "ument", "completeNode has expected value");

Expand Down
4 changes: 3 additions & 1 deletion devtools/server/actors/webconsole.js
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,8 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
cursor,
frameActorId,
selectedNodeActor,
authorizedEvaluations
authorizedEvaluations,
expressionVars = []
) {
let dbgObject = null;
let environment = null;
Expand Down Expand Up @@ -1393,6 +1394,7 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
webconsoleActor: this,
selectedNodeActor,
authorizedEvaluations,
expressionVars,
});

if (result === null) {
Expand Down
1 change: 1 addition & 0 deletions devtools/shared/specs/webconsole.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ const webconsoleSpecPrototype = {
frameActor: Arg(2, "nullable:string"),
selectedNodeActor: Arg(3, "nullable:string"),
authorizedEvaluations: Arg(4, "nullable:json"),
expressionVars: Arg(5, "nullable:json"),
},
response: RetVal("console.autocomplete"),
},
Expand Down
15 changes: 14 additions & 1 deletion devtools/shared/webconsole/js-property-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ const MAX_AUTOCOMPLETIONS = (exports.MAX_AUTOCOMPLETIONS = 1500);
* evaluation result or create a debuggee value.
* - {String}: selectedNodeActor
* The actor id of the selected node in the inspector.
* - {Array<string>}: expressionVars
* Optional array containing variable defined in the expression. Those variables
* are extracted from CodeMirror state.
* @returns null or object
* If the inputValue is an unsafe getter and invokeUnsafeGetter is false, the
* following form is returned:
Expand Down Expand Up @@ -88,6 +91,7 @@ function JSPropertyProvider({
authorizedEvaluations = [],
webconsoleActor,
selectedNodeActor,
expressionVars = [],
}) {
if (cursor === undefined) {
cursor = inputValue.length;
Expand Down Expand Up @@ -242,10 +246,19 @@ function JSPropertyProvider({

let obj = dbgObject;
if (properties.length === 0) {
const environmentProperties = getMatchedPropsInEnvironment(env, search);
const expressionVariables = new Set(
expressionVars.filter(variableName => variableName.startsWith(matchProp))
);

for (const prop of environmentProperties) {
expressionVariables.add(prop);
}

return {
isElementAccess,
matchProp,
matches: getMatchedPropsInEnvironment(env, search),
matches: expressionVariables,
};
}

Expand Down
21 changes: 21 additions & 0 deletions devtools/shared/webconsole/test/unit/test_js_property_provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,27 @@ function runChecks(dbgObject, environment, sandbox) {

results = propertyProvider(`/*I'm a comment\n \t */\n\nt`);
test_has_result(results, "testObject");

info("Test local expression variables");
results = propertyProvider("b", { expressionVars: ["a", "b", "c"] });
test_has_result(results, "b");
Assert.equal(results.matches.has("a"), false);
Assert.equal(results.matches.has("c"), false);

info(
"Test that local expression variables are not included when accessing an object properties"
);
results = propertyProvider("testObject.prop", {
expressionVars: ["propLocal"],
});
Assert.equal(results.matches.has("propLocal"), false);
test_has_result(results, "propA");

results = propertyProvider("testObject['prop", {
expressionVars: ["propLocal"],
});
test_has_result(results, "'propA'");
Assert.equal(results.matches.has("propLocal"), false);
}

/**
Expand Down

0 comments on commit c1dd141

Please sign in to comment.