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.

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

Expose TS server tracing #110534

Merged
merged 2 commits into from Nov 20, 2020
Merged

Expose TS server tracing #110534

merged 2 commits into from Nov 20, 2020

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Nov 13, 2020

Support microsoft/TypeScript#41374

Three commits, in increasing order of visibility:

  1. Add a setting
  2. Document the setting
  3. Add a command

Feel free to reject any or all of them.

}

try {
await vscode.commands.executeCommand('revealFileInOS', vscode.Uri.file(this.serverState.server.tsServerTraceDirectory));
Copy link
Member Author

Choose a reason for hiding this comment

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

I copied this from the server log handler, but it doesn't actually work. The command ignores its argument and reveals the current file (and only if a folder is open). Rather than fix it, I made the PR a draft until I get confirmation that we actually want a command (which I doubt).

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's start by just logging the server trace directory instead of adding a command.

I imagine we'll usually be asking user to go through a set of steps to collect server traces. If we find that getting to the trace file itself is confusing, we revisit adding a command to simplify things

@amcasey
Copy link
Member Author

amcasey commented Nov 13, 2020

FYI @mjbvz

@mjbvz mjbvz self-assigned this Nov 13, 2020
@mjbvz mjbvz added this to the November 2020 milestone Nov 13, 2020
@@ -216,6 +226,13 @@ export class TypeScriptServerSpawner {
}
}

if (configuration.enableTsServerTracing && !isWeb()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need a version check?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought the server would silently ignore unsupported arguments.

}

try {
await vscode.commands.executeCommand('revealFileInOS', vscode.Uri.file(this.serverState.server.tsServerTraceDirectory));
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's start by just logging the server trace directory instead of adding a command.

I imagine we'll usually be asking user to go through a set of steps to collect server traces. If we find that getting to the trace file itself is confusing, we revisit adding a command to simplify things

@amcasey
Copy link
Member Author

amcasey commented Nov 16, 2020

Dropped the command commit. Let me know if you want to drop the documentation commit too.

@amcasey
Copy link
Member Author

amcasey commented Nov 16, 2020

The failure looks unrelated to my change:

*** Please use node >=10 and <=12.

Copy link
Contributor

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Looks good. Just unmark it as a draft when it's ready to merge

@amcasey amcasey marked this pull request as ready for review November 16, 2020 22:33
@amcasey amcasey closed this Nov 20, 2020
@amcasey amcasey reopened this Nov 20, 2020
@mjbvz mjbvz merged commit 9195c9a into microsoft:master Nov 20, 2020
@mjbvz
Copy link
Contributor

mjbvz commented Nov 20, 2020

Thanks @amcasey!

@amcasey amcasey deleted the ServerTracing branch November 20, 2020 23:31
chenjigeng pushed a commit to chenjigeng/vscode that referenced this pull request Nov 22, 2020
* Add typescript.tsserver.enableTracing setting

* Document typescript.tsserver.enableTracing setting
@github-actions github-actions bot locked and limited conversation to collaborators Jan 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants