finos · texodus · Jun 15, 2021 · Jun 15, 2021
diff --git a/packages/perspective-viewer/test/results/linux.docker.json b/packages/perspective-viewer/test/results/linux.docker.json
@@ -54,7 +54,7 @@
     "Computed_Expressions_On_restore,_computed_expressions_in_classic_syntax_are_parsed_correctly_": "563f8c45e0ec32bc4b07d30ed5f0086e",
     "superstore_adds_computed_column_via_attribute": "f2fa526d6f0c168a47ab272bb7ae5771",
     "superstore_user_defined_aggregates_maintained_on_computed_columns": "c7aa2902feb512ceb1e1487de80878ff",
-    "__GIT_COMMIT__": "1c31a36cab7bb9452351984518e342dfd6c00d95",
+    "__GIT_COMMIT__": "55e80696b3f95ff9ce948fff91ae690895d9819e",
     "blank_Handles_reloading_with_a_schema_": "2f23d1416fc97e07a3a21e318411e0d1",
     "superstore_doesn_t_leak_tables_": "5bb762b2860eb5af067bb83afbca3264",
     "superstore_doesn_t_leak_elements_": "b8a3a84406fede4823cd51e07b817752",
@@ -94,17 +94,17 @@
     "filters_less_than_ISO_string_on_datetime_column": "b7a0f123b4d5dea1b710aeea7e61572a",
     "filters_less_than_US_locale_string_on_datetime_column": "13e4f63b1060272bff4f01382e565e82",
     "blank_Should_load_a_promise_to_a_table_": "a3841665c5fc438c5e959c9858540744",
-    "Expressions_click_on_add_column_button_opens_the_expression_UI_": "21a2a2193df81b31112548f7254a0243",
+    "Expressions_click_on_add_column_button_opens_the_expression_UI_": "b909fc09ef2d2062896bf0c5d902589f",
     "Expressions_click_on_close_button_closes_the_expression_UI_": "a02605db902dc06ea824fb013117ae6b",
-    "Expressions_An_expression_with_unknown_symbols_should_disable_the_save_button": "6caf7343204eeb2ede8ffb4fb774b196",
-    "Expressions_A_type-invalid_expression_should_disable_the_save_button": "999387fecc94a112cefa4de79502bb29",
-    "Expressions_An_expression_with_invalid_input_columns_should_disable_the_save_button": "bbe27f53050713f77dc5aa5364e7eb64",
+    "Expressions_An_expression_with_unknown_symbols_should_disable_the_save_button": "c79b2b1ea739fc3f4cfbb155c4768bf0",
+    "Expressions_A_type-invalid_expression_should_disable_the_save_button": "beca6e3c90f2032a24f95f7e7e175cee",
+    "Expressions_An_expression_with_invalid_input_columns_should_disable_the_save_button": "b53c4672d455f775848457078e05c755",
     "Expressions_Should_show_the_help_panel": "dc16ac2736a46205fa13a30cc2842510",
     "Expressions_Non-aliased_expressions_should_have_autogenerated_alias": "0478c89a614c402083c7bf0eca48a66d",
     "Expressions_Should_skip_if_trying_to_set_an_expression_without_an_alias": "d13e8556321b0a8bbbf5122ce4f95754",
-    "Expressions_Should_prevent_saving_a_duplicate_expression_alias": "fe8d36b483dff4c7be193b22157fbc49",
-    "Expressions_Should_not_prevent_saving_a_duplicate_expression_with_a_different_alias": "4ed53b7ffe61c1e4106c0eac364c85c9",
-    "Expressions_Should_prevent_saving_a_duplicate_expression": "52de710bcc741a76db68ec8c41f1e0ad",
+    "Expressions_Should_prevent_saving_a_duplicate_expression_alias": "f4975b44f2cebb81d4a6a99901f3c746",
+    "Expressions_Should_not_prevent_saving_a_duplicate_expression_with_a_different_alias": "3c27a1c305c2e5ce4c3e44c055aa15dd",
+    "Expressions_Should_prevent_saving_a_duplicate_expression": "fb0258131aa8514a8e05db26aeb01c14",
     "Expressions_Removing_expressions_should_reset_active_columns,_pivots,_sort,_and_filter_": "e31a7da6f65bf5edc7d6582a4514a069",
     "Expressions_Resetting_the_viewer_with_expressions_should_place_columns_in_the_inactive_list_": "17722593bb3bc9564a29edf1859c6951",
     "Expressions_Resetting_the_viewer_with_columns_in_active_columns_should_reset_columns_but_not_delete_columns_": "17722593bb3bc9564a29edf1859c6951",
