From a65019dc753f224b52c153d8e3c60977820ccaa8 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 24 Feb 2025 09:58:32 -0500 Subject: [PATCH 1/5] update error styles --- .../ui-components/src/components/Popover.tsx | 36 +++-- .../input/AssetSelectionInput.oss.tsx | 4 +- .../src/selection/CustomErrorListener.ts | 47 ++++-- .../ui-core/src/selection/SelectionInput.tsx | 30 ++-- .../selection/SelectionInputHighlighter.ts | 16 ++- .../src/selection/SelectionInputParser.ts | 10 +- .../src/selection/createSelectionLinter.ts | 42 +++--- ...seSelectionInputLintingAndHighlighting.tsx | 136 ++++++++++++++++++ 8 files changed, 236 insertions(+), 85 deletions(-) create mode 100644 js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx diff --git a/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx b/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx index cc5068ed58f68..6167c87548a1e 100644 --- a/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx +++ b/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx @@ -3,22 +3,40 @@ import {Popover2, Popover2Props} from '@blueprintjs/popover2'; import deepmerge from 'deepmerge'; import * as React from 'react'; -import {createGlobalStyle} from 'styled-components'; +import {createGlobalStyle, css} from 'styled-components'; import {Colors} from './Color'; import {FontFamily} from './styles'; import searchSVG from '../icon-svgs/search.svg'; +export const PopoverWrapperStyle = css` + box-shadow: ${Colors.shadowDefault()} 0px 2px 12px; +`; + +export const PopoverContentStyle = css` + background-color: ${Colors.popoverBackground()}; + border-radius: 4px; + + > :first-child { + border-top-left-radius: 4px; + border-top-right-radius: 4px; + } + + > :last-child { + border-bottom-left-radius: 4px; + border-bottom-right-radius: 4px; + } +`; + export const GlobalPopoverStyle = createGlobalStyle` .dagster-popover.bp5-popover, .dagster-popover.bp5-popover { - box-shadow: ${Colors.shadowDefault()} 0px 2px 12px; + ${PopoverWrapperStyle} } .dagster-popover .bp5-popover-content, .dagster-popover .bp5-popover-content { - background-color: ${Colors.popoverBackground()}; - border-radius: 4px; + ${PopoverContentStyle} .bp5-menu { background-color: ${Colors.popoverBackground()}; @@ -54,16 +72,6 @@ export const GlobalPopoverStyle = createGlobalStyle` } } - .dagster-popover .bp5-popover-content > :first-child { - border-top-left-radius: 4px; - border-top-right-radius: 4px; - } - - .dagster-popover .bp5-popover-content > :last-child { - border-bottom-left-radius: 4px; - border-bottom-right-radius: 4px; - } - .dagster-popover .bp5-popover-arrow-fill { fill: ${Colors.popoverBackground()}; } diff --git a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx index eebe3914ab890..ad3bd52248fcf 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx @@ -1,8 +1,8 @@ -import type {Linter} from 'codemirror/addon/lint/lint'; import {useAssetSelectionAutoCompleteProvider as defaultUseAssetSelectionAutoCompleteProvider} from 'shared/asset-selection/input/useAssetSelectionAutoCompleteProvider.oss'; import {assetSelectionSyntaxSupportedAttributes, unsupportedAttributeMessages} from './util'; import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData'; +import {SyntaxError} from '../../selection/CustomErrorListener'; import {SelectionAutoCompleteProvider} from '../../selection/SelectionAutoCompleteProvider'; import {SelectionAutoCompleteInput} from '../../selection/SelectionInput'; import {createSelectionLinter} from '../../selection/createSelectionLinter'; @@ -13,7 +13,7 @@ export interface AssetSelectionInputProps { assets: AssetGraphQueryItem[]; value: string; onChange: (value: string) => void; - linter?: Linter; + linter?: (content: string) => SyntaxError[]; useAssetSelectionAutoComplete?: ( assets: AssetGraphQueryItem[], ) => Pick; diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts index 39b40f9dcc68e..421f066cb4c72 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts @@ -1,33 +1,54 @@ -import {ANTLRErrorListener, RecognitionException, Recognizer} from 'antlr4ts'; -import {Token} from 'antlr4ts/Token'; +import {ANTLRErrorListener, Parser, RecognitionException, Recognizer} from 'antlr4ts'; -export interface SyntaxError { +export type SyntaxError = { message: string; - line: number; - column: number; - offendingSymbol: Token | null; -} + from: number; + to: number; +} & ( + | { + mismatchedInput?: undefined; + expectedInput?: undefined; + } + | { + mismatchedInput: string; + expectedInput: string[]; + } +); export class CustomErrorListener implements ANTLRErrorListener { private errors: SyntaxError[]; + private parser: Parser | undefined; - constructor() { + constructor({parser}: {parser?: Parser}) { + this.parser = parser; this.errors = []; } syntaxError( _recognizer: Recognizer, offendingSymbol: any, - line: number, + _line: number, charPositionInLine: number, msg: string, _e: RecognitionException | undefined, ): void { + const expectedTokens = this.parser?.getExpectedTokens()?.toArray(); + const message = msg; + let mismatchedInput; + let expectedInput; + if (this.parser && expectedTokens?.length && expectedTokens[0] !== -1 && offendingSymbol) { + mismatchedInput = offendingSymbol.text; + expectedInput = expectedTokens + .map((token) => this.parser?.vocabulary.getLiteralName(token)) + .filter(Boolean) as string[]; + } + this.errors.push({ - message: msg, - line, - column: charPositionInLine, - offendingSymbol, + message, + from: charPositionInLine, + to: charPositionInLine + (offendingSymbol?.text?.length ?? Infinity), + mismatchedInput, + expectedInput, }); } diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx index e957f1c96e8f2..bb638a663f9eb 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx @@ -1,17 +1,15 @@ import {Box, Colors, Icon, Popover, UnstyledButton} from '@dagster-io/ui-components'; import useResizeObserver from '@react-hook/resize-observer'; import CodeMirror, {Editor, EditorChange} from 'codemirror'; -import type {Linter} from 'codemirror/addon/lint/lint'; import debounce from 'lodash/debounce'; import React, {KeyboardEvent, useCallback, useLayoutEffect, useMemo, useRef, useState} from 'react'; import styled from 'styled-components'; +import {SyntaxError} from './CustomErrorListener'; import {SelectionAutoCompleteProvider} from './SelectionAutoCompleteProvider'; import {SelectionInputAutoCompleteResults} from './SelectionInputAutoCompleteResults'; -import { - SelectionAutoCompleteInputCSS, - applyStaticSyntaxHighlighting, -} from './SelectionInputHighlighter'; +import {SelectionAutoCompleteInputCSS} from './SelectionInputHighlighter'; +import {useSelectionInputLintingAndHighlighting} from './useSelectionInputLintingAndHighlighting'; import {useTrackEvent} from '../app/analytics'; import {useDangerousRenderEffect} from '../hooks/useDangerousRenderEffect'; import {useUpdatingRef} from '../hooks/useUpdatingRef'; @@ -25,7 +23,7 @@ import 'codemirror/addon/display/placeholder'; type SelectionAutoCompleteInputProps = { id: string; // Used for logging placeholder: string; - linter: Linter; + linter: (content: string) => SyntaxError[]; value: string; onChange: (value: string) => void; useAutoComplete: SelectionAutoCompleteProvider['useAutoComplete']; @@ -100,10 +98,6 @@ export const SelectionAutoCompleteInput = ({ lineWrapping: false, // Initially false; enable during focus scrollbarStyle: 'native', autoCloseBrackets: true, - lint: { - getAnnotations: linter, - async: false, - }, placeholder, extraKeys: { 'Ctrl-Space': 'autocomplete', @@ -160,7 +154,6 @@ export const SelectionAutoCompleteInput = ({ }); cmInstance.current.on('cursorActivity', (instance: Editor) => { - applyStaticSyntaxHighlighting(instance); const nextCursorPosition = instance.getCursor().ch; if (cursorPositionRef.current !== nextCursorPosition) { // If the cursor has moved then update the cursor position @@ -169,18 +162,16 @@ export const SelectionAutoCompleteInput = ({ setShowResults({current: true}); } }); - - requestAnimationFrame(() => { - if (!cmInstance.current) { - return; - } - - applyStaticSyntaxHighlighting(cmInstance.current); - }); } // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + const errorTooltip = useSelectionInputLintingAndHighlighting({ + cmInstance, + value: innerValue, + linter, + }); + const [currentHeight, setCurrentHeight] = useState(20); const adjustHeight = useCallback(() => { @@ -392,6 +383,7 @@ export const SelectionAutoCompleteInput = ({ + {errorTooltip} ); }; diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts index ecdd2f43e60c5..4cfb30a5e3c01 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts @@ -4,6 +4,7 @@ import {AbstractParseTreeVisitor} from 'antlr4ts/tree/AbstractParseTreeVisitor'; import CodeMirror from 'codemirror'; import {css} from 'styled-components'; +import {SyntaxError} from './CustomErrorListener'; import {parseInput} from './SelectionInputParser'; import { AllExpressionContext, @@ -188,15 +189,18 @@ export class SyntaxHighlightingVisitor } } -export function applyStaticSyntaxHighlighting(cm: CodeMirror.Editor): void { +export function applyStaticSyntaxHighlighting(cm: CodeMirror.Editor, errors: SyntaxError[]): void { const value = cm.getValue(); // Clear existing marks to avoid duplication cm.getAllMarks().forEach((mark) => { - // Don't clear error marks coming from the linter which uses the real grammar's parser - if ((mark as any)?.__annotation?.severity !== 'error') { - mark.clear(); - } + mark.clear(); + }); + + errors.forEach((error, idx) => { + cm.markText(cm.posFromIndex(error.from), cm.posFromIndex(error.to), { + className: `selection-input-error selection-input-error-${idx}`, + }); }); const cursorIndex = cm.getCursor().ch; @@ -255,7 +259,7 @@ export const SelectionAutoCompleteInputCSS = css` font-family: ${FontFamily.monospace}; } - .CodeMirror-lint-mark-error { + .selection-input-error { background: unset; text-decoration-line: underline; text-decoration-style: wavy; diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts index 0c09586b1bda4..bb0bae72b5ac9 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts @@ -42,9 +42,8 @@ export const parseInput = memoize((input: string): ParseResult => { const parser = new SelectionAutoCompleteParser(tokenStream); // Attach custom error listener - const errorListener = new CustomErrorListener(); + const errorListener = new CustomErrorListener({parser}); parser.removeErrorListeners(); - parser.addErrorListener(errorListener); // Set the error handler to bail on error to prevent infinite loops parser.errorHandler = new BailErrorStrategy(); @@ -64,7 +63,7 @@ export const parseInput = memoize((input: string): ParseResult => { if (currentErrors.length > 0) { const error = currentErrors[0]!; - const errorCharPos = error.column; + const errorCharPos = error.from; const errorIndex = currentPosition + errorCharPos; // Parse up to the error @@ -89,9 +88,8 @@ export const parseInput = memoize((input: string): ParseResult => { // Add the error to the errors array errors.push({ message: error.message, - line: error.line, - column: error.column + currentPosition, - offendingSymbol: error.offendingSymbol, + from: error.from + currentPosition, + to: error.to + currentPosition, }); // Advance currentPosition beyond the error diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts index 545ce00b9ff99..22acbb81eb3e7 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts @@ -1,8 +1,7 @@ import {CharStreams, CommonTokenStream, Lexer, Parser, ParserRuleContext} from 'antlr4ts'; import {AbstractParseTreeVisitor} from 'antlr4ts/tree/AbstractParseTreeVisitor'; -import CodeMirror from 'codemirror'; -import {CustomErrorListener} from './CustomErrorListener'; +import {CustomErrorListener, SyntaxError} from './CustomErrorListener'; import {parseInput} from './SelectionInputParser'; import {AttributeNameContext} from './generated/SelectionAutoCompleteParser'; import {SelectionAutoCompleteVisitor} from './generated/SelectionAutoCompleteVisitor'; @@ -27,17 +26,18 @@ export function createSelectionLinter({ if (!text.length) { return []; } - const errorListener = new CustomErrorListener(); const inputStream = CharStreams.fromString(text); const lexer = new LexerKlass(inputStream); - lexer.removeErrorListeners(); - lexer.addErrorListener(errorListener); - const tokens = new CommonTokenStream(lexer); const parser = new ParserKlass(tokens); + const errorListener = new CustomErrorListener({parser}); + + lexer.removeErrorListeners(); + lexer.addErrorListener(errorListener); + parser.removeErrorListeners(); // Remove default console error listener parser.addErrorListener(errorListener); @@ -45,10 +45,8 @@ export function createSelectionLinter({ // Map syntax errors to CodeMirror's lint format const lintErrors = errorListener.getErrors().map((error) => ({ + ...error, message: error.message.replace(', ', ''), - severity: 'error', - from: CodeMirror.Pos(0, error.column), - to: CodeMirror.Pos(0, text.length), })); const {parseTrees} = parseInput(text); @@ -67,22 +65,17 @@ class InvalidAttributeVisitor extends AbstractParseTreeVisitor implements SelectionAutoCompleteVisitor { - private errors: { - message: string; - severity: 'error' | 'warning'; - from: CodeMirror.Position; - to: CodeMirror.Position; - }[] = []; - private sortedLintErrors: {from: CodeMirror.Position; to: CodeMirror.Position}[]; + private errors: SyntaxError[] = []; + private sortedLintErrors: SyntaxError[]; constructor( private supportedAttributes: readonly string[], private unsupportedAttributeMessages: Record, - lintErrors: {from: CodeMirror.Position; to: CodeMirror.Position}[], + lintErrors: SyntaxError[], ) { super(); // Sort errors by start position for efficient searching - this.sortedLintErrors = [...lintErrors].sort((a, b) => a.from.ch - b.from.ch); + this.sortedLintErrors = [...lintErrors].sort((a, b) => a.from - b.from); } getErrors() { @@ -93,7 +86,7 @@ class InvalidAttributeVisitor return undefined; } - private hasOverlap(from: CodeMirror.Position, to: CodeMirror.Position): boolean { + private hasOverlap(from: number, to: number): boolean { // Binary search to find the first error that could potentially overlap let low = 0; let high = this.sortedLintErrors.length - 1; @@ -102,9 +95,9 @@ class InvalidAttributeVisitor const mid = Math.floor((low + high) / 2); const error = this.sortedLintErrors[mid]!; - if (error.to.ch < from.ch) { + if (error.to < from) { low = mid + 1; - } else if (error.from.ch > to.ch) { + } else if (error.from > to) { high = mid - 1; } else { // Found an overlapping error @@ -117,15 +110,14 @@ class InvalidAttributeVisitor visitAttributeName(ctx: AttributeNameContext) { const attributeName = ctx.IDENTIFIER().text; if (!this.supportedAttributes.includes(attributeName)) { - const from = CodeMirror.Pos(0, ctx.start.startIndex); - const to = CodeMirror.Pos(0, ctx.stop!.stopIndex + 1); + const from = ctx.start.startIndex; + const to = ctx.stop!.stopIndex + 1; if (!this.hasOverlap(from, to)) { this.errors.push({ message: this.unsupportedAttributeMessages[attributeName] ?? - `Unsupported attribute: ${attributeName}`, - severity: 'error', + `Unsupported attribute: "${attributeName}"`, from, to, }); diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx new file mode 100644 index 0000000000000..59968bf7130bd --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx @@ -0,0 +1,136 @@ +import { + BodySmall, + Box, + Colors, + Icon, + MonoSmall, + PopoverContentStyle, + PopoverWrapperStyle, +} from '@dagster-io/ui-components'; +import {useLayoutEffect, useMemo, useState} from 'react'; +import ReactDOM from 'react-dom'; +import styled from 'styled-components'; + +import {SyntaxError} from './CustomErrorListener'; +import {applyStaticSyntaxHighlighting} from './SelectionInputHighlighter'; +import {useUpdatingRef} from '../hooks/useUpdatingRef'; + +export const useSelectionInputLintingAndHighlighting = ({ + cmInstance, + value, + linter, +}: { + cmInstance: React.MutableRefObject; + value: string; + linter: (content: string) => SyntaxError[]; +}) => { + const errors = useMemo(() => { + const errors = linter(value); + return errors.map((error, idx) => ({ + ...error, + idx, + })); + }, [linter, value]); + + useLayoutEffect(() => { + if (!cmInstance.current) { + return; + } + applyStaticSyntaxHighlighting(cmInstance.current, errors); + }, [cmInstance, errors]); + + const errorsRef = useUpdatingRef(errors); + + const [error, setError] = useState<{ + error: SyntaxError; + x: number; + y: number; + } | null>(null); + + const errorRef = useUpdatingRef(error); + + useLayoutEffect(() => { + document.body.addEventListener('mousemove', (ev) => { + if (!(ev.target instanceof HTMLElement)) { + return; + } + const error = ev.target.closest('.selection-input-error') as HTMLElement | null; + if (error) { + const regex = /selection-input-error-(\d+)/; + const errorIdx = parseInt(error.className.match(regex)?.[1] ?? '0', 10); + const errorAnnotation = errorsRef.current[errorIdx]; + if (errorAnnotation) { + setError({ + error: errorAnnotation, + x: ev.clientX, + y: ev.clientY, + }); + return; + } + } + if (errorRef.current) { + setError(null); + } + }); + }, [cmInstance, errorsRef, errorRef]); + + const message = useMemo(() => { + if (!error) { + return null; + } + if (error.error.mismatchedInput && error.error.expectedInput) { + return ( + + Mismatched input{' '} + '{error.error.mismatchedInput}'. + Expected{' '} + {error.error.expectedInput.map((expectedInput, idx) => ( + <> + {idx > 0 && ', '} + + {expectedInput} + + + ))} + + ); + } + if (error.error.message) { + return error.error.message; + } + return null; + }, [error]); + + if (!error) { + return null; + } + + return ReactDOM.createPortal( + + + + {message} + + , + document.body, + ); +}; + +const PortalElement = styled.div<{$bottom: number; $left: number}>` + position: absolute; + top: ${({$bottom}) => $bottom - 32}px; + left: ${({$left}) => $left + 16}px; + min-width: 320px; + max-width: 600px; + z-index: 20; // Z-index 20 to match bp5-overlay + ${PopoverWrapperStyle} +`; + +const Content = styled.div` + ${PopoverContentStyle} +`; From 09c7cb3adcb315b8409e263915b755049dbb1688 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 24 Feb 2025 10:03:28 -0500 Subject: [PATCH 2/5] cleanup listener --- .../selection/useSelectionInputLintingAndHighlighting.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx index 59968bf7130bd..a019f3bc90925 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx @@ -50,7 +50,7 @@ export const useSelectionInputLintingAndHighlighting = ({ const errorRef = useUpdatingRef(error); useLayoutEffect(() => { - document.body.addEventListener('mousemove', (ev) => { + const listener = (ev: MouseEvent) => { if (!(ev.target instanceof HTMLElement)) { return; } @@ -71,7 +71,11 @@ export const useSelectionInputLintingAndHighlighting = ({ if (errorRef.current) { setError(null); } - }); + }; + document.body.addEventListener('mousemove', listener); + return () => { + document.body.removeEventListener('mousemove', listener); + }; }, [cmInstance, errorsRef, errorRef]); const message = useMemo(() => { From 465fbbc5eca1f532d413c782bddd33f5755d6d94 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 24 Feb 2025 13:49:12 -0500 Subject: [PATCH 3/5] feedback --- .../src/selection/CustomErrorListener.ts | 34 ++++--------------- ...seSelectionInputLintingAndHighlighting.tsx | 25 ++++++-------- 2 files changed, 17 insertions(+), 42 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts index 421f066cb4c72..1a3fdf0b125f0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts @@ -1,26 +1,16 @@ -import {ANTLRErrorListener, Parser, RecognitionException, Recognizer} from 'antlr4ts'; +import {ANTLRErrorListener, RecognitionException, Recognizer} from 'antlr4ts'; export type SyntaxError = { message: string; from: number; to: number; -} & ( - | { - mismatchedInput?: undefined; - expectedInput?: undefined; - } - | { - mismatchedInput: string; - expectedInput: string[]; - } -); + offendingSymbol?: string | null | undefined; +}; export class CustomErrorListener implements ANTLRErrorListener { private errors: SyntaxError[]; - private parser: Parser | undefined; - constructor({parser}: {parser?: Parser}) { - this.parser = parser; + constructor() { this.errors = []; } @@ -32,23 +22,11 @@ export class CustomErrorListener implements ANTLRErrorListener { msg: string, _e: RecognitionException | undefined, ): void { - const expectedTokens = this.parser?.getExpectedTokens()?.toArray(); - const message = msg; - let mismatchedInput; - let expectedInput; - if (this.parser && expectedTokens?.length && expectedTokens[0] !== -1 && offendingSymbol) { - mismatchedInput = offendingSymbol.text; - expectedInput = expectedTokens - .map((token) => this.parser?.vocabulary.getLiteralName(token)) - .filter(Boolean) as string[]; - } - this.errors.push({ - message, + message: msg, + offendingSymbol: offendingSymbol?.text, from: charPositionInLine, to: charPositionInLine + (offendingSymbol?.text?.length ?? Infinity), - mismatchedInput, - expectedInput, }); } diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx index a019f3bc90925..fcee167ad6922 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx @@ -3,6 +3,7 @@ import { Box, Colors, Icon, + MiddleTruncate, MonoSmall, PopoverContentStyle, PopoverWrapperStyle, @@ -82,20 +83,17 @@ export const useSelectionInputLintingAndHighlighting = ({ if (!error) { return null; } - if (error.error.mismatchedInput && error.error.expectedInput) { + if (error.error.offendingSymbol) { return ( - - Mismatched input{' '} - '{error.error.mismatchedInput}'. - Expected{' '} - {error.error.expectedInput.map((expectedInput, idx) => ( - <> - {idx > 0 && ', '} - - {expectedInput} - - - ))} + + Unexpected input +
+ +
+ +
+ + . ); } @@ -129,7 +127,6 @@ const PortalElement = styled.div<{$bottom: number; $left: number}>` position: absolute; top: ${({$bottom}) => $bottom - 32}px; left: ${({$left}) => $left + 16}px; - min-width: 320px; max-width: 600px; z-index: 20; // Z-index 20 to match bp5-overlay ${PopoverWrapperStyle} From 141a92c62f2d3cae61b01dfdbaa994d4c9de25a6 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 24 Feb 2025 13:56:59 -0500 Subject: [PATCH 4/5] update test --- .../__tests__/createSelectionLinter.test.ts | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts index a47084b5c3b12..0ee4730f43b4b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts @@ -1,5 +1,3 @@ -import CodeMirror from 'codemirror'; - import {AssetSelectionLexer} from '../../asset-selection/generated/AssetSelectionLexer'; import {AssetSelectionParser} from '../../asset-selection/generated/AssetSelectionParser'; import {createSelectionLinter} from '../createSelectionLinter'; @@ -39,21 +37,18 @@ describe('createSelectionLinter', () => { expect(errors).toEqual([ { message: 'tag filtering is not supported in this test', - severity: 'error', - from: CodeMirror.Pos(0, 0), - to: CodeMirror.Pos(0, 'tag'.length), + from: 0, + to: 'tag'.length, }, { message: 'column filtering is not supported in this test', - severity: 'error', - from: CodeMirror.Pos(0, 'tag:value or '.length), - to: CodeMirror.Pos(0, 'tag:value or column'.length), + from: 'tag:value or '.length, + to: 'tag:value or column'.length, }, { - message: 'Unsupported attribute: table_name', // default message - severity: 'error', - from: CodeMirror.Pos(0, 'tag:value or column:value or '.length), - to: CodeMirror.Pos(0, 'tag:value or column:value or table_name'.length), + message: 'Unsupported attribute: "table_name"', // default message + from: 'tag:value or column:value or '.length, + to: 'tag:value or column:value or table_name'.length, }, ]); }); @@ -71,10 +66,9 @@ describe('createSelectionLinter', () => { // Only expect syntax errors, not attribute errors expect(errors).toEqual([ expect.objectContaining({ - from: CodeMirror.Pos(0, 0), - message: expect.stringContaining("mismatched input 'fake'"), - severity: 'error', - to: CodeMirror.Pos(0, 10), + from: 0, + offendingSymbol: 'fake', + to: 4, }), ]); }); From 85437ad0e1d18bae92f0407635a6c80a5fbed1b2 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 24 Feb 2025 14:15:35 -0500 Subject: [PATCH 5/5] update test --- .../packages/ui-core/src/selection/SelectionInputParser.ts | 2 +- .../packages/ui-core/src/selection/createSelectionLinter.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts index bb0bae72b5ac9..3b7f09a819e34 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts @@ -42,7 +42,7 @@ export const parseInput = memoize((input: string): ParseResult => { const parser = new SelectionAutoCompleteParser(tokenStream); // Attach custom error listener - const errorListener = new CustomErrorListener({parser}); + const errorListener = new CustomErrorListener(); parser.removeErrorListeners(); // Set the error handler to bail on error to prevent infinite loops diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts index 22acbb81eb3e7..3da0e4360f00b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts @@ -33,7 +33,7 @@ export function createSelectionLinter({ const tokens = new CommonTokenStream(lexer); const parser = new ParserKlass(tokens); - const errorListener = new CustomErrorListener({parser}); + const errorListener = new CustomErrorListener(); lexer.removeErrorListeners(); lexer.addErrorListener(errorListener);