Skip to content

Commit

Permalink
[ES|QL] fix query highlight when wrapped in multi-line (#172080)
Browse files Browse the repository at this point in the history
## Summary

~~This PR edits a bit the grammar to make the highlight work again.~~

This PR fixes the highlight issue with the pipe wrapping in the editor.
The initial fix at grammar level didn't work, breaking some validation
tests.

The new approach operates at the editor level, keeping track of the line
number between each tokenize session and cleaning up the line from the
initial `|` for lines after the first one.
Note that with this approach the initial `|` remains "unstyled" for the
language (it still inherit some styling from the editor) but that seems
to still work with both themes.


![esql_highlight_dark](https://github.com/elastic/kibana/assets/924948/17779f2b-6eb6-4f89-a41f-e387ad1218cc)

![esql_highlight_light](https://github.com/elastic/kibana/assets/924948/4d1d3cdd-d5fc-4866-a55c-ca728c5635c9)


Note: It can become a problem if we decide to color the `|` with a
specific color in the future.


**TL;DR.**

The edit is required due to how Monaco works in this case.

**Long explanation**

In the grammar the `UNKNOWN_CMD` is a catch all place for all those
strings who match a new line in a query/statement. Due to ES always
receiving the query as single line they want to validate invalid strings
with a catch all trick like that.
ES|QL is defined to work as single line query language.

On the other hand Kibana uses Monaco which calls a `TokenProvider`
utility for each line, and each line is completely independent from each
other. When the multi-line configuration is enabled with the wrapped,
who puts the `|` at the beginning of each line (after the first one),
the grammar replies that the `|` is not a known command and it cann
handle anything else after worse. Removing the `UNKNOWN_CMD` from the
grammar definition will make the hightlight work again as the `|` string
is ignored.
I've asked @luigidellaquila some insight about that specific token and
if it was used in the ES codebase. From a quick investigation I did was
just a superficial validation layer, not used anywhere else in the code.
  • Loading branch information
dej611 authored Nov 29, 2023
1 parent 90699c8 commit 0e8f6cd
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 6 deletions.
5 changes: 3 additions & 2 deletions packages/kbn-monaco/src/esql/lib/monaco/esql_line_tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import { ESQLState } from './esql_state';

/** @internal **/
export class ESQLLineTokens implements monaco.languages.ILineTokens {
endState: monaco.languages.IState;
endState: ESQLState;
tokens: monaco.languages.IToken[];

constructor(tokens: monaco.languages.IToken[]) {
constructor(tokens: monaco.languages.IToken[], line: number) {
this.endState = new ESQLState();
this.endState.setLineNumber(line);
this.tokens = tokens;
}
}
14 changes: 13 additions & 1 deletion packages/kbn-monaco/src/esql/lib/monaco/esql_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,20 @@ import { monaco } from '../../../monaco_imports';

/** @internal **/
export class ESQLState implements monaco.languages.IState {
private lastLine: number = 0;

setLineNumber(n: number) {
this.lastLine = n;
}

getLineNumber() {
return this.lastLine;
}

clone(): monaco.languages.IState {
return new ESQLState();
const newState = new ESQLState();
newState.setLineNumber(this.lastLine);
return newState;
}

equals(other: monaco.languages.IState): boolean {
Expand Down
12 changes: 9 additions & 3 deletions packages/kbn-monaco/src/esql/lib/monaco/esql_tokens_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@ export class ESQLTokensProvider implements monaco.languages.TokensProvider {
return new ESQLState();
}

tokenize(line: string, state: monaco.languages.IState): monaco.languages.ILineTokens {
tokenize(line: string, prevState: ESQLState): monaco.languages.ILineTokens {
const errorStartingPoints: number[] = [];
const errorListener = new ANTLREErrorListener();
const inputStream = CharStreams.fromString(line);
// This has the drawback of not styling any ESQL wrong query as
// | from ...
const cleanedLine =
prevState.getLineNumber() && line.trimStart()[0] === '|'
? line.trimStart().substring(1)
: line;
const inputStream = CharStreams.fromString(cleanedLine);
const lexer = getLexer(inputStream, errorListener);

let done = false;
Expand Down Expand Up @@ -63,6 +69,6 @@ export class ESQLTokensProvider implements monaco.languages.TokensProvider {

myTokens.sort((a, b) => a.startIndex - b.startIndex);

return new ESQLLineTokens(myTokens);
return new ESQLLineTokens(myTokens, prevState.getLineNumber() + 1);
}
}

0 comments on commit 0e8f6cd

Please sign in to comment.