Skip to content

Commit

Permalink
[ES|QL] Improve support for Invoke completion trigger kind (#188877)
Browse files Browse the repository at this point in the history
## Summary

Resolve #188677

This PR extends our support for Monaco's "Invoke" completion trigger
kind
([ref](https://microsoft.github.io/monaco-editor/typedoc/enums/languages.CompletionTriggerKind.html)).

What this means for the user is that suggestions are now shown when the
user types a character in a word even if the suggestion menu isn't
already open. This situation often occurs when the user is editing a
query. For example, they may follow a flow like this:

1. `FROM my-index | LIMIT 10<cursor-here>`
2. `FROM <cursor-here> | LIMIT 10` (deleted the index name)
3. `FROM k<cursor-here> | LIMIT 10` (types the first character of the
index name)

Previously, they wouldn't get any help. Now, we show the list of
suggestions.

The following table shows the cases that I considered as well as the
ones that are now fixed. I also added test coverage, even for cases that
worked before to prevent subtle regressions.

| | Before | After |

|------------------------------|------------------------------------------|-----------------------|
| Source command | ✅ | ✅ |
| Pipe command | ❌ | ✅ |
| Function argument | ✅ | ✅ |
| FROM source | ❌ | ✅ |
| FROM source METADATA | ✅ | ✅ |
| FROM source METADATA field | ❌ (for `_`) | ✅ |
| EVAL argument | ✅ | ✅ |
| DISSECT field | ✅ | ✅ |
| DISSECT field pattern | ❌ | ❌ |
| DROP field | ✅ | ✅ |
| DROP field1, field2 | ❌ | ✅ |
| ENRICH policy | ✅ | ✅ |
| ENRICH policy ON | ✅ | ✅ |
| ENRICH policy ON field | ✅ | ✅ |
| ENRICH policy WITH | ✅ | ✅ |
| ENRICH policy WITH field | ✅ | ✅ |
| GROK field | ✅ | ✅ |
| GROK field pattern | ❌ | ❌ |
| KEEP field | ✅ | ✅ |
| KEEP field1, field2 | ❌ | ✅ |
| LIMIT number | ❌ | ❌ |
| MV_EXPAND field | ✅ | ✅ |
| RENAME field | ✅ | ✅ |
| RENAME field AS | ❌ | ✅ |
| RENAME field AS var0 | ✅ | ✅ |
| SORT field | ✅ | ✅ |
| SORT field order | ✅ | ✅ |
| SORT field order nulls-order | ✅ | ✅ |
| STATS argument | ✅ | ✅ |
| STATS argument BY | ✅ | ✅ |
| STATS argument BY expression | ✅ | ✅ |
| WHERE argument | ✅ | ✅ |
| WHERE argument comparison    | ✅  | ✅                     |

As I worked, I encountered a couple small bugs which I also fixed.

### Escaping field values in `ENRICH`

**Before**

https://github.com/user-attachments/assets/88f2bf35-a703-4cf4-8d45-a24452a1b59d

**After**

https://github.com/user-attachments/assets/69ad0770-f158-44fb-be8d-66b497c7f7f7

### Suggesting variable assignment in `ENRICH ... WITH`
**Before**

https://github.com/user-attachments/assets/fec47b6d-eae5-44e8-b739-9b2eecc7458b

**After**

https://github.com/user-attachments/assets/dff5afe1-37e7-470b-bb9c-edb5ac87e76d

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit c77cd80)

# Conflicts:
#	packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts
#	packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
  • Loading branch information
drewdaemon committed Jul 29, 2024
1 parent 3d73dee commit 610916d
Show file tree
Hide file tree
Showing 7 changed files with 356 additions and 97 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-esql-ast/src/ast_walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ export function visitRenameClauses(clausesCtx: RenameClauseContext[]): ESQLAstIt
return clausesCtx
.map((clause) => {
const asToken = clause.getToken(esql_parser.AS, 0);
if (asToken) {
if (asToken && textExistsAndIsValid(asToken.getText())) {
const fn = createOption(asToken.getText().toLowerCase(), clause);
for (const arg of [clause._oldName, clause._newName]) {
if (textExistsAndIsValid(arg.getText())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,15 @@ export function getFunctionSignaturesByReturnType(
{
agg,
grouping,
evalMath,
scalar,
builtin,
// skipAssign here is used to communicate to not propose an assignment if it's not possible
// within the current context (the actual logic has it, but here we want a shortcut)
skipAssign,
}: {
agg?: boolean;
grouping?: boolean;
evalMath?: boolean;
scalar?: boolean;
builtin?: boolean;
skipAssign?: boolean;
} = {},
Expand All @@ -152,7 +152,7 @@ export function getFunctionSignaturesByReturnType(
list.push(...groupingFunctionDefinitions);
}
// eval functions (eval is a special keyword in JS)
if (evalMath) {
if (scalar) {
list.push(...evalFunctionDefinitions);
}
if (builtin) {
Expand Down
Loading

0 comments on commit 610916d

Please sign in to comment.