Skip to content

Commit a1f5041

Browse files
karthiknadigeleanorjboyd
authored andcommitted
Use new logging API for python extension logger and LS logger (microsoft#21062)
In this PR: 1. Changes the python extension logging to use LogOutputChannel 2. Changes the language server logger with LogOutputChannel 3. Test output channel uses OutputChannel as it needs to show test output and not really logging. Also, using logging test output makes it pretty much unreadable. 4. Simplifies logging channel and output channel registration. We need to do this now to make it easier for new test work to integrate with output logging. For microsoft#20844 This still doesn't get rid of the log level setting.
1 parent 6403d20 commit a1f5041

39 files changed

+259
-146
lines changed

src/client/activation/common/analysisOptions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { DocumentFilter, LanguageClientOptions, RevealOutputChannelOn } from 'vs
55
import { IWorkspaceService } from '../../common/application/types';
66

77
import { PYTHON, PYTHON_LANGUAGE } from '../../common/constants';
8-
import { IOutputChannel, Resource } from '../../common/types';
8+
import { ILogOutputChannel, Resource } from '../../common/types';
99
import { debounceSync } from '../../common/utils/decorators';
1010
import { IEnvironmentVariablesProvider } from '../../common/variables/types';
1111
import { traceDecoratorError } from '../../logging';
@@ -14,7 +14,7 @@ import { ILanguageServerAnalysisOptions, ILanguageServerOutputChannel } from '..
1414

1515
export abstract class LanguageServerAnalysisOptionsBase implements ILanguageServerAnalysisOptions {
1616
protected readonly didChange = new EventEmitter<void>();
17-
private readonly output: IOutputChannel;
17+
private readonly output: ILogOutputChannel;
1818

1919
protected constructor(
2020
lsOutputChannel: ILanguageServerOutputChannel,

src/client/activation/common/outputChannel.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
import { inject, injectable } from 'inversify';
77
import { IApplicationShell, ICommandManager } from '../../common/application/types';
88
import '../../common/extensions';
9-
import { IDisposableRegistry, IOutputChannel } from '../../common/types';
9+
import { IDisposableRegistry, ILogOutputChannel } from '../../common/types';
1010
import { OutputChannelNames } from '../../common/utils/localize';
1111
import { ILanguageServerOutputChannel } from '../types';
1212

1313
@injectable()
1414
export class LanguageServerOutputChannel implements ILanguageServerOutputChannel {
15-
public output: IOutputChannel | undefined;
15+
public output: ILogOutputChannel | undefined;
1616

1717
private registered = false;
1818

@@ -22,7 +22,7 @@ export class LanguageServerOutputChannel implements ILanguageServerOutputChannel
2222
@inject(IDisposableRegistry) private readonly disposable: IDisposableRegistry,
2323
) {}
2424

25-
public get channel(): IOutputChannel {
25+
public get channel(): ILogOutputChannel {
2626
if (!this.output) {
2727
this.output = this.appShell.createOutputChannel(OutputChannelNames.languageServer);
2828
this.disposable.push(this.output);

src/client/activation/types.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { Event } from 'vscode';
77
import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node';
8-
import type { IDisposable, IOutputChannel, Resource } from '../common/types';
8+
import type { IDisposable, ILogOutputChannel, Resource } from '../common/types';
99
import { PythonEnvironment } from '../pythonEnvironments/info';
1010

1111
export const IExtensionActivationManager = Symbol('IExtensionActivationManager');
@@ -110,10 +110,10 @@ export interface ILanguageServerOutputChannel {
110110
/**
111111
* Creates output channel if necessary and returns it
112112
*
113-
* @type {IOutputChannel}
113+
* @type {ILogOutputChannel}
114114
* @memberof ILanguageServerOutputChannel
115115
*/
116-
readonly channel: IOutputChannel;
116+
readonly channel: ILogOutputChannel;
117117
}
118118

119119
export const IExtensionSingleActivationService = Symbol('IExtensionSingleActivationService');

src/client/common/application/applicationShell.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import {
1414
InputBoxOptions,
1515
languages,
1616
LanguageStatusItem,
17+
LogOutputChannel,
1718
MessageItem,
1819
MessageOptions,
1920
OpenDialogOptions,
20-
OutputChannel,
2121
Progress,
2222
ProgressOptions,
2323
QuickPick,
@@ -166,8 +166,8 @@ export class ApplicationShell implements IApplicationShell {
166166
public createTreeView<T>(viewId: string, options: TreeViewOptions<T>): TreeView<T> {
167167
return window.createTreeView<T>(viewId, options);
168168
}
169-
public createOutputChannel(name: string): OutputChannel {
170-
return window.createOutputChannel(name);
169+
public createOutputChannel(name: string): LogOutputChannel {
170+
return window.createOutputChannel(name, { log: true });
171171
}
172172
public createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem {
173173
return languages.createLanguageStatusItem(id, selector);

src/client/common/application/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ import {
2525
InputBox,
2626
InputBoxOptions,
2727
LanguageStatusItem,
28+
LogOutputChannel,
2829
MessageItem,
2930
MessageOptions,
3031
OpenDialogOptions,
31-
OutputChannel,
3232
Progress,
3333
ProgressOptions,
3434
QuickPick,
@@ -429,7 +429,7 @@ export interface IApplicationShell {
429429
*
430430
* @param name Human-readable string which will be used to represent the channel in the UI.
431431
*/
432-
createOutputChannel(name: string): OutputChannel;
432+
createOutputChannel(name: string): LogOutputChannel;
433433
createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem;
434434
}
435435

src/client/common/constants.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@ export namespace ThemeIcons {
9393

9494
export const DEFAULT_INTERPRETER_SETTING = 'python';
9595

96-
export const STANDARD_OUTPUT_CHANNEL = 'STANDARD_OUTPUT_CHANNEL';
97-
9896
export const isCI = process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined;
9997

10098
export function isTestExecution(): boolean {

src/client/common/installer/moduleInstaller.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@ import { sendTelemetryEvent } from '../../telemetry';
1212
import { EventName } from '../../telemetry/constants';
1313
import { IApplicationShell } from '../application/types';
1414
import { wrapCancellationTokens } from '../cancellation';
15-
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
1615
import { IFileSystem } from '../platform/types';
1716
import * as internalPython from '../process/internal/python';
1817
import { IProcessServiceFactory } from '../process/types';
1918
import { ITerminalServiceFactory, TerminalCreationOptions } from '../terminal/types';
20-
import { ExecutionInfo, IConfigurationService, IOutputChannel, Product } from '../types';
19+
import { ExecutionInfo, IConfigurationService, ILogOutputChannel, Product } from '../types';
2120
import { isResource } from '../utils/misc';
2221
import { ProductNames } from './productNames';
2322
import { IModuleInstaller, InstallOptions, InterpreterUri, ModuleInstallFlags } from './types';
@@ -152,7 +151,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
152151
const options = {
153152
name: 'VS Code Python',
154153
};
155-
const outputChannel = this.serviceContainer.get<IOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
154+
const outputChannel = this.serviceContainer.get<ILogOutputChannel>(ILogOutputChannel);
156155
const command = `"${execPath.replace(/\\/g, '/')}" ${args.join(' ')}`;
157156

158157
traceLog(`[Elevated] ${command}`);

src/client/common/process/rawProcessApis.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ export function plainExec(
121121
}
122122

123123
const stdoutBuffers: Buffer[] = [];
124-
on(proc.stdout, 'data', (data: Buffer) => stdoutBuffers.push(data));
124+
on(proc.stdout, 'data', (data: Buffer) => {
125+
stdoutBuffers.push(data);
126+
options.outputChannel?.append(data.toString());
127+
});
125128
const stderrBuffers: Buffer[] = [];
126129
on(proc.stderr, 'data', (data: Buffer) => {
127130
if (options.mergeStdOutErr) {
@@ -130,6 +133,7 @@ export function plainExec(
130133
} else {
131134
stderrBuffers.push(data);
132135
}
136+
options.outputChannel?.append(data.toString());
133137
});
134138

135139
proc.once('close', () => {

src/client/common/process/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { ChildProcess, ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process';
55
import { Observable } from 'rxjs/Observable';
6-
import { CancellationToken, Uri } from 'vscode';
6+
import { CancellationToken, OutputChannel, Uri } from 'vscode';
77
import { PythonExecInfo } from '../../pythonEnvironments/exec';
88
import { InterpreterInformation, PythonEnvironment } from '../../pythonEnvironments/info';
99
import { ExecutionInfo, IDisposable } from '../types';
@@ -24,6 +24,7 @@ export type SpawnOptions = ChildProcessSpawnOptions & {
2424
mergeStdOutErr?: boolean;
2525
throwOnStdErr?: boolean;
2626
extraVariables?: NodeJS.ProcessEnv;
27+
outputChannel?: OutputChannel;
2728
};
2829

2930
export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };

src/client/common/types.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@ import {
1515
Extension,
1616
ExtensionContext,
1717
Memento,
18-
OutputChannel,
18+
LogOutputChannel,
1919
Uri,
2020
WorkspaceEdit,
21+
OutputChannel,
2122
} from 'vscode';
2223
import { LanguageServerType } from '../activation/types';
2324
import type { InstallOptions, InterpreterUri, ModuleInstallFlags } from './installer/types';
@@ -29,8 +30,10 @@ export interface IDisposable {
2930
dispose(): void | undefined | Promise<void>;
3031
}
3132

32-
export const IOutputChannel = Symbol('IOutputChannel');
33-
export interface IOutputChannel extends OutputChannel {}
33+
export const ILogOutputChannel = Symbol('ILogOutputChannel');
34+
export interface ILogOutputChannel extends LogOutputChannel {}
35+
export const ITestOutputChannel = Symbol('ITestOutputChannel');
36+
export interface ITestOutputChannel extends OutputChannel {}
3437
export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider');
3538
export interface IDocumentSymbolProvider extends DocumentSymbolProvider {}
3639
export const IsWindows = Symbol('IS_WINDOWS');

src/client/extensionActivation.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,22 @@
33

44
'use strict';
55

6-
import {
7-
debug,
8-
DebugConfigurationProvider,
9-
DebugConfigurationProviderTriggerKind,
10-
languages,
11-
OutputChannel,
12-
window,
13-
} from 'vscode';
6+
import { debug, DebugConfigurationProvider, DebugConfigurationProviderTriggerKind, languages, window } from 'vscode';
147

158
import { registerTypes as activationRegisterTypes } from './activation/serviceRegistry';
169
import { IExtensionActivationManager } from './activation/types';
1710
import { registerTypes as appRegisterTypes } from './application/serviceRegistry';
1811
import { IApplicationDiagnostics } from './application/types';
1912
import { IApplicationEnvironment, ICommandManager, IWorkspaceService } from './common/application/types';
20-
import { Commands, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL, UseProposedApi } from './common/constants';
13+
import { Commands, PYTHON, PYTHON_LANGUAGE, UseProposedApi } from './common/constants';
2114
import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry';
2215
import { IFileSystem } from './common/platform/types';
2316
import {
2417
IConfigurationService,
2518
IDisposableRegistry,
2619
IExtensions,
2720
IInterpreterPathService,
28-
IOutputChannel,
21+
ILogOutputChannel,
2922
IPathUtils,
3023
} from './common/types';
3124
import { noop } from './common/utils/misc';
@@ -173,7 +166,7 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
173166
const dispatcher = new DebugSessionEventDispatcher(handlers, DebugService.instance, disposables);
174167
dispatcher.registerEventHandlers();
175168

176-
const outputChannel = serviceManager.get<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
169+
const outputChannel = serviceManager.get<ILogOutputChannel>(ILogOutputChannel);
177170
disposables.push(cmdManager.registerCommand(Commands.ViewOutput, () => outputChannel.show()));
178171
cmdManager.executeCommand('setContext', 'python.vscode.channel', applicationEnv.channel).then(noop, noop);
179172

src/client/extensionInit.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@
44
'use strict';
55

66
import { Container } from 'inversify';
7-
import { Disposable, Memento, OutputChannel, window } from 'vscode';
7+
import { Disposable, Memento, window } from 'vscode';
88
import { instance, mock } from 'ts-mockito';
9-
import { STANDARD_OUTPUT_CHANNEL } from './common/constants';
109
import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry';
1110
import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry';
1211
import { registerTypes as commonRegisterTypes } from './common/serviceRegistry';
@@ -16,7 +15,8 @@ import {
1615
IDisposableRegistry,
1716
IExtensionContext,
1817
IMemento,
19-
IOutputChannel,
18+
ILogOutputChannel,
19+
ITestOutputChannel,
2020
WORKSPACE_MEMENTO,
2121
} from './common/types';
2222
import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
@@ -26,7 +26,6 @@ import { ServiceContainer } from './ioc/container';
2626
import { ServiceManager } from './ioc/serviceManager';
2727
import { IServiceContainer, IServiceManager } from './ioc/types';
2828
import * as pythonEnvironments from './pythonEnvironments';
29-
import { TEST_OUTPUT_CHANNEL } from './testing/constants';
3029
import { IDiscoveryAPI } from './pythonEnvironments/base/locator';
3130
import { registerLogger } from './logging';
3231
import { OutputChannelLogger } from './logging/outputChannelLogger';
@@ -54,20 +53,20 @@ export function initializeGlobals(
5453
serviceManager.addSingletonInstance<Memento>(IMemento, context.workspaceState, WORKSPACE_MEMENTO);
5554
serviceManager.addSingletonInstance<IExtensionContext>(IExtensionContext, context);
5655

57-
const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python);
56+
const standardOutputChannel = window.createOutputChannel(OutputChannelNames.python, { log: true });
5857
disposables.push(standardOutputChannel);
5958
disposables.push(registerLogger(new OutputChannelLogger(standardOutputChannel)));
6059

6160
const workspaceService = new WorkspaceService();
6261
const unitTestOutChannel =
6362
workspaceService.isVirtualWorkspace || !workspaceService.isTrusted
6463
? // Do not create any test related output UI when using virtual workspaces.
65-
instance(mock<IOutputChannel>())
64+
instance(mock<ITestOutputChannel>())
6665
: window.createOutputChannel(OutputChannelNames.pythonTest);
6766
disposables.push(unitTestOutChannel);
6867

69-
serviceManager.addSingletonInstance<OutputChannel>(IOutputChannel, standardOutputChannel, STANDARD_OUTPUT_CHANNEL);
70-
serviceManager.addSingletonInstance<OutputChannel>(IOutputChannel, unitTestOutChannel, TEST_OUTPUT_CHANNEL);
68+
serviceManager.addSingletonInstance<ILogOutputChannel>(ILogOutputChannel, standardOutputChannel);
69+
serviceManager.addSingletonInstance<ITestOutputChannel>(ITestOutputChannel, unitTestOutChannel);
7170

7271
return {
7372
context,

src/client/linters/errorHandlers/standard.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { l10n, Uri } from 'vscode';
22
import { IApplicationShell } from '../../common/application/types';
3-
import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
4-
import { ExecutionInfo, IOutputChannel } from '../../common/types';
3+
import { ExecutionInfo, ILogOutputChannel } from '../../common/types';
54
import { traceError, traceLog } from '../../logging';
65
import { ILinterManager, LinterId } from '../types';
76
import { BaseErrorHandler } from './baseErrorHandler';
@@ -29,7 +28,7 @@ export class StandardErrorHandler extends BaseErrorHandler {
2928
private async displayLinterError(linterId: LinterId) {
3029
const message = l10n.t("There was an error in running the linter '{0}'", linterId);
3130
const appShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
32-
const outputChannel = this.serviceContainer.get<IOutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
31+
const outputChannel = this.serviceContainer.get<ILogOutputChannel>(ILogOutputChannel);
3332
const action = await appShell.showErrorMessage(message, 'View Errors');
3433
if (action === 'View Errors') {
3534
outputChannel.show();

src/client/logging/outputChannelLogger.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,29 @@
22
// Licensed under the MIT License.
33

44
import * as util from 'util';
5-
import { OutputChannel } from 'vscode';
5+
import { LogOutputChannel } from 'vscode';
66
import { Arguments, ILogging } from './types';
7-
import { getTimeForLogging } from './util';
8-
9-
function formatMessage(level?: string, ...data: Arguments): string {
10-
return level ? `[${level.toUpperCase()} ${getTimeForLogging()}]: ${util.format(...data)}` : util.format(...data);
11-
}
127

138
export class OutputChannelLogger implements ILogging {
14-
constructor(private readonly channel: OutputChannel) {}
9+
constructor(private readonly channel: LogOutputChannel) {}
1510

1611
public traceLog(...data: Arguments): void {
1712
this.channel.appendLine(util.format(...data));
1813
}
1914

2015
public traceError(...data: Arguments): void {
21-
this.channel.appendLine(formatMessage('error', ...data));
16+
this.channel.error(util.format(...data));
2217
}
2318

2419
public traceWarn(...data: Arguments): void {
25-
this.channel.appendLine(formatMessage('warn', ...data));
20+
this.channel.warn(util.format(...data));
2621
}
2722

2823
public traceInfo(...data: Arguments): void {
29-
this.channel.appendLine(formatMessage('info', ...data));
24+
this.channel.info(util.format(...data));
3025
}
3126

3227
public traceVerbose(...data: Arguments): void {
33-
this.channel.appendLine(formatMessage('debug', ...data));
28+
this.channel.debug(util.format(...data));
3429
}
3530
}

src/client/testing/constants.ts

Lines changed: 0 additions & 4 deletions
This file was deleted.

src/client/testing/testController/common/server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export class PythonTestServer implements ITestServer, Disposable {
9999
token: options.token,
100100
cwd: options.cwd,
101101
throwOnStdErr: true,
102+
outputChannel: options.outChannel,
102103
};
103104

104105
// Create the Python environment in which to execute the command.

0 commit comments

Comments
 (0)