diff --git a/src/client/components/configuration/annotations.ts b/src/client/components/configuration/annotations.ts index ea6c201dd..d2b489752 100644 --- a/src/client/components/configuration/annotations.ts +++ b/src/client/components/configuration/annotations.ts @@ -57,7 +57,10 @@ export class Annotations extends LitElement { description: 'Run cell inside terminal to allow for interactive input.', }, closeTerminalOnSuccess: { - description: 'Hide terminal after cell successful execution.', + description: 'Hide terminal panel after cell successful execution.', + }, + openTerminalOnError: { + description: 'open terminal panel after cell execution error.', }, promptEnv: { description: 'Prompt user input for exported environment variables.', diff --git a/src/extension/cell.ts b/src/extension/cell.ts index d296043bd..943d2ff24 100644 --- a/src/extension/cell.ts +++ b/src/extension/cell.ts @@ -224,7 +224,6 @@ export class NotebookCellOutputManager { return new NotebookCellOutput([NotebookCellOutputItem.json(json, OutputType.vercel)]) } - case OutputType.outputItems: case OutputType.terminal: { const terminalState = this.terminalState if (!terminalState) { @@ -276,24 +275,32 @@ export class NotebookCellOutputManager { 'runme.dev/id': cellId, }, ) - } else { - const terminalStateBase64 = terminalState.serialize() - const json: CellOutputPayload = { - type: OutputType.outputItems, - output: { - content: terminalStateBase64, - mime: 'text/plain', - id: cellId, - }, - } + } + } + + case OutputType.outputItems: { + const terminalState = this.terminalState + if (!terminalState) { + return + } - return new NotebookCellOutput([ - NotebookCellOutputItem.json(json, OutputType.outputItems), - NotebookCellOutputItem.stdout( - Buffer.from(terminalStateBase64, 'base64').toString('utf-8'), - ), - ]) + const { 'runme.dev/id': cellId } = getAnnotations(cell) + const terminalStateBase64 = terminalState.serialize() + const json: CellOutputPayload = { + type: OutputType.outputItems, + output: { + content: terminalStateBase64, + mime: 'text/plain', + id: cellId!, + }, } + + return new NotebookCellOutput([ + NotebookCellOutputItem.json(json, OutputType.outputItems), + NotebookCellOutputItem.stdout( + Buffer.from(terminalStateBase64, 'base64').toString('utf-8'), + ), + ]) } case OutputType.github: { @@ -631,7 +638,7 @@ export class NotebookCellOutputManager { const terminalOutput = this.terminalState?.outputType if (terminalOutput) { - this.terminalEnabled = this.hasOutputTypeUnsafe(terminalOutput) + this.terminalEnabled = this.hasOutputTypeUnsafe(OutputType.terminal) } if (!((await mutater?.()) ?? true)) { diff --git a/src/extension/executors/runner/index.ts b/src/extension/executors/runner/index.ts index 8143ac038..6e82893b7 100644 --- a/src/extension/executors/runner/index.ts +++ b/src/extension/executors/runner/index.ts @@ -10,7 +10,17 @@ import { TextDocument, window, } from 'vscode' -import { Subject, debounceTime } from 'rxjs' +import { + Observable, + debounceTime, + map, + filter, + from, + scan, + withLatestFrom, + Subscription, + takeLast, +} from 'rxjs' import { RpcError } from '@protobuf-ts/runtime-rpc' import getLogger from '../../logger' @@ -32,7 +42,7 @@ import { } from '../../../utils/configuration' import { ITerminalState } from '../../terminal/terminalState' import { toggleTerminal } from '../../commands' -import { closeTerminalByEnvID } from '../task' +import { closeTerminalByEnvID, openTerminalByEnvID } from '../task' import { getCellProgram, getNotebookSkipPromptEnvSetting, @@ -86,6 +96,7 @@ export const executeRunner: IKernelRunner = async ({ mimeType: cellMimeType, background, closeTerminalOnSuccess, + openTerminalOnError, } = getAnnotations(exec.cell) // enforce background tasks as singleton instances // to do this, @@ -131,13 +142,24 @@ export const executeRunner: IKernelRunner = async ({ let terminalState: ITerminalState | undefined - const writeToTerminalStdout = (data: string | Uint8Array) => { - postClientMessage(messaging, ClientMessages.terminalStdout, { - 'runme.dev/id': cellId, - data, - }) + let writeToTerminalStdout: (data: string | Uint8Array) => void - terminalState?.write(data) + if (interactive) { + // receives both stdout+stderr via tty + writeToTerminalStdout = (data: string | Uint8Array) => { + postClientMessage(messaging, ClientMessages.terminalStdout, { + 'runme.dev/id': cellId, + data, + }) + + terminalState?.write(data) + } + program.onDidWrite(writeToTerminalStdout) + } else { + writeToTerminalStdout = (data: string | Uint8Array) => { + terminalState?.write(data) + } + program.onStdoutRaw(writeToTerminalStdout) } program.onDidErr((data) => @@ -209,14 +231,10 @@ export const executeRunner: IKernelRunner = async ({ writeToTerminalStdout(`\x1B[7m * \x1B[0m ${text}`) }) - program.onDidWrite(writeToTerminalStdout) - - if (interactive) { - program.registerTerminalWindow('vscode') - await program.setActiveTerminalWindow('vscode') - } + program.registerTerminalWindow('vscode') + await program.setActiveTerminalWindow('vscode') - let revealNotebookTerminal = isNotebookTerminalEnabledForCell(exec.cell) + const revealNotebookTerminal = isNotebookTerminalEnabledForCell(exec.cell) terminalState = await kernel.registerCellTerminalState( exec.cell, @@ -230,142 +248,158 @@ export const executeRunner: IKernelRunner = async ({ await program.setActiveTerminalWindow('notebook') } - const t = OutputType[execKey as keyof typeof OutputType] - if (t) { - outputs.showOutput(t) - } - await outputs.showTerminal() } else { - const output: Buffer[] = [] - const outputItems$ = new Subject() - - // adapted from `shellExecutor` in `shell.ts` - const _handleOutput = async (data: Uint8Array) => { - mimeType = mimeType || (await program.mimeType) || CELL_MIME_TYPE_DEFAULT - output.push(Buffer.from(data)) - if (MIME_TYPES_WITH_CUSTOM_RENDERERS.includes(mimeType)) { - outputItems$.complete() - return - } - - const item = new NotebookCellOutputItem(Buffer.concat(output), mimeType) - outputItems$.next(item) - } + const mime = program.mimeType.then((mime) => mimeType || mime || CELL_MIME_TYPE_DEFAULT) + const mime$ = from(mime) + const raw$ = new Observable((observer) => { + program.onStdoutRaw((data) => observer.next(data)) + program.onDidClose(() => observer.complete()) + }).pipe( + scan((acc, data) => { + const combined = new Uint8Array(acc.length + data.length) + combined.set(acc) + combined.set(data, acc.length) + return combined + }, new Uint8Array()), + ) // debounce by 0.5s because human preception likely isn't as fast - const sub = outputItems$.pipe(debounceTime(500)).subscribe({ - next: (item) => outputs.replaceOutputs([new NotebookCellOutput([item])]), - complete: async () => { - const isCustomRenderer = MIME_TYPES_WITH_CUSTOM_RENDERERS.includes(mimeType || '') - return isCustomRenderer && (await outputs.showTerminal()) - }, - }) - context.subscriptions.push({ dispose: () => sub.unsubscribe() }) + let item$ = raw$.pipe(debounceTime(500)).pipe( + withLatestFrom(mime$), + map(([item, mime]) => new NotebookCellOutputItem(Buffer.from(item), mime)), + ) + + const isCustomMime = (mime: string) => { + // todo(sebastian): should we take execKey into account? + // const t = OutputType[execKey as keyof typeof OutputType] + // if (t) { + // await outputs.showOutput(t) + // } + return MIME_TYPES_WITH_CUSTOM_RENDERERS.includes(mime) + } - program.onStdoutRaw(_handleOutput) - program.onStderrRaw(_handleOutput) - program.onDidClose(() => outputItems$.complete()) + let subs: Subscription[] = [ + // render vanilla mime types, eg PNG/SVG + item$ + .pipe( + filter((item) => { + return !isCustomMime(item.mime) + }), + ) + .subscribe({ + next: (item) => outputs.replaceOutputs([new NotebookCellOutput([item])]), + }), + // render custom mime type for text/plain to show copy buttons etc + item$ + .pipe( + filter((item) => { + return isCustomMime(item.mime) + }), + takeLast(1), + ) + .subscribe({ + next: () => outputs.showOutput(OutputType.outputItems), + }), + ] + + context.subscriptions.push({ dispose: () => subs.forEach((s) => s.unsubscribe()) }) } - if (!interactive) { - exec.token.onCancellationRequested(() => { - program.close() - }) - } else { - await outputs.replaceOutputs([]) - - const cellText = runningCell.getText() - const RUNME_ID = getCellRunmeId(exec.cell) - const taskExecution = new Task( - { type: 'shell', name: `Runme Task (${RUNME_ID})` }, - TaskScope.Workspace, - (cellText.length > LABEL_LIMIT ? `${cellText.slice(0, LABEL_LIMIT)}...` : cellText) + - ` (RUNME_ID: ${RUNME_ID})`, - 'exec', - new CustomExecution(async () => program), - ) + const cellText = runningCell.getText() + const RUNME_ID = getCellRunmeId(exec.cell) + const taskExecution = new Task( + { type: 'shell', name: `Runme Task (${RUNME_ID})` }, + TaskScope.Workspace, + (cellText.length > LABEL_LIMIT ? `${cellText.slice(0, LABEL_LIMIT)}...` : cellText) + + ` (RUNME_ID: ${RUNME_ID})`, + 'exec', + new CustomExecution(async () => program), + ) - taskExecution.isBackground = background - taskExecution.presentationOptions = { - focus: revealNotebookTerminal ? false : true, - reveal: revealNotebookTerminal - ? TaskRevealKind.Never - : background - ? TaskRevealKind.Never - : TaskRevealKind.Always, - panel: background ? TaskPanelKind.Dedicated : TaskPanelKind.Shared, - } + taskExecution.isBackground = background + taskExecution.presentationOptions = { + focus: false, + reveal: TaskRevealKind.Silent, + panel: background ? TaskPanelKind.Dedicated : TaskPanelKind.Shared, + } - const execution = await tasks.executeTask(taskExecution) + const execution = await tasks.executeTask(taskExecution) - context.subscriptions.push({ - dispose: () => execution.terminate(), - }) + context.subscriptions.push({ + dispose: () => execution.terminate(), + }) - exec.token.onCancellationRequested(() => { - try { - // runs `program.close()` implicitly - execution.terminate() - } catch (err: any) { - log.error(`Failed to terminate task: ${(err as Error).message}`) - throw new Error(err) - } - }) + exec.token.onCancellationRequested(() => { + try { + // runs `program.close()` implicitly + execution.terminate() + } catch (err: any) { + log.error(`Failed to terminate task: ${(err as Error).message}`) + throw new Error(err) + } + }) - tasks.onDidStartTaskProcess((e) => { - const taskId = (e.execution as any)['_id'] - const executionId = (execution as any)['_id'] + tasks.onDidStartTaskProcess((e) => { + const taskId = (e.execution as any)['_id'] + const executionId = (execution as any)['_id'] - if (taskId !== executionId) { - return - } + if (taskId !== executionId) { + return + } - const terminal = getTerminalByCell(exec.cell) - if (!terminal) { - return - } + const terminal = getTerminalByCell(exec.cell) + if (!terminal) { + return + } - terminal.runnerSession = program - kernel.registerTerminal(terminal, executionId, RUNME_ID) + terminal.runnerSession = program + kernel.registerTerminal(terminal, executionId, RUNME_ID) - // proxy pid value - Object.defineProperty(terminal, 'processId', { - get: function () { - return program.pid - }, - }) + // proxy pid value + Object.defineProperty(terminal, 'processId', { + get: function () { + return program.pid + }, }) + }) - tasks.onDidEndTaskProcess((e) => { - const taskId = (e.execution as any)['_id'] - const executionId = (execution as any)['_id'] + tasks.onDidEndTaskProcess((e) => { + const taskId = (e.execution as any)['_id'] + const executionId = (execution as any)['_id'] + /** + * ignore if + */ + if ( /** - * ignore if + * VS Code is running a different task */ - if ( - /** - * VS Code is running a different task - */ - taskId !== executionId || - /** - * we don't have an exit code - */ - typeof e.exitCode === 'undefined' - ) { - return - } - + taskId !== executionId || /** - * only close terminal if execution passed and desired by user + * we don't have an exit code */ - const closeIt = getCloseTerminalOnSuccess() && closeTerminalOnSuccess - if (e.exitCode === 0 && closeIt && !background) { - closeTerminalByEnvID(RUNME_ID) - } - }) - } + typeof e.exitCode === 'undefined' + ) { + return + } + + /** + * only close terminal if execution passed and desired by user + */ + const closeIt = interactive && getCloseTerminalOnSuccess() && closeTerminalOnSuccess + if (e.exitCode === 0 && closeIt && !background) { + closeTerminalByEnvID(RUNME_ID) + } + + /** + * open non-interactive terminal if execution exited with non-zero + */ + const openIt = !interactive && openTerminalOnError + if (e.exitCode !== 0 && openIt && !background) { + openTerminalByEnvID(RUNME_ID) + } + }) if (program.numTerminalWindows === 0) { await program.run() diff --git a/src/extension/executors/task.ts b/src/extension/executors/task.ts index 12cd394c9..ab3d501e9 100644 --- a/src/extension/executors/task.ts +++ b/src/extension/executors/task.ts @@ -44,6 +44,13 @@ export function closeTerminalByEnvID(id: string, kill?: boolean) { } } +export function openTerminalByEnvID(id: string) { + const terminal = window.terminals.find((t) => getTerminalRunmeId(t) === id) + if (terminal) { + terminal.show() + } +} + export const taskExecutor: IKernelExecutor = async (executor) => { const { context, exec, doc } = executor const { interactive: isInteractive, promptEnv } = getAnnotations(exec.cell) diff --git a/src/extension/provider/background.ts b/src/extension/provider/background.ts index cc73d2990..2dac0d837 100644 --- a/src/extension/provider/background.ts +++ b/src/extension/provider/background.ts @@ -29,6 +29,7 @@ export class ToggleTerminalProvider async provideCellStatusBarItems( cell: vscode.NotebookCell, ): Promise { + const { interactive } = getAnnotations(cell) const terminalState = await this.kernel.getTerminalState(cell) if (!terminalState) { @@ -43,6 +44,14 @@ export class ToggleTerminalProvider ) item.command = 'runme.toggleTerminal' + if (!interactive) { + const terminal = getTerminalByCell(cell) + if (!terminal) { + return undefined + } + item.command = 'runme.openIntegratedTerminal' + } + return item } diff --git a/src/extension/runner/index.ts b/src/extension/runner/index.ts index 70d7051fa..0ace38e48 100644 --- a/src/extension/runner/index.ts +++ b/src/extension/runner/index.ts @@ -143,12 +143,12 @@ export interface IRunnerProgramSession extends IRunnerChild, Pseudoterminal { /** * Implementers should **still call `onDidWrite`** to stay compatible with - * VSCode pseudoterminal interface + * VS Code pseudoterminal interface */ readonly onStdoutRaw: Event /** * Implementers should **still call `onDidErr`** to stay compatible with - * VSCode pseudoterminal interface + * VS Code pseudoterminal interface */ readonly onStderrRaw: Event @@ -432,6 +432,11 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession { // TODO: web compat const stderr = Buffer.from(data).toString('utf-8') this._onDidErr.fire(stderr) + + // onDidErr is **not** part of VS Code's PTY interface + // for non-interactive we deliberately write stderr to the PTY + const yellowStderr = `\x1b[33m${stderr}\x1b[0m` + this._onDidWrite.fire(yellowStderr) }), ) @@ -660,7 +665,7 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession { /** * Manually closed by the user * - * Implemented for compatibility with VSCode's `Pseudoterminal` interface; + * Implemented for compatibility with VS Code's `Pseudoterminal` interface; * please use `_close` internally */ close() { @@ -700,7 +705,7 @@ export class GrpcRunnerProgramSession implements IRunnerProgramSession { // // if(terminalWindow === 'vscode' && this.initialized) { // if (terminalWindowState.hasSetDimensions) { - // // VSCode terminal window calls `setDimensions` only when focused - this + // // VS Code terminal window calls `setDimensions` only when focused - this // // can be conveniently used to set the active window to the terminal // this._setActiveTerminalWindow(terminalWindow) // } else { diff --git a/src/schema.ts b/src/schema.ts index a034943e6..081c92da2 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -54,6 +54,7 @@ export const AnnotationSchema = { background: boolify(false), interactive: boolify(true), closeTerminalOnSuccess: boolify(true), + openTerminalOnError: boolify(true), promptEnv: z.preprocess((subject) => { if (typeof subject === 'string') { subject = cleanAnnotation(subject, ',') @@ -117,6 +118,7 @@ export const SafeCellAnnotationsSchema = z.object({ background: falseyBoolean(false), interactive: falseyBoolean(true), closeTerminalOnSuccess: falseyBoolean(true), + openTerminalOnError: falseyBoolean(true), }) export const SafeNotebookAnnotationsSchema = z.object({ diff --git a/src/types.ts b/src/types.ts index fc6560289..3439ae5f5 100644 --- a/src/types.ts +++ b/src/types.ts @@ -96,6 +96,7 @@ export namespace Serializer { background?: string interactive?: string closeTerminalOnSuccess?: string + openTerminalOnError?: string mimeType?: string promptEnv?: string category?: string diff --git a/src/utils/configuration.ts b/src/utils/configuration.ts index 7a2b46b11..72362f8fd 100644 --- a/src/utils/configuration.ts +++ b/src/utils/configuration.ts @@ -294,11 +294,13 @@ const getNotebookTerminalConfigurations = (metadata: Serializer.Metadata) => { const isNotebookTerminalEnabledForCell = (cell: NotebookCell): boolean => { const { interactive, background } = getAnnotations(cell) - return interactive - ? background + if (interactive) { + return background ? isNotebookTerminalFeatureEnabled('backgroundTask') : isNotebookTerminalFeatureEnabled('interactive') - : isNotebookTerminalFeatureEnabled('nonInteractive') + } + + return false } const getCloseTerminalOnSuccess = () => { diff --git a/tests/extension/provider/annotations.test.ts b/tests/extension/provider/annotations.test.ts index 9eb0e7044..6793437e7 100644 --- a/tests/extension/provider/annotations.test.ts +++ b/tests/extension/provider/annotations.test.ts @@ -20,6 +20,7 @@ vi.mock('../../../src/extension/utils', () => ({ background: false, interactive: true, closeTerminalOnSuccess: true, + openTerminalOnError: true, mimeType: 'text/plain', name: 'npm-install', 'runme.dev/id': '01HGVC6M8Y76XAGAY6MQ06F5XS', diff --git a/tests/extension/provider/notebook.test.ts b/tests/extension/provider/notebook.test.ts index 848788b58..55c598454 100644 --- a/tests/extension/provider/notebook.test.ts +++ b/tests/extension/provider/notebook.test.ts @@ -16,6 +16,7 @@ vi.mock('../../../src/extension/utils', () => ({ background: false, interactive: true, closeTerminalOnSuccess: true, + openTerminalOnError: true, mimeType: 'text/plain', name: 'npm-install', 'runme.dev/id': '01HGVC6M8Y76XAGAY6MQ06F5XS', diff --git a/tests/extension/provider/sessionOutputs.test.ts b/tests/extension/provider/sessionOutputs.test.ts index 4b1130aef..0dfb2efc2 100644 --- a/tests/extension/provider/sessionOutputs.test.ts +++ b/tests/extension/provider/sessionOutputs.test.ts @@ -16,6 +16,7 @@ vi.mock('../../../src/extension/utils', () => ({ background: false, interactive: true, closeTerminalOnSuccess: true, + openTerminalOnError: true, mimeType: 'text/plain', name: 'npm-install', 'runme.dev/id': '01HGVC6M8Y76XAGAY6MQ06F5XS', diff --git a/tests/extension/schema.test.ts b/tests/extension/schema.test.ts index f635339cf..363fb154c 100644 --- a/tests/extension/schema.test.ts +++ b/tests/extension/schema.test.ts @@ -98,11 +98,13 @@ suite('AnnotationSchema', () => { background: false, interactive: true, closeTerminalOnSuccess: true, + openTerminalOnError: true, } const parseResult = SafeCellAnnotationsSchema.safeParse({ background: 'invalid', interactive: 'invalid', closeTerminalOnSuccess: 'invalid', + openTerminalOnError: 'invalid', }) expect(parseResult.success).toBeTruthy() @@ -117,9 +119,17 @@ suite('AnnotationSchema', () => { const parseResult = SafeCellAnnotationsSchema.safeParse({}) expect(parseResult.success).toBeTruthy() if (parseResult.success) { - const { background, closeTerminalOnSuccess, interactive, mimeType, name } = parseResult.data + const { + background, + closeTerminalOnSuccess, + openTerminalOnError, + interactive, + mimeType, + name, + } = parseResult.data expect(background).toBeFalsy() expect(closeTerminalOnSuccess).toBeTruthy() + expect(openTerminalOnError).toBeTruthy() expect(interactive).toBeTruthy() expect(mimeType).toBeUndefined() expect(name).toBe('') @@ -133,6 +143,7 @@ suite('AnnotationSchema', () => { background: 'invalid', interactive: 'invalid', closeTerminalOnSuccess: 'invalid', + openTerminalOnError: 'invalid', } const parseResult = CellAnnotationsSchema.safeParse(input) as SafeParseError expect(parseResult.success).toBeFalsy() @@ -141,6 +152,7 @@ suite('AnnotationSchema', () => { background: ['expected a boolean value'], interactive: ['expected a boolean value'], closeTerminalOnSuccess: ['expected a boolean value'], + openTerminalOnError: ['expected a boolean value'], }) }) @@ -148,9 +160,17 @@ suite('AnnotationSchema', () => { const parseResult = SafeCellAnnotationsSchema.safeParse({}) expect(parseResult.success).toBeTruthy() if (parseResult.success) { - const { background, closeTerminalOnSuccess, interactive, mimeType, name } = parseResult.data + const { + background, + closeTerminalOnSuccess, + openTerminalOnError, + interactive, + mimeType, + name, + } = parseResult.data expect(background).toBeFalsy() expect(closeTerminalOnSuccess).toBeTruthy() + expect(openTerminalOnError).toBeTruthy() expect(interactive).toBeTruthy() expect(mimeType).toBeUndefined() expect(name).toBe('') diff --git a/tests/extension/utils.test.ts b/tests/extension/utils.test.ts index e13e45c08..5af37b70f 100644 --- a/tests/extension/utils.test.ts +++ b/tests/extension/utils.test.ts @@ -323,6 +323,7 @@ suite('#getAnnotations', () => { expect(d).toStrictEqual({ background: false, closeTerminalOnSuccess: true, + openTerminalOnError: true, cwd: '', interactive: true, name: 'command-123', @@ -347,6 +348,7 @@ suite('#getAnnotations', () => { id: undefined, background: false, closeTerminalOnSuccess: true, + openTerminalOnError: true, cwd: '', interactive: true, name: 'echo-hello', @@ -516,6 +518,7 @@ suite('validateAnnotations', () => { background: 'invalid', interactive: 'invalid', closeTerminalOnSuccess: 'invalid', + openTerminalOnError: 'invalid', promptEnv: 'invalid', mimeType: 'application/', }, @@ -523,7 +526,7 @@ suite('validateAnnotations', () => { } const result = validateAnnotations(cell) expect(result.hasErrors).toBe(true) - expect(result.errors && Object.entries(result.errors).length).toBe(4) + expect(result.errors && Object.entries(result.errors).length).toBe(5) }) test('it should pass for valid annotations values', () => { @@ -532,6 +535,7 @@ suite('validateAnnotations', () => { background: false, interactive: true, closeTerminalOnSuccess: true, + openTerminalOnError: true, promptEnv: false, mimeType: 'text/plain', },