Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add a disableLanguageService setting.#2392

Merged
sean-mcmanus merged 8 commits into
mastermicrosoft/vscode-cpptools:masterfrom
seanmcm/disableLanguageServiceSettingmicrosoft/vscode-cpptools:seanmcm/disableLanguageServiceSettingCopy head branch name to clipboard
Aug 16, 2018
Merged

Add a disableLanguageService setting.#2392
sean-mcmanus merged 8 commits into
mastermicrosoft/vscode-cpptools:masterfrom
seanmcm/disableLanguageServiceSettingmicrosoft/vscode-cpptools:seanmcm/disableLanguageServiceSettingCopy head branch name to clipboard

Conversation

@sean-mcmanus

Copy link
Copy Markdown
Contributor

Fix for #785 .

Comment thread Extension/package.json Outdated
"description": "The value to use for the system include path. If set, it overrides the system include path acquired via \"compilerPath\" and \"compileCommands\" settings.",
"scope": "resource"
},
"C_Cpp.disableLanguageService": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was planning to expose this differently, via C_Cpp.intelliSenseEngine (until we refactor settings and have intelliSense.enabled).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You mean a "Disabled" setting for intelliSenseEngine? I could do that.

Comment thread Extension/src/main.ts Outdated
}

async function finalizeExtensionActivation(): Promise<void> {
if (vscode.workspace.getConfiguration("C_Cpp", null).get<boolean>("disableLanguageService")) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like this feature should be a little more involved than just this. What happens to all the commands that are registered in package.json? Are we able to remove the context menu items?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, all the package.json and context menu stuff remains activated, but they don't do anything. I was thinking that didn't matter, and/or wait till people complain about that. I don't think there's a way to remove the package.json stuff without modifying the file and doing a Reload Window and/or splitting the extension into multiple ones and disabling the separate sub-extension.

Comment thread Extension/src/main.ts
}

async function finalizeExtensionActivation(): Promise<void> {
if (vscode.workspace.getConfiguration("C_Cpp", null).get<string>("intelliSenseEngine") === "Disabled") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There doesn't seem to be a way to re-enable the language server after it's been disabled. It also appears that disabling the language server after it has already been loaded will have no effect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that was "by design" :)...my assumption was that users who wanted to use this setting wouldn't mind having to Reload Window for it to work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added the reload prompt.

Comment thread Extension/src/commands.ts Outdated
// Used to save/re-execute commands used before the extension has activated (e.g. delayed by dependency downloading).
private delayedCommandsToExecute: Set<string>;
private tempCommands: vscode.Disposable[]; // Need to save this to unregister/dispose the temporary commands.
private isDisabled: boolean;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this is just about disabling the language server, I think this variable name should reflect that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Comment thread Extension/src/commands.ts Outdated
public registerTempCommand(command: string): void {
this.tempCommands.push(vscode.commands.registerCommand(command, () => {
if (this.isDisabled) {
vscode.window.showInformationMessage("The C/C++ command is disabled due to C_Cpp.intelliSenseEngine set to Disabled.");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"This command is disabled when "C_Cpp.intelliSenseEngine" is set to "Disabled""

Comment thread Extension/src/main.ts Outdated
util.promptForReloadWindowDueToSettingsChange();
}
}));
Telemetry.logLanguageServerEvent("intelliSenseEngine disabled");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we don't have any events with spaces in them that I am aware of and the normal telemetry should cover this already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah.

Comment thread Extension/src/main.ts Outdated
Telemetry.logLanguageServerEvent("intelliSenseEngine disabled");
return;
} else {
disposables.push(vscode.workspace.onDidChangeConfiguration(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

since the "true" case does an early return, you don't need to put this part in an "else" block.

@sean-mcmanus sean-mcmanus added this to the August 2018 milestone Aug 16, 2018
@sean-mcmanus sean-mcmanus removed this from the August 2018 milestone Aug 16, 2018
@sean-mcmanus sean-mcmanus merged commit 3790fa9 into master Aug 16, 2018
@sean-mcmanus sean-mcmanus deleted the seanmcm/disableLanguageServiceSetting branch August 16, 2018 16:12
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.