From 96dea97e9214ee82a94d0ea6d2e97ff9b9e99bd7 Mon Sep 17 00:00:00 2001 From: Oleg Solomko Date: Fri, 28 Mar 2025 11:20:22 -0700 Subject: [PATCH] add doc comments, refactor and cleanup todo items --- src/vs/editor/common/codecs/baseToken.ts | 14 ++- .../common/codecs/linesCodec/linesDecoder.ts | 2 +- .../codecs/simpleCodec/simpleDecoder.ts | 22 ++-- .../test/common/codecs/simpleDecoder.test.ts | 118 +++++++++--------- .../parsers/promptTemplateVariableParser.ts | 15 ++- .../codecs/tokens/promptTemplateVariable.ts | 8 +- 6 files changed, 89 insertions(+), 90 deletions(-) diff --git a/src/vs/editor/common/codecs/baseToken.ts b/src/vs/editor/common/codecs/baseToken.ts index 25a28043097..26e120725da 100644 --- a/src/vs/editor/common/codecs/baseToken.ts +++ b/src/vs/editor/common/codecs/baseToken.ts @@ -62,8 +62,9 @@ export abstract class BaseToken { } /** - * TODO: @legomushroom + * Render a list of tokens into a string. */ + // TODO: @legomushroom - add unit tests public static render(tokens: readonly BaseToken[]): string { return tokens.map((token) => { return token.text; @@ -71,9 +72,16 @@ export abstract class BaseToken { } /** - * TODO: @legomushroom - * @throws + * Returns the full range of a list of tokens in which the first token is + * used as the start of a tokens sequence and the last token reflects the end. + * + * @throws if: + * - provided {@link tokens} list is empty + * - the first token starts on a line that is greater than the last token + * - if the first and last token are on the same line, the first token must + * start column must be smaller or equal to the start column of the last token */ + // TODO: @legomushroom - add unit tests public static fullRange(tokens: readonly BaseToken[]): Range { assert( tokens.length > 0, diff --git a/src/vs/editor/common/codecs/linesCodec/linesDecoder.ts b/src/vs/editor/common/codecs/linesCodec/linesDecoder.ts index e26b72f05e0..2faf59028ab 100644 --- a/src/vs/editor/common/codecs/linesCodec/linesDecoder.ts +++ b/src/vs/editor/common/codecs/linesCodec/linesDecoder.ts @@ -13,7 +13,7 @@ import { assertDefined } from '../../../../base/common/types.js'; import { BaseDecoder } from '../../../../base/common/codecs/baseDecoder.js'; /** - * Tokens produced by the `LinesDecoder`. + * Tokens produced by the {@link LinesDecoder}. */ export type TLineToken = Line | CarriageReturn | NewLine; diff --git a/src/vs/editor/common/codecs/simpleCodec/simpleDecoder.ts b/src/vs/editor/common/codecs/simpleCodec/simpleDecoder.ts index d5893bddbb0..b5f786e45d8 100644 --- a/src/vs/editor/common/codecs/simpleCodec/simpleDecoder.ts +++ b/src/vs/editor/common/codecs/simpleCodec/simpleDecoder.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Line } from '../linesCodec/tokens/line.js'; import { NewLine } from '../linesCodec/tokens/newLine.js'; import { VSBuffer } from '../../../../base/common/buffer.js'; import { ReadableStream } from '../../../../base/common/stream.js'; @@ -35,6 +36,7 @@ import { LeftAngleBracket, RightAngleBracket, } from './tokens/index.js'; +import { pick } from '../../../../base/common/arrays.js'; /** * A token type that this decoder can handle. @@ -55,19 +57,13 @@ const WELL_KNOWN_TOKENS = Object.freeze([ ]); /** - * Characters that stop a "word" sequence. - * Note! the `\r` and `\n` are excluded from the list because this decoder based on `LinesDecoder` which - * already handles the `CR`/`newline` cases and emits lines that don't contain them. + * A {@link Word} sequence stops when one of the well-known tokens are encountered. + * Note! the `\r` and `\n` are excluded from the list because this decoder based on + * the {@link LinesDecoder} which emits {@link Line} tokens without them. */ -// TODO: @legomushroom - refactor to account for the `WELL_KNOWN_TOKENS`? -const WORD_STOP_CHARACTERS: readonly string[] = Object.freeze([ - Space.symbol, Tab.symbol, VerticalTab.symbol, FormFeed.symbol, At.symbol, Slash.symbol, - Colon.symbol, Hash.symbol, Dash.symbol, ExclamationMark.symbol, DollarSign.symbol, - LeftCurlyBrace.symbol, RightCurlyBrace.symbol, - LeftBracket.symbol, RightBracket.symbol, - LeftParenthesis.symbol, RightParenthesis.symbol, - LeftAngleBracket.symbol, RightAngleBracket.symbol, -]); +const WORD_STOP_CHARACTERS: readonly string[] = Object.freeze( + WELL_KNOWN_TOKENS.map(pick('symbol')), +); /** * A decoder that can decode a stream of `Line`s into a stream @@ -88,7 +84,7 @@ export class SimpleDecoder extends BaseDecoder { return; } - // loop through the text separating it into `Word` and `Space` tokens + // loop through the text separating it into `Word` and `well-known` tokens let i = 0; while (i < line.text.length) { // index is 0-based, but column numbers are 1-based diff --git a/src/vs/editor/test/common/codecs/simpleDecoder.test.ts b/src/vs/editor/test/common/codecs/simpleDecoder.test.ts index 42743267c5a..62573013af9 100644 --- a/src/vs/editor/test/common/codecs/simpleDecoder.test.ts +++ b/src/vs/editor/test/common/codecs/simpleDecoder.test.ts @@ -68,7 +68,7 @@ export class TestSimpleDecoder extends TestDecoder suite('SimpleDecoder', () => { const testDisposables = ensureNoDisposablesAreLeakedInTestSuite(); - test('produces expected tokens', async () => { + test('produces expected tokens #1', async () => { const test = testDisposables.add( new TestSimpleDecoder(), ); @@ -167,65 +167,63 @@ suite('SimpleDecoder', () => { ); }); - suite('slash commands', () => { - test('produces expected tokens', async () => { - const test = testDisposables.add( - new TestSimpleDecoder(), - ); + test('produces expected tokens #2', async () => { + const test = testDisposables.add( + new TestSimpleDecoder(), + ); - await test.run( - [ - 'your command is /catch', - '\t\t/command1/command2 ', - ' /cmd#var ', - '/test@github\t\t', - '/update\r', - '', - ], - [ - // first line - new Word(new Range(1, 1, 1, 5), 'your'), - new Space(new Range(1, 5, 1, 6)), - new Word(new Range(1, 6, 1, 6 + 7), 'command'), - new Space(new Range(1, 13, 1, 14)), - new Word(new Range(1, 14, 1, 14 + 2), 'is'), - new Space(new Range(1, 16, 1, 17)), - new Slash(new Range(1, 17, 1, 18)), - new Word(new Range(1, 18, 1, 18 + 5), 'catch'), - new NewLine(new Range(1, 23, 1, 24)), - // second line - new Tab(new Range(2, 1, 2, 2)), - new Tab(new Range(2, 2, 2, 3)), - new Slash(new Range(2, 3, 2, 4)), - new Word(new Range(2, 4, 2, 4 + 8), 'command1'), - new Slash(new Range(2, 12, 2, 13)), - new Word(new Range(2, 13, 2, 13 + 8), 'command2'), - new Space(new Range(2, 21, 2, 22)), - new NewLine(new Range(2, 22, 2, 23)), - // third line - new Space(new Range(3, 1, 3, 2)), - new Space(new Range(3, 2, 3, 3)), - new Slash(new Range(3, 3, 3, 4)), - new Word(new Range(3, 4, 3, 4 + 3), 'cmd'), - new Hash(new Range(3, 7, 3, 8)), - new Word(new Range(3, 8, 3, 8 + 3), 'var'), - new Space(new Range(3, 11, 3, 12)), - new NewLine(new Range(3, 12, 3, 13)), - // fourth line - new Slash(new Range(4, 1, 4, 2)), - new Word(new Range(4, 2, 4, 2 + 4), 'test'), - new At(new Range(4, 6, 4, 7)), - new Word(new Range(4, 7, 4, 7 + 6), 'github'), - new Tab(new Range(4, 13, 4, 14)), - new Tab(new Range(4, 14, 4, 15)), - new NewLine(new Range(4, 15, 4, 16)), - // fifth line - new Slash(new Range(5, 1, 5, 2)), - new Word(new Range(5, 2, 5, 2 + 6), 'update'), - new CarriageReturn(new Range(5, 8, 5, 9)), - new NewLine(new Range(5, 9, 5, 10)), - ], - ); - }); + await test.run( + [ + 'your command is /catch', + '\t\t/command1/command2 ', + ' /cmd#var ', + '/test@github\t\t', + '/update\r', + '', + ], + [ + // first line + new Word(new Range(1, 1, 1, 5), 'your'), + new Space(new Range(1, 5, 1, 6)), + new Word(new Range(1, 6, 1, 6 + 7), 'command'), + new Space(new Range(1, 13, 1, 14)), + new Word(new Range(1, 14, 1, 14 + 2), 'is'), + new Space(new Range(1, 16, 1, 17)), + new Slash(new Range(1, 17, 1, 18)), + new Word(new Range(1, 18, 1, 18 + 5), 'catch'), + new NewLine(new Range(1, 23, 1, 24)), + // second line + new Tab(new Range(2, 1, 2, 2)), + new Tab(new Range(2, 2, 2, 3)), + new Slash(new Range(2, 3, 2, 4)), + new Word(new Range(2, 4, 2, 4 + 8), 'command1'), + new Slash(new Range(2, 12, 2, 13)), + new Word(new Range(2, 13, 2, 13 + 8), 'command2'), + new Space(new Range(2, 21, 2, 22)), + new NewLine(new Range(2, 22, 2, 23)), + // third line + new Space(new Range(3, 1, 3, 2)), + new Space(new Range(3, 2, 3, 3)), + new Slash(new Range(3, 3, 3, 4)), + new Word(new Range(3, 4, 3, 4 + 3), 'cmd'), + new Hash(new Range(3, 7, 3, 8)), + new Word(new Range(3, 8, 3, 8 + 3), 'var'), + new Space(new Range(3, 11, 3, 12)), + new NewLine(new Range(3, 12, 3, 13)), + // fourth line + new Slash(new Range(4, 1, 4, 2)), + new Word(new Range(4, 2, 4, 2 + 4), 'test'), + new At(new Range(4, 6, 4, 7)), + new Word(new Range(4, 7, 4, 7 + 6), 'github'), + new Tab(new Range(4, 13, 4, 14)), + new Tab(new Range(4, 14, 4, 15)), + new NewLine(new Range(4, 15, 4, 16)), + // fifth line + new Slash(new Range(5, 1, 5, 2)), + new Word(new Range(5, 2, 5, 2 + 6), 'update'), + new CarriageReturn(new Range(5, 8, 5, 9)), + new NewLine(new Range(5, 9, 5, 10)), + ], + ); }); }); diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/codecs/parsers/promptTemplateVariableParser.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/codecs/parsers/promptTemplateVariableParser.ts index 23becf1df3c..94c9fa4ee30 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/codecs/parsers/promptTemplateVariableParser.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/codecs/parsers/promptTemplateVariableParser.ts @@ -11,12 +11,14 @@ import { DollarSign, LeftCurlyBrace, RightCurlyBrace } from '../../../../../../. import { assertNotConsumed, ParserBase, TAcceptTokenResult } from '../../../../../../../editor/common/codecs/simpleCodec/parserBase.js'; /** - * TODO: @legomushroom + * Parsers of the `${variable}` token sequence in a prompt text. */ export type TPromptTemplateVariableParser = PartialPromptTemplateVariableStart | PartialPromptTemplateVariable; /** - * TODO: @legomushroom + * Parser that handles start sequence of a `${variable}` token sequence in + * a prompt text. Transitions to {@link PartialPromptTemplateVariable} parser + * as soon as the `${` character sequence is found. */ export class PartialPromptTemplateVariableStart extends ParserBase { constructor(token: DollarSign) { @@ -44,7 +46,7 @@ export class PartialPromptTemplateVariableStart extends ParserBase { constructor(tokens: (DollarSign | LeftCurlyBrace)[]) { @@ -66,8 +68,6 @@ export class PartialPromptTemplateVariable extends ParserBase