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.

Sign up for GitHub

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

Allow semantic tokens to be provided by more than one provider #135602

Merged
merged 12 commits into from
Oct 25, 2021
Merged

Allow semantic tokens to be provided by more than one provider #135602

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5f161f2
Multiplex providers into one
rchiodo Oct 21, 2021
02b5209
Build problem
rchiodo Oct 21, 2021
3daf66b
Cache last used provider for legends call
rchiodo Oct 21, 2021
6da697c
Cache last provider for range token provider too
rchiodo Oct 21, 2021
e9d68a1
Put back old method for use in viewportSemanticTokens
rchiodo Oct 21, 2021
eef5a74
Add test for multiple providers
rchiodo Oct 21, 2021
ae6066d
Double check called both providers
rchiodo Oct 21, 2021
31662e1
Make sure to cache result id and use the appropriate legend
rchiodo Oct 22, 2021
a0d62ed
:lipstick:
alexdima Oct 25, 2021
3703c27
Use `CompositeDocumentRangeSemanticTokensProvider` only when having 2…
alexdima Oct 25, 2021
21b22af
Add support for invoking multiple equal scored `DocumentRangeSemantic…
alexdima Oct 25, 2021
42aa33c
Add support for invoking multiple equal scored `DocumentSemanticToken…
alexdima Oct 25, 2021
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
123 changes: 112 additions & 11 deletions src/vs/editor/common/services/getSemanticTokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { assertType } from 'vs/base/common/types';
import { VSBuffer } from 'vs/base/common/buffer';
import { encodeSemanticTokensDto } from 'vs/editor/common/services/semanticTokensDto';
import { Range } from 'vs/editor/common/core/range';
import { Emitter, Event } from 'vs/base/common/event';
import { DisposableStore } from 'vs/base/common/lifecycle';

