-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
fix #1229 disable installer prompts via do not show again option #1243
Conversation
src/client/common/configSettings.ts
Outdated
@@ -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 : [];; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the double ;;
for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
src/client/common/installer.ts
Outdated
|
||
const installOption = 'Install ' + productName; | ||
const disableOption = 'Disable ' + productTypeName; | ||
const disableOptionGlobally = `Disable ${productTypeName} globally`; | ||
const dontShowAgain = `Don't show this again`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Don't show this prompt again"?
@@ -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> { |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
…. (#1241) (#1243) Forward-port from 2018.3.1 Fixes #1239
…. (#1241) (#1243) Forward-port from 2018.3.1 Fixes #1239
…. (#1241) (DonJayamanne#1243) Forward-port from 2018.3.1 Fixes #1239
fix #1229