Skip to content

fix #1229 disable installer prompts via do not show again option #1243

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
DonJayamanne merged 4 commits into from
Sep 27, 2017
Merged

fix #1229 disable installer prompts via do not show again option #1243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7af49ef
disable installer prompts via do not show again option
DonJayamanne Sep 26, 2017
774fd3b
removed unwanted resolved promise
DonJayamanne Sep 26, 2017
8f40e39
updated message
DonJayamanne Sep 27, 2017
31feebc
removed redundant semicolon
DonJayamanne Sep 27, 2017
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
19 changes: 19 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,25 @@
"type": "string"
}
},
"python.disablePromptForFeatures": {
"type": "array",
"default": [],
"description": "Do not display a prompt to install these features",
"items": {
"type": "string",
"default": "pylint",
"description": "Feature",
"enum": [
"flake8",
"mypy",
"pep8",
"pylama",
"prospector",
"pydocstyle",
"pylint"
]
}
},
"python.linting.enabled": {
"type": "boolean",
"default": true,
Expand Down
6 changes: 5 additions & 1 deletion src/client/common/configSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface IPythonSettings {
sortImports: ISortImportSettings;
workspaceSymbols: IWorkspaceSymbolSettings;
envFile: string;
disablePromptForFeatures: string[];
}
export interface ISortImportSettings {
path: string;
Expand Down Expand Up @@ -166,6 +167,8 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
this.devOptions = systemVariables.resolveAny(pythonSettings.get<any[]>('devOptions'))!;
this.devOptions = Array.isArray(this.devOptions) ? this.devOptions : [];
let lintingSettings = systemVariables.resolveAny(pythonSettings.get<ILintingSettings>('linting'))!;
this.disablePromptForFeatures = pythonSettings.get<string[]>('disablePromptForFeatures')!;
this.disablePromptForFeatures = Array.isArray(this.disablePromptForFeatures) ? this.disablePromptForFeatures : [];
if (this.linting) {
Object.assign<ILintingSettings, ILintingSettings>(this.linting, lintingSettings);
}
Expand Down Expand Up @@ -353,6 +356,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
}
public jediPath: string;
public envFile: string;
public disablePromptForFeatures: string[];
public venvPath: string;
public devOptions: string[];
public linting: ILintingSettings;
Expand Down Expand Up @@ -422,4 +426,4 @@ function isValidPythonPath(pythonPath: string): boolean {
catch (ex) {
return false;
}
}
}
76 changes: 46 additions & 30 deletions src/client/common/installer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as vscode from 'vscode';
import * as settings from './configSettings';
import { createDeferred, isNotInstalledError } from './helpers';
import { execPythonFile, getFullyQualifiedPythonInterpreterPath } from './utils';
import * as os from 'os';
import { isNotInstalledError } from './helpers';
import { execPythonFile, getFullyQualifiedPythonInterpreterPath } from './utils';
import { Documentation } from './constants';