export function isSemanticTokens(v: SemanticTokens | SemanticTokensEdits): v is SemanticTokens {
return v && !!((<SemanticTokens>v).data);
Expand All @@ -29,26 +31,125 @@ export interface IDocumentSemanticTokensResult {
}

export function getDocumentSemanticTokens(model: ITextModel, lastResultId: string | null, token: CancellationToken): IDocumentSemanticTokensResult | null {
const provider = _getDocumentSemanticTokensProvider(model);
if (!provider) {
const providerGroup = _getDocumentSemanticTokensProviderHighestGroup(model);
if (!providerGroup) {
return null;
}
const compositeProvider = new CompositeDocumentSemanticTokensProvider(model, providerGroup);
return {
provider: provider,
request: Promise.resolve(provider.provideDocumentSemanticTokens(model, lastResultId, token))
provider: compositeProvider,
request: Promise.resolve(compositeProvider.provideDocumentSemanticTokens(model, lastResultId, token))
};
}

function _getDocumentSemanticTokensProvider(model: ITextModel): DocumentSemanticTokensProvider | null {
const result = DocumentSemanticTokensProviderRegistry.ordered(model);
class CompositeDocumentSemanticTokensProvider implements DocumentSemanticTokensProvider {
private disposables = new DisposableStore();
private didChangeEmitter = new Emitter<void>();
private lastUsedProvider: DocumentSemanticTokensProvider | undefined = undefined;
private static providerToLastResult = new WeakMap<DocumentSemanticTokensProvider, string>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This static introduces another subtle bug in the case of having two files opened at the same time, even when having a single provider.

If file1.ts and file2.ts are shown side-by-side, and if file1.ts makes a request and has the resultId 1, and then file2.ts makes a request and has the resultId 2, then after typing in file1.ts the request will not use resultId 1 (which would be correct) because the last request to this provider returned the resultId 2 for file2.ts.

IMHO there is a problem with the lifecycle of CompositeDocumentSemanticTokensProvider here. Before, getDocumentSemanticTokens could be implemented stateless, but IMHO that is no longer the case now, as some state needs to be captured between semantic tokens requests (like e.g. what was the last selected provider).

I will try to refactor how ModelSemanticColoring interacts with the DocumentSemanticTokensProviderRegistry, but IMHO this can no longer be done with a stateless getDocumentSemanticTokens call. I am working directly on your branch, please hold off from pushing for now.

constructor(model: ITextModel, private readonly providerGroup: DocumentSemanticTokensProvider[]) {
// Lifetime of this provider is tied to the text model
model.onWillDispose(() => this.disposables.clear());

// Mirror did change events
providerGroup.forEach(p => {
if (p.onDidChange) {
p.onDidChange(() => this.didChangeEmitter.fire(), this, this.disposables);
}
});
}
public async provideDocumentSemanticTokens(model: ITextModel, lastResultId: string | null, token: CancellationToken): Promise<SemanticTokens | SemanticTokensEdits | null | undefined> {
// Get tokens from the group all at the same time. Return the first
// that actually returned tokens
const list = await Promise.all(this.providerGroup.map(async provider => {
try {
// If result id is passed in, make sure it's for this provider
const localLastResultId = lastResultId && CompositeDocumentSemanticTokensProvider.providerToLastResult.get(provider) === lastResultId ? lastResultId : null;

// Get the result for this provider
const result = await provider.provideDocumentSemanticTokens(model, localLastResultId, token);

// Save result id for this provider
if (result?.resultId) {
CompositeDocumentSemanticTokensProvider.providerToLastResult.set(provider, result.resultId);
}

return result;
} catch (err) {
onUnexpectedExternalError(err);
}
return undefined;
}));

const hasTokensIndex = list.findIndex(l => l);

// Save last used provider. Use it for the legend if called
this.lastUsedProvider = this.providerGroup[hasTokensIndex];
return list[hasTokensIndex];
}
public get onDidChange(): Event<void> {
return this.didChangeEmitter.event;
}
public getLegend(): SemanticTokensLegend {
return this.lastUsedProvider?.getLegend() || this.providerGroup[0].getLegend();
}
public releaseDocumentSemanticTokens(resultId: string | undefined): void {
this.providerGroup.forEach(p => {
// If this result is for this provider, release it
if (resultId) {
if (CompositeDocumentSemanticTokensProvider.providerToLastResult.get(p) === resultId) {
p.releaseDocumentSemanticTokens(resultId);
CompositeDocumentSemanticTokensProvider.providerToLastResult.delete(p);
}
// Else if the result is empty, release for all providers that aren't waiting for a result id
} else if (CompositeDocumentSemanticTokensProvider.providerToLastResult.get(p) === undefined) {
p.releaseDocumentSemanticTokens(undefined);
}
});
}
}

class CompositeDocumentRangeSemanticTokensProvider implements DocumentRangeSemanticTokensProvider {
private lastUsedProvider: DocumentRangeSemanticTokensProvider | undefined = undefined;
constructor(private readonly providerGroup: DocumentRangeSemanticTokensProvider[]) { }
public async provideDocumentRangeSemanticTokens(model: ITextModel, range: Range, token: CancellationToken): Promise<SemanticTokens | null | undefined> {
// Get tokens from the group all at the same time. Return the first
// that actually returned tokens
const list = await Promise.all(this.providerGroup.map(async provider => {
try {
return await provider.provideDocumentRangeSemanticTokens(model, range, token);
} catch (err) {
onUnexpectedExternalError(err);
}
return undefined;
}));

const hasTokensIndex = list.findIndex(l => l);

// Save last used provider. Use it for the legend if called
this.lastUsedProvider = this.providerGroup[hasTokensIndex];
return list[hasTokensIndex];
}
getLegend(): SemanticTokensLegend {
return this.lastUsedProvider?.getLegend() || this.providerGroup[0].getLegend();
}
}

function _getDocumentSemanticTokensProviderHighestGroup(model: ITextModel): DocumentSemanticTokensProvider[] | null {
const result = DocumentSemanticTokensProviderRegistry.orderedGroups(model);
return (result.length > 0 ? result[0] : null);
}

export function getDocumentRangeSemanticTokensProvider(model: ITextModel): DocumentRangeSemanticTokensProvider | null {
const result = DocumentRangeSemanticTokensProviderRegistry.ordered(model);
function _getDocumentRangeSemanticTokensProviderHighestGroup(model: ITextModel): DocumentRangeSemanticTokensProvider[] | null {
const result = DocumentRangeSemanticTokensProviderRegistry.orderedGroups(model);
return (result.length > 0 ? result[0] : null);
}

export function getDocumentRangeSemanticTokensProvider(model: ITextModel): DocumentRangeSemanticTokensProvider | null {
const highestGroup = _getDocumentRangeSemanticTokensProviderHighestGroup(model);
return highestGroup ? new CompositeDocumentRangeSemanticTokensProvider(highestGroup) : null;
}

CommandsRegistry.registerCommand('_provideDocumentSemanticTokensLegend', async (accessor, ...args): Promise<SemanticTokensLegend | undefined> => {
const [uri] = args;
assertType(uri instanceof URI);
Expand All @@ -58,13 +159,13 @@ CommandsRegistry.registerCommand('_provideDocumentSemanticTokensLegend', async (
return undefined;
}

const provider = _getDocumentSemanticTokensProvider(model);
if (!provider) {
const providers = _getDocumentSemanticTokensProviderHighestGroup(model);
if (!providers) {
// there is no provider => fall back to a document range semantic tokens provider
return accessor.get(ICommandService).executeCommand('_provideDocumentRangeSemanticTokensLegend', uri);
}

return provider.getLegend();
return providers[0].getLegend();
});

CommandsRegistry.registerCommand('_provideDocumentSemanticTokens', async (accessor, ...args): Promise<VSBuffer | undefined> => {
Expand Down
3 changes: 1 addition & 2 deletions src/vs/editor/common/services/modelServiceImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -840,10 +840,9 @@ export class ModelSemanticColoring extends Disposable {
pendingChanges.push(e);
});

const styling = this._semanticStyling.get(provider);

request.then((res) => {
this._currentDocumentRequestCancellationTokenSource = null;
const styling = this._semanticStyling.get(provider); // Do this after the provider gets results to ensure legend matches
contentChangeListener.dispose();
this._setDocumentSemanticTokens(provider, res || null, styling, pendingChanges);
}, (err) => {
Expand Down
65 changes: 65 additions & 0 deletions src/vs/editor/test/common/services/modelService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { IModelService } from 'vs/editor/common/services/modelService';
import { IModeService } from 'vs/editor/common/services/modeService';
import { TestTextResourcePropertiesService } from 'vs/editor/test/common/services/testTextResourcePropertiesService';
import { TestLanguageConfigurationService } from 'vs/editor/test/common/modes/testLanguageConfigurationService';
import { getDocumentSemanticTokens } from 'vs/editor/common/services/getSemanticTokens';

const GENERATE_TESTS = false;

Expand Down Expand Up @@ -486,6 +487,70 @@ suite('ModelSemanticColoring', () => {
// assert that it got called twice
assert.strictEqual(callCount, 2);
});

test('DocumentSemanticTokens should be pick the token provider with actual items', async () => {

let calledBoth = false;
disposables.add(ModesRegistry.registerLanguage({ id: 'testMode2' }));
disposables.add(DocumentSemanticTokensProviderRegistry.register('testMode2', new class implements DocumentSemanticTokensProvider {
getLegend(): SemanticTokensLegend {
return { tokenTypes: ['class1'], tokenModifiers: [] };
}
async provideDocumentSemanticTokens(model: ITextModel, lastResultId: string | null, token: CancellationToken): Promise<SemanticTokens | SemanticTokensEdits | null> {
// For a secondary request return a different value
if (lastResultId) {
return {
data: new Uint32Array([2, 1, 1, 1, 1, 0, 2, 1, 1, 1])
};
}
return {
resultId: '1',
data: new Uint32Array([0, 1, 1, 1, 1, 0, 2, 1, 1, 1])
};
}
releaseDocumentSemanticTokens(resultId: string | undefined): void {
}
}));
disposables.add(DocumentSemanticTokensProviderRegistry.register('testMode2', new class implements DocumentSemanticTokensProvider {
getLegend(): SemanticTokensLegend {
return { tokenTypes: ['class2'], tokenModifiers: [] };
}
async provideDocumentSemanticTokens(model: ITextModel, lastResultId: string | null, token: CancellationToken): Promise<SemanticTokens | SemanticTokensEdits | null> {
calledBoth = true;
return null;
}
releaseDocumentSemanticTokens(resultId: string | undefined): void {
}
}));

const textModel = modelService.createModel('Hello world 2', modeService.create('testMode2'));
try {
let request = getDocumentSemanticTokens(textModel, null, CancellationToken.None);
assert.deepStrictEqual(request?.provider.getLegend(), { tokenTypes: ['class2'], tokenModifiers: [] }, `Legend does not match prior to request`);
let tokens = await request?.request;

// We should have tokens
assert.ok(tokens, `Tokens not found from multiple providers`);
assert.ok(tokens.resultId, `Token result id not found from multiple providers`);
assert.deepStrictEqual([...(tokens as any).data], [0, 1, 1, 1, 1, 0, 2, 1, 1, 1], `Token data not returned for multiple providers`);
assert.ok(calledBoth, `Did not actually call both token providers`);
assert.deepStrictEqual(request?.provider.getLegend(), { tokenTypes: ['class1'], tokenModifiers: [] }, `Legend did not update after match`);

// Make a second request. Make sure we get the secondary value
request = getDocumentSemanticTokens(textModel, tokens!.resultId!, CancellationToken.None);
tokens = await request?.request;
assert.deepStrictEqual([...(tokens as any).data], [2, 1, 1, 1, 1, 0, 2, 1, 1, 1], `Token data not returned for second request for multiple providers`);

} finally {
disposables.clear();

// Wait for scheduler to finish
await timeout(0);

// Now dispose the text model
textModel.dispose();
}
});
});

function assertComputeEdits(lines1: string[], lines2: string[]): void {
Expand Down