Эх сурвалжийг харах

[editor] Properly mark error lines in Editor V2

Johan Ahlen 5 жил өмнө
parent
commit
61252e641a

+ 75 - 45
desktop/core/src/desktop/js/apps/notebook2/components/aceEditor/AceAnchoredRange.ts

@@ -16,10 +16,11 @@
 
 import ace from 'ext/aceHelper';
 import { Ace } from 'ext/ace';
+
 import { Disposable } from 'components/utils/SubscriptionTracker';
 import { ParsedLocation } from 'parse/types';
 
-const clearGutterCss = (
+const removeGutterDecoration = (
   cssClass: string,
   session: Ace.Session,
   startRow: number,
@@ -30,7 +31,7 @@ const clearGutterCss = (
   }
 };
 
-const setGutterCss = (
+const addGutterDecoration = (
   cssClass: string,
   session: Ace.Session,
   startRow: number,
@@ -43,19 +44,23 @@ const setGutterCss = (
 
 export default class AceAnchoredRange implements Disposable {
   editor: Ace.Editor;
-  startAnchor: Ace.Anchor;
-  endAnchor: Ace.Anchor;
+  gutterStart: Ace.Anchor;
+  gutterEnd: Ace.Anchor;
+  rowStart: Ace.Anchor;
+  rowEnd: Ace.Anchor;
 
   changed = false;
-  markerCssClasses: { [clazz: string]: number } = {};
-  gutterCssClasses: { [clazz: string]: { start: number; end: number } } = {};
+  rowMarkerSpec?: { cssClass: string; rowOffset: number; marker: number };
+  gutterSpec?: { cssClass: string; span: { start: number; end: number } };
   refreshThrottle = -1;
 
   constructor(editor: Ace.Editor) {
     this.editor = editor;
     const doc = this.editor.getSession().doc;
-    this.startAnchor = doc.createAnchor(0, 0);
-    this.endAnchor = doc.createAnchor(0, 0);
+    this.gutterStart = doc.createAnchor(0, 0);
+    this.gutterEnd = doc.createAnchor(0, 0);
+    this.rowStart = doc.createAnchor(0, 0);
+    this.rowEnd = doc.createAnchor(0, 0);
 
     this.attachChangeHandler();
   }
@@ -66,72 +71,97 @@ export default class AceAnchoredRange implements Disposable {
       this.refreshThrottle = window.setTimeout(this.refresh.bind(this), 10);
     };
 
-    this.startAnchor.on('change', throttledRefresh);
-    this.endAnchor.on('change', throttledRefresh);
+    this.gutterStart.on('change', throttledRefresh);
+    this.gutterEnd.on('change', throttledRefresh);
+    this.rowStart.on('change', throttledRefresh);
+    this.rowEnd.on('change', throttledRefresh);
   }
 
   refresh(): void {
     const session = this.editor.getSession();
-    const newStart = this.startAnchor.getPosition();
-    const newEnd = this.endAnchor.getPosition();
-
-    Object.keys(this.gutterCssClasses).forEach(cssClass => {
-      const rowSpan = this.gutterCssClasses[cssClass];
-      clearGutterCss(cssClass, session, rowSpan.start, rowSpan.end);
+    const newStart = this.gutterStart.getPosition();
+    const newEnd = this.gutterEnd.getPosition();
+    if (this.gutterSpec) {
+      const rowSpan = this.gutterSpec.span;
+      removeGutterDecoration(this.gutterSpec.cssClass, session, rowSpan.start, rowSpan.end);
       rowSpan.start = newStart.row;
       rowSpan.end = newEnd.row;
-      setGutterCss(cssClass, session, rowSpan.start, rowSpan.end);
-    });
+      addGutterDecoration(this.gutterSpec.cssClass, session, rowSpan.start, rowSpan.end);
+    }
+    if (this.rowMarkerSpec) {
+      const offset = this.rowMarkerSpec.rowOffset;
+      const cssClass = this.rowMarkerSpec.cssClass;
+      this.removeMarkerRowCss();
+      this.setMarkerRowCss(cssClass, offset);
+    }
   }
 
   move(parseLocation: ParsedLocation, leadingEmptyLineCount?: number): void {
     const lastRow = parseLocation.last_line - 1;
     const firstRow = Math.min(lastRow, parseLocation.first_line - 1 + (leadingEmptyLineCount || 0));
     const firstCol = leadingEmptyLineCount ? 0 : parseLocation.first_column;
-    this.startAnchor.setPosition(firstRow, firstCol);
-    this.endAnchor.setPosition(lastRow, parseLocation.last_column);
+    this.gutterStart.setPosition(firstRow, firstCol);
+    this.gutterEnd.setPosition(lastRow, parseLocation.last_column);
+    if (this.rowMarkerSpec) {
+      this.refreshRowAnchors(this.rowMarkerSpec.rowOffset);
+    }
   }
 
-  addGutterCss(cssClass: string): void {
+  setGutterCss(cssClass: string): void {
+    if (this.gutterSpec) {
+      this.removeGutterCss();
+    }
     const session = this.editor.getSession();
-    const startRow = this.startAnchor.getPosition().row;
-    const endRow = this.endAnchor.getPosition().row;
-    this.gutterCssClasses[cssClass] = { start: startRow, end: endRow };
-    setGutterCss(cssClass, session, startRow, endRow);
+    const startRow = this.gutterStart.getPosition().row;
+    const endRow = this.gutterEnd.getPosition().row;
+    this.gutterSpec = { cssClass, span: { start: startRow, end: endRow } };
+    addGutterDecoration(cssClass, session, startRow, endRow);
+  }
+
+  refreshRowAnchors(rowOffset: number): void {
+    const markerRow = this.gutterStart.row + rowOffset;
+    this.rowStart.setPosition(markerRow, 0);
+    this.rowEnd.setPosition(markerRow, this.editor.getSession().getLine(markerRow).length);
   }
 
-  addMarkerCss(cssClass: string): void {
-    if (!this.markerCssClasses[cssClass]) {
-      const AceRange = ace.require('ace/range').Range;
-      const range = new AceRange(0, 0, 0, 0);
-      range.start = this.startAnchor;
-      range.end = this.endAnchor;
-      this.markerCssClasses[cssClass] = this.editor.getSession().addMarker(range, cssClass);
+  setMarkerRowCss(cssClass: string, rowOffset: number): void {
+    if (this.rowMarkerSpec) {
+      this.removeMarkerRowCss();
     }
+    this.refreshRowAnchors(rowOffset);
+    const AceRange = ace.require('ace/range').Range;
+    const range: Ace.Range = new AceRange(0, 0, 0, 0);
+    range.start = this.rowStart;
+    range.end = this.rowEnd;
+    const marker = this.editor.getSession().addMarker(range, cssClass);
+    this.rowMarkerSpec = { cssClass, rowOffset, marker };
+    this.rowMarkerSpec.marker = marker;
   }
 
-  removeMarkerCss(cssClass: string): void {
-    if (this.markerCssClasses[cssClass]) {
-      this.editor.getSession().removeMarker(this.markerCssClasses[cssClass]);
-      delete this.markerCssClasses[cssClass];
+  removeMarkerRowCss(): void {
+    if (this.rowMarkerSpec) {
+      this.editor.getSession().removeMarker(this.rowMarkerSpec.marker);
+      this.rowMarkerSpec = undefined;
     }
   }
 
-  removeGutterCss(cssClass: string): void {
-    if (this.gutterCssClasses[cssClass]) {
+  removeGutterCss(): void {
+    if (this.gutterSpec) {
       const session = this.editor.getSession();
-      const rowSpan = this.gutterCssClasses[cssClass];
-      delete this.gutterCssClasses[cssClass];
-      clearGutterCss(cssClass, session, rowSpan.start, rowSpan.end);
+      const rowSpan = this.gutterSpec.span;
+      removeGutterDecoration(this.gutterSpec.cssClass, session, rowSpan.start, rowSpan.end);
+      this.gutterSpec = undefined;
     }
   }
 
   dispose(): void {
     window.clearTimeout(this.refreshThrottle);
-    this.startAnchor.detach();
-    this.endAnchor.detach();
+    this.gutterStart.detach();
+    this.gutterEnd.detach();
+    this.rowStart.detach();
+    this.rowEnd.detach();
 
-    Object.keys(this.gutterCssClasses).forEach(this.removeGutterCss.bind(this));
-    Object.keys(this.markerCssClasses).forEach(this.removeMarkerCss.bind(this));
+    this.removeGutterCss();
+    this.removeMarkerRowCss();
   }
 }

+ 0 - 46
desktop/core/src/desktop/js/apps/notebook2/components/aceEditor/AceEditor.vue

@@ -168,52 +168,6 @@
         }
       }
 
-      // const processErrorsAndWarnings = (
-      //   type: string,
-      //   list: { line: number; message: string; col: number | null }[]
-      // ): void => {
-      //   editor.clearErrorsAndWarnings(type);
-      //   let offset = 0;
-      //   if ((<EditorInterpreter>this.executor.connector()).is_sql && editor.getSelectedText()) {
-      //     const selectionRange = editor.getSelectionRange();
-      //     offset = Math.min(selectionRange.start.row, selectionRange.end.row);
-      //   }
-      //   if (list.length > 0) {
-      //     list.forEach((item, cnt) => {
-      //       if (item.line !== null) {
-      //         if (type === 'error') {
-      //           editor.addError(item.message, item.line + offset);
-      //         } else {
-      //           editor.addWarning(item.message, item.line + offset);
-      //         }
-      //         if (cnt === 0) {
-      //           editor.scrollToLine(item.line + offset, true, true, () => {
-      //             /* empty */
-      //           });
-      //           if (item.col !== null) {
-      //             editor.renderer.scrollCursorIntoView(
-      //               { row: item.line + offset, column: item.col + 10 },
-      //               0.5
-      //             );
-      //           }
-      //         }
-      //       }
-      //     });
-      //   }
-      // };
-
-      // const errorsSub = snippet.errors.subscribe(newErrors => {
-      //   processErrorsAndWarnings('error', newErrors);
-      // });
-      //
-      // const aceWarningsSub = snippet.aceWarnings.subscribe(newWarnings => {
-      //   processErrorsAndWarnings('warning', newWarnings);
-      // });
-      //
-      // const aceErrorsSub = snippet.aceErrors.subscribe(newErrors => {
-      //   processErrorsAndWarnings('error', newErrors);
-      // });
-
       const triggerChange = () => {
         this.$emit('value-changed', removeUnicodes(editor.getValue()));
       };

+ 28 - 23
desktop/core/src/desktop/js/apps/notebook2/components/aceEditor/AceGutterHandler.ts

@@ -14,13 +14,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import { Ace } from 'ext/ace';
+
+import AceAnchoredRange from './AceAnchoredRange';
+import { ACTIVE_STATEMENT_CHANGED_EVENT } from './AceLocationHandler';
+import { EXECUTABLE_UPDATED_EVENT } from 'apps/notebook2/execution/executable';
 import Executor from 'apps/notebook2/execution/executor';
+import SqlExecutable from 'apps/notebook2/execution/sqlExecutable';
 import SubscriptionTracker, { Disposable } from 'components/utils/SubscriptionTracker';
-import { Ace } from 'ext/ace';
 import { ParsedSqlStatement } from 'parse/sqlStatementsParser';
-import { EXECUTABLE_UPDATED_EVENT } from 'apps/notebook2/execution/executable';
-import { ACTIVE_STATEMENT_CHANGED_EVENT } from './AceLocationHandler';
-import AceAnchoredRange from 'ko/bindings/ace/aceAnchoredRange';
 
 const LINE_BREAK_REGEX = /(\r\n)|(\n)|(\r)/g;
 const LEADING_WHITE_SPACE_REGEX = /^\s+/;
@@ -47,6 +49,7 @@ export default class AceGutterHandler implements Disposable {
   editor: Ace.Editor;
   editorId: string;
   executor: Executor;
+  trackedAnchors: Map<string, AceAnchoredRange> = new Map();
 
   subTracker: SubscriptionTracker = new SubscriptionTracker();
 
@@ -56,7 +59,7 @@ export default class AceGutterHandler implements Disposable {
     this.executor = options.executor;
 
     const activeStatementAnchor = new AceAnchoredRange(this.editor);
-    activeStatementAnchor.addGutterCss(ACTIVE_CSS);
+    activeStatementAnchor.setGutterCss(ACTIVE_CSS);
 
     this.subTracker.subscribe(ACTIVE_STATEMENT_CHANGED_EVENT, statementDetails => {
       if (statementDetails.id !== this.editorId || !statementDetails.activeStatement) {
@@ -69,35 +72,37 @@ export default class AceGutterHandler implements Disposable {
     this.subTracker.addDisposable(activeStatementAnchor);
 
     if (this.executor) {
-      this.subTracker.subscribe(EXECUTABLE_UPDATED_EVENT, executable => {
+      this.subTracker.subscribe(EXECUTABLE_UPDATED_EVENT, (executable: SqlExecutable) => {
         if (executable.executor === this.executor) {
+          let anchor = this.trackedAnchors.get(executable.id);
+          if (!anchor) {
+            anchor = new AceAnchoredRange(this.editor);
+            this.trackedAnchors.set(executable.id, anchor);
+          }
+
           if (executable.lost) {
-            if (executable.observerState.aceAnchor) {
-              executable.observerState.aceAnchor.dispose();
-              delete executable.observerState.aceAnchor;
-            }
+            anchor.dispose();
+            this.trackedAnchors.delete(executable.id);
             return;
           }
 
+          anchor.removeGutterCss();
+          anchor.removeMarkerRowCss();
+
           const statement = executable.parsedStatement;
-          if (!executable.observerState.aceAnchor) {
-            executable.observerState.aceAnchor = new AceAnchoredRange(this.editor);
-          }
           const leadingEmptyLineCount = getLeadingEmptyLineCount(statement);
-          executable.observerState.aceAnchor.move(statement.location, leadingEmptyLineCount);
-          const anchoredRange = executable.observerState.aceAnchor;
-          anchoredRange.removeGutterCss(COMPLETED_CSS);
-          anchoredRange.removeGutterCss(EXECUTING_CSS);
-          anchoredRange.removeGutterCss(FAILED_CSS);
-          anchoredRange.removeMarkerCss(FAILED_MARKER_CSS);
+          anchor.move(statement.location, leadingEmptyLineCount);
 
           if (executable.isRunning()) {
-            anchoredRange.addGutterCss(EXECUTING_CSS);
+            anchor.setGutterCss(EXECUTING_CSS);
           } else if (executable.isSuccess()) {
-            anchoredRange.addGutterCss(COMPLETED_CSS);
+            anchor.setGutterCss(COMPLETED_CSS);
           } else if (executable.isFailed()) {
-            anchoredRange.addMarkerCss(FAILED_MARKER_CSS);
-            anchoredRange.addGutterCss(FAILED_CSS);
+            anchor.setGutterCss(FAILED_CSS);
+            if (executable.logs && executable.logs.errors.length) {
+              const error = executable.logs.errors[0];
+              anchor.setMarkerRowCss(FAILED_MARKER_CSS, error.row - leadingEmptyLineCount - 1);
+            }
           }
         }
       });

+ 1 - 1
desktop/core/src/desktop/js/apps/notebook2/components/ko.executableLogs.js

@@ -34,7 +34,7 @@ const TEMPLATE = `
     <button type="button" class="close" data-bind="click: reset" aria-label="${I18n('Clear')}"><span aria-hidden="true">&times;</span></button>
   </div>
   <ul class="unstyled" data-bind="foreach: errors">
-    <li data-bind="text: $data"></li>
+    <li data-bind="text: $data.message"></li>
   </ul>
 </div>
 

+ 8 - 9
desktop/core/src/desktop/js/apps/notebook2/execution/executable.ts

@@ -29,7 +29,10 @@ import { hueWindow } from 'types/types';
 import hueAnalytics from 'utils/hueAnalytics';
 import huePubSub from 'utils/huePubSub';
 import sessionManager from 'apps/notebook2/execution/sessionManager';
-import ExecutionLogs, { ExecutionLogsRaw } from 'apps/notebook2/execution/executionLogs';
+import ExecutionLogs, {
+  ExecutionError,
+  ExecutionLogsRaw
+} from 'apps/notebook2/execution/executionLogs';
 import hueUtils, { UUID } from 'utils/hueUtils';
 import Executor from 'apps/notebook2/execution/executor';
 
@@ -69,10 +72,6 @@ export interface ExecutableRaw {
   type: string;
 }
 
-// const INITIAL_HANDLE: ExecutionHandle = {
-//   statement_id: 0
-// };
-
 export default abstract class Executable {
   id: string = UUID();
   database?: string;
@@ -222,9 +221,9 @@ export default abstract class Executable {
           });
         }
       } catch (err) {
-        if (typeof err === 'string') {
-          err = this.adaptError(err);
-          this.logs.errors.push(err);
+        if (err && (err.message || typeof err === 'string')) {
+          const adapted = this.adaptError((err.message && err.message) || err);
+          this.logs.errors.push(adapted);
           this.logs.notify();
         }
         throw err;
@@ -341,7 +340,7 @@ export default abstract class Executable {
 
   abstract async internalExecute(): Promise<ExecuteApiResponse>;
 
-  abstract adaptError(err: string): string;
+  abstract adaptError(err: string): ExecutionError;
 
   abstract canExecuteInBatch(): boolean;
 

+ 8 - 2
desktop/core/src/desktop/js/apps/notebook2/execution/executionLogs.ts

@@ -20,6 +20,12 @@ import Executable, { EXECUTION_STATUS } from './executable';
 
 export const LOGS_UPDATED_EVENT = 'hue.executable.logs.updated';
 
+export interface ExecutionError {
+  row: number;
+  column: number;
+  message: string;
+}
+
 export interface ExecutionLogsRaw {
   jobs: ExecutionJob[];
   errors: string[];
@@ -30,7 +36,7 @@ export default class ExecutionLogs {
   fullLog = '';
   logLines = 0;
   jobs: ExecutionJob[] = [];
-  errors: string[] = [];
+  errors: ExecutionError[] = [];
 
   constructor(executable: Executable) {
     this.executable = executable;
@@ -97,7 +103,7 @@ export default class ExecutionLogs {
   toJs(): ExecutionLogsRaw {
     return {
       jobs: this.jobs,
-      errors: this.errors
+      errors: this.errors.map(err => err.message)
     };
   }
 }

+ 7 - 9
desktop/core/src/desktop/js/apps/notebook2/execution/sqlExecutable.ts

@@ -16,6 +16,7 @@
 
 import { ExecuteApiResponse, executeStatement } from 'apps/notebook2/execution/apiUtils';
 import Executable, { ExecutableRaw } from 'apps/notebook2/execution/executable';
+import { ExecutionError } from 'apps/notebook2/execution/executionLogs';
 import Executor from 'apps/notebook2/execution/executor';
 import { ParsedSqlStatement } from 'parse/sqlStatementsParser';
 
@@ -126,17 +127,14 @@ export default class SqlExecutable extends Executable {
     });
   }
 
-  adaptError(err: string): string {
-    const match = ERROR_REGEX.exec(err);
+  adaptError(message: string): ExecutionError {
+    const match = ERROR_REGEX.exec(message);
     if (match) {
-      const errorLine = parseInt(match[1]) + this.parsedStatement.location.first_line - 1;
-      let errorCol = match[3] && parseInt(match[3]);
-      if (errorCol && errorLine === 1) {
-        errorCol += this.parsedStatement.location.first_column;
-      }
+      const row = parseInt(match[1]);
+      const column = (match[3] && parseInt(match[3])) || 0;
 
-      return err.replace(match[0], 'line ' + errorLine + (errorCol !== null ? ':' + errorCol : ''));
+      return { message, column: column || 0, row };
     }
-    return err;
+    return { message, column: 0, row: this.parsedStatement.location.first_line };
   }
 }

+ 0 - 31
desktop/core/src/desktop/js/apps/notebook2/snippet.js

@@ -954,37 +954,6 @@ export default class Snippet {
       });
     } else if (data.status === 1 || data.status === -1) {
       this.status(STATUS.failed);
-      const match = ERROR_REGEX.exec(data.message);
-      if (match) {
-        let errorLine = parseInt(match[1]);
-        let errorCol;
-        if (typeof match[3] !== 'undefined') {
-          errorCol = parseInt(match[3]);
-        }
-        if (this.positionStatement()) {
-          if (errorCol && errorLine === 1) {
-            errorCol += this.positionStatement().location.first_column;
-          }
-          errorLine += this.positionStatement().location.first_line - 1;
-        }
-
-        this.errors.push({
-          message: data.message.replace(
-            match[0],
-            'line ' + errorLine + (errorCol !== null ? ':' + errorCol : '')
-          ),
-          help: null,
-          line: errorLine - 1,
-          col: errorCol
-        });
-      } else {
-        this.errors.push({
-          message: data.message,
-          help: data.help,
-          line: null,
-          col: null
-        });
-      }
     } else {
       $(document).trigger('error', data.message);
       this.status(STATUS.failed);

+ 3 - 3
desktop/core/src/desktop/js/ext/ace.d.ts

@@ -73,7 +73,7 @@ declare namespace Ace {
       textToScreenCoordinates(row: number, column: number): { pageX: number; pageY: number }
     };
     resize(force?: boolean): void;
-    scrollToLine(line: number, u: boolean, v: boolean, callback: () => void): void;
+    scrollToLine(line: number, center: boolean, animate: boolean, callback?: () => void): void;
     selection: {
       getRange(): Range;
       getAllRanges(): Range[];
@@ -93,7 +93,7 @@ declare namespace Ace {
   }
 
   export interface AceUtil {
-    retrievePrecedingIdentifier(row: number, column: number, regex?: RegExp): string;
+    retrievePrecedingIdentifier(line: string, column: number, regex?: RegExp): string;
   }
 
   export interface Position {
@@ -142,7 +142,7 @@ declare namespace Ace {
     addMarker(range: Range, clazz: string): number;
     doc: Document;
     getDocument(): Document;
-    getLine(row: number): number;
+    getLine(row: number): string;
     getTextRange(range: SimpleRange): string;
     getTokenAt(row: number, column: number): HueToken | null;
     getTokens(line?: number): HueToken[];