Skip to content
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

Promote extension log API to stable #43276

Closed
wants to merge 2 commits into from
Closed

Promote extension log API to stable #43276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e45f55
Promote extension log API to stable - Fix #43275
roblourens Feb 9, 2018
1fd0f50
Add comments for Logger methods
roblourens Feb 13, 2018
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
57 changes: 57 additions & 0 deletions src/vs/vscode.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4091,6 +4091,63 @@ declare module 'vscode' {
* [`globalState`](#ExtensionContext.globalState) to store key value data.
*/
storagePath: string | undefined;

/**
* This extension's logger
*/
logger: Logger;
}

/**
* The severity level of a log message
*/
export enum LogLevel {
Trace = 1,
Debug = 2,
Info = 3,
Warning = 4,
Error = 5,
Critical = 6,
Off = 7
}

/**
* A logger for writing to an extension's log file, and accessing its dedicated log directory.
*/
export interface Logger {
readonly onDidChangeLogLevel: Event<LogLevel>;
readonly currentLevel: LogLevel;
Copy link
Member

Choose a reason for hiding this comment

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

Why is the log level be exposed as API ?

Copy link
Member Author

Choose a reason for hiding this comment

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

One use case might be when for when an extension is managing its own logging and writing to a file on its own.

Copy link
Member

Choose a reason for hiding this comment

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

Has anyone asked for this? There are very many things we could add to the API, but IMHO it is a good practice to be as limited as possible and add something only if really needed...

Copy link
Member Author

Choose a reason for hiding this comment

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

If we expose the log directory, we have to expose this, it makes no sense to only provide one.

readonly logDirectory: Thenable<string>;
Copy link
Member

Choose a reason for hiding this comment

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

Typically, we make read API synchronous. Why is logDirectory a Thenable ?

Copy link
Member Author

@roblourens roblourens Feb 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The directory is only created when this is called or when the logger is used. Otherwise there will be a ton of empty folders sitting around.

Copy link
Member

Choose a reason for hiding this comment

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

@roblourens IMHO that is super weird. I've never seen a lazy getter field that creates a directory when accessed. Is there no other way to solve the problem of the empty folders ? Why does each extension need to get its own folder ? Can't we use the extension id as the filename and thus avoid collisions ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If an extension owns it own log file(s), where would it write to? The main logging directory? Then it would have to know about the window id or rotate files itself.

It could be a get() method too.


/**
* Log a message with level `Trace`
*/
trace(message: string, ...args: any[]): void;

/**
* Log a message with level `Debug`
*/
debug(message: string, ...args: any[]): void;

/**
* Log a message with level `Info`
*/
info(message: string, ...args: any[]): void;

/**
* Log a message with level `Warn`
*/
warn(message: string, ...args: any[]): void;

/**
* Log a message with level `Error`
*/
error(message: string | Error, ...args: any[]): void;

/**
* Log a message with level `Critical`
*/
critical(message: string | Error, ...args: any[]): void;
}

/**
Expand Down
36 changes: 0 additions & 36 deletions src/vs/vscode.proposed.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,42 +395,6 @@ declare module 'vscode' {
debugAdapterExecutable?(folder: WorkspaceFolder | undefined, token?: CancellationToken): ProviderResult<DebugAdapterExecutable>;
}

/**
* The severity level of a log message
*/
export enum LogLevel {
Trace = 1,
Debug = 2,
Info = 3,
Warning = 4,
Error = 5,
Critical = 6,
Off = 7
}

/**
* A logger for writing to an extension's log file, and accessing its dedicated log directory.
*/
export interface Logger {
readonly onDidChangeLogLevel: Event<LogLevel>;
readonly currentLevel: LogLevel;
readonly logDirectory: Thenable<string>;

trace(message: string, ...args: any[]): void;
debug(message: string, ...args: any[]): void;
info(message: string, ...args: any[]): void;
warn(message: string, ...args: any[]): void;
error(message: string | Error, ...args: any[]): void;
critical(message: string | Error, ...args: any[]): void;
}

export interface ExtensionContext {
/**
* This extension's logger
*/
logger: Logger;
}

export interface RenameInitialValue {
range: Range;
text?: string;
Expand Down