Skip to content
Snippets Groups Projects
Unverified Commit 4615909d authored by Ariya Hidayat's avatar Ariya Hidayat Committed by GitHub
Browse files

Custom expression: refactor the logic for the help text (#15897)

Use the lexical determination of enclosing function, which makes it more
robust agains syntax error or incompleteness. Also, increase the test
coverage.

* Fix testing the operator

* Offer the help text upon a complete field reference (with some tests)
parent 1c8292f3
No related branches found
No related tags found
No related merge requests found
import { tokenize, TOKEN } from "./tokenizer";
import { tokenize, TOKEN, OPERATOR as OP } from "./tokenizer";
import _ from "underscore";
// Given an expression, get the last identifier as the prefix match.
......@@ -19,3 +19,35 @@ export function partialMatch(expression) {
return null;
}
// Given an expression, find the inner-most function name.
// Examples:
// "Concat([FirstName]," returns "Concat"
// "Concat([Category], Lower([Type]" returns "Lower"
// "X() + Concat(Type,Upper(Vendor),Y()" return "Concat"
// "[Tax] / 3" returns null (not a function call)
export function enclosingFunction(expression) {
const { tokens } = tokenize(expression);
const isOpen = t => t.op === OP.OpenParenthesis;
const isClose = t => t.op === OP.CloseParenthesis;
let parenCount = 0;
for (let i = tokens.length - 1; i > 0; --i) {
const token = tokens[i];
if (isClose(token)) {
--parenCount;
} else if (isOpen(token)) {
++parenCount;
if (parenCount === 1) {
const prev = tokens[i - 1];
if (prev.type === TOKEN.Identifier) {
return expression.slice(prev.start, prev.end);
}
}
}
}
return null;
}
......@@ -32,7 +32,7 @@ import {
lexerWithRecovery,
} from "./lexer";
import { partialMatch } from "./completer";
import { partialMatch, enclosingFunction } from "./completer";
import getHelpText from "./helper_text_strings";
......@@ -43,6 +43,7 @@ import {
MBQL_CLAUSES,
isExpressionType,
getFunctionArgType,
getMBQLName,
EXPRESSION_TYPES,
EDITOR_FK_SYMBOLS,
} from "./config";
......@@ -67,18 +68,29 @@ export function suggest({
expressionName,
} = {}) {
const partialSource = source.slice(0, targetOffset);
const matchPrefix = partialMatch(partialSource);
const partialSuggestionMode =
matchPrefix && matchPrefix.length > 0 && _.last(matchPrefix) !== "]";
if (!partialSuggestionMode) {
const functionDisplayName = enclosingFunction(partialSource);
if (functionDisplayName) {
const helpText = getHelpText(getMBQLName(functionDisplayName));
if (helpText) {
return { helpText };
}
}
}
const lexResult = lexerWithRecovery.tokenize(partialSource);
if (lexResult.errors.length > 0) {
throw lexResult.errors;
}
let tokenVector = lexResult.tokens;
const matchPrefix = partialMatch(partialSource);
const partialSuggestionMode = matchPrefix && matchPrefix.length > 0;
if (partialSuggestionMode) {
tokenVector = tokenVector.slice(0, -1);
}
const context = getContext({
cst,
tokenVector,
......@@ -86,10 +98,6 @@ export function suggest({
startRule,
}) || { expectedType: startRule };
const helpText = context.clause && getHelpText(context.clause.name);
if (!partialSuggestionMode && helpText) {
return { helpText };
}
const { expectedType } = context;
let finalSuggestions = [];
......
import { partialMatch } from "metabase/lib/expressions/completer";
import {
partialMatch,
enclosingFunction,
} from "metabase/lib/expressions/completer";
describe("metabase/lib/expressions/completer", () => {
describe("partialMatch", () => {
......@@ -28,4 +31,35 @@ describe("metabase/lib/expressions/completer", () => {
expect(partialMatch(" ")).toEqual(null);
});
});
describe("enclosingFunction", () => {
it("should get the correct name", () => {
expect(enclosingFunction("isnull([ID")).toEqual("isnull");
});
it("should ignore completed function construct", () => {
expect(enclosingFunction("Upper([Name])")).toEqual(null);
});
it("should handle multiple arguments", () => {
expect(enclosingFunction("Concat(First,Middle,Last")).toEqual("Concat");
});
it("should handle nested function calls", () => {
expect(enclosingFunction("Concat(X,Lower(Y,Z")).toEqual("Lower");
expect(enclosingFunction("P() + Q(R,S(7),T(")).toEqual("T");
expect(enclosingFunction("P() + Q(R,S(7),T()")).toEqual("Q");
});
it("should ignore non-function calls", () => {
expect(enclosingFunction("1")).toEqual(null);
expect(enclosingFunction("2 +")).toEqual(null);
expect(enclosingFunction("X OR")).toEqual(null);
});
it("should handle empty input", () => {
expect(enclosingFunction("")).toEqual(null);
expect(enclosingFunction(" ")).toEqual(null);
});
});
});
......@@ -944,6 +944,29 @@ describe("scenarios > question > notebook", () => {
});
});
describe("help text", () => {
it("should appear while inside a function", () => {
openProductsTable({ mode: "notebook" });
cy.findByText("Custom column").click();
cy.get("[contenteditable='true']").type("Lower(");
cy.findByText("lower(text)");
});
it("should not appear while outside a function", () => {
openProductsTable({ mode: "notebook" });
cy.findByText("Custom column").click();
cy.get("[contenteditable='true']").type("Lower([Category])");
cy.findByText("lower(text)").should("not.exist");
});
it("should appear after a field reference", () => {
openProductsTable({ mode: "notebook" });
cy.findByText("Custom column").click();
cy.get("[contenteditable='true']").type("Lower([Category]");
cy.findByText("lower(text)");
});
});
it("should correctly insert function suggestion with the opening parenthesis", () => {
openProductsTable({ mode: "notebook" });
cy.findByText("Custom column").click();
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment