mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
[Console] Fix autocomplete inserting comma in triple quotes (#123572)
* Fix autocomplete inserting comma in triple quotes * Fix inserting commas and flaky test * Fixed problems on triple quotes and single quotes replacement. * Fixed cursor position after adding a comma to the prefix. * Final generic solution for multiple edge cases. Co-authored-by: Muhammad Ibragimov <muhammad.ibragimov@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yaroslav Kuznietsov <kuznetsov.yaroslav.yk@gmail.com>
This commit is contained in:
parent
7afae17948
commit
17134697a0
4 changed files with 112 additions and 46 deletions
|
@ -324,7 +324,7 @@ describe('Integration', () => {
|
|||
' "field": "something"\n' +
|
||||
' },\n' +
|
||||
' "facets": {},\n' +
|
||||
' "size": 20 \n' +
|
||||
' "size": 20\n' +
|
||||
'}',
|
||||
MAPPING,
|
||||
SEARCH_KB,
|
||||
|
@ -357,31 +357,18 @@ describe('Integration', () => {
|
|||
autoCompleteSet: ['facets', 'query', 'size'],
|
||||
},
|
||||
{
|
||||
name: 'prefix comma, beginning of line',
|
||||
name: 'prefix comma, end of line',
|
||||
cursor: { lineNumber: 7, column: 1 },
|
||||
initialValue: '',
|
||||
addTemplate: true,
|
||||
prefixToAdd: '',
|
||||
prefixToAdd: ',\n',
|
||||
suffixToAdd: '',
|
||||
rangeToReplace: {
|
||||
start: { lineNumber: 7, column: 1 },
|
||||
start: { lineNumber: 6, column: 14 },
|
||||
end: { lineNumber: 7, column: 1 },
|
||||
},
|
||||
autoCompleteSet: ['facets', 'query', 'size'],
|
||||
},
|
||||
{
|
||||
name: 'prefix comma, end of line',
|
||||
cursor: { lineNumber: 6, column: 15 },
|
||||
initialValue: '',
|
||||
addTemplate: true,
|
||||
prefixToAdd: '',
|
||||
suffixToAdd: '',
|
||||
rangeToReplace: {
|
||||
start: { lineNumber: 6, column: 15 },
|
||||
end: { lineNumber: 6, column: 15 },
|
||||
},
|
||||
autoCompleteSet: ['facets', 'query', 'size'],
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
|
|
|
@ -335,6 +335,20 @@ export default function ({
|
|||
});
|
||||
}
|
||||
|
||||
function replaceLinesWithPrefixPieces(prefixPieces: string[], startLineNumber: number) {
|
||||
const middlePiecesCount = prefixPieces.length - 1;
|
||||
prefixPieces.forEach((piece, index) => {
|
||||
if (index >= middlePiecesCount) {
|
||||
return;
|
||||
}
|
||||
const line = startLineNumber + index + 1;
|
||||
const column = editor.getLineValue(line).length - 1;
|
||||
const start = { lineNumber: line, column: 0 };
|
||||
const end = { lineNumber: line, column };
|
||||
editor.replace({ start, end }, piece);
|
||||
});
|
||||
}
|
||||
|
||||
function applyTerm(term: {
|
||||
value?: string;
|
||||
context?: AutoCompleteContext;
|
||||
|
@ -390,12 +404,36 @@ export default function ({
|
|||
templateInserted = false;
|
||||
}
|
||||
}
|
||||
const linesToMoveDown = (context.prefixToAdd ?? '').match(/\n|\r/g)?.length ?? 0;
|
||||
|
||||
valueToInsert = context.prefixToAdd + valueToInsert + context.suffixToAdd;
|
||||
let prefix = context.prefixToAdd ?? '';
|
||||
|
||||
// disable listening to the changes we are making.
|
||||
editor.off('changeSelection', editorChangeListener);
|
||||
|
||||
// if should add chars on the previous not empty line
|
||||
if (linesToMoveDown) {
|
||||
const [firstPart = '', ...prefixPieces] = context.prefixToAdd?.split(/\n|\r/g) ?? [];
|
||||
const lastPart = _.last(prefixPieces) ?? '';
|
||||
const { start } = context.rangeToReplace!;
|
||||
const end = { ...start, column: start.column + firstPart.length };
|
||||
|
||||
// adding only the content of prefix before newlines
|
||||
editor.replace({ start, end }, firstPart);
|
||||
|
||||
// replacing prefix pieces without the last one, which is handled separately
|
||||
if (prefixPieces.length - 1 > 0) {
|
||||
replaceLinesWithPrefixPieces(prefixPieces, start.lineNumber);
|
||||
}
|
||||
|
||||
// and the last prefix line, keeping the editor's own newlines.
|
||||
prefix = lastPart;
|
||||
context.rangeToReplace!.start.lineNumber = context.rangeToReplace!.end.lineNumber;
|
||||
context.rangeToReplace!.start.column = 0;
|
||||
}
|
||||
|
||||
valueToInsert = prefix + valueToInsert + context.suffixToAdd;
|
||||
|
||||
if (context.rangeToReplace!.start.column !== context.rangeToReplace!.end.column) {
|
||||
editor.replace(context.rangeToReplace!, valueToInsert);
|
||||
} else {
|
||||
|
@ -410,7 +448,7 @@ export default function ({
|
|||
column:
|
||||
context.rangeToReplace!.start.column +
|
||||
termAsString.length +
|
||||
context.prefixToAdd!.length +
|
||||
prefix.length +
|
||||
(templateInserted ? 0 : context.suffixToAdd!.length),
|
||||
};
|
||||
|
||||
|
@ -671,6 +709,47 @@ export default function ({
|
|||
}
|
||||
}
|
||||
|
||||
function addCommaToPrefixOnAutocomplete(
|
||||
nonEmptyToken: Token | null,
|
||||
context: AutoCompleteContext,
|
||||
charsToSkipOnSameLine: number = 1
|
||||
) {
|
||||
if (nonEmptyToken && nonEmptyToken.type.indexOf('url') < 0) {
|
||||
const { position } = nonEmptyToken;
|
||||
// if not on the first line
|
||||
if (context.rangeToReplace && context.rangeToReplace.start?.lineNumber > 1) {
|
||||
const prevTokenLineNumber = position.lineNumber;
|
||||
const line = context.editor?.getLineValue(prevTokenLineNumber) ?? '';
|
||||
const prevLineLength = line.length;
|
||||
const linesToEnter = context.rangeToReplace.end.lineNumber - prevTokenLineNumber;
|
||||
|
||||
const isTheSameLine = linesToEnter === 0;
|
||||
let startColumn = prevLineLength + 1;
|
||||
let spaces = context.rangeToReplace.start.column - 1;
|
||||
|
||||
if (isTheSameLine) {
|
||||
// prevent last char line from replacing
|
||||
startColumn = position.column + charsToSkipOnSameLine;
|
||||
// one char for pasted " and one for ,
|
||||
spaces = context.rangeToReplace.end.column - startColumn - 2;
|
||||
}
|
||||
|
||||
// go back to the end of the previous line
|
||||
context.rangeToReplace = {
|
||||
start: { lineNumber: prevTokenLineNumber, column: startColumn },
|
||||
end: { ...context.rangeToReplace.end },
|
||||
};
|
||||
|
||||
spaces = spaces >= 0 ? spaces : 0;
|
||||
const spacesToEnter = isTheSameLine ? (spaces === 0 ? 1 : spaces) : spaces;
|
||||
const newLineChars = `\n`.repeat(linesToEnter >= 0 ? linesToEnter : 0);
|
||||
const whitespaceChars = ' '.repeat(spacesToEnter);
|
||||
// add a comma at the end of the previous line, a new line and indentation
|
||||
context.prefixToAdd = `,${newLineChars}${whitespaceChars}`;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function addBodyPrefixSuffixToContext(context: AutoCompleteContext) {
|
||||
// Figure out what happens next to the token to see whether it needs trailing commas etc.
|
||||
|
||||
|
@ -758,23 +837,19 @@ export default function ({
|
|||
case 'paren.lparen':
|
||||
case 'punctuation.comma':
|
||||
case 'punctuation.colon':
|
||||
case 'punctuation.start_triple_quote':
|
||||
case 'method':
|
||||
break;
|
||||
case 'text':
|
||||
case 'string':
|
||||
case 'constant.numeric':
|
||||
case 'constant.language.boolean':
|
||||
case 'punctuation.end_triple_quote':
|
||||
addCommaToPrefixOnAutocomplete(nonEmptyToken, context, nonEmptyToken?.value.length);
|
||||
break;
|
||||
default:
|
||||
if (nonEmptyToken && nonEmptyToken.type.indexOf('url') < 0) {
|
||||
const { position, value } = nonEmptyToken;
|
||||
|
||||
// We can not rely on prefixToAdd here, because it adds a comma at the beginning of the new token
|
||||
// Since we have access to the position of the previous token here, this could be a good place to insert a comma manually
|
||||
context.prefixToAdd = '';
|
||||
editor.insert(
|
||||
{
|
||||
column: position.column + value.length,
|
||||
lineNumber: position.lineNumber,
|
||||
},
|
||||
', '
|
||||
);
|
||||
}
|
||||
addCommaToPrefixOnAutocomplete(nonEmptyToken, context);
|
||||
break;
|
||||
}
|
||||
|
||||
return context;
|
||||
|
|
|
@ -92,24 +92,24 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
|
|||
);
|
||||
});
|
||||
|
||||
// Flaky, see https://github.com/elastic/kibana/issues/123556
|
||||
it.skip('should add comma after previous non empty line on autocomplete', async () => {
|
||||
const LINE_NUMBER = 2;
|
||||
it('should add comma after previous non empty line on autocomplete', async () => {
|
||||
const LINE_NUMBER = 4;
|
||||
|
||||
await PageObjects.console.dismissTutorial();
|
||||
await PageObjects.console.clearTextArea();
|
||||
await PageObjects.console.enterRequest();
|
||||
|
||||
await PageObjects.console.enterText(`{\n\t"query": {\n\t\t"match": {}`);
|
||||
await PageObjects.console.pressEnter();
|
||||
await PageObjects.console.pressEnter();
|
||||
await PageObjects.console.pressEnter();
|
||||
await PageObjects.console.promptAutocomplete();
|
||||
await PageObjects.console.pressEnter();
|
||||
|
||||
await retry.try(async () => {
|
||||
const textOfPreviousNonEmptyLine = await PageObjects.console.getVisibleTextAt(LINE_NUMBER);
|
||||
log.debug(textOfPreviousNonEmptyLine);
|
||||
const lastChar = textOfPreviousNonEmptyLine.charAt(textOfPreviousNonEmptyLine.length - 1);
|
||||
expect(lastChar).to.be.equal(',');
|
||||
});
|
||||
const textOfPreviousNonEmptyLine = await PageObjects.console.getVisibleTextAt(LINE_NUMBER);
|
||||
log.debug(textOfPreviousNonEmptyLine);
|
||||
const lastChar = textOfPreviousNonEmptyLine.charAt(textOfPreviousNonEmptyLine.length - 1);
|
||||
expect(lastChar).to.be.equal(',');
|
||||
});
|
||||
|
||||
describe('with a data URI in the load_from query', () => {
|
||||
|
|
|
@ -110,9 +110,11 @@ export class ConsolePageObject extends FtrService {
|
|||
|
||||
private async getEditorTextArea() {
|
||||
// This focusses the cursor on the bottom of the text area
|
||||
const editor = await this.getEditor();
|
||||
const content = await editor.findByCssSelector('.ace_content');
|
||||
await content.click();
|
||||
await this.retry.try(async () => {
|
||||
const editor = await this.getEditor();
|
||||
const content = await editor.findByCssSelector('.ace_content');
|
||||
await content.click();
|
||||
});
|
||||
return await this.testSubjects.find('console-textarea');
|
||||
}
|
||||
|
||||
|
@ -137,6 +139,8 @@ export class ConsolePageObject extends FtrService {
|
|||
|
||||
public async clearTextArea() {
|
||||
const textArea = await this.getEditorTextArea();
|
||||
await textArea.clearValueWithKeyboard();
|
||||
await this.retry.try(async () => {
|
||||
await textArea.clearValueWithKeyboard();
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue