Repository: stateful/vscode-runme
Pull request #380 (2023-04-04 22:59:20)

Support multiple terminal windows in Notebook Terminal
Supports multiple terminal windows in the notebook terminal. Distinguishes an "active" terminal, which is the one that controls PTY rows/cols.

I turned the `examples/README.md` script which gives rows/cols into a watch script. That way, you can see the effect of multiple terminal windows more easily.

The behavior is as follows:

 - If notebook terminal exists, it will use that size by default
   - When you switch to the integrated terminal, it will then switch to the terminal size
   - To switch back to the notebook terminal's sizing, you have to focus it in the notebook (by e.g. left clicking on it)
 - If no notebook terminal exists, it will use integrated terminal sizing
 - If no integrated terminal exists at all (non interactive old output), program should run normally

Note that I have discovered some weird behavior with the watch script and the native integrated terminal. I can file this as a separate issue, since I'm pretty sure it's an unrelated issue.


Patch:
diff --git a/examples/README.md b/examples/README.md
index 06cc7c7f..2f8e15e9 100644
--- a/examples/README.md
+++ b/examples/README.md
@@ -140,7 +140,9 @@ curl https://lever-client-logos.s3.us-west-2.amazonaws.com/a8ff9b1f-f313-4632-b9
 
 ## Terminal Dimensions
 
