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

Custom expression editor: do not always suggest an open parenthesis (#20533)

For functions which do not accept any arguments, i.e. COUNT and
CUMULATIVECOUNT, do not suggest the completion with the "(" character.
parent 2ebb5958
No related branches found
No related tags found
No related merge requests found
...@@ -19,6 +19,12 @@ import { ...@@ -19,6 +19,12 @@ import {
EDITOR_FK_SYMBOLS, EDITOR_FK_SYMBOLS,
} from "./config"; } from "./config";
const suggestionText = func => {
const { displayName, args } = func;
const suffix = args.length > 0 ? "(" : " ";
return displayName + suffix;
};
export function suggest({ export function suggest({
source, source,
query, query,
...@@ -59,7 +65,7 @@ export function suggest({ ...@@ -59,7 +65,7 @@ export function suggest({
.map(func => ({ .map(func => ({
type: "functions", type: "functions",
name: func.displayName, name: func.displayName,
text: func.displayName + "(", text: suggestionText(func),
index: targetOffset, index: targetOffset,
icon: "function", icon: "function",
order: 1, order: 1,
...@@ -73,7 +79,7 @@ export function suggest({ ...@@ -73,7 +79,7 @@ export function suggest({
.map(func => ({ .map(func => ({
type: "aggregations", type: "aggregations",
name: func.displayName, name: func.displayName,
text: func.displayName + "(", text: suggestionText(func),
index: targetOffset, index: targetOffset,
icon: "function", icon: "function",
order: 1, order: 1,
......
...@@ -138,7 +138,7 @@ export default class ExpressionEditorTextfield extends React.Component { ...@@ -138,7 +138,7 @@ export default class ExpressionEditorTextfield extends React.Component {
const extraTrim = openParen && alreadyOpenParen ? 1 : 0; const extraTrim = openParen && alreadyOpenParen ? 1 : 0;
const replacement = suggested.slice(0, suggested.length - extraTrim); const replacement = suggested.slice(0, suggested.length - extraTrim);
const updatedExpression = prefix + replacement.trim() + postfix; const updatedExpression = prefix + replacement + postfix;
this.handleExpressionChange(updatedExpression); this.handleExpressionChange(updatedExpression);
const caretPos = updatedExpression.length - postfix.length; const caretPos = updatedExpression.length - postfix.length;
......
...@@ -207,9 +207,9 @@ describe("metabase/lib/expression/suggest", () => { ...@@ -207,9 +207,9 @@ describe("metabase/lib/expression/suggest", () => {
it("should suggest partial matches after an aggregation", () => { it("should suggest partial matches after an aggregation", () => {
expect(suggest({ source: "average(c", ...aggregationOpts })).toEqual([ expect(suggest({ source: "average(c", ...aggregationOpts })).toEqual([
// FIXME: the next four should not appear // FIXME: the next four should not appear
{ type: "aggregations", text: "Count(" }, { type: "aggregations", text: "Count " },
{ type: "aggregations", text: "CountIf(" }, { type: "aggregations", text: "CountIf(" },
{ type: "aggregations", text: "CumulativeCount(" }, { type: "aggregations", text: "CumulativeCount " },
{ type: "aggregations", text: "CumulativeSum(" }, { type: "aggregations", text: "CumulativeSum(" },
{ type: "fields", text: "[C] " }, { type: "fields", text: "[C] " },
{ type: "fields", text: "[count] " }, { type: "fields", text: "[count] " },
...@@ -223,9 +223,9 @@ describe("metabase/lib/expression/suggest", () => { ...@@ -223,9 +223,9 @@ describe("metabase/lib/expression/suggest", () => {
it("should suggest partial matches in aggregation", () => { it("should suggest partial matches in aggregation", () => {
expect(suggest({ source: "1 + C", ...aggregationOpts })).toEqual([ expect(suggest({ source: "1 + C", ...aggregationOpts })).toEqual([
{ type: "aggregations", text: "Count(" }, { type: "aggregations", text: "Count " },
{ type: "aggregations", text: "CountIf(" }, { type: "aggregations", text: "CountIf(" },
{ type: "aggregations", text: "CumulativeCount(" }, { type: "aggregations", text: "CumulativeCount " },
{ type: "aggregations", text: "CumulativeSum(" }, { type: "aggregations", text: "CumulativeSum(" },
{ type: "fields", text: "[C] " }, { type: "fields", text: "[C] " },
{ type: "fields", text: "[count] " }, { type: "fields", text: "[count] " },
...@@ -258,7 +258,7 @@ describe("metabase/lib/expression/suggest", () => { ...@@ -258,7 +258,7 @@ describe("metabase/lib/expression/suggest", () => {
startRule: "aggregation", startRule: "aggregation",
}), }),
).toEqual([ ).toEqual([
{ type: "aggregations", text: "Count(" }, { type: "aggregations", text: "Count " },
{ type: "aggregations", text: "CountIf(" }, { type: "aggregations", text: "CountIf(" },
]); ]);
}); });
......
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