Repository: stateful/vscode-runme (ts)

Issue:
Open Terminal can become stale
After creating a terminal for a background process, the button/PID will linger even after the terminal is destroyed:

<img src="https://user-images.githubusercontent.com/16108792/212572480-e0d08633-2038-4174-bcb3-2534126a2973.gif" />


Pull request:
Remove stale open terminal button
Fixes #124 

This may conflict with #114, but it shouldn't be hard to integrate this if/when that PR gets approved 👍 

Patch:
diff --git a/__mocks__/vscode.ts b/__mocks__/vscode.ts
index a7365127..b3db1976 100644
--- a/__mocks__/vscode.ts
+++ b/__mocks__/vscode.ts
@@ -69,7 +69,8 @@ export const window = {
   showNotebookDocument: vi.fn(),
   showTextDocument: vi.fn(),
   onDidChangeActiveNotebookEditor: vi.fn().mockReturnValue({ dispose: vi.fn() }),
-  registerTreeDataProvider: vi.fn()
+  registerTreeDataProvider: vi.fn(),
+  onDidCloseTerminal: vi.fn()
 }
 
 export const tasks = {
@@ -100,7 +101,7 @@ export enum NotebookCellKind {
 }
 
 export const TreeItem = vi.fn()
-export const EventEmitter = vi.fn()
+export class EventEmitter {}
 export const Event = vi.fn()
 
 export enum TreeItemCollapsibleState {
@@ -134,7 +135,7 @@ export enum NotebookCellStatusBarAlignment {
 }
 
 export class NotebookCellStatusBarItem {
-  text: string
+  label: string
 
   alignment: NotebookCellStatusBarAlignment
 
@@ -143,7 +144,10 @@ export class NotebookCellStatusBarItem {
   priority?: number
 
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  constructor(text: string, alignment: NotebookCellStatusBarAlignment) {}
+  constructor(label: string, alignment: NotebookCellStatusBarAlignment) {
+    this.label = label
+    this.alignment = alignment
+  }
 }
 
 
diff --git a/src/extension/extension.ts b/src/extension/extension.ts
index 801261b3..292945aa 100644
--- a/src/extension/extension.ts
+++ b/src/extension/extension.ts
@@ -21,7 +21,6 @@ import {
 import { WasmSerializer, GrpcSerializer } from './serializer'
 import { RunmeLauncherProvider } from './provider/launcher'
 
-
 export class RunmeExtension {
   async initialize(context: ExtensionContext) {
     const kernel = new Kernel(context)
diff --git a/src/extension/provider/background.ts b/src/extension/provider/background.ts
index 4736573e..6c33306f 100644
--- a/src/extension/provider/background.ts
+++ b/src/extension/provider/background.ts
@@ -1,8 +1,19 @@
-import vscode from 'vscode'
+import vscode, { window, EventEmitter, NotebookCellStatusBarItem, NotebookCellStatusBarAlignment } from 'vscode'
 
 import { getAnnotations, getTerminalByCell } from '../utils'
 
-export class ShowTerminalProvider implements vscode.NotebookCellStatusBarItemProvider {
+export class ShowTerminalProvider implements vscode.NotebookCellStatusBarItemProvider, vscode.Disposable {
+  private _onDidChangeCellStatusBarItems = new EventEmitter<void>()
+  onDidChangeCellStatusBarItems = this._onDidChangeCellStatusBarItems.event
+
+  private _closeTerminalSubscription: vscode.Disposable
+
+  constructor() {
+    this._closeTerminalSubscription = window.onDidCloseTerminal(() =>
+      this.refreshStatusBarItems()
+    )
+  }
+
   async provideCellStatusBarItems(cell: vscode.NotebookCell): Promise<vscode.NotebookCellStatusBarItem | undefined> {
     /**
      * don't show status item if we run it in non-interactive mode where there is no terminal to open
@@ -18,13 +29,22 @@ export class ShowTerminalProvider implements vscode.NotebookCellStatusBarItemPro
       return
     }
 
-    const item = new vscode.NotebookCellStatusBarItem(
+    const item = new NotebookCellStatusBarItem(
       `$(terminal) Open Terminal (PID: ${pid})`,
-      vscode.NotebookCellStatusBarAlignment.Right
+      NotebookCellStatusBarAlignment.Right
     )
     item.command = 'runme.openTerminal'
     return item
   }
+
+  refreshStatusBarItems() {
+    this._onDidChangeCellStatusBarItems.fire()
+  }
+
+	public dispose() {
+    this._onDidChangeCellStatusBarItems.dispose()
+    this._closeTerminalSubscription.dispose()
+	}
 }
 
 export class BackgroundTaskProvider implements vscode.NotebookCellStatusBarItemProvider {
@@ -38,9 +58,9 @@ export class BackgroundTaskProvider implements vscode.NotebookCellStatusBarItemP
       return
     }
 
-    const item = new vscode.NotebookCellStatusBarItem(
+    const item = new NotebookCellStatusBarItem(
       'Background Task',
-      vscode.NotebookCellStatusBarAlignment.Right
+      NotebookCellStatusBarAlignment.Right
     )
     return item
   }
@@ -56,9 +76,9 @@ export class StopBackgroundTaskProvider implements vscode.NotebookCellStatusBarI
       return
     }
 
-    const item = new vscode.NotebookCellStatusBarItem(
+    const item = new NotebookCellStatusBarItem(
       '$(circle-slash) Stop Task',
-      vscode.NotebookCellStatusBarAlignment.Right
+      NotebookCellStatusBarAlignment.Right
     )
     item.command = 'runme.stopBackgroundTask'
     return item


Test patch:
diff --git a/tests/extension/provider/annotations.test.ts b/tests/extension/provider/annotations.test.ts
index 23150d18..11726acf 100644
--- a/tests/extension/provider/annotations.test.ts
+++ b/tests/extension/provider/annotations.test.ts
@@ -68,6 +68,8 @@ describe('Runme Annotations', () => {
       }
 
       const expectedItem = {
+        label: '$(gear) Configure',
+        alignment: 2,
         command: {
           title: 'Configure cell behavior',
           command: 'runme.toggleCellAnnotations',
diff --git a/tests/extension/provider/background.test.ts b/tests/extension/provider/background.test.ts
index d6d5af7b..626c4cd5 100644
--- a/tests/extension/provider/background.test.ts
+++ b/tests/extension/provider/background.test.ts
@@ -1,3 +1,4 @@
+import { window } from 'vscode'
 import { vi, describe, expect, beforeEach, it } from 'vitest'
 
 import { getAnnotations, getTerminalByCell } from '../../../src/extension/utils'
@@ -7,21 +8,13 @@ import {
   StopBackgroundTaskProvider
 } from '../../../src/extension/provider/background'
 
-vi.mock('vscode', () => ({
-  default: {
-    NotebookCellStatusBarItem: class {
-      constructor (public label: string, public position: number) {}
-    },
-    NotebookCellStatusBarAlignment: {
-      Right: 'right'
-    }
-  }
-}))
-
-vi.mock('../../../src/extension/utils', () => ({
-  getTerminalByCell: vi.fn(),
-  getAnnotations: vi.fn()
-}))
+vi.mock('vscode')
+vi.mock('../../../src/extension/utils', async () => {
+  return ({
+    getTerminalByCell: vi.fn(),
+    getAnnotations: vi.fn()
+  })
+})
 
 describe('ShowTerminalProvider', () => {
   beforeEach(() => {
@@ -55,9 +48,31 @@ describe('ShowTerminalProvider', () => {
     expect(item).toEqual({
       label: '$(terminal) Open Terminal (PID: 123)',
       command: 'runme.openTerminal',
-      position: 'right'
+      alignment: 2
     })
   })
+
+  it('will stop showing pid if terminal is destroyed', async () => {
+    let changeActiveTerminal: (() => void)[] = []
+    vi.mocked<any>(window.onDidCloseTerminal).mockImplementationOnce(c => changeActiveTerminal.push(c))
+
+    vi.mocked(getAnnotations).mockReturnValueOnce({ interactive: true } as any)
+    vi.mocked(getTerminalByCell).mockReturnValueOnce({ processId: Promise.resolve(123) } as any)
+    const p = new ShowTerminalProvider()
+    p.refreshStatusBarItems = vi.fn()
+    const item = await p.provideCellStatusBarItems('cell' as any)
+    expect(item).toBeTruthy()
+
+    changeActiveTerminal.forEach((c) => c())
+    expect(p.refreshStatusBarItems).toBeCalledTimes(1)
+
+    vi.mocked(getAnnotations).mockReturnValueOnce({ interactive: true } as any)
+    vi.mocked(getTerminalByCell).mockReturnValueOnce(undefined)
+    {
+      const p = new ShowTerminalProvider()
+      expect(await p.provideCellStatusBarItems('cell' as any)).toBe(undefined)
+    }
+  })
 })
 
 describe('BackgroundTaskProvider', () => {
@@ -96,7 +111,7 @@ describe('BackgroundTaskProvider', () => {
     const item = await p.provideCellStatusBarItems(cell as any)
     expect(item).toEqual({
       label: 'Background Task',
-      position: 'right'
+      alignment: 2
     })
   })
 })
@@ -147,7 +162,7 @@ describe('StopBackgroundTaskProvider', () => {
     const item = await p.provideCellStatusBarItems(cell)
     expect(item).toEqual({
       label: '$(circle-slash) Stop Task',
-      position: 'right',
+      alignment: 2,
       command: 'runme.stopBackgroundTask'
     })
   })