Skip to content

Commit

Permalink
Fix autocomplete triggering on URL tokens (elastic#168956)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes autocomplete triggering on URL tokens ~~for url parameters
to work with a single character~~.

Fixes elastic#168017 (which is a regression introduced by elastic#163233)

![fix-autocomplete-168017](https://github.com/elastic/kibana/assets/721858/94e6f773-53c9-4bc1-991c-fb572ddf7ffd)

### 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

### Notes

- No functional tests are added because they would also go flaky.
- ~~No unit tests are added because of the lack of existing unit
tests.~~
- ~~The change is kept minimal by accepting the growing if-else block.~~

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 8fe2e1a)
  • Loading branch information
sakurai-youhei committed Oct 23, 2023
1 parent 8c58d18 commit 8e6e91c
Show file tree
Hide file tree
Showing 3 changed files with 323 additions and 38 deletions.
41 changes: 3 additions & 38 deletions src/plugins/console/public/lib/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import * as utils from '../utils';
import { populateContext } from './engine';
import type { AutoCompleteContext, DataAutoCompleteRulesOneOf, ResultTerm } from './types';
import { URL_PATH_END_MARKER, ConstantComponent } from './components';
import { looksLikeTypingIn } from './looks_like_typing_in';

let lastEvaluatedToken: Token | null = null;

Expand Down Expand Up @@ -1137,44 +1138,8 @@ export default function ({
return; // wait for the next typing.
}

if (
lastEvaluatedToken.position.column + 1 === currentToken.position.column &&
lastEvaluatedToken.position.lineNumber === currentToken.position.lineNumber &&
(lastEvaluatedToken.type === 'url.slash' || lastEvaluatedToken.type === 'url.comma') &&
currentToken.type === 'url.part' &&
currentToken.value.length === 1
) {
// do not suppress autocomplete for a single character immediately following a slash or comma in URL
} else if (
lastEvaluatedToken.position.column < currentToken.position.column &&
lastEvaluatedToken.position.lineNumber === currentToken.position.lineNumber &&
lastEvaluatedToken.type === 'method' &&
currentToken.type === 'url.part' &&
currentToken.value.length === 1
) {
// do not suppress autocomplete for a single character following method in URL
} else if (
lastEvaluatedToken.position.column < currentToken.position.column &&
lastEvaluatedToken.position.lineNumber === currentToken.position.lineNumber &&
!lastEvaluatedToken.type &&
currentToken.type === 'method' &&
currentToken.value.length === 1
) {
// do not suppress autocompletion for the first character of method
} else if (
// if the column or the line number have changed for the last token or
// user did not provided a new value, then we should not show autocomplete
// this guards against triggering autocomplete when clicking around the editor
lastEvaluatedToken.position.column !== currentToken.position.column ||
lastEvaluatedToken.position.lineNumber !== currentToken.position.lineNumber ||
lastEvaluatedToken.value === currentToken.value
) {
tracer(
'not starting autocomplete since the change looks like a click',
lastEvaluatedToken,
'->',
currentToken
);
if (!looksLikeTypingIn(lastEvaluatedToken, currentToken, editor)) {
tracer('not starting autocomplete', lastEvaluatedToken, '->', currentToken);
// not on the same place or nothing changed, cache and wait for the next time
lastEvaluatedToken = currentToken;
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import '../../application/models/sense_editor/sense_editor.test.mocks';

import { looksLikeTypingIn } from './looks_like_typing_in';
import { create } from '../../application/models';
import type { SenseEditor } from '../../application/models';
import type { CoreEditor, Position, Token, TokensProvider } from '../../types';

describe('looksLikeTypingIn', () => {
let editor: SenseEditor;
let coreEditor: CoreEditor;
let tokenProvider: TokensProvider;

beforeEach(() => {
document.body.innerHTML = `<div>
<div id="ConAppEditor" />
<div id="ConAppEditorActions" />
<div id="ConCopyAsCurl" />
</div>`;
editor = create(document.getElementById('ConAppEditor')!);
coreEditor = editor.getCoreEditor();
tokenProvider = coreEditor.getTokenProvider();
});

afterEach(async () => {
await editor.update('', true);
});

describe('general typing in', () => {
interface RunTestArgs {
preamble: string;
autocomplete?: string;
input: string;
}

const runTest = async ({ preamble, autocomplete, input }: RunTestArgs) => {
const pos: Position = { lineNumber: 1, column: 1 };

await editor.update(preamble, true);
pos.column += preamble.length;
const lastEvaluatedToken = tokenProvider.getTokenAt(pos);

if (autocomplete !== undefined) {
await editor.update(coreEditor.getValue() + autocomplete, true);
pos.column += autocomplete.length;
}

await editor.update(coreEditor.getValue() + input, true);
pos.column += input.length;
const currentToken = tokenProvider.getTokenAt(pos);

expect(lastEvaluatedToken).not.toBeNull();
expect(currentToken).not.toBeNull();
expect(looksLikeTypingIn(lastEvaluatedToken!, currentToken!, coreEditor)).toBe(true);
};

const cases: RunTestArgs[] = [
{ preamble: 'G', input: 'E' },
{ preamble: 'GET .kibana', input: '/' },
{ preamble: 'GET .kibana', input: ',' },
{ preamble: 'GET .kibana', input: '?' },
{ preamble: 'GET .kibana/', input: '_' },
{ preamble: 'GET .kibana/', input: '?' },
{ preamble: 'GET .kibana,', input: '.' },
{ preamble: 'GET .kibana,', input: '?' },
{ preamble: 'GET .kibana?', input: 'k' },
{ preamble: 'GET .kibana?k', input: '=' },
{ preamble: 'GET .kibana?k=', input: 'v' },
{ preamble: 'GET .kibana?k=v', input: '&' },
{ preamble: 'GET .kibana?k', input: '&' },
{ preamble: 'GET .kibana?k&', input: 'k' },
{ preamble: 'GET ', autocomplete: '.kibana', input: '/' },
{ preamble: 'GET ', autocomplete: '.kibana', input: ',' },
{ preamble: 'GET ', autocomplete: '.kibana', input: '?' },
{ preamble: 'GET .ki', autocomplete: 'bana', input: '/' },
{ preamble: 'GET .ki', autocomplete: 'bana', input: ',' },
{ preamble: 'GET .ki', autocomplete: 'bana', input: '?' },
{ preamble: 'GET _nodes/', autocomplete: 'stats', input: '/' },
{ preamble: 'GET _nodes/sta', autocomplete: 'ts', input: '/' },
{ preamble: 'GET _nodes/', autocomplete: 'jvm', input: ',' },
{ preamble: 'GET _nodes/j', autocomplete: 'vm', input: ',' },
{ preamble: 'GET _nodes/jvm,', autocomplete: 'os', input: ',' },
{ preamble: 'GET .kibana,', autocomplete: '.security', input: ',' },
{ preamble: 'GET .kibana,.sec', autocomplete: 'urity', input: ',' },
{ preamble: 'GET .kibana,', autocomplete: '.security', input: '/' },
{ preamble: 'GET .kibana,.sec', autocomplete: 'urity', input: '/' },
{ preamble: 'GET .kibana,', autocomplete: '.security', input: '?' },
{ preamble: 'GET .kibana,.sec', autocomplete: 'urity', input: '?' },
{ preamble: 'GET .kibana/', autocomplete: '_search', input: '?' },
{ preamble: 'GET .kibana/_se', autocomplete: 'arch', input: '?' },
{ preamble: 'GET .kibana/_search?', autocomplete: 'expand_wildcards', input: '=' },
{ preamble: 'GET .kibana/_search?exp', autocomplete: 'and_wildcards', input: '=' },
{ preamble: 'GET .kibana/_search?expand_wildcards=', autocomplete: 'all', input: '&' },
{ preamble: 'GET .kibana/_search?expand_wildcards=a', autocomplete: 'll', input: '&' },
{ preamble: 'GET _cat/indices?s=index&', autocomplete: 'expand_wildcards', input: '=' },
{ preamble: 'GET _cat/indices?s=index&exp', autocomplete: 'and_wildcards', input: '=' },
{ preamble: 'GET _cat/indices?v&', autocomplete: 'expand_wildcards', input: '=' },
{ preamble: 'GET _cat/indices?v&exp', autocomplete: 'and_wildcards', input: '=' },
];
for (const c of cases) {
const name =
c.autocomplete === undefined
? `'${c.preamble}' -> '${c.input}'`
: `'${c.preamble}' -> '${c.autocomplete}' (autocomplte) -> '${c.input}'`;
test(name, async () => runTest(c));
}
});

describe('first typing in', () => {
test(`'' -> 'G'`, () => {
// this is based on an implementation within the evaluateCurrentTokenAfterAChange function
const lastEvaluatedToken = { position: { column: 0, lineNumber: 0 }, value: '', type: '' };
lastEvaluatedToken.position.lineNumber = coreEditor.getCurrentPosition().lineNumber;

const currentToken = { position: { column: 1, lineNumber: 1 }, value: 'G', type: 'method' };
expect(looksLikeTypingIn(lastEvaluatedToken, currentToken, coreEditor)).toBe(true);
});
});

const matrices = [
`
GET .kibana/
`
.slice(1, -1)
.split('\n'),
`
POST test/_doc
{"message": "test"}
GET /_cat/indices?v&s=
DE
`
.slice(1, -1)
.split('\n'),
`
PUT test/_doc/1
{"field": "value"}
`
.slice(1, -1)
.split('\n'),
];

describe('navigating the editor via keyboard arrow keys', () => {
const runHorizontalZigzagWalkTest = async (matrix: string[]) => {
const width = matrix[0].length;
const height = matrix.length;

await editor.update(matrix.join('\n'), true);
let lastEvaluatedToken = tokenProvider.getTokenAt(coreEditor.getCurrentPosition());
let currentToken: Token | null;

for (let i = 1; i < height * width * 2; i++) {
const pos = {
column: 1 + (i % width),
lineNumber: 1 + Math.floor(i / width),
};
if (pos.lineNumber % 2 === 0) {
pos.column = width - pos.column + 1;
}
if (pos.lineNumber > height) {
pos.lineNumber = 2 * height - pos.lineNumber + 1;
}

currentToken = tokenProvider.getTokenAt(pos);
expect(lastEvaluatedToken).not.toBeNull();
expect(currentToken).not.toBeNull();
expect(looksLikeTypingIn(lastEvaluatedToken!, currentToken!, coreEditor)).toBe(false);
lastEvaluatedToken = currentToken;
}
};

for (const matrix of matrices) {
test(`horizontal zigzag walk ${matrix[0].length}x${matrix.length} map`, () =>
runHorizontalZigzagWalkTest(matrix));
}
});

describe('clicking around the editor', () => {
const runRandomClickingTest = async (matrix: string[], attempts: number) => {
const width = matrix[0].length;
const height = matrix.length;

await editor.update(matrix.join('\n'), true);
let lastEvaluatedToken = tokenProvider.getTokenAt(coreEditor.getCurrentPosition());
let currentToken: Token | null;

for (let i = 1; i < attempts; i++) {
const pos = {
column: Math.ceil(Math.random() * width),
lineNumber: Math.ceil(Math.random() * height),
};

currentToken = tokenProvider.getTokenAt(pos);
expect(lastEvaluatedToken).not.toBeNull();
expect(currentToken).not.toBeNull();
expect(looksLikeTypingIn(lastEvaluatedToken!, currentToken!, coreEditor)).toBe(false);
lastEvaluatedToken = currentToken;
}
};

for (const matrix of matrices) {
const attempts = 4 * matrix[0].length * matrix.length;
test(`random clicking ${matrix[0].length}x${matrix.length} map ${attempts} times`, () =>
runRandomClickingTest(matrix, attempts));
}
});
});
101 changes: 101 additions & 0 deletions src/plugins/console/public/lib/autocomplete/looks_like_typing_in.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { CoreEditor, Position, Token } from '../../types';

enum Move {
ForwardOneCharacter = 1,
ForwardOneToken, // the column position may jump to the next token by autocomplete
}

const knownTypingInTokenTypes = new Map<Move, Map<string, Set<string>>>([
[
Move.ForwardOneCharacter,
new Map<string, Set<string>>([
// a pair of the last evaluated token type and a set of the current token types
['', new Set(['method'])],
['url.amp', new Set(['url.param'])],
['url.comma', new Set(['url.part', 'url.questionmark'])],
['url.equal', new Set(['url.value'])],
['url.param', new Set(['url.amp', 'url.equal'])],
['url.questionmark', new Set(['url.param'])],
['url.slash', new Set(['url.part', 'url.questionmark'])],
['url.value', new Set(['url.amp'])],
]),
],
[
Move.ForwardOneToken,
new Map<string, Set<string>>([
['method', new Set(['url.part'])],
['url.amp', new Set(['url.amp', 'url.equal'])],
['url.comma', new Set(['url.comma', 'url.questionmark', 'url.slash'])],
['url.equal', new Set(['url.amp'])],
['url.param', new Set(['url.equal'])],
['url.part', new Set(['url.comma', 'url.questionmark', 'url.slash'])],
['url.questionmark', new Set(['url.equal'])],
['url.slash', new Set(['url.comma', 'url.questionmark', 'url.slash'])],
['url.value', new Set(['url.amp'])],
['whitespace', new Set(['url.comma', 'url.questionmark', 'url.slash'])],
]),
],
]);

const getOneCharacterNextOnTheRight = (pos: Position, coreEditor: CoreEditor): string => {
const range = {
start: { column: pos.column + 1, lineNumber: pos.lineNumber },
end: { column: pos.column + 2, lineNumber: pos.lineNumber },
};
return coreEditor.getValueInRange(range);
};

/**
* Examines a change from the last evaluated to the current token and one
* character next to the current token position on the right. Returns true if
* the change looks like typing in, false otherwise.
*
* This function is supposed to filter out situations where autocomplete is not
* preferable, such as clicking around the editor, navigating the editor via
* keyboard arrow keys, etc.
*/
export const looksLikeTypingIn = (
lastEvaluatedToken: Token,
currentToken: Token,
coreEditor: CoreEditor
): boolean => {
// if the column position moves to the right in the same line and the current
// token length is 1, then user is possibly typing in a character.
if (
lastEvaluatedToken.position.column < currentToken.position.column &&
lastEvaluatedToken.position.lineNumber === currentToken.position.lineNumber &&
currentToken.value.length === 1 &&
getOneCharacterNextOnTheRight(currentToken.position, coreEditor) === ''
) {
const move =
lastEvaluatedToken.position.column + 1 === currentToken.position.column
? Move.ForwardOneCharacter
: Move.ForwardOneToken;
const tokenTypesPairs = knownTypingInTokenTypes.get(move) ?? new Map<string, Set<string>>();
const currentTokenTypes = tokenTypesPairs.get(lastEvaluatedToken.type) ?? new Set<string>();
if (currentTokenTypes.has(currentToken.type)) {
return true;
}
}

// if the column or the line number have changed for the last token or
// user did not provided a new value, then we should not show autocomplete
// this guards against triggering autocomplete when clicking around the editor
if (
lastEvaluatedToken.position.column !== currentToken.position.column ||
lastEvaluatedToken.position.lineNumber !== currentToken.position.lineNumber ||
lastEvaluatedToken.value === currentToken.value
) {
return false;
}

return true;
};

0 comments on commit 8e6e91c

Please sign in to comment.