Skip to content

Commit

Permalink
Deprecate the 'Build Workspace Symbols' command.
Browse files Browse the repository at this point in the history 
- Fixes microsoft#2267
- new Python Language Server rebuilds symbols in background
- remove registrations/command functionality
- add deprecation popup
- cleanup prior deprecated code
  • Loading branch information
@d3r3kk
d3r3kk committed Aug 21, 2018
1 parent 6707eaa commit e429a8e
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 84 deletions.
1 change: 1 addition & 0 deletions news/3 Code Health/2267.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate command 'Python: Build Workspace Symbols'.
1 change: 0 additions & 1 deletion src/client/common/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export namespace Commands {
export const Refactor_Extract_Variable = 'python.refactorExtractVariable';
export const Refaactor_Extract_Method = 'python.refactorExtractMethod';
export const Update_SparkLibrary = 'python.updateSparkLibrary';
export const Build_Workspace_Symbols = 'python.buildWorkspaceSymbols';
export const Start_REPL = 'python.startREPL';
export const Create_Terminal = 'python.createTerminal';
export const Set_Linter = 'python.setLinter';
Expand Down
8 changes: 7 additions & 1 deletion src/client/common/featureDeprecationManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ const deprecatedFeatures: deprecatedFeatureInfo[] = [
message: 'The setting \'python.linting.lintOnTextChange\' is deprecated, please enable \'python.linting.lintOnSave\' and \'files.autoSave\'.',
moreInfoUrl: 'https://github.com/Microsoft/vscode-python/issues/313',
setting: { setting: 'linting.lintOnTextChange', values: ['true', true] }
},
{
doNotDisplayPromptStateKey: 'SHOW_DEPRECATED_FEATURE_PROMPT_BUILD_WORKSPACE_SYMBOLS',
message: 'The command \'Python: Build Workspace Symbols\' is deprecated as the new Python Language Server builds symbols in the workspace in the background.',
moreInfoUrl: 'https://github.com/Microsoft/vscode-python/issues/2267#issuecomment-408996859',
commands: ['python.buildWorkspaceSymbols']
}
];

