Skip to content

Add typescript.implementationsCodeLens.showOnInterfaceMethods setting (#136282) #198419

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

Merged
mjbvz merged 3 commits into from
Nov 16, 2023
Merged

Add typescript.implementationsCodeLens.showOnInterfaceMethods setting (#136282) #198419

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
baa6b98
Add `typescript.implementationsCodeLens.showOnInterfaceMethods` setti…
gjsjohnmurray Nov 16, 2023
76ae941
Update codelenses when `typescript.referencesCodeLens.showOnAllFuncti…
gjsjohnmurray Nov 16, 2023
186754c
Improve handling of disposables
gjsjohnmurray Nov 16, 2023
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
6 changes: 6 additions & 0 deletions extensions/typescript-language-features/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@
"description": "%typescript.implementationsCodeLens.enabled%",
"scope": "window"
},
"typescript.implementationsCodeLens.showOnInterfaceMethods": {
"type": "boolean",
"default": false,
"description": "%typescript.implementationsCodeLens.showOnInterfaceMethods%",
"scope": "window"
},
"typescript.tsserver.enableTracing": {
"type": "boolean",
"default": false,
Expand Down
1 change: 1 addition & 0 deletions extensions/typescript-language-features/package.nls.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"typescript.referencesCodeLens.enabled": "Enable/disable references CodeLens in TypeScript files.",
"typescript.referencesCodeLens.showOnAllFunctions": "Enable/disable references CodeLens on all functions in TypeScript files.",
"typescript.implementationsCodeLens.enabled": "Enable/disable implementations CodeLens. This CodeLens shows the implementers of an interface.",
"typescript.implementationsCodeLens.showOnInterfaceMethods": "Enable/disable implementations CodeLens on interface methods.",
"typescript.openTsServerLog.title": "Open TS Server log",
"typescript.restartTsServer": "Restart TS Server",
"typescript.selectTypeScriptVersion.title": "Select TypeScript Version...",
Expand Down
6 changes: 6 additions & 0 deletions ...nsions/typescript-language-features/src/languageFeatures/codeLens/baseCodeLensProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ export class ReferencesCodeLens extends vscode.CodeLens {
}

export abstract class TypeScriptBaseCodeLensProvider implements vscode.CodeLensProvider<ReferencesCodeLens> {
protected readonly subscriptions: vscode.Disposable[] = [];
protected changeEmitter = new vscode.EventEmitter<void>();
Copy link
Collaborator

@mjbvz mjbvz Nov 16, 2023

Choose a reason for hiding this comment

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

This emitter should also get disposed of. You can inherit from Disposable too to make managing the disposables a little cleaner

Copy link
Contributor Author

@gjsjohnmurray gjsjohnmurray Nov 16, 2023

Choose a reason for hiding this comment

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

Thanks for the feedback.

This emitter should also get disposed of.

I derived the code from here:

vscode/extensions/npm/src/npmScriptLens.ts

Lines 32 to 35 in 0d552b5

export class NpmScriptLensProvider implements CodeLensProvider, Disposable {
private lensLocation = getFreshLensLocation();
private changeEmitter = new EventEmitter<void>();
private subscriptions: Disposable[] = [];

Does that code also fail to dispose its emitter, or did I overlook how/where it does the right thing?

You can inherit from Disposable too to make managing the disposables a little cleaner

By simply appending , Disposable to what TypeScriptBaseCodeLensProvider implements? Please help me understand what that will achieves.

Copy link
Collaborator

@mjbvz mjbvz Nov 16, 2023

Choose a reason for hiding this comment

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

Yes it's likely missing there too

If you inherit from Disposable, then you don't have to implement dispose() yourself. Instead you can just call this._register with every disposable value

Copy link
Contributor Author

@gjsjohnmurray gjsjohnmurray Nov 16, 2023

Choose a reason for hiding this comment

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

I have pushed what I think you meant.

public onDidChangeCodeLenses = this.changeEmitter.event;

public static readonly cancelledCommand: vscode.Command = {
// Cancellation is not an error. Just show nothing until we can properly re-compute the code lens
Expand All @@ -39,6 +42,9 @@ export abstract class TypeScriptBaseCodeLensProvider implements vscode.CodeLensP
private readonly cachedResponse: CachedResponse<Proto.NavTreeResponse>
) { }

public dispose() {
this.subscriptions.forEach(s => s.dispose());
}

async provideCodeLenses(document: vscode.TextDocument, token: vscode.CancellationToken): Promise<ReferencesCodeLens[]> {
const filepath = this.client.toOpenTsFilePath(document);
Expand Down
21 changes: 19 additions & 2 deletions ...ons/typescript-language-features/src/languageFeatures/codeLens/implementationsCodeLens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,20 @@ import { ExecutionTarget } from '../../tsServer/server';


export default class TypeScriptImplementationsCodeLensProvider extends TypeScriptBaseCodeLensProvider {
public constructor(
client: ITypeScriptServiceClient,
protected _cachedResponse: CachedResponse<Proto.NavTreeResponse>,
private readonly language: LanguageDescription
) {
super(client, _cachedResponse);
this.subscriptions.push(
vscode.workspace.onDidChangeConfiguration(evt => {
if (evt.affectsConfiguration(`${language.id}.implementationsCodeLens.showOnInterfaceMethods`)) {
this.changeEmitter.fire();
}
})
);
}

public async resolveCodeLens(
codeLens: ReferencesCodeLens,
Expand Down Expand Up @@ -71,8 +85,11 @@ export default class TypeScriptImplementationsCodeLensProvider extends TypeScrip
protected extractSymbol(
document: vscode.TextDocument,
item: Proto.NavigationTree,
_parent: Proto.NavigationTree | undefined
parent: Proto.NavigationTree | undefined
): vscode.Range | undefined {
if (item.kind === PConst.Kind.method && parent && parent.kind === PConst.Kind.interface && vscode.workspace.getConfiguration(this.language.id).get<boolean>('implementationsCodeLens.showOnInterfaceMethods')) {
return getSymbolRange(document, item);
}
switch (item.kind) {
case PConst.Kind.interface:
return getSymbolRange(document, item);
Expand Down Expand Up @@ -102,6 +119,6 @@ export function register(
requireSomeCapability(client, ClientCapability.Semantic),
], () => {
return vscode.languages.registerCodeLensProvider(selector.semantic,
new TypeScriptImplementationsCodeLensProvider(client, cachedResponse));
new TypeScriptImplementationsCodeLensProvider(client, cachedResponse, language));
});
}
7 changes: 7 additions & 0 deletions extensions/typescript-language-features/src/languageFeatures/codeLens/referencesCodeLens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ export class TypeScriptReferencesCodeLensProvider extends TypeScriptBaseCodeLens
private readonly language: LanguageDescription
) {
super(client, _cachedResponse);
this.subscriptions.push(
vscode.workspace.onDidChangeConfiguration(evt => {
if (evt.affectsConfiguration(`${language.id}.referencesCodeLens.showOnAllFunctions`)) {
this.changeEmitter.fire();
}
})
);
}

public async resolveCodeLens(codeLens: ReferencesCodeLens, token: vscode.CancellationToken): Promise<vscode.CodeLens> {
Expand Down