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

Jump to bottom

Organize Static Features #1073

Merged
merged 2 commits into from
Apr 25, 2022
Merged

Organize Static Features #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9fb8ff
Organize Static Features
jpogran Apr 25, 2022
b213869
hook up manifest
jpogran Apr 25, 2022
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
11 changes: 5 additions & 6 deletions src/clientHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import {
State,
} from 'vscode-languageclient/node';
import { ServerPath } from './serverPath';
import { ShowReferencesFeature } from './showReferences';
import { TelemetryFeature } from './telemetry';
import { ShowReferencesFeature } from './features/showReferences';
import { TelemetryFeature } from './features/telemetry';
import { config } from './vscodeUtils';
import { CustomSemanticTokens } from './semanticTokens';
import { CustomSemanticTokens, PartialManifest } from './features/semanticTokens';

export interface TerraformLanguageClient {
commandPrefix: string;
Expand All @@ -37,6 +37,7 @@ export class ClientHandler {
private lsPath: ServerPath,
private outputChannel: vscode.OutputChannel,
private reporter: TelemetryReporter,
private manifest: PartialManifest,
Copy link
Member

Choose a reason for hiding this comment

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

Since this is now used outside the scope of semantic tokens I wonder if it's worth calling it e.g. ManifestWithSemanticTokens? or making a copy of that interface here which can be expanded and used elsewhere later - presumably one that is compatible with PartialManifest within semanticTokens.

I suppose there are other places where we access the manifest for some other properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would agree if we didn't have more work to break up the dependencies inside ClientHandler.

ClientHandler only needs to know about this because of the StaticHandler registrations. Building up a StaticHandler should be done outside ClientHandler and passed in rather than passing in all information required to build several StaticHandlers.

I could do this here now, or do that work with the next planned PR for refactoring ClientHandler. I prefer to make those changes in the next PR.

) {
if (lsPath.hasCustomBinPath()) {
this.reporter.sendTelemetryEvent('usePathToBinary');
Expand Down Expand Up @@ -90,9 +91,7 @@ export class ClientHandler {
const id = `terraform`;
const client = new LanguageClient(id, serverOptions, clientOptions);

client.registerFeature(
new CustomSemanticTokens(client, this.extSemanticTokenTypes, this.extSemanticTokenModifiers),
);
client.registerFeature(new CustomSemanticTokens(client, this.manifest));

const codeLensReferenceCount = config('terraform').get<boolean>('codelens.referenceCount');
if (codeLensReferenceCount) {
Expand Down
30 changes: 1 addition & 29 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
}

const lsPath = new ServerPath(context);
clientHandler = new ClientHandler(lsPath, outputChannel, reporter);
clientHandler.extSemanticTokenTypes = tokenTypesFromExtManifest(manifest);
clientHandler.extSemanticTokenModifiers = tokenModifiersFromExtManifest(manifest);
clientHandler = new ClientHandler(lsPath, outputChannel, reporter, manifest);

// get rid of pre-2.0.0 settings
if (config('terraform').has('languageServer.enabled')) {
Expand Down Expand Up @@ -314,32 +312,6 @@ async function terraformCommand(command: string, languageServerExec = true): Pro
}
}

interface PartialManifest {
contributes: {
semanticTokenTypes?: ObjectWithId[];
semanticTokenModifiers?: ObjectWithId[];
};
}

interface ObjectWithId {
id: string;
}

function tokenTypesFromExtManifest(manifest: PartialManifest): string[] {
if (!manifest.contributes.semanticTokenTypes) {
return [];
}
return manifest.contributes.semanticTokenTypes.map((token: ObjectWithId) => token.id);
}

function tokenModifiersFromExtManifest(manifest: PartialManifest): string[] {
if (!manifest.contributes.semanticTokenModifiers) {
return [];
}

return manifest.contributes.semanticTokenModifiers.map((modifier: ObjectWithId) => modifier.id);
}

function enabled(): boolean {
return config('terraform').get('languageServer.external', false);
}
55 changes: 55 additions & 0 deletions src/features/semanticTokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { BaseLanguageClient, ClientCapabilities, ServerCapabilities, StaticFeature } from 'vscode-languageclient';

export interface PartialManifest {
contributes: {
semanticTokenTypes?: ObjectWithId[];
semanticTokenModifiers?: ObjectWithId[];
};
}

interface ObjectWithId {
id: string;
}

export class CustomSemanticTokens implements StaticFeature {
constructor(private _client: BaseLanguageClient, private manifest: PartialManifest) {}

public fillClientCapabilities(capabilities: ClientCapabilities): void {
if (!capabilities.textDocument || !capabilities.textDocument.semanticTokens) {
return;
}

const extSemanticTokenTypes = this.tokenTypesFromExtManifest(this.manifest);
const extSemanticTokenModifiers = this.tokenModifiersFromExtManifest(this.manifest);

const tokenTypes = capabilities.textDocument.semanticTokens.tokenTypes;
capabilities.textDocument.semanticTokens.tokenTypes = tokenTypes.concat(extSemanticTokenTypes);

const tokenModifiers = capabilities.textDocument.semanticTokens.tokenModifiers;
capabilities.textDocument.semanticTokens.tokenModifiers = tokenModifiers.concat(extSemanticTokenModifiers);
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
public initialize(capabilities: ServerCapabilities): void {
return;
}

public dispose(): void {
return;
}

tokenTypesFromExtManifest(manifest: PartialManifest): string[] {
if (!manifest.contributes.semanticTokenTypes) {
return [];
}
return manifest.contributes.semanticTokenTypes.map((token: ObjectWithId) => token.id);
}

tokenModifiersFromExtManifest(manifest: PartialManifest): string[] {
if (!manifest.contributes.semanticTokenModifiers) {
return [];
}

return manifest.contributes.semanticTokenModifiers.map((modifier: ObjectWithId) => modifier.id);
}
}
0 src/showReferences.ts → src/features/showReferences.ts
View file
Open in desktop
File renamed without changes.
0 src/telemetry.ts → src/features/telemetry.ts
View file
Open in desktop
File renamed without changes.
0 src/types.ts → src/features/types.ts
View file
Open in desktop
File renamed without changes.
30 changes: 0 additions & 30 deletions src/semanticTokens.ts
View file
Open in desktop

This file was deleted.