@@ -114,7 +114,7 @@
     "Expressions_saving_without_an_expression_should_fail_as_button_is_disabled_": "462c0e101f72c021987def75335a967c",
     "Expressions_saving_a_single_expression_should_add_it_to_inactive_columns_": "28d815bfccfcd54f6f2ed6b683d86055",
     "Expressions_saving_multiple_expressions_should_add_it_to_inactive_columns_": "74b4a63079bc47b9809a92967335d3bf",
-    "Expressions_Expression_columns_should_persist_when_new_views_are_created_": "52a1d084281078b18aef439f3dd15875",
+    "Expressions_Expression_columns_should_persist_when_new_views_are_created_": "6aa47661329b23d7f0fd8f243ee89ca2",
     "Expressions_Expression_columns_should_persist_when_new_columns_are_added_": "951b31155154d97badbda8dea3a28feb",
     "Expressions_aggregates_by_expression_column_": "e7ba48d18a68dd96282d79fc0ba609bd",
     "Expressions_sets_column_attribute_with_expression_column_": "516a4292d1784b97fe4de8fa37483e4f",

diff --git a/rust/perspective-vieux/src/less/expression-editor.less b/rust/perspective-vieux/src/less/expression-editor.less
@@ -13,7 +13,6 @@
     position: absolute;
     z-index: 10000;
     outline: none;
-    padding-top: 6px;
     font-family: "Open Sans";
     font-size: 12px;
     font-weight: 300;
@@ -31,8 +30,11 @@
             appearance: var(--monaco-container--appearance, auto);
         }
     }
