From 9e020ed5ecc4c4695c275975e54c7793f649ad24 Mon Sep 17 00:00:00 2001 From: Josh Nichols Date: Tue, 26 Nov 2024 11:44:59 -0500 Subject: [PATCH 1/5] Fix 'Maximum call stack size exceeded' during `run` function --- src/extension.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index e1b53a8..6d78e1d 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -9,7 +9,8 @@ let channel: vscode.OutputChannel; function run(file: string | vscode.Uri | null | undefined) { if (!file) { - run(vscode.window.activeTextEditor?.document.uri); + if (!vscode.window.activeTextEditor) return; + run(vscode.window.activeTextEditor.document.uri); return; } @@ -254,7 +255,7 @@ class Worker implements vscode.Disposable { constructor( private readonly workspace: vscode.WorkspaceFolder, private readonly statusProvider: StatusProvider, - ) {} + ) { } workspaceHas(file: vscode.Uri): boolean { return file.fsPath.startsWith(this.workspace.uri.fsPath); From b81fc69a17f62148912f9b37f5f971d53ecf3952 Mon Sep 17 00:00:00 2001 From: Josh Nichols Date: Tue, 26 Nov 2024 11:55:14 -0500 Subject: [PATCH 2/5] first pass at handling. at least it's not an error --- src/extension.ts | 121 +++++++++++++++++++++++++++-------------------- 1 file changed, 70 insertions(+), 51 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 6d78e1d..d5ceb8a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -264,79 +264,98 @@ class Worker implements vscode.Disposable { async run(file: vscode.Uri): Promise { if (this.workspaceHas(file)) { this.statusProvider.status = 'working'; - await new Promise((r) => setTimeout(r, 50)); - // bin/codeownership currenlty wants relative paths const cwd = this.workspace.uri.fsPath; const relativePath = relative(cwd, file.fsPath); logSpace(); + log('info', `Checking ownership for ${relativePath}`); log('debug', `cwd: ${cwd}`); log('debug', `workspace: ${this.workspace.uri.fsPath}`); log('debug', `file path: ${file.fsPath}`); - log('debug', `relative path: ${relativePath}`); - try { - const output = await runCommand( - cwd, - `bin/codeownership for_file "${relativePath}" --json`, - this.statusProvider, - ); + // Check if binary exists + const checkBinary = await runCommand( + cwd, + 'test -f bin/codeownership && echo "exists"', + this.statusProvider + ); + + if (!checkBinary) { + log('info', 'No code ownership binary found in project'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } + // Run ownership check + const output = await runCommand( + cwd, + `bin/codeownership for_file "${relativePath}" --json`, + this.statusProvider, + ); + + if (!output) { + log('info', 'Code ownership check returned no output'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } + + try { const obj = JSON.parse(output); - if (typeof obj.team_name !== 'string') { - log( - 'warning', - 'Missing expected property `team_name` in command output', - ); - } - if (typeof obj.team_yml !== 'string') { - log( - 'warning', - 'Missing expected property `team_yml` in command output', - ); + if (!obj.team_name) { + log('info', 'No team name found in ownership data'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; } - if ( - typeof obj.team_name === 'string' && - typeof obj.team_yml === 'string' && - obj.team_name !== 'Unowned' - ) { - const teamConfig = resolve(this.workspace.uri.fsPath, obj.team_yml); - - const actions: UserAction[] = []; + if (!obj.team_yml) { + log('info', 'No team config file found in ownership data'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } - const slackChannel = await getSlackChannel(teamConfig); + if (obj.team_name === 'Unowned') { + log('info', 'File is explicitly unowned'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } - if (slackChannel) { - actions.push({ - title: `Slack: #${slackChannel}`, - uri: vscode.Uri.parse( - `https://slack.com/app_redirect?channel=${slackChannel}`, - ), - }); - } + const teamConfig = resolve(this.workspace.uri.fsPath, obj.team_yml); + const actions: UserAction[] = []; + const slackChannel = await getSlackChannel(teamConfig); + if (slackChannel) { actions.push({ - title: 'View team config', - uri: vscode.Uri.parse(teamConfig), + title: `Slack: #${slackChannel}`, + uri: vscode.Uri.parse( + `https://slack.com/app_redirect?channel=${slackChannel}`, + ), }); - - this.statusProvider.owner = { - filepath: file.fsPath, - teamName: obj.team_name, - teamConfig, - actions, - }; - } else { - this.statusProvider.owner = undefined; } + + actions.push({ + title: 'View team config', + uri: vscode.Uri.parse(teamConfig), + }); + + this.statusProvider.owner = { + filepath: file.fsPath, + teamName: obj.team_name, + teamConfig, + actions, + }; + this.statusProvider.status = 'idle'; + } catch (error) { + log('info', `Invalid ownership data format: ${error.message}`); + this.statusProvider.owner = undefined; this.statusProvider.status = 'idle'; - } catch { - this.statusProvider.status = 'error'; - log('error', 'Error parsing command output'); } } } From 0986bd3b57a9aa48d0ef0ce0e99e0962302aa575 Mon Sep 17 00:00:00 2001 From: Josh Nichols Date: Tue, 26 Nov 2024 12:03:16 -0500 Subject: [PATCH 3/5] check for binary/configuration much earlier --- src/extension.ts | 174 +++++++++++++++++++++++++---------------------- 1 file changed, 93 insertions(+), 81 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index d5ceb8a..a4b3239 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,6 +1,7 @@ import { relative, resolve, sep } from 'path'; import * as cp from 'child_process'; import { readFile } from 'fs/promises'; +import { existsSync } from 'fs'; import * as yaml from 'js-yaml'; import * as vscode from 'vscode'; @@ -252,111 +253,122 @@ class StatusProvider implements vscode.Disposable { } class Worker implements vscode.Disposable { + private isConfigured: boolean | null = null; + constructor( private readonly workspace: vscode.WorkspaceFolder, private readonly statusProvider: StatusProvider, - ) { } + ) { + this.checkConfiguration(); + } + + private async checkConfiguration(): Promise { + const binaryPath = resolve(this.workspace.uri.fsPath, 'bin/codeownership'); + this.isConfigured = existsSync(binaryPath); + + if (!this.isConfigured) { + log('info', `No code ownership binary found in workspace: ${this.workspace.name}`); + } else { + log('info', `Code ownership binary found in workspace: ${this.workspace.name}`); + } + } workspaceHas(file: vscode.Uri): boolean { return file.fsPath.startsWith(this.workspace.uri.fsPath); } async run(file: vscode.Uri): Promise { - if (this.workspaceHas(file)) { - this.statusProvider.status = 'working'; - await new Promise((r) => setTimeout(r, 50)); - - const cwd = this.workspace.uri.fsPath; - const relativePath = relative(cwd, file.fsPath); - - logSpace(); - log('info', `Checking ownership for ${relativePath}`); - log('debug', `cwd: ${cwd}`); - log('debug', `workspace: ${this.workspace.uri.fsPath}`); - log('debug', `file path: ${file.fsPath}`); - - // Check if binary exists - const checkBinary = await runCommand( - cwd, - 'test -f bin/codeownership && echo "exists"', - this.statusProvider - ); - - if (!checkBinary) { - log('info', 'No code ownership binary found in project'); + if (!this.workspaceHas(file)) return; + + if (this.isConfigured === null) { + await this.checkConfiguration(); + } + + if (!this.isConfigured) { + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } + + this.statusProvider.status = 'working'; + await new Promise((r) => setTimeout(r, 50)); + + const cwd = this.workspace.uri.fsPath; + const relativePath = relative(cwd, file.fsPath); + + logSpace(); + log('info', `Checking ownership for ${relativePath}`); + log('debug', `cwd: ${cwd}`); + log('debug', `workspace: ${this.workspace.uri.fsPath}`); + log('debug', `file path: ${file.fsPath}`); + + // Run ownership check + const output = await runCommand( + cwd, + `bin/codeownership for_file "${relativePath}" --json`, + this.statusProvider, + ); + + if (!output) { + log('info', 'Code ownership check returned no output'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } + + try { + const obj = JSON.parse(output); + + if (!obj.team_name) { + log('info', 'No team name found in ownership data'); this.statusProvider.owner = undefined; this.statusProvider.status = 'idle'; return; } - // Run ownership check - const output = await runCommand( - cwd, - `bin/codeownership for_file "${relativePath}" --json`, - this.statusProvider, - ); - - if (!output) { - log('info', 'Code ownership check returned no output'); + if (!obj.team_yml) { + log('info', 'No team config file found in ownership data'); this.statusProvider.owner = undefined; this.statusProvider.status = 'idle'; return; } - try { - const obj = JSON.parse(output); - - if (!obj.team_name) { - log('info', 'No team name found in ownership data'); - this.statusProvider.owner = undefined; - this.statusProvider.status = 'idle'; - return; - } - - if (!obj.team_yml) { - log('info', 'No team config file found in ownership data'); - this.statusProvider.owner = undefined; - this.statusProvider.status = 'idle'; - return; - } - - if (obj.team_name === 'Unowned') { - log('info', 'File is explicitly unowned'); - this.statusProvider.owner = undefined; - this.statusProvider.status = 'idle'; - return; - } + if (obj.team_name === 'Unowned') { + log('info', 'File is explicitly unowned'); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; + return; + } - const teamConfig = resolve(this.workspace.uri.fsPath, obj.team_yml); - const actions: UserAction[] = []; - - const slackChannel = await getSlackChannel(teamConfig); - if (slackChannel) { - actions.push({ - title: `Slack: #${slackChannel}`, - uri: vscode.Uri.parse( - `https://slack.com/app_redirect?channel=${slackChannel}`, - ), - }); - } + const teamConfig = resolve(this.workspace.uri.fsPath, obj.team_yml); + const actions: UserAction[] = []; + const slackChannel = await getSlackChannel(teamConfig); + if (slackChannel) { actions.push({ - title: 'View team config', - uri: vscode.Uri.parse(teamConfig), + title: `Slack: #${slackChannel}`, + uri: vscode.Uri.parse( + `https://slack.com/app_redirect?channel=${slackChannel}`, + ), }); - - this.statusProvider.owner = { - filepath: file.fsPath, - teamName: obj.team_name, - teamConfig, - actions, - }; - this.statusProvider.status = 'idle'; - } catch (error) { - log('info', `Invalid ownership data format: ${error.message}`); - this.statusProvider.owner = undefined; - this.statusProvider.status = 'idle'; } + + actions.push({ + title: 'View team config', + uri: vscode.Uri.parse(teamConfig), + }); + + this.statusProvider.owner = { + filepath: file.fsPath, + teamName: obj.team_name, + teamConfig, + actions, + }; + this.statusProvider.status = 'idle'; + } catch (error) { + log('info', `Invalid ownership data format: ${error.message}`); + this.statusProvider.owner = undefined; + this.statusProvider.status = 'idle'; } } From 09019a5eb7934a60d41623af2045e9ed1cd8e8d0 Mon Sep 17 00:00:00 2001 From: Josh Nichols Date: Tue, 26 Nov 2024 12:10:33 -0500 Subject: [PATCH 4/5] show when it's not configured vs no owner --- src/extension.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/extension.ts b/src/extension.ts index a4b3239..275afc6 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -202,6 +202,7 @@ class StatusProvider implements vscode.Disposable { private _status: Status = 'idle'; private _owner: Owner | undefined = undefined; + private _isConfigured: boolean | null = null; get status(): Status { return this._status; @@ -219,6 +220,14 @@ class StatusProvider implements vscode.Disposable { this.update(); } + get isConfigured(): boolean | null { + return this._isConfigured; + } + set isConfigured(value: boolean | null) { + this._isConfigured = value; + this.update(); + } + private update() { if (this.status === 'error') { this.statusBarItem.command = 'code-ownership-vscode.showOutputChannel'; @@ -239,9 +248,13 @@ class StatusProvider implements vscode.Disposable { this.statusBarItem.text = `$(account) Owner: ${this.owner.teamName}`; this.statusBarItem.tooltip = undefined; this.statusBarItem.show(); + } else if (this.isConfigured === false) { + this.statusBarItem.text = `$(info) Ownership: not configured`; + this.statusBarItem.tooltip = 'This workspace is not configured for code ownership'; + this.statusBarItem.show(); } else { this.statusBarItem.text = `$(warning) Owner: none`; - this.statusBarItem.tooltip = undefined; + this.statusBarItem.tooltip = 'This file has no assigned team ownership'; this.statusBarItem.show(); } } @@ -265,6 +278,7 @@ class Worker implements vscode.Disposable { private async checkConfiguration(): Promise { const binaryPath = resolve(this.workspace.uri.fsPath, 'bin/codeownership'); this.isConfigured = existsSync(binaryPath); + this.statusProvider.isConfigured = this.isConfigured; if (!this.isConfigured) { log('info', `No code ownership binary found in workspace: ${this.workspace.name}`); From 6ab9caedf906a8a7f9c943459b742c9d5b798335 Mon Sep 17 00:00:00 2001 From: Josh Nichols Date: Tue, 26 Nov 2024 12:14:37 -0500 Subject: [PATCH 5/5] run prettier --- README.md | 1 + src/extension.ts | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 55fb33e..2a65b13 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ View code ownership for every file right in the status bar. You'll get the name Quick access to the owning team's config file. Clicking on the status bar item will open a popup that includes a button that opens the team's config file. See [Code Teams](https://github.com/rubyatscale/code_teams) for more information on team config files. ## Installation + [Install from Marketplace](https://marketplace.visualstudio.com/items?itemName=Gusto.code-ownership-vscode) ## Requirements diff --git a/src/extension.ts b/src/extension.ts index 275afc6..96c18cd 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -250,7 +250,8 @@ class StatusProvider implements vscode.Disposable { this.statusBarItem.show(); } else if (this.isConfigured === false) { this.statusBarItem.text = `$(info) Ownership: not configured`; - this.statusBarItem.tooltip = 'This workspace is not configured for code ownership'; + this.statusBarItem.tooltip = + 'This workspace is not configured for code ownership'; this.statusBarItem.show(); } else { this.statusBarItem.text = `$(warning) Owner: none`; @@ -281,9 +282,15 @@ class Worker implements vscode.Disposable { this.statusProvider.isConfigured = this.isConfigured; if (!this.isConfigured) { - log('info', `No code ownership binary found in workspace: ${this.workspace.name}`); + log( + 'info', + `No code ownership binary found in workspace: ${this.workspace.name}`, + ); } else { - log('info', `Code ownership binary found in workspace: ${this.workspace.name}`); + log( + 'info', + `Code ownership binary found in workspace: ${this.workspace.name}`, + ); } }