Skip to content

Commit

Permalink
[SelectionAutoCompleteInput] Fix auto-complete for tags that have bot…
Browse files Browse the repository at this point in the history
…h a key and value (#26847)

## Summary & Motivation

1. The current SelectionAutoComplete grammar doesn't allow for
attribute:value=value so add that to the grammar
2. Fix the suggestions so that we end up with `tag:"key"="value"`
instead of `tag:"key=value"`
3. Add icons for the other attributes

## How I Tested These Changes
<img width="1200" alt="Screenshot 2025-01-06 at 12 15 19 PM"
src="https://github.com/user-attachments/assets/3f8fabc3-0e55-4340-a757-8e85e0da09a8"
/>
  • Loading branch information
salazarm authored Jan 6, 2025
1 parent 5514eba commit ed3d48d
Show file tree
Hide file tree
Showing 11 changed files with 446 additions and 303 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ export const AssetSelectionInput = ({value, onChange, assets}: AssetSelectionInp
assetNamesSet.add(asset.name);
asset.node.tags.forEach((tag) => {
if (tag.key && tag.value) {
tagNamesSet.add(`${tag.key}=${tag.value}`);
// We add quotes around the equal sign here because the auto-complete suggestion already wraps the entire value in quotes.
// So wer end up with tag:"key"="value" as the final suggestion
tagNamesSet.add(`${tag.key}"="${tag.value}`);
} else {
tagNamesSet.add(tag.key);
}
Expand Down Expand Up @@ -106,6 +108,22 @@ export const AssetSelectionInput = ({value, onChange, assets}: AssetSelectionInp

const WrapperDiv = styled.div`
.attribute-owner {
${iconStyle(Icons.owner.src)}
${iconStyle(Icons.owner.src)};
}
.attribute-tag {
${iconStyle(Icons.tag.src)};
}
.attribute-key_substring,
.attribute-key {
${iconStyle(Icons.asset.src)};
}
.attribute-group {
${iconStyle(Icons.asset_group.src)};
}
.attribute-code_location {
${iconStyle(Icons.code_location.src)};
}
.attribute-kind {
${iconStyle(Icons.compute_kind.src)};
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,21 @@ expr:

// Allowed expressions within traversal contexts
traversalAllowedExpr:
attributeName colonToken attributeValue postAttributeValueWhitespace # AttributeExpression
| functionName parenthesizedExpr # FunctionCallExpression
| parenthesizedExpr # TraversalAllowedParenthesizedExpression
| incompleteExpr # IncompleteExpression;
attributeName colonToken attributeValue (
EQUAL attributeValue
)? postAttributeValueWhitespace # AttributeExpression
| functionName parenthesizedExpr # FunctionCallExpression
| parenthesizedExpr # TraversalAllowedParenthesizedExpression
| incompleteExpr # IncompleteExpression;

parenthesizedExpr:
leftParenToken postLogicalOperatorWhitespace expr rightParenToken postExpressionWhitespace #
ParenthesizedExpression;

incompleteExpr:
attributeName colonToken attributeValueWhitespace # IncompleteAttributeExpressionMissingValue
attributeName colonToken attributeValueWhitespace # IncompleteAttributeExpressionMissingValue
| attributeName colonToken attributeValue EQUAL attributeValueWhitespace #
IncompleteAttributeExpressionMissingSecondValue
| functionName expressionLessParenthesizedExpr # ExpressionlessFunctionExpression
| functionName leftParenToken postLogicalOperatorWhitespace #
UnclosedExpressionlessFunctionExpression
Expand Down Expand Up @@ -112,12 +116,12 @@ COLON: ':';
LPAREN: '(';
RPAREN: ')';

EQUAL: '=';

// Tokens for strings
QUOTED_STRING: '"' (~["\\\r\n])* '"';
INCOMPLETE_LEFT_QUOTED_STRING: '"' (~["\\\r\n():])*;
INCOMPLETE_RIGHT_QUOTED_STRING: (~["\\\r\n:()])* '"';
INCOMPLETE_LEFT_QUOTED_STRING: '"' (~["\\\r\n():=])*;
INCOMPLETE_RIGHT_QUOTED_STRING: (~["\\\r\n:()=])* '"';
EQUAL: '=';
// Identifiers (attributes and functions)
IDENTIFIER: [a-zA-Z_][a-zA-Z0-9_]*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export function createSelectionLinter({
const lintErrors = errorListener.getErrors().map((error) => ({
message: error.message.replace('<EOF>, ', ''),
severity: 'error',
from: CodeMirror.Pos(error.line, error.column),
to: CodeMirror.Pos(error.line, text.length),
from: CodeMirror.Pos(0, error.column),
to: CodeMirror.Pos(0, text.length),
}));

return lintErrors;
Expand Down

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

1 comment on commit ed3d48d

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-46smi5qmk-elementl.vercel.app

Built with commit ed3d48d.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.