export enum Product {
Expand Down Expand Up @@ -86,9 +86,6 @@ export const Linters: Product[] = [
Product.pydocstyle
];

const Formatters: Product[] = [Product.autopep8, Product.yapf];
const TestFrameworks: Product[] = [Product.pytest, Product.nosetest, Product.unittest];

const ProductNames = new Map<Product, string>();
ProductNames.set(Product.autopep8, 'autopep8');
ProductNames.set(Product.flake8, 'flake8');
Expand Down Expand Up @@ -146,9 +143,9 @@ ProductTypes.set(Product.yapf, ProductType.Formatter);
ProductTypes.set(Product.rope, ProductType.RefactoringLibrary);

export class Installer implements vscode.Disposable {
private static terminal: vscode.Terminal;
private static terminal: vscode.Terminal | undefined | null;
Copy link
Contributor

@brettcannon brettcannon Sep 27, 2017

Choose a reason for hiding this comment

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

Out of curiosity, what does this pattern do?

Copy link
Owner Author

@DonJayamanne DonJayamanne Sep 27, 2017

Choose a reason for hiding this comment

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

Allow it to be nullable (i'm slowly introducing some strict checks.
I don't want to turn this on globally, else we'll have 100s of errors.
New --strict master option

Copy link
Contributor

@brettcannon brettcannon Sep 27, 2017

Choose a reason for hiding this comment

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

Is the errors from dependencies or just old code that needs an update?

private disposables: vscode.Disposable[] = [];
constructor(private outputChannel: vscode.OutputChannel = null) {
constructor(private outputChannel?: vscode.OutputChannel) {
this.disposables.push(vscode.window.onDidCloseTerminal(term => {
if (term === Installer.terminal) {
Installer.terminal = null;
Expand All @@ -158,46 +155,65 @@ export class Installer implements vscode.Disposable {
public dispose() {
this.disposables.forEach(d => d.dispose());
}
public shouldDisplayPrompt(product: Product) {
const productName = ProductNames.get(product)!;
return settings.PythonSettings.getInstance().disablePromptForFeatures.indexOf(productName) === -1;
}

promptToInstall(product: Product): Thenable<any> {
const productType = ProductTypes.get(product);
async promptToInstall(product: Product) {
const productType = ProductTypes.get(product)!;
const productTypeName = ProductTypeNames.get(productType);
const productName = ProductNames.get(product);
const productName = ProductNames.get(product)!;

if (!this.shouldDisplayPrompt(product)) {
const message = `${productTypeName} '${productName}' not installed.`;
if (this.outputChannel) {
this.outputChannel.appendLine(message);
}
else {
console.warn(message);
}
return;
}

const installOption = 'Install ' + productName;
const disableOption = 'Disable ' + productTypeName;
const disableOptionGlobally = `Disable ${productTypeName} globally`;
const dontShowAgain = `Don't show this prompt again`;
const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8';
const useOtherFormatter = `Use '${alternateFormatter}' formatter`;
const options = [];
options.push(installOption);
if (productType === ProductType.Formatter) {
options.push(...[installOption, useOtherFormatter]);
options.push(...[useOtherFormatter]);
}
if (SettingToDisableProduct.has(product)) {
options.push(...[disableOption, disableOptionGlobally]);
options.push(...[disableOption, dontShowAgain]);
}
return vscode.window.showErrorMessage(`${productTypeName} ${productName} is not installed`, ...options).then(item => {
switch (item) {
case installOption: {
return this.install(product);
}
case disableOptionGlobally:
case disableOption: {
const global = item === disableOptionGlobally;
if (Linters.indexOf(product) >= 0) {
return disableLinter(product, global);
return disableLinter(product);
}
else {
const pythonConfig = vscode.workspace.getConfiguration('python');
const settingToDisable = SettingToDisableProduct.get(product);
return pythonConfig.update(settingToDisable, false, global);
const settingToDisable = SettingToDisableProduct.get(product)!;
return pythonConfig.update(settingToDisable, false);
}
}
case useOtherFormatter: {
const pythonConfig = vscode.workspace.getConfiguration('python');
return pythonConfig.update('formatting.provider', alternateFormatter);
}
case dontShowAgain: {
const pythonConfig = vscode.workspace.getConfiguration('python');
const features = pythonConfig.get('disablePromptForFeatures', [] as string[]);
features.push(productName);
return pythonConfig.update('disablePromptForFeatures', features, true);
}
case 'Help': {
return Promise.resolve();
}
Expand All @@ -215,7 +231,7 @@ export class Installer implements vscode.Disposable {
return Promise.resolve();
}

let installArgs = ProductInstallScripts.get(product);
let installArgs = ProductInstallScripts.get(product)!;
let pipIndex = installArgs.indexOf('pip');
if (pipIndex > 0) {
installArgs = installArgs.slice();
Expand All @@ -228,8 +244,8 @@ export class Installer implements vscode.Disposable {
if (this.outputChannel && installArgs[0] === '-m') {
// Errors are just displayed to the user
this.outputChannel.show();
return execPythonFile(settings.PythonSettings.getInstance().pythonPath, installArgs, vscode.workspace.rootPath, true, (data) => {
this.outputChannel.append(data);
return execPythonFile(settings.PythonSettings.getInstance().pythonPath, installArgs, vscode.workspace.rootPath!, true, (data) => {
this.outputChannel!.append(data);
});
}
else {
Expand All @@ -249,8 +265,8 @@ export class Installer implements vscode.Disposable {
installScript = `${pythonPath} ${installScript}`;
}
}
Installer.terminal.sendText(installScript);
Installer.terminal.show(false);
Installer.terminal!.sendText(installScript);
Installer.terminal!.show(false);
});
}
}
Expand All @@ -266,7 +282,7 @@ export class Installer implements vscode.Disposable {

export function disableLinter(product: Product, global?: boolean) {
const pythonConfig = vscode.workspace.getConfiguration('python');
const settingToDisable = SettingToDisableProduct.get(product);
const settingToDisable = SettingToDisableProduct.get(product)!;
if (vscode.workspace.rootPath) {
return pythonConfig.update(settingToDisable, false, global);
}
Expand All @@ -275,12 +291,12 @@ export function disableLinter(product: Product, global?: boolean) {
}
}

function isProductInstalled(product: Product): Promise<boolean | undefined> {
async function isProductInstalled(product: Product): Promise<boolean | undefined> {
Copy link
Contributor

@brettcannon brettcannon Sep 27, 2017

Choose a reason for hiding this comment

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

To help me learn, what are the rules for when to use null versus undefined?

Copy link
Owner Author

@DonJayamanne DonJayamanne Sep 27, 2017

Choose a reason for hiding this comment

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

lol, undefined is the similar to None in Python, null is more or less an actual value.
Generally in JS people use either, as both evaluate to nothing.
Don't you love JS.

Copy link
Contributor

@brettcannon brettcannon Sep 27, 2017

Choose a reason for hiding this comment

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

Figures. Wasn't sure if there was a move towards just null or something to overlook the weirdness of having both values or to use the ! operator or something.

Copy link
Owner Author

@DonJayamanne DonJayamanne Sep 27, 2017

Choose a reason for hiding this comment

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

e.g. if you declare a variable or call a function without passing in a value, the default value of those variables or parameters is undefined.
null is a value that you'd assign manually to something.

if (!ProductExecutableAndArgs.has(product)) {
return;
}
const prodExec = ProductExecutableAndArgs.get(product);
return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), vscode.workspace.rootPath, false)
const prodExec = ProductExecutableAndArgs.get(product)!;
return execPythonFile(prodExec.executable, prodExec.args.concat(['--version']), vscode.workspace.rootPath!, false)
.then(() => {
return true;
}).catch(reason => {
Expand All @@ -289,6 +305,6 @@ function isProductInstalled(product: Product): Promise<boolean | undefined> {
}

function uninstallproduct(product: Product): Promise<any> {
const uninstallArgs = ProductUninstallScripts.get(product);
return execPythonFile('python', uninstallArgs, vscode.workspace.rootPath, false);
}
const uninstallArgs = ProductUninstallScripts.get(product)!;
return execPythonFile('python', uninstallArgs, vscode.workspace.rootPath!, false);
}
4 changes: 2 additions & 2 deletions src/client/common/systemVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class SystemVariables extends AbstractSystemVariables {

constructor() {
super();
this._workspaceRoot = typeof vscode.workspace.rootPath === 'string' ? vscode.workspace.rootPath : __dirname;;
this._workspaceRoot = typeof vscode.workspace.rootPath === 'string' ? vscode.workspace.rootPath : __dirname;
this._workspaceRootFolderName = Path.basename(this._workspaceRoot);
Object.keys(process.env).forEach(key => {
this[`env:${key}`] = this[`env.${key}`] = process.env[key];
Expand All @@ -155,4 +155,4 @@ export class SystemVariables extends AbstractSystemVariables {
public get workspaceRootFolderName(): string {
return this._workspaceRootFolderName;
}
}
}
2 changes: 1 addition & 1 deletion src/client/interpreter/display/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class InterpreterDisplay implements Disposable {
await Promise.all([interpreterExists, displayName, virtualEnvName])
.then(([interpreterExists, displayName, virtualEnvName]) => {
const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : '';
this.statusBar.text = `${displayName}${dislayNameSuffix}`;;
this.statusBar.text = `${displayName}${dislayNameSuffix}`;

if (!interpreterExists && displayName === defaultDisplayName && interpreters.length > 0) {
this.statusBar.color = 'yellow';
Expand Down
2 changes: 1 addition & 1 deletion src/client/providers/jediProxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ function handleError(source: string, errorMessage: string) {
logger.error(source + ' jediProxy', `Error (${source}) ${errorMessage}`);
}

let spawnRetryAttempts = 0;;
let spawnRetryAttempts = 0;
function spawnProcess(dir: string) {
try {
let environmentVariables = { 'PYTHONUNBUFFERED': '1' };
Expand Down