Expand All @@ -39,7 +45,7 @@ export interface IFeatureDeprecationManager extends Disposable {

export class FeatureDeprecationManager implements IFeatureDeprecationManager {
private disposables: Disposable[] = [];
constructor(private persistentStateFactory: IPersistentStateFactory, private jupyterExtensionInstalled: boolean) { }
constructor(private persistentStateFactory: IPersistentStateFactory) { }
public dispose() {
this.disposables.forEach(disposable => disposable.dispose());
}
Expand Down
10 changes: 6 additions & 4 deletions src/client/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ import { registerTypes as platformRegisterTypes } from './common/platform/servic
import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry';
import { registerTypes as commonRegisterTypes } from './common/serviceRegistry';
import { ITerminalHelper } from './common/terminal/types';
import { GLOBAL_MEMENTO, IConfigurationService, IDisposableRegistry,
import {
GLOBAL_MEMENTO, IConfigurationService, IDisposableRegistry,
IExtensionContext, ILogger, IMemento, IOutputChannel,
IPersistentStateFactory, WORKSPACE_MEMENTO } from './common/types';
IPersistentStateFactory, WORKSPACE_MEMENTO
} from './common/types';
import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
import { AttachRequestArguments, LaunchRequestArguments } from './debugger/Common/Contracts';
import { BaseConfigurationProvider } from './debugger/configProviders/baseProvider';
Expand Down Expand Up @@ -142,9 +144,9 @@ export async function activate(context: ExtensionContext) {
context.subscriptions.push(languages.registerOnTypeFormattingEditProvider(PYTHON, new OnEnterFormatter(), '\n'));

const persistentStateFactory = serviceManager.get<IPersistentStateFactory>(IPersistentStateFactory);
const deprecationMgr = new FeatureDeprecationManager(persistentStateFactory, !!jupyterExtension);
const deprecationMgr = new FeatureDeprecationManager(persistentStateFactory);
deprecationMgr.initialize();
context.subscriptions.push(new FeatureDeprecationManager(persistentStateFactory, !!jupyterExtension));
context.subscriptions.push(new FeatureDeprecationManager(persistentStateFactory));

context.subscriptions.push(serviceContainer.get<IInterpreterSelector>(IInterpreterSelector));
context.subscriptions.push(activateUpdateSparkLibraryProvider());
Expand Down
68 changes: 4 additions & 64 deletions src/client/workspaceSymbols/main.ts
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { CancellationToken, commands, Disposable, languages, OutputChannel, workspace } from 'vscode';
import { Commands, STANDARD_OUTPUT_CHANNEL } from '../common/constants';
import { isNotInstalledError } from '../common/helpers';
import { Disposable, languages, OutputChannel, workspace } from 'vscode';
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
import { IProcessServiceFactory } from '../common/process/types';
import { IInstaller, InstallerResponse, IOutputChannel, Product } from '../common/types';
import { fsExistsAsync } from '../common/utils';
import { IOutputChannel } from '../common/types';
import { IServiceContainer } from '../ioc/types';
import { Generator } from './generator';
import { WorkspaceSymbolProvider } from './provider';

const MAX_NUMBER_OF_ATTEMPTS_TO_INSTALL_AND_BUILD = 2;

export class WorkspaceSymbols implements Disposable {
private disposables: Disposable[];
private generators: Generator[] = [];
Expand All @@ -18,9 +14,8 @@ export class WorkspaceSymbols implements Disposable {
this.outputChannel = this.serviceContainer.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
this.disposables = [];
this.disposables.push(this.outputChannel);
this.registerCommands();
this.initializeGenerators();
languages.registerWorkspaceSymbolProvider(new WorkspaceSymbolProvider(this.generators, this.outputChannel));
languages.registerWorkspaceSymbolProvider(new WorkspaceSymbolProvider(this.generators));
this.disposables.push(workspace.onDidChangeWorkspaceFolders(() => this.initializeGenerators()));
}
public dispose() {
Expand All @@ -39,59 +34,4 @@ export class WorkspaceSymbols implements Disposable {
});
}
}
private registerCommands() {
this.disposables.push(commands.registerCommand(Commands.Build_Workspace_Symbols, async (rebuild: boolean = true, token?: CancellationToken) => {
const promises = this.buildWorkspaceSymbols(rebuild, token);
return Promise.all(promises);
}));
}
// tslint:disable-next-line:no-any
private buildWorkspaceSymbols(rebuild: boolean = true, token?: CancellationToken): Promise<any>[] {
if (token && token.isCancellationRequested) {
return [];
}
if (this.generators.length === 0) {
return [];
}

let promptPromise: Promise<InstallerResponse>;
let promptResponse: InstallerResponse;
return this.generators.map(async generator => {
if (!generator.enabled) {
return;
}
const exists = await fsExistsAsync(generator.tagFilePath);
// If file doesn't exist, then run the ctag generator,
// or check if required to rebuild.
if (!rebuild && exists) {
return;
}
for (let counter = 0; counter < MAX_NUMBER_OF_ATTEMPTS_TO_INSTALL_AND_BUILD; counter += 1) {
try {
await generator.generateWorkspaceTags();
return;
} catch (error) {
if (!isNotInstalledError(error)) {
this.outputChannel.show();
return;
}
}
if (!token || token.isCancellationRequested) {
return;
}
// Display prompt once for all workspaces.
if (promptPromise) {
promptResponse = await promptPromise;
continue;
} else {
const installer = this.serviceContainer.get<IInstaller>(IInstaller);
promptPromise = installer.promptToInstall(Product.ctags, workspace.workspaceFolders![0]!.uri);
promptResponse = await promptPromise;
}
if (promptResponse !== InstallerResponse.Installed || (!token || token.isCancellationRequested)) {
return;
}
}
});
}
}
12 changes: 4 additions & 8 deletions src/client/workspaceSymbols/provider.ts
View file
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
'use strict';

import * as _ from 'lodash';
import * as vscode from 'vscode';
import { Commands } from '../common/constants';
import { fsExistsAsync } from '../common/utils';
import { captureTelemetry } from '../telemetry';
import { WORKSPACE_SYMBOLS_GO_TO } from '../telemetry/constants';
import { Generator } from './generator';
import { parseTags } from './parser';

export class WorkspaceSymbolProvider implements vscode.WorkspaceSymbolProvider {
public constructor(private tagGenerators: Generator[], private outputChannel: vscode.OutputChannel) {
public constructor(private tagGenerators: Generator[]) {
}

@captureTelemetry(WORKSPACE_SYMBOLS_GO_TO)
public async provideWorkspaceSymbols(query: string, token: vscode.CancellationToken): Promise<vscode.SymbolInformation[]> {
if (this.tagGenerators.length === 0) {
return [];
}
const generatorsWithTagFiles = await Promise.all(this.tagGenerators.map(generator => fsExistsAsync(generator.tagFilePath)));
if (generatorsWithTagFiles.filter(exists => exists).length !== this.tagGenerators.length) {
await vscode.commands.executeCommand(Commands.Build_Workspace_Symbols, true, token);
}

const generators = await Promise.all(this.tagGenerators.map(async generator => {
const tagFileExists = await fsExistsAsync(generator.tagFilePath);
return tagFileExists ? generator : undefined;
Expand All @@ -30,7 +26,7 @@ export class WorkspaceSymbolProvider implements vscode.WorkspaceSymbolProvider {
.filter(generator => generator !== undefined && generator.enabled)
.map(async generator => {
// load tags
const items = await parseTags(generator.workspaceFolder.fsPath, generator.tagFilePath, query, token);
const items = await parseTags(generator!.workspaceFolder.fsPath, generator!.tagFilePath, query, token);
if (!Array.isArray(items)) {
return [];
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/workspaceSymbols/multiroot.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ suite('Multiroot Workspace Symbols', () => {
await updateSetting('workspaceSymbols.enabled', false, childWorkspaceUri, ConfigurationTarget.WorkspaceFolder);

let generator = new Generator(childWorkspaceUri, outputChannel, processServiceFactory);
let provider = new WorkspaceSymbolProvider([generator], outputChannel);
let provider = new WorkspaceSymbolProvider([generator]);
let symbols = await provider.provideWorkspaceSymbols('', new CancellationTokenSource().token);
assert.equal(symbols.length, 0, 'Symbols returned even when workspace symbols are turned off');
generator.dispose();

await updateSetting('workspaceSymbols.enabled', true, childWorkspaceUri, ConfigurationTarget.WorkspaceFolder);

generator = new Generator(childWorkspaceUri, outputChannel, processServiceFactory);
provider = new WorkspaceSymbolProvider([generator], outputChannel);
provider = new WorkspaceSymbolProvider([generator]);
symbols = await provider.provideWorkspaceSymbols('', new CancellationTokenSource().token);
assert.notEqual(symbols.length, 0, 'Symbols should be returned when workspace symbols are turned on');
});
Expand All @@ -70,7 +70,7 @@ suite('Multiroot Workspace Symbols', () => {
const generators = [
new Generator(childWorkspaceUri, outputChannel, processServiceFactory),
new Generator(workspace2Uri, outputChannel, processServiceFactory)];
const provider = new WorkspaceSymbolProvider(generators, outputChannel);
const provider = new WorkspaceSymbolProvider(generators);
const symbols = await provider.provideWorkspaceSymbols('meth1Of', new CancellationTokenSource().token);

assert.equal(symbols.length, 2, 'Incorrect number of symbols returned');
Expand Down
6 changes: 3 additions & 3 deletions src/test/workspaceSymbols/standard.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ suite('Workspace Symbols', () => {
settings.workspaceSymbols!.tagFilePath = path.join(workspaceUri.fsPath, '.vscode', 'tags');

let generator = new Generator(workspaceUri, outputChannel, processServiceFactory);
let provider = new WorkspaceSymbolProvider([generator], outputChannel);
let provider = new WorkspaceSymbolProvider([generator]);
let symbols = await provider.provideWorkspaceSymbols('', new CancellationTokenSource().token);
assert.equal(symbols.length, 0, 'Symbols returned even when workspace symbols are turned off');
generator.dispose();
Expand All @@ -58,7 +58,7 @@ suite('Workspace Symbols', () => {
settings.workspaceSymbols!.tagFilePath = path.join(workspaceUri.fsPath, '.vscode', 'tags');

generator = new Generator(workspaceUri, outputChannel, processServiceFactory);
provider = new WorkspaceSymbolProvider([generator], outputChannel);
provider = new WorkspaceSymbolProvider([generator]);
symbols = await provider.provideWorkspaceSymbols('', new CancellationTokenSource().token);
assert.notEqual(symbols.length, 0, 'Symbols should be returned when workspace symbols are turned on');
});
Expand All @@ -73,7 +73,7 @@ suite('Workspace Symbols', () => {
settings.workspaceSymbols!.tagFilePath = path.join(workspaceUri.fsPath, '.vscode', 'tags');

const generators = [new Generator(workspaceUri, outputChannel, processServiceFactory)];
const provider = new WorkspaceSymbolProvider(generators, outputChannel);
const provider = new WorkspaceSymbolProvider(generators);
const symbols = await provider.provideWorkspaceSymbols('meth1Of', new CancellationTokenSource().token);

assert.equal(symbols.length >= 2, true, 'Incorrect number of symbols returned');
Expand Down

0 comments on commit e429a8e

Please sign in to comment.