-    .monaco-editor .cursors-layer .cursor {
-        visibility: var(--monaco-cursor--visibility, inherit) !important;
+    .monaco-editor {
+        padding-top: 6px;
+        .cursors-layer .cursor {
+            visibility: var(--monaco-cursor--visibility, inherit) !important;
+        }
     }
 
     #psp-expression-editor-actions {
@@ -53,6 +55,7 @@
 
         &:hover {
             cursor: pointer;
+            background-color: rgba(0,0,0,0.05);
         }
 
         &[disabled] {
@@ -67,7 +70,7 @@
             font-size: 12px;
             height: 36px;
             opacity: 0.1;
-            background-color: var(--plugin--background, #fff);
+            background-color: inherit;
             transition: background-color 0.2s;
             padding-right: 0 6px;
 

diff --git a/rust/perspective-vieux/src/rust/components/expression_editor.rs b/rust/perspective-vieux/src/rust/components/expression_editor.rs
@@ -9,6 +9,7 @@
 use crate::exprtk::*;
 use crate::session::Session;
 use crate::utils::monaco::*;
+use crate::utils::perspective::*;
 use crate::utils::*;
 
 use std::cell::RefCell;
@@ -43,7 +44,7 @@ pub struct ExpressionEditor {
     top: u32,
     left: u32,
     container: NodeRef,
-    editor: Rc<RefCell<Option<MonacoEditor>>>,
+    editor: Rc<RefCell<Option<(Editor, MonacoEditor)>>>,
     props: ExpressionEditorProps,
     link: ComponentLink<Self>,
     save_enabled: bool,
@@ -84,7 +85,7 @@ impl Component for ExpressionEditor {
                 self.top = top;
                 self.left = left;
                 match self.editor.borrow().as_ref() {
-                    Some(x) => x.set_value(""),
+                    Some((_, x)) => x.set_value(""),
                     None => {}
                 }
 
@@ -103,7 +104,7 @@ impl Component for ExpressionEditor {
                 if self.save_enabled {
                     match self.editor.borrow().as_ref() {
                         None => {}
-                        Some(x) => {
+                        Some((_, x)) => {
                             let expr = x.get_value();
                             (self.props.on_save)(expr);
                             x.set_value("");
@@ -123,7 +124,7 @@ impl Component for ExpressionEditor {
         if first_render {
             let _promise = future_to_promise(self.clone().init_monaco_editor());
         } else if self.editor.borrow().is_some() {
-            self.editor.borrow().as_ref().unwrap().focus();
+            self.editor.borrow().as_ref().unwrap().1.focus();
         }
     }
 
@@ -172,7 +173,7 @@ impl ExpressionEditor {
         editor.add_command(cmd, self.on_save.as_ref().as_ref().unchecked_ref());
         let cb = self.on_validate.as_ref().as_ref().unchecked_ref();
         editor.get_model().on_did_change_content(cb);
-        *self.editor.borrow_mut() = Some(editor.clone());
+        *self.editor.borrow_mut() = Some((monaco, editor.clone()));
         await_animation_frame().await?;
         editor.focus();
         (self.props.on_init)();
@@ -182,11 +183,34 @@ impl ExpressionEditor {
     /// Validate the editor's current value, and toggle the Save button state
     /// if the expression is valid.
     async fn validate_expr(self) -> Result<JsValue, JsValue> {
-        let expr = self.editor.borrow().as_ref().unwrap().get_value();
+        let (monaco, editor) = self.editor.borrow().as_ref().unwrap().clone();
+        let expr = editor.get_value();
         (self.props.on_validate)(true);
-        let result = self.props.session.validate_expr(expr).await?;
-        let msg = ExpressionEditorMsg::EnableSave(result);
-        self.link.send_message(msg);
+        let model = editor.get_model();
+        let arr = js_sys::Array::new();
+        let msg = match self.props.session.validate_expr(expr).await? {
+            None => true,
+            Some(err) => {
+                let marker = error_to_market(err);
+                arr.push(&JsValue::from_serde(&marker).unwrap());
+                false
+            }
+        };
+
+        monaco.set_model_markers(&model, "exprtk", &arr);
+        self.link.send_message(ExpressionEditorMsg::EnableSave(msg));
         Ok(JsValue::UNDEFINED)
     }
 }
+
+fn error_to_market(err: PerspectiveValidationError) -> MonacoModelMarker<'static> {
+    MonacoModelMarker {
+        code: "".to_owned(),
+        start_line_number: err.line + 1,
+        end_line_number: err.line + 1,
+        start_column: err.column,
+        end_column: err.column,
+        severity: "error",
+        message: err.error_message,
+    }
+}
diff --git a/rust/perspective-vieux/src/rust/exprtk/language.rs b/rust/perspective-vieux/src/rust/exprtk/language.rs
@@ -41,6 +41,10 @@ thread_local! {
             //     open: "\"",
             //     close: "\""
             // },
+            AutoClosingPairs {
+                open: "'",
+                close: "'"
+            },
             AutoClosingPairs {
                 open: "(",
                 close: ")"

diff --git a/rust/perspective-vieux/src/rust/session.rs b/rust/perspective-vieux/src/rust/session.rs
@@ -84,7 +84,7 @@ impl Session {
         }
     }
 
-    pub async fn validate_expr(self, expr: JsValue) -> Result<bool, JsValue> {
+    pub async fn validate_expr(self, expr: JsValue) -> Result<Option<PerspectiveValidationError>, JsValue> {
         let arr = Array::new();
         arr.push(&expr);
         let result = self
@@ -94,8 +94,16 @@ impl Session {
             .unwrap()
             .validate_expressions(arr)
             .await?;
-        let error_count = js_sys::Object::keys(&result.errors()).length();
-        Ok(error_count == 0)
+
+        let errors = result.errors();
+        let error_keys = js_sys::Object::keys(&errors);
+        if error_keys.length() > 0 {
+            let js_err = js_sys::Reflect::get(&errors, &error_keys.get(0))?;
+            let error: PerspectiveValidationError = js_err.into_serde().unwrap();
+            Ok(Some(error))
+        } else {
+            Ok(None)
+        }
     }
 
     pub fn copy_to_clipboard(&self, flat: bool) {

diff --git a/rust/perspective-vieux/src/rust/utils/monaco.rs b/rust/perspective-vieux/src/rust/utils/monaco.rs
@@ -65,6 +65,7 @@ extern "C" {
     #[wasm_bindgen(method, getter)]
     pub fn languages(this: &MonacoModule) -> Languages;
 
+    #[derive(Clone)]
     pub type Editor;
 
     #[wasm_bindgen(method)]
@@ -73,6 +74,9 @@ extern "C" {
     #[wasm_bindgen(method, js_name = "defineTheme")]
     pub fn define_theme(this: &Editor, id: &str, options: JsValue);
 
+    #[wasm_bindgen(method, js_name = "setModelMarkers")]
+    pub fn set_model_markers(this: &Editor, model: &MonacoModel, id: &str, errors: &js_sys::Array);
+
     pub type Languages;
 
     #[wasm_bindgen(method)]
@@ -236,3 +240,15 @@ pub struct DefineThemeToken<'a> {
     pub foreground: &'a str,
     pub font_style: Option<&'a str>,
 }
+
+#[derive(Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct MonacoModelMarker<'a> {
+    pub code: String,
+    pub start_line_number: u32,
+    pub end_line_number: u32,
+    pub start_column: u32,
+    pub end_column: u32,
+    pub severity: &'a str,
+    pub message: String
+}
diff --git a/rust/perspective-vieux/src/rust/utils/perspective.rs b/rust/perspective-vieux/src/rust/utils/perspective.rs
@@ -7,6 +7,7 @@
 // file.
 
 use js_sys::Array;
+use serde::Deserialize;
 use wasm_bindgen::prelude::*;
 use wasm_bindgen::JsCast;
 
@@ -119,3 +120,11 @@ impl PerspectiveJsView {
     async_typed!(_delete, delete() -> ());
     async_typed!(_get_config, get_config() -> PerspectiveJsViewConfig);
 }
+
+#[derive(Deserialize)]
+#[serde()]
+pub struct PerspectiveValidationError {
+    pub error_message: String,
+    pub line: u32,
+    pub column: u32,
+}