Repository: stateful/vscode-runme
Pull request #63 (2022-11-16 23:29:14)

reimplemented stop task btn ux
covers #62  and https://github.com/stateful/vscode-runme/pull/48

lorenzejay: > LGTM 👍

@christian-bromann  Can you merge? 

Patch:
diff --git a/package.json b/package.json
index 76fa86cc..51738ee4 100644
--- a/package.json
+++ b/package.json
@@ -91,6 +91,12 @@
         "category": "Runme",
         "title": "Open Runme notebook in text editor",
         "icon": "$(markdown)"
+      },
+      {
+        "command": "runme.stopBackgroundTask",
+        "category": "Runme",
+        "title": "Dispose terminal session to stop background task."
+
       }
     ],
     "menus": {
@@ -106,6 +112,10 @@
         {
           "command": "runme.openSplitViewAsMarkdownText",
           "when": "false"
+        },
+        {
+          "command": "runme.stopBackgroundTask",
+          "when": "false"
         }
       ],
       "editor/title": [
diff --git a/src/extension/commands/index.ts b/src/extension/commands/index.ts
index 9f8b3b82..afc3cb40 100644
--- a/src/extension/commands/index.ts
+++ b/src/extension/commands/index.ts
@@ -5,10 +5,14 @@ import { NotebookCell, Uri, window, env, NotebookDocument, TextDocument, ViewCol
 import { CliProvider } from '../provider/cli'
 import { getTerminalByCell } from '../utils'
 
+function showWarningMessage () {
+  return window.showWarningMessage('Couldn\'t find terminal! Was it already closed?')
+}
+
 export function openTerminal (cell: NotebookCell) {
   const terminal = getTerminalByCell(cell)
   if (!terminal) {
-    return window.showWarningMessage('Couldn\'t find terminal! Was it already closed?')
+    return showWarningMessage()
   }
   return terminal.show()
 }
@@ -18,6 +22,15 @@ export function copyCellToClipboard (cell: NotebookCell) {
   return window.showInformationMessage('Copied cell to clipboard!')
 }
 
+export function stopBackgroundTask (cell: NotebookCell) {
+  const terminal = getTerminalByCell(cell)
+  if (!terminal) {
+    return showWarningMessage()
+  }
+  terminal.dispose()
+  return window.showInformationMessage(`${terminal?.name} task terminated!`)
+}
+
 export async function runCLICommand (cell: NotebookCell) {
   if (!await CliProvider.isCliInstalled()) {
     return window.showInformationMessage(
diff --git a/src/extension/extension.ts b/src/extension/extension.ts
index e9514c0a..795c3095 100644
--- a/src/extension/extension.ts
+++ b/src/extension/extension.ts
@@ -3,8 +3,7 @@ import { workspace, notebooks, commands, ExtensionContext } from 'vscode'
 
 import { Serializer } from './notebook'
 import { Kernel } from './kernel'
-import { ShowTerminalProvider, BackgroundTaskProvider } from './provider/background'
-import { PidStatusProvider } from './provider/pid'
+import { ShowTerminalProvider, BackgroundTaskProvider, StopBackgroundTaskProvider} from './provider/background'
 import { CopyProvider } from './provider/copy'
 import { resetEnv } from './utils'
 import { CliProvider } from './provider/cli'
@@ -13,16 +12,10 @@ import {
   runCLICommand, 
   copyCellToClipboard, 
   openAsRunmeNotebook, 
-  openSplitViewAsMarkdownText 
+  openSplitViewAsMarkdownText ,
+  stopBackgroundTask
 } from './commands'
 
-export function activate(context: ExtensionContext) {
-  context.subscriptions.push(
-    commands.registerCommand('runme.openSplitViewAsMarkdownText', openSplitViewAsMarkdownText),
-    commands.registerCommand('runme.openAsRunmeNotebook', openAsRunmeNotebook),
-  )
-}
-export function deactivate() {}
 
 export class RunmeExtension {
   async initialise (context: ExtensionContext) {
@@ -38,14 +31,15 @@ export class RunmeExtension {
       }),
 
       notebooks.registerNotebookCellStatusBarItemProvider('runme', new ShowTerminalProvider()),
-      notebooks.registerNotebookCellStatusBarItemProvider('runme', new PidStatusProvider()),
       notebooks.registerNotebookCellStatusBarItemProvider('runme', new CliProvider()),
       notebooks.registerNotebookCellStatusBarItemProvider('runme', new BackgroundTaskProvider()),
       notebooks.registerNotebookCellStatusBarItemProvider('runme', new CopyProvider()),
+      notebooks.registerNotebookCellStatusBarItemProvider('runme', new StopBackgroundTaskProvider()),
       commands.registerCommand('runme.resetEnv', resetEnv),
       commands.registerCommand('runme.openTerminal', openTerminal),
       commands.registerCommand('runme.runCliCommand', runCLICommand),
       commands.registerCommand('runme.copyCellToClipboard', copyCellToClipboard),
+      commands.registerCommand('runme.stopBackgroundTask', stopBackgroundTask),
       commands.registerCommand('runme.openSplitViewAsMarkdownText', openSplitViewAsMarkdownText),
       commands.registerCommand('runme.openAsRunmeNotebook', openAsRunmeNotebook),
     )
diff --git a/src/extension/provider/background.ts b/src/extension/provider/background.ts
index ed70cfbe..72a9cf70 100644
--- a/src/extension/provider/background.ts
+++ b/src/extension/provider/background.ts
@@ -3,7 +3,7 @@ import vscode from 'vscode'
 import { getExecutionProperty, getTerminalByCell } from '../utils'
 
 export class ShowTerminalProvider implements vscode.NotebookCellStatusBarItemProvider {
-  provideCellStatusBarItems(cell: vscode.NotebookCell): vscode.NotebookCellStatusBarItem | undefined {
+  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
      */
@@ -12,12 +12,14 @@ export class ShowTerminalProvider implements vscode.NotebookCellStatusBarItemPro
     }
 
     const terminal = getTerminalByCell(cell)
-    if (!Boolean(terminal)) {
+    const pid = await terminal?.processId
+    
+    if (!Boolean(terminal) || !pid) {
       return
     }
 
     const item = new vscode.NotebookCellStatusBarItem(
-      '$(terminal) Open Terminal',
+      `$(terminal) Open Terminal (PID: ${pid})`,
       vscode.NotebookCellStatusBarAlignment.Right
     )
     item.command = 'runme.openTerminal'
@@ -42,3 +44,21 @@ export class BackgroundTaskProvider implements vscode.NotebookCellStatusBarItemP
     return item
   }
 }
+export class StopBackgroundTaskProvider implements vscode.NotebookCellStatusBarItemProvider {
+  provideCellStatusBarItems(cell: vscode.NotebookCell): vscode.NotebookCellStatusBarItem | undefined {
+    const isBackground = cell.metadata.attributes?.['background'] === 'true'
+    /**
+     * don't show if not a background task & if not command currently running
+     */
+    if (!isBackground || !getExecutionProperty('interactive', cell) || !cell.executionSummary?.success) {
+      return
+    }
+
+    const item = new vscode.NotebookCellStatusBarItem(
+      '$(circle-slash) Stop Task',
+      vscode.NotebookCellStatusBarAlignment.Right
+    )
+    item.command = 'runme.stopBackgroundTask'
+    return item
+  }
+}


Test patch:
diff --git a/tests/extension/commands/index.test.ts b/tests/extension/commands/index.test.ts
index 591c0a94..8ae66f02 100644
--- a/tests/extension/commands/index.test.ts
+++ b/tests/extension/commands/index.test.ts
@@ -13,7 +13,8 @@ import {
   copyCellToClipboard, 
   runCLICommand, 
   openAsRunmeNotebook, 
-  openSplitViewAsMarkdownText } from '../../../src/extension/commands'
+  openSplitViewAsMarkdownText,
+  stopBackgroundTask } from '../../../src/extension/commands'
 import { getTerminalByCell } from '../../../src/extension/utils'
 import { CliProvider } from '../../../src/extension/provider/cli'
 
@@ -78,3 +79,15 @@ test('open Runme notebook in text editor', (file: TextDocument) => {
   openSplitViewAsMarkdownText(file)
   expect(window.showTextDocument).toBeCalledWith(file, {viewColumn: ViewColumn.Beside})
 })
+
+test('stopBackgroundTask if terminal exists', () => {
+  vi.mocked(getTerminalByCell).mockReturnValue({ dispose: vi.fn() } as any)
+  stopBackgroundTask({} as any)
+  expect(window.showInformationMessage).toBeCalledTimes(1)
+})
+
+test('stopBackgroundTask if terminal does not exist', () => {
+  vi.mocked(getTerminalByCell).mockReturnValue(undefined)
+  stopBackgroundTask({} as any)
+  expect(window.showWarningMessage).toBeCalledTimes(1)
+})
diff --git a/tests/extension/extension.test.ts b/tests/extension/extension.test.ts
index 67d72044..4b1f7bfe 100644
--- a/tests/extension/extension.test.ts
+++ b/tests/extension/extension.test.ts
@@ -12,5 +12,5 @@ test('initialises all providers', async () => {
   await ext.initialise(context)
   expect(notebooks.registerNotebookCellStatusBarItemProvider).toBeCalledTimes(5)
   expect(workspace.registerNotebookSerializer).toBeCalledTimes(1)
-  expect(commands.registerCommand).toBeCalledTimes(6)
+  expect(commands.registerCommand).toBeCalledTimes(7)
 })
diff --git a/tests/extension/provider/background.test.ts b/tests/extension/provider/background.test.ts
index 23e00e85..9d171807 100644
--- a/tests/extension/provider/background.test.ts
+++ b/tests/extension/provider/background.test.ts
@@ -1,7 +1,11 @@
 import { vi, describe, expect, beforeEach, it } from 'vitest'
 
 import { getExecutionProperty, getTerminalByCell } from '../../../src/extension/utils'
-import { ShowTerminalProvider, BackgroundTaskProvider } from '../../../src/extension/provider/background'
+import { 
+  ShowTerminalProvider, 
+  BackgroundTaskProvider, 
+  StopBackgroundTaskProvider 
+} from '../../../src/extension/provider/background'
 
 vi.mock('vscode', () => ({
   default: {
@@ -49,7 +53,7 @@ describe('ShowTerminalProvider', () => {
     const p = new ShowTerminalProvider()
     const item = await p.provideCellStatusBarItems('cell' as any)
     expect(item).toEqual({
-      label: '$(terminal) Open Terminal',
+      label: '$(terminal) Open Terminal (PID: 123)',
       command: 'runme.openTerminal',
       position: 'right'
     })
@@ -91,3 +95,52 @@ describe('BackgroundTaskProvider', () => {
     })
   })
 })
+
+describe('StopBackgroundTaskProvider', () => {
+  const cell: any = {
+    metadata: { attributes: { background: undefined } },
+    executionSummary: { success: undefined }
+  }
+
+  beforeEach(() => {
+    vi.mocked(getExecutionProperty).mockClear()
+    vi.mocked(getTerminalByCell).mockClear()
+  })
+
+  it('dont show bg task label if cell is non a background task', async () => {
+    const p = new StopBackgroundTaskProvider()
+    expect(await p.provideCellStatusBarItems(cell as any)).toBe(undefined)
+    expect(getExecutionProperty).toBeCalledTimes(0)
+  })
+
+  it('dont show bg task label if cell is non interactive', async () => {
+    cell.metadata.attributes.background = 'true'
+    vi.mocked(getExecutionProperty).mockReturnValueOnce(false)
+    const p = new StopBackgroundTaskProvider()
+    expect(await p.provideCellStatusBarItems(cell as any)).toBe(undefined)
+    expect(getExecutionProperty).toBeCalledTimes(1)
+    expect(getExecutionProperty).toBeCalledWith('interactive', cell)
+  })
+
+  it('dont show if cell was not yet executed', async () => {
+    cell.metadata.attributes.background = 'true'
+    cell.executionSummary.success = false
+    const p = new StopBackgroundTaskProvider()
+    expect(await p.provideCellStatusBarItems(cell as any)).toBe(undefined)
+    expect(getExecutionProperty).toBeCalledTimes(1)
+    expect(cell.executionSummary.success).toBe(false)
+  })
+
+  it('return with button to close', async () => {
+    cell.metadata.attributes.background = 'true'
+    cell.executionSummary.success = true
+    vi.mocked(getExecutionProperty).mockReturnValueOnce(true)
+    const p = new StopBackgroundTaskProvider()
+    const item = await p.provideCellStatusBarItems(cell)
+    expect(item).toEqual({
+      label: '$(circle-slash) Stop Task',
+      position: 'right',
+      command: 'runme.stopBackgroundTask'
+    })
+  })
+})