Skip to content

Commit

Permalink
[prompt snippets]: pre PR cleanups
Browse files Browse the repository at this point in the history
  • Loading branch information
legomushroom committed Nov 19, 2024
1 parent e3a4a4f commit a927e4e
Show file tree
Hide file tree
Showing 16 changed files with 72 additions and 55 deletions.
19 changes: 11 additions & 8 deletions src/vs/base/common/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export interface ReadableStream<T> extends ReadableStreamEvents<T> {
/**
* Allows to remove a listener that was previously added.
*/
removeEventListener(event: string, callback: Function): void;
removeListener(event: string, callback: Function): void;
}

/**
Expand Down Expand Up @@ -221,10 +221,13 @@ class WriteableStreamImpl<T> implements WriteableStream<T> {

private readonly pendingWritePromises: Function[] = [];

/**
* @param reducer a function that reduces the buffered data into a single object;
* because some objects can be complex and non-reducible, we also
* allow passing the explicit `null` value to skip the reduce step
* @param options stream options
*/
constructor(
// some messages can be complex objects so are non-reducable,
// hence `reducer` cabe be `null`, the `null` value is used
// here to make the user intention more explicit
private reducer: IReducer<T> | null,
private options?: WriteableStreamOptions,
) { }
Expand Down Expand Up @@ -372,7 +375,7 @@ class WriteableStreamImpl<T> implements WriteableStream<T> {
}
}

removeEventListener(event: string, callback: Function): void {
removeListener(event: string, callback: Function): void {
if (this.state.destroyed) {
return;
}
Expand Down Expand Up @@ -652,16 +655,16 @@ export function peekStream<T>(stream: ReadableStream<T>, maxChunks: number): Pro
return resolve({ stream, buffer, ended: true });
};

streamListeners.add(toDisposable(() => stream.removeEventListener('error', errorListener)));
streamListeners.add(toDisposable(() => stream.removeListener('error', errorListener)));
stream.on('error', errorListener);

streamListeners.add(toDisposable(() => stream.removeEventListener('end', endListener)));
streamListeners.add(toDisposable(() => stream.removeListener('end', endListener)));
stream.on('end', endListener);

// Important: leave the `data` listener last because
// this can turn the stream into flowing mode and we
// want `error` events to be received as well.
streamListeners.add(toDisposable(() => stream.removeEventListener('data', dataListener)));
streamListeners.add(toDisposable(() => stream.removeListener('data', dataListener)));
stream.on('data', dataListener);
});
}
Expand Down
6 changes: 3 additions & 3 deletions src/vs/base/test/common/stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ suite('Stream', () => {
assert.strictEqual(data, true);

data = false;
stream.removeEventListener('data', dataListener);
stream.removeListener('data', dataListener);

stream.write('World');
assert.strictEqual(data, false);
Expand All @@ -235,7 +235,7 @@ suite('Stream', () => {
assert.strictEqual(error, true);

error = false;
stream.removeEventListener('error', errorListener);
stream.removeListener('error', errorListener);

// always leave at least one error listener to streams to avoid unexpected errors during test running
stream.on('error', () => { });
Expand Down Expand Up @@ -558,7 +558,7 @@ suite('Stream', () => {
let listener1Called = false;
let listener2Called = false;

const listener1 = () => { stream.removeEventListener('end', listener1); listener1Called = true; };
const listener1 = () => { stream.removeListener('end', listener1); listener1Called = true; };
const listener2 = () => { listener2Called = true; };
stream.on('end', listener1);
stream.on('end', listener2);
Expand Down
36 changes: 23 additions & 13 deletions src/vs/workbench/common/codecs/baseDecoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import { Disposable, IDisposable } from '../../../base/common/lifecycle.js';
import { TStreamListenerNames } from './types/TStreamListenerEventNames.js';

/**
* Base decoder class that can be used to decode a stream of data of one type into another.
* Aimed to be a part of a "codec" implementation and not to be used directly.
* Base decoder class that can be used to convert stream messages data type
* from one type to another. For instance, a stream of binary data can be
* "decoded" into a stream of well defined objects.
* Intended to be a part of "codec" implementation rather than used directly.
*/
export abstract class BaseDecoder<T extends NonNullable<unknown>, K extends NonNullable<unknown> = NonNullable<unknown>> extends Disposable implements ReadableStream<T> {
/**
Expand All @@ -28,10 +30,10 @@ export abstract class BaseDecoder<T extends NonNullable<unknown>, K extends NonN
*/
private readonly _listeners: Map<TStreamListenerNames, Map<Function, IDisposable>> = new Map();

/**
* @param stream The input stream to decode.
*/
constructor(
/**
* The input stream to decode.
*/
protected readonly stream: ReadableStream<K>,
) {
super();
Expand Down Expand Up @@ -174,9 +176,9 @@ export abstract class BaseDecoder<T extends NonNullable<unknown>, K extends NonN
*/
public removeAllListeners(): void {
// remove listeners set up by this class
this.stream.removeEventListener('data', this.tryOnStreamData);
this.stream.removeEventListener('error', this.onStreamError);
this.stream.removeEventListener('end', this.onStreamEnd);
this.stream.removeListener('data', this.tryOnStreamData);
this.stream.removeListener('error', this.onStreamError);
this.stream.removeListener('end', this.onStreamEnd);

// remove listeners set up by external consumers
for (const [name, listeners] of this._listeners.entries()) {
Expand Down Expand Up @@ -226,7 +228,7 @@ export abstract class BaseDecoder<T extends NonNullable<unknown>, K extends NonN
* not found, therefore passing incorrect `callback` function may
* result in silent unexpected behaviour
*/
public removeEventListener(event: string, callback: Function): void {
public removeListener(event: string, callback: Function): void {
for (const [nameName, listeners] of this._listeners.entries()) {
if (nameName !== event) {
continue;
Expand Down Expand Up @@ -326,6 +328,9 @@ class AsyncDecoder<T extends NonNullable<unknown>, K extends NonNullable<unknown
*/
private resolveOnNewEvent?: (value: void) => void;

/**
* @param decoder The decoder instance to wrap.
*/
constructor(
private readonly decoder: BaseDecoder<T, K>,
) {
Expand All @@ -336,14 +341,17 @@ class AsyncDecoder<T extends NonNullable<unknown>, K extends NonNullable<unknown
* Async iterator implementation.
*/
async *[Symbol.asyncIterator](): AsyncIterator<T | null> {
// callback is called when `data` or `end` event is received
const callback = (data?: T) => {
if (data !== undefined) {
this.messages.push(data);
} else {
this.decoder.removeEventListener('data', callback);
this.decoder.removeEventListener('end', callback);
this.decoder.removeListener('data', callback);
this.decoder.removeListener('end', callback);
}

// is the promise resolve callback is present,
// then call it and remove the reference
if (this.resolveOnNewEvent) {
this.resolveOnNewEvent();
delete this.resolveOnNewEvent;
Expand All @@ -352,6 +360,7 @@ class AsyncDecoder<T extends NonNullable<unknown>, K extends NonNullable<unknown
this.decoder.on('data', callback);
this.decoder.on('end', callback);

// start flowing the decoder stream
this.decoder.start();

while (true) {
Expand All @@ -361,12 +370,13 @@ class AsyncDecoder<T extends NonNullable<unknown>, K extends NonNullable<unknown
continue;
}

// no data and stream ended, so we're done
// if no data and stream ended, so we're done
if (this.decoder.isEnded) {
return null;
}

// otherwise wait for new data to be available
// stream isn't ended so wait for the new
// `data` or `end` event to be received
await new Promise((resolve) => {
this.resolveOnNewEvent = resolve;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
import { Range } from '../../../editor/common/core/range.js';

/**
* Base class for all tokens with a `range` that reflects
* token's position in the original data.
* Base class for all tokens with a `range` that
* reflects token position in the original data.
*/
export class RangedToken {
export class BaseToken {
constructor(
public readonly range: Range,
) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { RangedToken } from '../../rangedToken.js';
import { BaseToken } from '../../baseToken.js';
import { Word } from '../../simpleCodec/tokens/index.js';
import { assert } from '../../../../../base/common/assert.js';
import { Range } from '../../../../../editor/common/core/range.js';
Expand All @@ -14,7 +14,7 @@ const TOKEN_START: string = '#file:';
/**
* A file reference token inside a prompt.
*/
export class FileReference extends RangedToken {
export class FileReference extends BaseToken {
// Start sequence for a file reference token in a prompt.
public static readonly TOKEN_START = TOKEN_START;

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/common/codecs/linesCodec/tokens/line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { RangedToken } from '../../rangedToken.js';
import { BaseToken } from '../../baseToken.js';
import { assert } from '../../../../../base/common/assert.js';
import { Range } from '../../../../../editor/common/core/range.js';

/**
* Token representing a line of text with a `range` which
* reflects the line's position in the original data.
*/
export class Line extends RangedToken {
export class Line extends BaseToken {
constructor(
// the line index
// Note! 1-based indexing
Expand Down
9 changes: 6 additions & 3 deletions src/vs/workbench/common/codecs/linesCodec/tokens/newLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { Line } from './line.js';
import { RangedToken } from '../../rangedToken.js';
import { BaseToken } from '../../baseToken.js';
import { Range } from '../../../../../editor/common/core/range.js';
import { Position } from '../../../../../editor/common/core/position.js';
import { VSBuffer } from '../../../../../base/common/buffer.js';
Expand All @@ -13,12 +13,15 @@ import { VSBuffer } from '../../../../../base/common/buffer.js';
* A token that represent a `new line` with a `range`. The `range`
* value reflects the position of the token in the original data.
*/
export class NewLine extends RangedToken {
export class NewLine extends BaseToken {
/**
* The underlying symbol of the `NewLine` token.
*/
public static readonly symbol: string = '\n';
// TODO: @legomushroom

/**
* The byte representation of the {@link symbol}.
*/
public static readonly byte = VSBuffer.fromString(NewLine.symbol);

/**
Expand Down
8 changes: 5 additions & 3 deletions src/vs/workbench/common/codecs/simpleCodec/simpleDecoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import { LinesDecoder, TLineToken } from '../linesCodec/linesDecoder.js';
*/
export type TSimpleToken = Word | Space | Tab | NewLine;

// Note! the `\n` is excluded because this decoder based on lines
// Characters that stop a word sequence.
// hence can't ever receive a line that contains a `newline`.
/**
* Characters that stop a "word" sequence.
* Note! the `\n` is excluded because this decoder based on `LinesDecoder` that already
* handles the `newline` case and emits lines that don't contain the `\n` anymore.
*/
const STOP_CHARACTERS = [Space.symbol, Tab.symbol];

/**
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/common/codecs/simpleCodec/tokens/space.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { RangedToken } from '../../rangedToken.js';
import { BaseToken } from '../../baseToken.js';
import { Line } from '../../linesCodec/tokens/line.js';
import { Range } from '../../../../../editor/common/core/range.js';
import { Position } from '../../../../../editor/common/core/position.js';
Expand All @@ -12,7 +12,7 @@ import { Position } from '../../../../../editor/common/core/position.js';
* A token that represent a `space` with a `range`. The `range`
* value reflects the position of the token in the original data.
*/
export class Space extends RangedToken {
export class Space extends BaseToken {
/**
* The underlying symbol of the `Space` token.
*/
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/common/codecs/simpleCodec/tokens/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { RangedToken } from '../../rangedToken.js';
import { BaseToken } from '../../baseToken.js';
import { Line } from '../../linesCodec/tokens/line.js';
import { Range } from '../../../../../editor/common/core/range.js';
import { Position } from '../../../../../editor/common/core/position.js';
Expand All @@ -12,7 +12,7 @@ import { Position } from '../../../../../editor/common/core/position.js';
* A token that represent a `tab` with a `range`. The `range`
* value reflects the position of the token in the original data.
*/
export class Tab extends RangedToken {
export class Tab extends BaseToken {
/**
* The underlying symbol of the `Tab` token.
*/
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/common/codecs/simpleCodec/tokens/word.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { RangedToken } from '../../rangedToken.js';
import { BaseToken } from '../../baseToken.js';
import { Line } from '../../linesCodec/tokens/line.js';
import { Range } from '../../../../../editor/common/core/range.js';
import { Position } from '../../../../../editor/common/core/position.js';
Expand All @@ -13,7 +13,7 @@ import { Position } from '../../../../../editor/common/core/position.js';
* characters without stop characters, like a `space`,
* a `tab`, or a `new line`.
*/
export class Word extends RangedToken {
export class Word extends BaseToken {
constructor(
/**
* The word range.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
// Well defined event names of `ReadableStream<T>`.
/**
* Event names of {@link ReadableStream} stream.
*/
export type TStreamListenerNames = 'data' | 'error' | 'end';
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export class ImplicitContextAttachmentWidget extends Disposable {
}

/**
* Get file URIs label.
* Get file URIs label, including its possible nested file references.
*/
private getUriLabel(
file: URI,
Expand Down
10 changes: 4 additions & 6 deletions src/vs/workbench/contrib/chat/browser/chatVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'
import { ThemeIcon } from '../../../../base/common/themables.js';
import { URI } from '../../../../base/common/uri.js';
import { Location } from '../../../../editor/common/languages.js';
import { IFileService } from '../../../../platform/files/common/files.js';
import { IViewsService } from '../../../services/views/common/viewsService.js';
import { ChatAgentLocation } from '../common/chatAgents.js';
import { IChatModel, IChatRequestVariableData, IChatRequestVariableEntry } from '../common/chatModel.js';
Expand All @@ -28,8 +27,8 @@ interface IChatData {
}

/**
* Helper to run provided `jobs` in parallel and return the `successful`
* results in the same order as the original jobs.
* Helper to run provided `jobs` in parallel and return only
* the `successful` results in the same order as the original jobs.
*/
const getJobResults = async (
jobs: Promise<IChatRequestVariableEntry | null>[],
Expand Down Expand Up @@ -63,7 +62,6 @@ export class ChatVariablesService implements IChatVariablesService {
constructor(
@IChatWidgetService private readonly chatWidgetService: IChatWidgetService,
@IViewsService private readonly viewsService: IViewsService,
@IFileService _: IFileService,
) { }

public async resolveVariables(
Expand Down Expand Up @@ -183,8 +181,8 @@ export class ChatVariablesService implements IChatVariablesService {
});

// run all jobs in paralle and get results in the original order
// Note! the `getJobResults` call supposed to never fail, therefore
// it's ok to `Promise.all` here
// Note! the `getJobResults` call supposed to never fail, hence it's ok to do
// `Promise.all()`, otherwise we have a logic error that would be caught here
const [resolvedVariables, resolvedAttachedContext] = await Promise.all(
[
getJobResults(resolvedVariableJobs),
Expand Down
Loading

0 comments on commit a927e4e

Please sign in to comment.