Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Selection input] Update error styles #28012

Merged
merged 5 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
Expand Down Expand Up @@ -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()};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -13,7 +13,7 @@ export interface AssetSelectionInputProps {
assets: AssetGraphQueryItem[];
value: string;
onChange: (value: string) => void;
linter?: Linter<any>;
linter?: (content: string) => SyntaxError[];
useAssetSelectionAutoComplete?: (
assets: AssetGraphQueryItem[],
) => Pick<SelectionAutoCompleteProvider, 'useAutoComplete'>;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<any> {
private errors: SyntaxError[];
private parser: Parser | undefined;

constructor() {
constructor({parser}: {parser?: Parser}) {
this.parser = parser;
this.errors = [];
}

syntaxError(
_recognizer: Recognizer<any, any>,
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,
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -25,7 +23,7 @@ import 'codemirror/addon/display/placeholder';
type SelectionAutoCompleteInputProps = {
id: string; // Used for logging
placeholder: string;
linter: Linter<any>;
linter: (content: string) => SyntaxError[];
value: string;
onChange: (value: string) => void;
useAutoComplete: SelectionAutoCompleteProvider['useAutoComplete'];
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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
Expand All @@ -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(() => {
Expand Down Expand Up @@ -392,6 +383,7 @@ export const SelectionAutoCompleteInput = ({
</Box>
</InputDiv>
</Popover>
{errorTooltip}
</div>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading