From 2f0ef803ca8a1037f00f7a6f98a20366b1dd27c9 Mon Sep 17 00:00:00 2001
From: Ariya Hidayat <ariya@metabase.com>
Date: Wed, 5 Jan 2022 08:48:36 -0800
Subject: [PATCH] Custom expression editor: show error marker (#19520)
---
frontend/src/metabase/css/core/colors.css | 1 +
.../src/metabase/lib/expressions/tokenizer.js | 8 ++++---
.../expressions/ExpressionEditorTextfield.jsx | 21 +++++++++++++++++++
.../components/expressions/expressions.css | 7 +++++++
.../lib/expressions/tokenizer.unit.spec.js | 3 +++
5 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/frontend/src/metabase/css/core/colors.css b/frontend/src/metabase/css/core/colors.css
index 0700adbd558..5cffdd5e9ca 100644
--- a/frontend/src/metabase/css/core/colors.css
+++ b/frontend/src/metabase/css/core/colors.css
@@ -16,6 +16,7 @@
--color-black: #2e353b;
--color-success: #84bb4c;
--color-error: #ed6e6e;
+ --color-error-background: #ed6e6e55;
--color-warning: #f9cf48;
--color-text-dark: #4c5773;
--color-text-medium: #949aab;
diff --git a/frontend/src/metabase/lib/expressions/tokenizer.js b/frontend/src/metabase/lib/expressions/tokenizer.js
index 971ed5ab632..64ae04aae82 100644
--- a/frontend/src/metabase/lib/expressions/tokenizer.js
+++ b/frontend/src/metabase/lib/expressions/tokenizer.js
@@ -322,7 +322,8 @@ export function tokenize(expression) {
if (error) {
const message = error;
const pos = t.start;
- errors.push({ message, pos });
+ const len = t.end - t.start;
+ errors.push({ message, pos, len });
}
} else {
const char = source[index];
@@ -330,6 +331,7 @@ export function tokenize(expression) {
break;
}
const pos = index;
+ const len = 1;
if (char === "]") {
const prev = tokens[tokens.length - 1];
const ref =
@@ -339,10 +341,10 @@ export function tokenize(expression) {
const message = ref
? t`Missing an opening bracket for ${ref}`
: t`Missing an opening bracket`;
- errors.push({ message, pos });
+ errors.push({ message, pos, len });
} else {
const message = t`Invalid character: ${char}`;
- errors.push({ message, pos });
+ errors.push({ message, pos, len });
}
++index;
}
diff --git a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
index 15a314cf8fb..62cf52ca593 100644
--- a/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
+++ b/frontend/src/metabase/query_builder/components/expressions/ExpressionEditorTextfield.jsx
@@ -333,6 +333,26 @@ export default class ExpressionEditorTextfield extends React.Component {
});
}
+ errorAsMarkers(errorMessage = null) {
+ if (errorMessage) {
+ const { pos, len } = errorMessage;
+ // Because not every error message offers location info (yet)
+ if (typeof pos === "number") {
+ return [
+ {
+ startRow: 0,
+ startCol: pos,
+ endRow: 0,
+ endCol: pos + len,
+ className: "error",
+ type: "text",
+ },
+ ];
+ }
+ }
+ return [];
+ }
+
commands = [
{
name: "arrowDown",
@@ -375,6 +395,7 @@ export default class ExpressionEditorTextfield extends React.Component {
commands={this.commands}
ref={this.input}
value={source}
+ markers={this.errorAsMarkers(errorMessage)}
focus={true}
highlightActiveLine={false}
wrapEnabled={true}
diff --git a/frontend/src/metabase/query_builder/components/expressions/expressions.css b/frontend/src/metabase/query_builder/components/expressions/expressions.css
index a6d977b1756..22d4089ed20 100644
--- a/frontend/src/metabase/query_builder/components/expressions/expressions.css
+++ b/frontend/src/metabase/query_builder/components/expressions/expressions.css
@@ -27,3 +27,10 @@
.expression-editor-textfield .ace_hidden-cursors .ace_cursor {
opacity: 0;
}
+
+.expression-editor-textfield .ace_content .error {
+ position: absolute;
+ border-bottom: 2px solid var(--color-error);
+ border-radius: 0px;
+ background-color: var(--color-error-background);
+}
diff --git a/frontend/test/metabase/lib/expressions/tokenizer.unit.spec.js b/frontend/test/metabase/lib/expressions/tokenizer.unit.spec.js
index 5f96a8a9e46..87aa050f75a 100644
--- a/frontend/test/metabase/lib/expressions/tokenizer.unit.spec.js
+++ b/frontend/test/metabase/lib/expressions/tokenizer.unit.spec.js
@@ -50,10 +50,12 @@ describe("metabase/lib/expressions/tokenizer", () => {
expect(errors("2e")[0].message).toEqual("Missing exponent");
expect(errors("3e+")[0].message).toEqual("Missing exponent");
expect(errors("4E-")[0].message).toEqual("Missing exponent");
+ expect(errors("4E-")[0].len).toEqual(3);
});
it("should catch a lone decimal point", () => {
expect(errors(".")[0].message).toEqual("Invalid character: .");
+ expect(errors(".")[0].len).toEqual(1);
});
it("should tokenize string literals", () => {
@@ -157,5 +159,6 @@ describe("metabase/lib/expressions/tokenizer", () => {
expect(errors("!")[0].message).toEqual("Invalid character: !");
expect(errors(" % @")[1].message).toEqual("Invalid character: @");
expect(errors(" #")[0].pos).toEqual(4);
+ expect(errors(" #")[0].len).toEqual(1);
});
});
--
GitLab