Conversation
EhabY
left a comment
EhabY
left a comment
There was a problem hiding this comment.
Thanks for contributing to this repo, appreciated
Also make sure to run format/lint:fix (I think the pipeline would fail now)
| /** | ||
| * Build CLI auth flags for execFile (no shell escaping). | ||
| */ | ||
| function authArgs(auth: CliAuth): string[] { |
There was a problem hiding this comment.
I'm pretty sure that this function exists elsewhere already, getGlobalFlags in the cli settings
| @@ -0,0 +1,6 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Maybe make this a JS file and run it using node so it's cross-platform
| const featureSet = featureSetForVersion(version); | ||
| const configDir = this.pathResolver.getGlobalConfigDir(safeHost); | ||
| const configs = vscode.workspace.getConfiguration(); | ||
| const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir); |
There was a problem hiding this comment.
Here we only pass the auth but not the global flags which might break if users have proxy rules or headers. See openAppStatus and how it uses getGlobalFlags
| const configDir = this.pathResolver.getGlobalConfigDir(safeHost); | ||
| const configs = vscode.workspace.getConfiguration(); | ||
| const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir); | ||
| const workspaceName = createWorkspaceIdentifier(this.workspace!); |
There was a problem hiding this comment.
We should avoid these kinds of null assertions (I thought we had a rule for that actually 🤔). Also might make sense to capture this.workspace in a variable in case it changes during one of the await operations (and would make the null assertion unnecessary as well)
|
|
||
| /** | ||
| * Run a speed test against the specified workspace and return the JSON output. | ||
| * Throw if unable to execute the binary or parse the output. |
There was a problem hiding this comment.
This here does not parse the output so this is inaccurate
| const doc = await vscode.workspace.openTextDocument({ | ||
| content: stdout, | ||
| language: "json", | ||
| }); | ||
| await vscode.window.showTextDocument(doc); | ||
| } catch (error) { |
There was a problem hiding this comment.
IMO I think we should show a save dialog instead of opening an untitled window.
We can show the save dialog then an info dialog that confirms it's been saved with a button to open the file in the editor
| await withProgress( | ||
| { | ||
| location: vscode.ProgressLocation.Notification, | ||
| title: "Running speed test...", | ||
| }, | ||
| async () => { |
There was a problem hiding this comment.
Should we pass the cancellation token to the CLI here? Also possibly use the -t argument and mention that in the title: https://coder.com/docs/reference/cli/speedtest#-t---time
2 participants
Closes #750