Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try better fix for syncing code blocks once complete #233455

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/vs/editor/common/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,11 @@ export class TextModelResolvedOptions {
readonly trimAutoWhitespace: boolean;
readonly bracketPairColorizationOptions: BracketPairColorizationOptions;

/**
* @internal
*/
readonly isForSimpleWidget: boolean;

public get originalIndentSize(): number | 'tabSize' {
return this._indentSizeIsTabSize ? 'tabSize' : this.indentSize;
}
Expand All @@ -537,6 +542,7 @@ export class TextModelResolvedOptions {
defaultEOL: DefaultEndOfLine;
trimAutoWhitespace: boolean;
bracketPairColorizationOptions: BracketPairColorizationOptions;
isForSimpleWidget: boolean;
}) {
this.tabSize = Math.max(1, src.tabSize | 0);
if (src.indentSize === 'tabSize') {
Expand All @@ -550,6 +556,7 @@ export class TextModelResolvedOptions {
this.defaultEOL = src.defaultEOL | 0;
this.trimAutoWhitespace = Boolean(src.trimAutoWhitespace);
this.bracketPairColorizationOptions = src.bracketPairColorizationOptions;
this.isForSimpleWidget = src.isForSimpleWidget;
}

/**
Expand All @@ -563,6 +570,7 @@ export class TextModelResolvedOptions {
&& this.insertSpaces === other.insertSpaces
&& this.defaultEOL === other.defaultEOL
&& this.trimAutoWhitespace === other.trimAutoWhitespace
&& this.isForSimpleWidget === other.isForSimpleWidget
&& equals(this.bracketPairColorizationOptions, other.bracketPairColorizationOptions)
);
}
Expand All @@ -576,6 +584,7 @@ export class TextModelResolvedOptions {
indentSize: this.indentSize !== newOpts.indentSize,
insertSpaces: this.insertSpaces !== newOpts.insertSpaces,
trimAutoWhitespace: this.trimAutoWhitespace !== newOpts.trimAutoWhitespace,
isForSimpleWidget: this.isForSimpleWidget !== newOpts.isForSimpleWidget,
};
}
}
Expand Down Expand Up @@ -606,6 +615,11 @@ export interface ITextModelUpdateOptions {
insertSpaces?: boolean;
trimAutoWhitespace?: boolean;
bracketColorizationOptions?: BracketPairColorizationOptions;

/**
* @internal
*/
isForSimpleWidget?: boolean;
}

export class FindMatch {
Expand Down
9 changes: 7 additions & 2 deletions src/vs/editor/common/model/textModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
trimAutoWhitespace: options.trimAutoWhitespace,
defaultEOL: options.defaultEOL,
bracketPairColorizationOptions: options.bracketPairColorizationOptions,
isForSimpleWidget: options.isForSimpleWidget,
});
}

Expand Down Expand Up @@ -241,7 +242,10 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
//#endregion

public readonly id: string;
public readonly isForSimpleWidget: boolean;
public get isForSimpleWidget() {
return this._options.isForSimpleWidget;
}

private readonly _associatedResource: URI;
private _attachedEditorCount: number;
private _buffer: model.ITextBuffer;
Expand Down Expand Up @@ -308,7 +312,6 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
// Generate a new unique model id
MODEL_ID++;
this.id = '$model' + MODEL_ID;
this.isForSimpleWidget = creationOptions.isForSimpleWidget;
if (typeof associatedResource === 'undefined' || associatedResource === null) {
this._associatedResource = URI.parse('inmemory://model/' + MODEL_ID);
} else {
Expand Down Expand Up @@ -653,6 +656,7 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
const insertSpaces = (typeof _newOpts.insertSpaces !== 'undefined') ? _newOpts.insertSpaces : this._options.insertSpaces;
const trimAutoWhitespace = (typeof _newOpts.trimAutoWhitespace !== 'undefined') ? _newOpts.trimAutoWhitespace : this._options.trimAutoWhitespace;
const bracketPairColorizationOptions = (typeof _newOpts.bracketColorizationOptions !== 'undefined') ? _newOpts.bracketColorizationOptions : this._options.bracketPairColorizationOptions;
const isForSimpleWidget = (typeof _newOpts.isForSimpleWidget !== 'undefined') ? _newOpts.isForSimpleWidget : this._options.isForSimpleWidget;

const newOpts = new model.TextModelResolvedOptions({
tabSize: tabSize,
Expand All @@ -661,6 +665,7 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
defaultEOL: this._options.defaultEOL,
trimAutoWhitespace: trimAutoWhitespace,
bracketPairColorizationOptions,
isForSimpleWidget,
});

if (this._options.equals(newOpts)) {
Expand Down
5 changes: 5 additions & 0 deletions src/vs/editor/common/textModelEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ export interface IModelOptionsChangedEvent {
readonly indentSize: boolean;
readonly insertSpaces: boolean;
readonly trimAutoWhitespace: boolean;

/**
* @internal
*/
readonly isForSimpleWidget: boolean;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/chat/browser/codeBlockPart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ export class CodeBlockPart extends Disposable {

private async updateEditor(data: ICodeBlockData): Promise<void> {
const textModel = await data.textModel;
console.log(textModel.id);
this.editor.setModel(textModel);
if (data.range) {
this.editor.setSelection(data.range);
Expand Down Expand Up @@ -455,7 +454,8 @@ export class ChatCodeBlockContentProvider extends Disposable implements ITextMod
if (existing) {
return existing;
}
return this._modelService.createModel('', null, resource);
// Start models as simple so they aren't synced until complete
return this._modelService.createModel('', null, resource, true);
}
}

Expand Down
52 changes: 11 additions & 41 deletions src/vs/workbench/contrib/chat/common/codeBlockModelCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { URI } from '../../../../base/common/uri.js';
import { Range } from '../../../../editor/common/core/range.js';
import { ILanguageService } from '../../../../editor/common/languages/language.js';
import { EndOfLinePreference, ITextModel } from '../../../../editor/common/model.js';
import { IModelService } from '../../../../editor/common/services/model.js';
import { IResolvedTextEditorModel, ITextModelService } from '../../../../editor/common/services/resolverService.js';
import { extractCodeblockUrisFromText, extractVulnerabilitiesFromText, IMarkdownVulnerability } from './annotations.js';
import { IChatRequestViewModel, IChatResponseViewModel, isResponseVM } from './chatViewModel.js';
Expand All @@ -28,18 +27,10 @@ interface CodeBlockEntry {
readonly codemapperUri?: URI;
}

type CodeBlockTextModel = {
readonly type: 'incomplete';
readonly value: ITextModel;
} | {
readonly type: 'complete';
readonly value: Promise<IReference<IResolvedTextEditorModel>>;
};

export class CodeBlockModelCollection extends Disposable {

private readonly _models = new Map<string, {
model: CodeBlockTextModel;
model: Promise<IReference<IResolvedTextEditorModel>>;
vulns: readonly IMarkdownVulnerability[];
codemapperUri?: URI;
}>();
Expand All @@ -53,7 +44,6 @@ export class CodeBlockModelCollection extends Disposable {

constructor(
@ILanguageService private readonly languageService: ILanguageService,
@IModelService private readonly modelService: IModelService,
@ITextModelService private readonly textModelService: ITextModelService,
) {
super();
Expand All @@ -70,7 +60,7 @@ export class CodeBlockModelCollection extends Disposable {
return;
}
return {
model: entry.model.type === 'incomplete' ? Promise.resolve(entry.model.value) : entry.model.value.then(ref => ref.object.textEditorModel),
model: entry.model.then(ref => ref.object.textEditorModel),
vulns: entry.vulns,
codemapperUri: entry.codemapperUri
};
Expand All @@ -82,10 +72,10 @@ export class CodeBlockModelCollection extends Disposable {
return existing;
}

const uri = this.getIncompleteModelUri(sessionId, chat, codeBlockIndex);
const model = this.modelService.createModel('', null, uri, true);
const uri = this.getCodeBlockUri(sessionId, chat, codeBlockIndex);
const model = this.textModelService.createModelReference(uri);
this._models.set(this.getKey(sessionId, chat, codeBlockIndex), {
model: { type: 'incomplete', value: model },
model: model,
vulns: [],
codemapperUri: undefined,
});
Expand All @@ -98,7 +88,7 @@ export class CodeBlockModelCollection extends Disposable {
this.delete(first);
}

return { model: Promise.resolve(model), vulns: [], codemapperUri: undefined };
return { model: model.then(x => x.object.textEditorModel), vulns: [], codemapperUri: undefined };
}

private delete(key: string) {
Expand All @@ -107,21 +97,13 @@ export class CodeBlockModelCollection extends Disposable {
return;
}

this.disposeModel(entry.model);
entry.model.then(ref => ref.object.dispose());

this._models.delete(key);
}

private disposeModel(model: CodeBlockTextModel) {
if (model.type === 'complete') {
model.value.then(ref => ref.dispose());
} else {
this.modelService.destroyModel(model.value.uri);
}
}

clear(): void {
this._models.forEach(async entry => this.disposeModel(entry.model));
this._models.forEach(async entry => (await entry.model).dispose());
this._models.clear();
}

Expand All @@ -146,15 +128,11 @@ export class CodeBlockModelCollection extends Disposable {

markCodeBlockCompleted(sessionId: string, chat: IChatRequestViewModel | IChatResponseViewModel, codeBlockIndex: number): void {
const entry = this._models.get(this.getKey(sessionId, chat, codeBlockIndex));
if (!entry || entry.model.type === 'complete') {
if (!entry) {
return;
}

this.disposeModel(entry.model);

const uri = this.getCompletedModelUri(sessionId, chat, codeBlockIndex);
const newModel = this.textModelService.createModelReference(uri);
entry.model = { type: 'complete', value: newModel };
entry.model.then(ref => ref.object.textEditorModel.updateOptions({ isForSimpleWidget: false }));
}

async update(sessionId: string, chat: IChatRequestViewModel | IChatResponseViewModel, codeBlockIndex: number, content: CodeBlockContent): Promise<CodeBlockEntry> {
Expand Down Expand Up @@ -222,15 +200,7 @@ export class CodeBlockModelCollection extends Disposable {
return `${sessionId}/${chat.id}/${index}`;
}

private getIncompleteModelUri(sessionId: string, chat: IChatRequestViewModel | IChatResponseViewModel, index: number): URI {
return URI.from({
scheme: Schemas.inMemory,
authority: 'chat-code-block',
path: `/${sessionId}/${chat.id}/${index}`
});
}

private getCompletedModelUri(sessionId: string, chat: IChatRequestViewModel | IChatResponseViewModel, index: number): URI {
private getCodeBlockUri(sessionId: string, chat: IChatRequestViewModel | IChatResponseViewModel, index: number): URI {
const metadata = this.getUriMetaData(chat);
return URI.from({
scheme: Schemas.vscodeChatCodeBlock,
Expand Down
Loading