Skip to content

Conversation

award999
Copy link
Contributor

@award999 award999 commented Jun 11, 2024

Issue: #833

IFF sourcekit logMessage notification includes a logName property, we'll
use a different output channel to not polute the langauge server's main
log. For a known list of logs we'll set them up on startup so we can
control if the log level and/or timestamp is included in the log message.
For the current known indexing log, for example, all messages will be
"Info" so we'll just add timestamp. If new logs are introduced, we'll
show log level and timestamp by default

@award999 award999 marked this pull request as ready for review June 11, 2024 18:59
@award999 award999 self-assigned this Jun 11, 2024
@award999
Copy link
Contributor Author

@swift-server-bot test this please

client.onNotification(langclient.LogMessageNotification.type, params => {
const sourceKitParams = params as SourceKitLogMessageParams;
if (sourceKitParams.logName) {
const outputChannel =
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create the log channels in advance. We should know what logs SourceKit-LSP is going to want to open.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After a chat will Alex, he asked if we could show timestamp but not log level for the Indexing log because it will always be info. So, for known logs like that, I'm letting set that configuration on startup. My idea behind allowing unknown logs to be created is if sourcekit-LSP changes, for development or as a user, they don't need to wait for an extension release to see the new log. Let me know if you still have concerns

Issue: swiftlang#833

IFF sourcekit logMessage notification includes a logName property, we'll
use a different output channel to not polute the langauge server's main
log. For a known list of logs we'll set them up on startup so we can
control if the log level and/or timestamp is included in the log message.
For the current known indexing log, for example, all messages will be
"Info" so we'll just add timestamp. If new logs are introduced, we'll
show log level and timestamp by default
Copy link
Contributor

@plemarquand plemarquand left a comment

Choose a reason for hiding this comment

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

LGTM

@award999 award999 merged commit e8308d6 into swiftlang:main Jun 14, 2024
@award999 award999 deleted the sourcekit-outputchannels branch June 14, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants