Update naming of console type to be consistent with vscode (#5245)

thomasgassmann · Kartik Raj · commit b25eeed7bb6d · 2019-04-16T10:29:01.000-07:00
* Added new Diagnostic code

* update naming of console type from none to internalConsole

* add news item for change

* Modify intellisense to show internalConsole instead of none

* Modified diagnostic file names
diff --git a/news/3 Code Health/4321.md b/news/3 Code Health/4321.md
@@ -0,0 +1 @@
+Update launch.json to use internalConsole instead of none
diff --git a/package.json b/package.json
@@ -1052,7 +1052,7 @@
                             },
                             "console": {
                                 "enum": [
-                                    "none",
+                                    "internalConsole",
                                     "integratedTerminal",
                                     "externalTerminal"
                                 ],
diff --git a/package.nls.json b/package.nls.json
@@ -175,6 +175,7 @@
     "diagnostics.invalidPythonPathInDebuggerSettings": "You need to select a Python interpreter before you start debugging.\n\nTip: click on \"Select Python Interpreter\" in the status bar.",
     "diagnostics.invalidPythonPathInDebuggerLaunch": "The Python path in your debug configuration is invalid.",
     "diagnostics.invalidDebuggerTypeDiagnostic": "Your launch.json file needs to be updated to change the \"pythonExperimental\" debug configurations to use the \"python\" debugger type, otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?",
+    "diagnostics.consoleTypeDiagnostic": "Your launch.json file needs to be updated to change the console type string from \"none\" to \"internalConsole\", otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?",
     "diagnostics.justMyCodeDiagnostic": "Configuration \"debugStdLib\" in launch.json is no longer supported. It's recommended to replace it with \"justMyCode\", which is the exact opposite of using \"debugStdLib\". Would you like to automatically update your launch.json file to do that?",
     "diagnostics.yesUpdateLaunch": "Yes, update launch.json",
     "diagnostics.bannerLabelNo": "No, I will do it later",
@@ -250,4 +251,4 @@
     "DataScience.liveShareInvalid": "One or more guests in the session do not have the Python [extension](https://marketplace.visualstudio.com/itemdetails?itemName=ms-python.python) installed.\r\nYour Live Share session cannot continue and will be closed.",
     "diagnostics.updateSettings": "Yes, update settings",
     "Common.noIWillDoItLater": "No, I will do it later"
-}
+}
diff --git a/src/client/application/diagnostics/checks/invalidLaunchJsonDebugger.ts b/src/client/application/diagnostics/checks/invalidLaunchJsonDebugger.ts
@@ -21,11 +21,13 @@ const messages = {
     [DiagnosticCodes.InvalidDebuggerTypeDiagnostic]:
         Diagnostics.invalidDebuggerTypeDiagnostic(),
     [DiagnosticCodes.JustMyCodeDiagnostic]:
-        Diagnostics.justMyCodeDiagnostic()
+        Diagnostics.justMyCodeDiagnostic(),
+    [DiagnosticCodes.ConsoleTypeDiagnostic]:
+        Diagnostics.consoleTypeDiagnostic()
 };
 
 export class InvalidLaunchJsonDebuggerDiagnostic extends BaseDiagnostic {
-    constructor(code: DiagnosticCodes.InvalidDebuggerTypeDiagnostic | DiagnosticCodes.JustMyCodeDiagnostic, resource: Resource) {
+    constructor(code: DiagnosticCodes.InvalidDebuggerTypeDiagnostic | DiagnosticCodes.JustMyCodeDiagnostic | DiagnosticCodes.ConsoleTypeDiagnostic, resource: Resource) {
         super(
             code,
             messages[code],
@@ -50,7 +52,7 @@ export class InvalidLaunchJsonDebuggerService extends BaseDiagnosticsService {
         @named(DiagnosticCommandPromptHandlerServiceId)
         private readonly messageService: IDiagnosticHandlerService<MessageCommandPrompt>
     ) {
-        super([DiagnosticCodes.InvalidDebuggerTypeDiagnostic, DiagnosticCodes.JustMyCodeDiagnostic], serviceContainer, disposableRegistry, true);
+        super([DiagnosticCodes.InvalidDebuggerTypeDiagnostic, DiagnosticCodes.JustMyCodeDiagnostic, DiagnosticCodes.ConsoleTypeDiagnostic], serviceContainer, disposableRegistry, true);
     }
     public async diagnose(resource: Resource): Promise<IDiagnostic[]> {
         if (!this.workspaceService.hasWorkspaceFolders) {
@@ -85,6 +87,9 @@ export class InvalidLaunchJsonDebuggerService extends BaseDiagnosticsService {
         if (fileContents.indexOf('"debugStdLib"') > 0) {
             diagnostics.push(new InvalidLaunchJsonDebuggerDiagnostic(DiagnosticCodes.JustMyCodeDiagnostic, resource));
         }
+        if (fileContents.indexOf('"console": "none"') > 0) {
+            diagnostics.push(new InvalidLaunchJsonDebuggerDiagnostic(DiagnosticCodes.ConsoleTypeDiagnostic, resource));
+        }
         return diagnostics;
     }
     private async handleDiagnostic(diagnostic: IDiagnostic): Promise<void> {
@@ -125,6 +130,10 @@ export class InvalidLaunchJsonDebuggerService extends BaseDiagnosticsService {
                 fileContents = this.findAndReplace(fileContents, '"debugStdLib": true', '"justMyCode": false');
                 break;
             }
+            case DiagnosticCodes.ConsoleTypeDiagnostic: {
+                fileContents = this.findAndReplace(fileContents, '"console": "none"', '"console": "internalConsole"');
+                break;
+            }
             default: {
                 return;
             }
diff --git a/src/client/application/diagnostics/constants.ts b/src/client/application/diagnostics/constants.ts
@@ -15,5 +15,6 @@ export enum DiagnosticCodes {
     EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic',
     NoCurrentlySelectedPythonInterpreterDiagnostic = 'InvalidPythonInterpreterDiagnostic',
     LSNotSupportedDiagnostic = 'LSNotSupportedDiagnostic',
-    JustMyCodeDiagnostic = 'JustMyCodeDiagnostic'
+    JustMyCodeDiagnostic = 'JustMyCodeDiagnostic',
+    ConsoleTypeDiagnostic = 'ConsoleTypeDiagnostic'
 }
diff --git a/src/client/application/diagnostics/serviceRegistry.ts b/src/client/application/diagnostics/serviceRegistry.ts
@@ -7,7 +7,7 @@ import { IServiceManager } from '../../ioc/types';
 import { IApplicationDiagnostics } from '../types';
 import { ApplicationDiagnostics } from './applicationDiagnostics';
 import { EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId } from './checks/envPathVariable';
-import { InvalidLaunchJsonDebuggerService, InvalidLaunchJsonDebuggerServiceId } from './checks/invalidDebuggerType';
+import { InvalidLaunchJsonDebuggerService, InvalidLaunchJsonDebuggerServiceId } from './checks/invalidLaunchJsonDebugger';
 import { InvalidPythonPathInDebuggerService, InvalidPythonPathInDebuggerServiceId } from './checks/invalidPythonPathInDebugger';
 import { LSNotSupportedDiagnosticService, LSNotSupportedDiagnosticServiceId } from './checks/lsNotSupported';
 import { InvalidMacPythonInterpreterService, InvalidMacPythonInterpreterServiceId } from './checks/macPythonInterpreter';
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -17,6 +17,7 @@ export namespace Diagnostics {
     export const invalidPythonPathInDebuggerSettings = localize('diagnostics.invalidPythonPathInDebuggerSettings', 'You need to select a Python interpreter before you start debugging.\n\nTip: click on "Select Python Interpreter" in the status bar.');
     export const invalidPythonPathInDebuggerLaunch = localize('diagnostics.invalidPythonPathInDebuggerLaunch', 'The Python path in your debug configuration is invalid.');
     export const invalidDebuggerTypeDiagnostic = localize('diagnostics.invalidDebuggerTypeDiagnostic', 'Your launch.json file needs to be updated to change the "pythonExperimental" debug configurations to use the "python" debugger type, otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?');
+    export const consoleTypeDiagnostic = localize('diagnostics.consoleTypeDiagnostic', 'Your launch.json file needs to be updated to change the console type string from \"none\" to \"internalConsole\", otherwise Python debugging may not work. Would you like to automatically update your launch.json file now?');
     export const justMyCodeDiagnostic = localize('diagnostics.justMyCodeDiagnostic', 'Configuration "debugStdLib" in launch.json is no longer supported. It\'s recommended to replace it with "justMyCode", which is the exact opposite of using "debugStdLib". Would you like to automatically update your launch.json file to do that?');
     export const yesUpdateLaunch = localize('diagnostics.yesUpdateLaunch', 'Yes, update launch.json');
     export const invalidTestSettings = localize('diagnostics.invalidTestSettings', 'Your settings needs to be updated to change the setting "python.unitTest." to "python.testing.", otherwise testing Python code using the extension may not work. Would you like to automatically update your settings now?');
diff --git a/src/client/debugger/debugAdapter/DebugClients/helper.ts b/src/client/debugger/debugAdapter/DebugClients/helper.ts
@@ -31,7 +31,7 @@ export class DebugClientHelper {
             this.envParser.appendPythonPath(env, this.process.env.PYTHONPATH!);
         }
 
-        if (typeof args.console !== 'string' || args.console === 'none') {
+        if (typeof args.console !== 'string' || args.console === 'internalConsole') {
             // For debugging, when not using any terminal, then we need to provide all env variables.
             // As we're spawning the process, we need to ensure all env variables are passed.
             // Including those from the current process (i.e. everything, not just custom vars).
diff --git a/src/client/debugger/extension/configuration/resolvers/launch.ts b/src/client/debugger/extension/configuration/resolvers/launch.ts
@@ -75,7 +75,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
             debugConfiguration.console = 'integratedTerminal';
         }
         // If using a terminal, then never open internal console.
-        if (debugConfiguration.console !== 'none' && !debugConfiguration.internalConsoleOptions) {
+        if (debugConfiguration.console !== 'internalConsole' && !debugConfiguration.internalConsoleOptions) {
             debugConfiguration.internalConsoleOptions = 'neverOpen';
         }
         if (!Array.isArray(debugConfiguration.debugOptions)) {
diff --git a/src/client/debugger/types.ts b/src/client/debugger/types.ts
@@ -76,4 +76,4 @@ export interface AttachRequestArguments extends DebugProtocol.AttachRequestArgum
 // tslint:disable-next-line:interface-name
 export interface DebugConfigurationArguments extends LaunchRequestArguments, AttachRequestArguments { }
 
-export type ConsoleType = 'none' | 'integratedTerminal' | 'externalTerminal';
+export type ConsoleType = 'internalConsole' | 'integratedTerminal' | 'externalTerminal';
diff --git a/src/client/extension.ts b/src/client/extension.ts
@@ -195,7 +195,7 @@ async function activateUnsafe(context: ExtensionContext): Promise<IExtensionApi>
         ]
     });
 
-    if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'none') {
+    if (pythonSettings && pythonSettings.formatting && pythonSettings.formatting.provider !== 'internalConsole') {
         const formatProvider = new PythonFormattingEditProvider(context, serviceContainer);
         context.subscriptions.push(languages.registerDocumentFormattingEditProvider(PYTHON, formatProvider));
         context.subscriptions.push(languages.registerDocumentRangeFormattingEditProvider(PYTHON, formatProvider));
diff --git a/src/client/linters/linterCommands.ts b/src/client/linters/linterCommands.ts
@@ -59,10 +59,10 @@ export class LinterCommands implements IDisposable {
 
         const selection = await this.appShell.showQuickPick(linterList, quickPickOptions);
         if (selection !== undefined) {
-            if (selection === 'Disable Linting'){
+            if (selection === 'Disable Linting') {
                 await this.linterManager.enableLintingAsync(false);
-                sendTelemetryEvent(EventName.SELECT_LINTER, undefined, {enabled: false});
-            } else{
+                sendTelemetryEvent(EventName.SELECT_LINTER, undefined, { enabled: false });
+            } else {
                 const index = linters.findIndex(x => x.id === selection);
                 if (activeLinters.length > 1) {
                     const response = await this.appShell.showWarningMessage(Linters.replaceWithSelectedLinter().format(selection), 'Yes', 'No');
@@ -71,7 +71,7 @@ export class LinterCommands implements IDisposable {
                     }
                 }
                 await this.linterManager.setActiveLintersAsync([linters[index].product], this.settingsUri);
-                sendTelemetryEvent(EventName.SELECT_LINTER, undefined, {tool: selection as LinterId, enabled: true});
+                sendTelemetryEvent(EventName.SELECT_LINTER, undefined, { tool: selection as LinterId, enabled: true });
             }
         }
     }
diff --git a/src/client/telemetry/types.ts b/src/client/telemetry/types.ts
@@ -5,6 +5,7 @@
 import { DiagnosticCodes } from '../application/diagnostics/constants';
 import { TerminalShellType } from '../common/terminal/types';
 import { DebugConfigurationType } from '../debugger/extension/types';
+import { ConsoleType } from '../debugger/types';
 import { AutoSelectionRule } from '../interpreter/autoSelection/types';
 import { InterpreterType } from '../interpreter/contracts';
 import { LinterId } from '../linters/types';
@@ -72,7 +73,7 @@ export type CodeExecutionTelemetry = {
 };
 export type DebuggerTelemetry = {
     trigger: 'launch' | 'attach' | 'test';
-    console?: 'none' | 'integratedTerminal' | 'externalTerminal';
+    console?: ConsoleType;
     hasEnvVars: boolean;
     hasArgs: boolean;
     django: boolean;
diff --git a/src/client/testing/common/debugLauncher.ts b/src/client/testing/common/debugLauncher.ts
@@ -129,7 +129,7 @@ export class DebugLauncher implements ITestDebugLauncher {
         // Default value of justMyCode is not provided intentionally, for now we derive its value required for launchArgs using debugStdLib
         // Have to provide it if and when we remove complete support for debugStdLib
         if (!cfg.console) {
-            cfg.console = 'none';
+            cfg.console = 'internalConsole';
         }
         if (!cfg.cwd) {
             cfg.cwd = workspaceFolder.uri.fsPath;
diff --git a/src/test/application/diagnostics/checks/invalidLaunchJsonDebugger.unit.test.ts b/src/test/application/diagnostics/checks/invalidLaunchJsonDebugger.unit.test.ts
@@ -7,7 +7,7 @@ import { expect } from 'chai';
 import * as TypeMoq from 'typemoq';
 import { Uri, WorkspaceFolder } from 'vscode';
 import { BaseDiagnosticsService } from '../../../../client/application/diagnostics/base';
-import { InvalidLaunchJsonDebuggerDiagnostic, InvalidLaunchJsonDebuggerService } from '../../../../client/application/diagnostics/checks/invalidDebuggerType';
+import { InvalidLaunchJsonDebuggerDiagnostic, InvalidLaunchJsonDebuggerService } from '../../../../client/application/diagnostics/checks/invalidLaunchJsonDebugger';
 import { IDiagnosticsCommandFactory } from '../../../../client/application/diagnostics/commands/types';
 import { DiagnosticCodes } from '../../../../client/application/diagnostics/constants';
 import { MessageCommandPrompt } from '../../../../client/application/diagnostics/promptHandler';
@@ -55,7 +55,8 @@ suite('Application Diagnostics - Checks if launch.json is invalid', () => {
     test('Can handle all InvalidLaunchJsonDebugger diagnostics', async () => {
         for (const code of [
             DiagnosticCodes.InvalidDebuggerTypeDiagnostic,
-            DiagnosticCodes.JustMyCodeDiagnostic
+            DiagnosticCodes.JustMyCodeDiagnostic,
+            DiagnosticCodes.ConsoleTypeDiagnostic
         ]) {
             const diagnostic = TypeMoq.Mock.ofType<IDiagnostic>();
             diagnostic
@@ -199,7 +200,8 @@ suite('Application Diagnostics - Checks if launch.json is invalid', () => {
     test('All InvalidLaunchJsonDebugger diagnostics should display 2 options to with one command', async () => {
         for (const code of [
             DiagnosticCodes.InvalidDebuggerTypeDiagnostic,
-            DiagnosticCodes.JustMyCodeDiagnostic
+            DiagnosticCodes.JustMyCodeDiagnostic,
+            DiagnosticCodes.ConsoleTypeDiagnostic
         ]) {
             const diagnostic = TypeMoq.Mock.ofType<IDiagnostic>();
             let options: MessageCommandPrompt | undefined;
@@ -231,7 +233,8 @@ suite('Application Diagnostics - Checks if launch.json is invalid', () => {
     test('All InvalidLaunchJsonDebugger diagnostics should display message twice if invoked twice', async () => {
         for (const code of [
             DiagnosticCodes.InvalidDebuggerTypeDiagnostic,
-            DiagnosticCodes.JustMyCodeDiagnostic
+            DiagnosticCodes.JustMyCodeDiagnostic,
+            DiagnosticCodes.ConsoleTypeDiagnostic
         ]) {
             const diagnostic = TypeMoq.Mock.ofType<IDiagnostic>();
             diagnostic
@@ -264,7 +267,8 @@ suite('Application Diagnostics - Checks if launch.json is invalid', () => {
     test('Function fixLaunchJson() returns if there are no workspace folders', async () => {
         for (const code of [
             DiagnosticCodes.InvalidDebuggerTypeDiagnostic,
-            DiagnosticCodes.JustMyCodeDiagnostic
+            DiagnosticCodes.JustMyCodeDiagnostic,
+            DiagnosticCodes.ConsoleTypeDiagnostic
         ]) {
             workspaceService.setup(w => w.hasWorkspaceFolders)
                 .returns(() => false)
@@ -280,7 +284,8 @@ suite('Application Diagnostics - Checks if launch.json is invalid', () => {
     test('Function fixLaunchJson() returns if file launch.json does not exist', async () => {
         for (const code of [
             DiagnosticCodes.InvalidDebuggerTypeDiagnostic,
-            DiagnosticCodes.JustMyCodeDiagnostic
+            DiagnosticCodes.JustMyCodeDiagnostic,
+            DiagnosticCodes.ConsoleTypeDiagnostic
         ]) {
             workspaceService.setup(w => w.hasWorkspaceFolders)
                 .returns(() => true)
@@ -345,4 +350,27 @@ suite('Application Diagnostics - Checks if launch.json is invalid', () => {
         workspaceService.verifyAll();
         fs.verifyAll();
     });
+
+    test('File launch.json is fixed correctly when code equals ConsoleTypeDiagnostic ', async () => {
+        const launchJson = '{"console": "none"}';
+        const correctedlaunchJson = '{"console": "internalConsole"}';
+        workspaceService.setup(w => w.hasWorkspaceFolders)
+            .returns(() => true)
+            .verifiable(TypeMoq.Times.once());
+        workspaceService.setup(w => w.workspaceFolders)
+            .returns(() => [workspaceFolder])
+            .verifiable(TypeMoq.Times.once());
+        fs.setup(w => w.fileExists(TypeMoq.It.isAny()))
+            .returns(() => Promise.resolve(true))
+            .verifiable(TypeMoq.Times.once());
+        fs.setup(w => w.readFile(TypeMoq.It.isAny()))
+            .returns(() => Promise.resolve(launchJson))
+            .verifiable(TypeMoq.Times.atLeastOnce());
+        fs.setup(w => w.writeFile(TypeMoq.It.isAnyString(), correctedlaunchJson))
+            .returns(() => Promise.resolve())
+            .verifiable(TypeMoq.Times.once());
+        await (diagnosticService as any).fixLaunchJson(DiagnosticCodes.InvalidDebuggerTypeDiagnostic);
+        workspaceService.verifyAll();
+        fs.verifyAll();
+    });
 });
diff --git a/src/test/debugger/envVars.test.ts b/src/test/debugger/envVars.test.ts
diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Update launch.json to use internalConsole instead of none`
Original file line number	Diff line number	Diff line change
`@@ -15,5 +15,6 @@ export enum DiagnosticCodes {`
`15`	`15`	`EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic = 'EnvironmentActivationInPowerShellWithBatchFilesNotSupportedDiagnostic',`
`16`	`16`	`NoCurrentlySelectedPythonInterpreterDiagnostic = 'InvalidPythonInterpreterDiagnostic',`
`17`	`17`	`LSNotSupportedDiagnostic = 'LSNotSupportedDiagnostic',`
`18`		`- JustMyCodeDiagnostic = 'JustMyCodeDiagnostic'`
	`18`	`+ JustMyCodeDiagnostic = 'JustMyCodeDiagnostic',`
	`19`	`+ ConsoleTypeDiagnostic = 'ConsoleTypeDiagnostic'`
`19`	`20`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ export class DebugClientHelper {`
`31`	`31`	`this.envParser.appendPythonPath(env, this.process.env.PYTHONPATH!);`
`32`	`32`	`}`
`33`	`33`
`34`		`- if (typeof args.console !== 'string' \|\| args.console === 'none') {`
	`34`	`+ if (typeof args.console !== 'string' \|\| args.console === 'internalConsole') {`
`35`	`35`	`// For debugging, when not using any terminal, then we need to provide all env variables.`
`36`	`36`	`// As we're spawning the process, we need to ensure all env variables are passed.`
`37`	`37`	`// Including those from the current process (i.e. everything, not just custom vars).`
Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc`
`75`	`75`	`debugConfiguration.console = 'integratedTerminal';`
`76`	`76`	`}`
`77`	`77`	`// If using a terminal, then never open internal console.`
`78`		`- if (debugConfiguration.console !== 'none' && !debugConfiguration.internalConsoleOptions) {`
	`78`	`+ if (debugConfiguration.console !== 'internalConsole' && !debugConfiguration.internalConsoleOptions) {`
`79`	`79`	`debugConfiguration.internalConsoleOptions = 'neverOpen';`
`80`	`80`	`}`
`81`	`81`	`if (!Array.isArray(debugConfiguration.debugOptions)) {`