-```sh { closeTerminalOnSuccess=false }
-echo Rows: $(tput lines)
-echo Columns: $(tput cols)
+```sh { background=true closeTerminalOnSuccess=false }
+watch -n 0.1 "
+echo Rows: \$(tput lines)
+echo Columns: \$(tput cols)
+"
 ```
diff --git a/package-lock.json b/package-lock.json
index 09374942..c8b72f3a 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "runme",
-  "version": "1.0.5",
+  "version": "1.0.6-edge.0",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "runme",
-      "version": "1.0.5",
+      "version": "1.0.6-edge.0",
       "hasInstallScript": true,
       "license": "Apache-2.0",
       "dependencies": {
diff --git a/package.json b/package.json
index 86686e5a..0f3f56c9 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "runme",
-  "version": "1.0.5",
+  "version": "1.0.6-edge.0",
   "publisher": "stateful",
   "displayName": "Runme",
   "description": "Run your README.md as an interactive notebook",
diff --git a/src/client/components/terminal.ts b/src/client/components/terminal.ts
index 4e334d76..22760f85 100644
--- a/src/client/components/terminal.ts
+++ b/src/client/components/terminal.ts
@@ -1,12 +1,12 @@
 import { LitElement, css, html, PropertyValues, unsafeCSS } from 'lit'
 import { customElement, property } from 'lit/decorators.js'
-import { Disposable } from 'vscode'
+import { Disposable, TerminalDimensions } from 'vscode'
 import { ITheme, Terminal as XTermJS } from 'xterm'
 import { SerializeAddon } from 'xterm-addon-serialize'
 import { Unicode11Addon } from 'xterm-addon-unicode11'
 import { WebLinksAddon } from 'xterm-addon-web-links'
 
-import { FitAddon } from '../fitAddon'
+import { FitAddon, type ITerminalDimensions } from '../fitAddon'
 import { ClientMessages } from '../../constants'
 import { getContext } from '../utils'
 import { onClientMessage, postClientMessage } from '../../utils/messaging'
@@ -352,15 +352,22 @@ export class TerminalView extends LitElement {
     super.firstUpdated(props)
     const terminalContainer = this.#getTerminalElement() as HTMLElement
 
-    window.addEventListener('focus', () => {
-      this.terminal?.focus()
-    })
+    window.addEventListener('focus', () => { this.#onFocusWindow() })
+    window.addEventListener('click', () => { this.#onFocusWindow(false) })
 
     this.terminal!.open(terminalContainer)
     this.terminal!.focus()
     this.fitAddon?.fit()
     this.#updateTerminalTheme()
 
+    const ctx = getContext()
+    ctx.postMessage && postClientMessage(ctx, ClientMessages.terminalOpen, {
+      'runme.dev/uuid': this.uuid!,
+      terminalDimensions: convertXTermDimensions(
+        this.fitAddon?.proposeDimensions()
+      )
+    })
+
     if (this.lastLine) {
       this.terminal!.scrollToLine(this.lastLine)
     }
@@ -392,15 +399,44 @@ export class TerminalView extends LitElement {
     return getComputedStyle(terminalContainer!).getPropertyValue(variableName) ?? undefined
   }
 
-  #onResizeWindow(): void {
+  async #onResizeWindow(): Promise<void> {
+    if (!this.fitAddon) { return }
+
     const { innerWidth } = window
+
     // Prevent adjusting the terminal size if the width remains the same
-    if (Math.abs(this.windowSize.width - innerWidth) > Number.EPSILON) {
-      this.windowSize.width = innerWidth
-      this.fitAddon?.fit()
+    if (Math.abs(this.windowSize.width - innerWidth) <= Number.EPSILON) {
+      return
+    }
+
+    this.windowSize.width = innerWidth
+    this.fitAddon.fit()
+
+    const proposedDimensions = this.fitAddon.proposeDimensions()
+    if (proposedDimensions) {
+      const ctx = getContext()
+      if (!ctx.postMessage) { return }
+
+      await postClientMessage(ctx, ClientMessages.terminalResize, {
+        'runme.dev/uuid': this.uuid!,
+        terminalDimensions: convertXTermDimensions(proposedDimensions)
+      })
     }
   }
 
+  async #onFocusWindow(focusTerminal = true): Promise<void> {
+    if (focusTerminal) {
+      this.terminal?.focus()
+    }
+
+    const ctx = getContext()
+    if (!ctx.postMessage) { return }
+
+    await postClientMessage(ctx, ClientMessages.terminalFocus, {
+      'runme.dev/uuid': this.uuid!
+    })
+  }
+
   #onWebLinkClick(event: MouseEvent, uri: string): void {
     postClientMessage(getContext(), ClientMessages.openLink, uri)
   }
@@ -442,3 +478,13 @@ export class TerminalView extends LitElement {
     )
   }
 }
+
+function convertXTermDimensions(dimensions: ITerminalDimensions): TerminalDimensions
+function convertXTermDimensions(dimensions: undefined): undefined
+function convertXTermDimensions(dimensions: ITerminalDimensions|undefined): TerminalDimensions|undefined
+function convertXTermDimensions(dimensions?: ITerminalDimensions): TerminalDimensions|undefined {
+  if (!dimensions) { return undefined }
+
+  const { rows, cols } = dimensions
+  return { columns: cols, rows }
+}
diff --git a/src/client/fitAddon.ts b/src/client/fitAddon.ts
index abf445a5..8e330ea3 100644
--- a/src/client/fitAddon.ts
+++ b/src/client/fitAddon.ts
@@ -35,7 +35,7 @@ interface IRenderDimensions {
   }
 }
 
-interface ITerminalDimensions {
+export interface ITerminalDimensions {
   /**
    * The number of rows in the terminal.
    */
diff --git a/src/constants.ts b/src/constants.ts
index d07e3f60..aadb6af1 100644
--- a/src/constants.ts
+++ b/src/constants.ts
@@ -23,8 +23,11 @@ export enum ClientMessages {
   terminalStdout = 'terminal:stdout',
   terminalStderr = 'terminal:stderr',
   terminalStdin = 'terminal:stdin',
-  activeThemeChanged = 'theme:changed',
-  openLink = 'terminal:openLink'
+  terminalResize = 'terminal:resize',
+  terminalFocus = 'terminal:focus',
+  terminalOpen = 'terminal:open',
+  openLink = 'terminal:openLink',
+  activeThemeChanged = 'theme:changed'
 }
 
 // [pretty print, languageId, destination]
diff --git a/src/extension/executors/runner.ts b/src/extension/executors/runner.ts
index 5e08e06e..212660ba 100644
--- a/src/extension/executors/runner.ts
+++ b/src/extension/executors/runner.ts
@@ -118,16 +118,40 @@ export async function executeRunner(
   messaging.onDidReceiveMessage(({ message }: { message: ClientMessage<ClientMessages> }) => {
     const { type, output } = message
 
-    if (type !== ClientMessages.terminalStdin) { return }
+    if (typeof output === 'object' && 'runme.dev/uuid' in output) {
+      const uuid = output['runme.dev/uuid']
+      if (uuid !== cellUUID) { return }
+    }
+
+    switch (type) {
+      case ClientMessages.terminalStdin: {
+        const { input } = output
 
-    const { 'runme.dev/uuid': uuid, input } = output
+        program.handleInput(input)
+        terminalState?.input(input, true)
+      } break
 
-    if (uuid !== cellUUID) { return }
+      case ClientMessages.terminalFocus: {
+        program.setActiveTerminalWindow('notebook')
+      } break
 
-    program.handleInput(input)
-    terminalState?.input(input, true)
+      case ClientMessages.terminalResize: {
+        const { terminalDimensions } = output
+        program.setDimensions(terminalDimensions, 'notebook')
+      } break
+
+      case ClientMessages.terminalOpen: {
+        const { terminalDimensions } = output
+        program.open(terminalDimensions, 'notebook')
+      } break
+    }
   })
 
+  if (interactive) {
+    program.registerTerminalWindow('vscode')
+    await program.setActiveTerminalWindow('vscode')
+  }
+
   let revealNotebookTerminal = isNotebookTerminalEnabledForCell(exec.cell)
 
   if (revealNotebookTerminal) {
@@ -140,8 +164,10 @@ export async function executeRunner(
     } else {
       revealNotebookTerminal = false
     }
-  }
 
+    program.registerTerminalWindow('notebook')
+    await program.setActiveTerminalWindow('notebook')
+  }
 
   if (!interactive) {
     const output: Buffer[] = []
@@ -182,8 +208,6 @@ export async function executeRunner(
     exec.token.onCancellationRequested(() => {
       program.close()
     })
-
-    await program.run()
   } else {
     const taskExecution = new Task(
       { type: 'shell', name: `Runme Task (${RUNME_ID})` },
@@ -245,6 +269,10 @@ export async function executeRunner(
     })
   }
 
+  if (program.numTerminalWindows === 0) {
+    await program.run()
+  }
+
   return await new Promise<boolean>((resolve, reject) => {
     program.onDidClose((code) => {
       resolve(code === 0)
diff --git a/src/extension/kernel.ts b/src/extension/kernel.ts
index 29b6cbc4..71e5ecdb 100644
--- a/src/extension/kernel.ts
+++ b/src/extension/kernel.ts
@@ -239,9 +239,7 @@ export class Kernel implements Disposable {
     } else if (message.type === ClientMessages.openLink) {
       return env.openExternal(Uri.parse(message.output))
     } else if (
-      message.type === ClientMessages.terminalStdin ||
-      message.type === ClientMessages.terminalStdout ||
-      message.type === ClientMessages.terminalStderr
+      message.type.startsWith('terminal:')
     ) {
       return
     }
diff --git a/src/extension/runner.ts b/src/extension/runner.ts
index 256432f8..44e12314 100644
--- a/src/extension/runner.ts
+++ b/src/extension/runner.ts
@@ -34,6 +34,8 @@ export type RunProgramExecution = {
   script: string
 }
 
+export type TerminalWindow = 'vscode'|'notebook'
+
 export interface RunProgramOptions {
   programName: string
   args?: string[]
@@ -113,11 +115,36 @@ export interface IRunnerProgramSession extends IRunnerChild, Pseudoterminal {
    */
   readonly onStderrRaw: Event<Uint8Array>
 
+  /**
+   * Number of registered terminal windows
+   */
+  readonly numTerminalWindows: number
+
   handleInput(message: string): Promise<void>
 
   setRunOptions(opts: RunProgramOptions): void
   run(): Promise<void>
   hasExited(): RunnerExitReason|undefined
+
+  setDimensions(dimensions: TerminalDimensions, terminalWindow?: TerminalWindow): void|Promise<void>
+
+  /**
+   * Register terminal window for usage with this program
+   *
+   * When windows are registered, the program will be run after all registered
+   * windows have made a call to `open`
+   *
+   * @param window Type of terminal window
+   */
+  registerTerminalWindow(window: TerminalWindow, initialDimensions?: TerminalDimensions): void
+
+  /**
+   * The "active" terminal window is the one whose dimensions are used for the
+   * underlying program sesssion's shell process
+   */
+  setActiveTerminalWindow(window: TerminalWindow): Promise<void>
+
+  open(initialDimensions?: TerminalDimensions, terminalWindow?: TerminalWindow): void|Promise<void>
 }
 
 export class GrpcRunner implements IRunner {
@@ -291,6 +318,16 @@ export class GrpcRunner implements IRunner {
   }
 }
 
+interface TerminalWindowState {
+  dimensions?: TerminalDimensions
+  opened: boolean
+  /**
+   * Used in VSCode to determine if this is part of the initial call to
+   * `setDimensions`, which generally should be ignored
+   */
+  hasSetDimensions?: boolean
+}
+
 export class GrpcRunnerProgramSession implements IRunnerProgramSession {
   private disposables: Disposable[] = []
 
@@ -318,6 +355,9 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession {
 
   protected buffer = ''
 
+  protected activeTerminalWindow?: TerminalWindow
+  protected terminalWindows = new Map<TerminalWindow, TerminalWindowState>()
+
   constructor(
     private readonly client: RunnerServiceClient,
     protected opts: RunProgramOptions
@@ -461,7 +501,47 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession {
     }))
   }
 
-  open(initialDimensions?: TerminalDimensions): void {
+  registerTerminalWindow(window: TerminalWindow, initialDimensions?: TerminalDimensions): void {
+    this.terminalWindows.set(window, {
+      dimensions: initialDimensions,
+      opened: false,
+    })
+  }
+
+  private _setActiveTerminalWindow(window: TerminalWindow): void {
+    this.activeTerminalWindow = window
+  }
+
+  async setActiveTerminalWindow(window: TerminalWindow): Promise<void> {
+    const terminalWindowState = this.terminalWindows.get(window)
+
+    if(!terminalWindowState) {
+      console.error(`Attempted to set active terminal window to unregistered window '${window}'`)
+      return
+    }
+
+    this._setActiveTerminalWindow(window)
+
+    if (terminalWindowState.dimensions && this.initialized) {
+      await this.setDimensions(terminalWindowState.dimensions, window)
+    }
+  }
+
+  open(initialDimensions?: TerminalDimensions, terminalWindow: TerminalWindow = 'vscode'): void {
+    const terminalWindowState = this.terminalWindows.get(terminalWindow)
+
+    if (!terminalWindowState) {
+      console.error(`Attempted to open unregistered terminal window '${terminalWindow}'!`)
+      return
+    }
+
+    if (terminalWindowState.opened) {
+      console.warn(`Attempted to open terminal window '${terminalWindow}' that has already opened!`)
+      return
+    }
+
+    terminalWindowState.opened = true
+
     // Workaround to force terminal to close if opened after early exit
     // TODO(mxs): find a better solution here
     if(this.hasExited()) {
@@ -469,10 +549,18 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession {
       return
     }
 
-    this.opts.terminalDimensions = initialDimensions
+    if (initialDimensions) {
+      terminalWindowState.dimensions = initialDimensions
+    }
+
+    if (terminalWindow === this.activeTerminalWindow) {
+      this.opts.terminalDimensions = terminalWindowState.dimensions
+    }
 
-    // in pty, we wait for open to run
-    this.run()
+    if ([ ...this.terminalWindows.values() ].every(({ opened }) => opened)) {
+      // in pty, we wait for open to run
+      this.run()
+    }
   }
 
   async dispose() {
@@ -519,10 +607,32 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession {
     this.dispose()
   }
 
-  setDimensions(dimensions: TerminalDimensions): void {
-    this.session.requests.send(ExecuteRequest.create({
-      winsize: terminalDimensionsToWinsize(dimensions)
-    }))
+  async setDimensions(dimensions: TerminalDimensions, terminalWindow: TerminalWindow = 'vscode'): Promise<void> {
+    const terminalWindowState = this.terminalWindows.get(terminalWindow)
+
+    if (!terminalWindowState) {
+      throw new Error(`Tried to set dimensions for unregistered terminal window ${terminalWindow}`)
+    }
+
+    // removed this functionality in favor of preferring notebook terminal
+    //
+    // if(terminalWindow === 'vscode' && this.initialized) {
+    //   if (terminalWindowState.hasSetDimensions) {
+    //     // VSCode terminal window calls `setDimensions` only when focused - this
+    //     // can be conveniently used to set the active window to the terminal
+    //     this._setActiveTerminalWindow(terminalWindow)
+    //   } else {
+    //     terminalWindowState.hasSetDimensions = true
+    //   }
+    // }
+
+    terminalWindowState.dimensions = dimensions
+
+    if (this.activeTerminalWindow === terminalWindow && this.initialized) {
+      await this.session.requests.send(ExecuteRequest.create({
+        winsize: terminalDimensionsToWinsize(dimensions)
+      }))
+    }
   }
 
   hasExited() {
@@ -553,6 +663,10 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession {
       ...terminalDimensions && { winsize: terminalDimensionsToWinsize(terminalDimensions) },
     })
   }
+
+  get numTerminalWindows() {
+    return this.terminalWindows.size
+  }
 }
 
 export class GrpcRunnerEnvironment implements IRunnerEnvironment {
diff --git a/src/types.ts b/src/types.ts
index 7d506c63..d91f6e12 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -1,4 +1,4 @@
-import { NotebookCellKind, TaskDefinition } from 'vscode'
+import { NotebookCellKind, TaskDefinition, TerminalDimensions } from 'vscode'
 import { z } from 'zod'
 
 import { OutputType, ClientMessages } from './constants'
@@ -109,6 +109,15 @@ export interface ClientMessagePayload {
     ['runme.dev/uuid']: string
     input: string
   }
+  [ClientMessages.terminalFocus]: { ['runme.dev/uuid']: string }
+  [ClientMessages.terminalResize]: {
+    ['runme.dev/uuid']: string
+    terminalDimensions: TerminalDimensions
+  }
+  [ClientMessages.terminalOpen]: {
+    ['runme.dev/uuid']: string
+    terminalDimensions?: TerminalDimensions
+  }
   [ClientMessages.activeThemeChanged]: string
   [ClientMessages.openLink]: string
 }


Test patch:
diff --git a/tests/extension/runner.test.ts b/tests/extension/runner.test.ts
index 51bdab76..d25e7197 100644
--- a/tests/extension/runner.test.ts
+++ b/tests/extension/runner.test.ts
@@ -263,6 +263,9 @@ suite('grpc Runner', () => {
         tty: true
       })
 
+      session.registerTerminalWindow('vscode')
+      session.setActiveTerminalWindow('vscode')
+
       session.open({ columns: 50, rows: 20 })
 
       expect(duplex.requests.send).toBeCalledTimes(1)
@@ -280,8 +283,10 @@ suite('grpc Runner', () => {
         tty: true
       })
 
-      session.open({ columns: 50, rows: 20 })
+      session.registerTerminalWindow('vscode')
+      session.setActiveTerminalWindow('vscode')
 
+      session.open({ columns: 50, rows: 20 })
       session.setDimensions({ columns: 60, rows: 30 })
 
       expect(duplex.requests.send).toBeCalledTimes(2)
@@ -292,6 +297,138 @@ suite('grpc Runner', () => {
         }
       })
     })
+
+    test('requires all active windows to open before starting', async () => {
+      const { duplex, session } = await createNewSession({
+        tty: true
+      })
+
+      session.registerTerminalWindow('vscode')
+
+      session.registerTerminalWindow('notebook')
+      session.setActiveTerminalWindow('notebook')
+
+      session.open({ columns: 50, rows: 20 })
+      expect(duplex.requests.send).toBeCalledTimes(0)
+
+      session.open({ columns: 60, rows: 30 }, 'notebook')
+      expect(duplex.requests.send).toBeCalledTimes(1)
+      expect((duplex.requests.send.mock.calls[0] as any)[0]).toMatchObject({
+        tty: true,
+        winsize: {
+          cols: 60,
+          rows: 30,
+        }
+      })
+      duplex.requests.send.mockClear()
+
+      session.setDimensions({ columns: 70, rows: 40 })
+      expect(duplex.requests.send).toBeCalledTimes(0)
+
+      duplex.requests.send.mockClear()
+
+      session.setDimensions({ columns: 80, rows: 50 }, 'notebook')
+      expect(duplex.requests.send).toBeCalledTimes(1)
+      expect((duplex.requests.send.mock.calls[0] as any)[0]).toStrictEqual({
+        winsize: {
+          cols: 80,
+          rows: 50,
+        }
+      })
+      duplex.requests.send.mockClear()
+
+      await session.setActiveTerminalWindow('vscode')
+      expect(duplex.requests.send).toBeCalledTimes(1)
+      expect(((duplex.requests.send.mock.calls[0]) as any)[0]).toStrictEqual({
+        winsize: { cols: 70, rows: 40 }
+      })
+    })
+
+    test('active window does not send dimensions if program is uninitialized', async () => {
+      const { duplex, session } = await createNewSession({
+        tty: true
+      })
+
+      session.registerTerminalWindow('vscode', { rows: 40, columns: 40 })
+      session.registerTerminalWindow('notebook', { rows: 50, columns: 50 })
+
+      await session.setActiveTerminalWindow('vscode')
+      expect(duplex.requests.send).toBeCalledTimes(0)
+      duplex.requests.send.mockClear()
+
+      session.open(undefined, 'vscode')
+      session.open(undefined, 'notebook')
+
+      expect(duplex.requests.send).toBeCalledTimes(1)
+      expect((duplex.requests.send.mock.calls[0] as any)[0]).toMatchObject({
+        tty: true,
+        winsize: {
+          cols: 40,
+          rows: 40,
+        }
+      })
+      duplex.requests.send.mockClear()
+
+      await session.setActiveTerminalWindow('notebook')
+      expect((duplex.requests.send.mock.calls[0] as any)[0]).toStrictEqual({
+        winsize: {
+          cols: 50,
+          rows: 50,
+        }
+      })
+      duplex.requests.send.mockClear()
+    })
+
+    test('cannot set dimensions of unregistered terminal window', async () => {
+      const { session } = await createNewSession({
+        tty: true
+      })
+
+      expect( session.setDimensions({ columns: 10, rows: 10 }, 'vscode'))
+        .rejects.toThrowError('Tried to set dimensions for unregistered terminal window vscode')
+    })
+
+    test('cannot set active terminal to unregistered window', async () => {
+      const { session } = await createNewSession({
+        tty: true
+      })
+
+      session.registerTerminalWindow('vscode')
+      session.setActiveTerminalWindow('vscode')
+
+      const consoleError = vi.spyOn(console, 'error')
+      session.setActiveTerminalWindow('notebook')
+
+      expect(consoleError).toBeCalledWith('Attempted to set active terminal window to unregistered window \'notebook\'')
+    })
+
+    test('cannot open unregistered terminal window', async () => {
+      const { session } = await createNewSession({
+        tty: true
+      })
+
+      const consoleError = vi.spyOn(console, 'error')
+      session.open(undefined, 'notebook')
+
+      expect(consoleError).toBeCalledWith('Attempted to open unregistered terminal window \'notebook\'!')
+      expect(session['terminalWindows'].get('notebook')?.opened).toBeUndefined()
+    })
+
+    test('cannot open terminal window twice', async () => {
+      const { session } = await createNewSession({
+        tty: true
+      })
+
+      session.registerTerminalWindow('notebook')
+
+      const consoleWarn = vi.spyOn(console, 'warn')
+      session.open(undefined, 'notebook')
+      expect(consoleWarn).toBeCalledTimes(0)
+
+      session.open(undefined, 'notebook')
+
+      expect(consoleWarn).toBeCalledWith('Attempted to open terminal window \'notebook\' that has already opened!')
+    })
   })
 })