Skip to content

Commit

Permalink
New settings and clean up (#1820)
Browse files Browse the repository at this point in the history
  • Loading branch information
sourishkrout authored Nov 23, 2024
1 parent b4dad17 commit 0c8fd3d
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 12 deletions.
9 changes: 7 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@
"category": "Runme",
"title": "Let the Runme team know what you think",
"icon": "$(feedback)",
"shortTitle": "Send Feedback"
"shortTitle": "Feedback"
},
{
"command": "runme.surveyShebangComingSoon",
Expand Down Expand Up @@ -354,7 +354,7 @@
"category": "Runme",
"title": "Reset Session including Environment Variables",
"icon": "$(search-refresh)",
"shortTitle": "Reset Session"
"shortTitle": "Reset"
},
{
"command": "runme.runWithPrompts",
Expand Down Expand Up @@ -842,6 +842,11 @@
"default": true,
"markdownDescription": "Enable Runme notebook terminal in interactive scripts (experimental)"
},
"runme.terminal.interactiveIsDefault": {
"type": "boolean",
"default": true,
"markdownDescription": "If set to 'true', any cell run will be interactive by default"
},
"runme.terminal.fontSize": {
"type": [
"number",
Expand Down
91 changes: 82 additions & 9 deletions src/client/components/terminal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,21 @@ import { ITheme, Terminal as XTermJS } from '@xterm/xterm'
import { SerializeAddon } from '@xterm/addon-serialize'
import { Unicode11Addon } from '@xterm/addon-unicode11'
import { WebLinksAddon } from '@xterm/addon-web-links'
import { Observable } from 'rxjs'
import { debounceTime, distinctUntilChanged, filter, map, share } from 'rxjs/operators'

import { FitAddon, type ITerminalDimensions } from '../../fitAddon'
import { ClientMessages, RENDERERS, OutputType, WebViews } from '../../../constants'
import { closeOutput, getContext } from '../../utils'
import { onClientMessage, postClientMessage } from '../../../utils/messaging'
import { stripANSI } from '../../../utils/ansi'
import { APIMethod, FeatureObserver, FeatureName } from '../../../types'
import {
APIMethod,
FeatureObserver,
FeatureName,
CellAnnotations,
ClientMessage,
} from '../../../types'
import type { TerminalConfiguration } from '../../../utils/configuration'
import '../closeCellButton'
import '../copyButton'
Expand All @@ -26,6 +34,7 @@ import {
UpdateCellOutputMutation,
} from '../../../extension/__generated-platform__/graphql'
import features from '../../../features'
import { CellAnnotationsSchema } from '../../../schema'

interface IWindowSize {
width: number
Expand Down Expand Up @@ -462,7 +471,11 @@ export class TerminalView extends LitElement {

const ctx = getContext()

window.addEventListener('resize', this.#onResizeWindow.bind(this))
const dims = new Observable<TerminalDimensions | undefined>((observer) =>
window.addEventListener('resize', () => observer.next(this.#getWindowDimensions())),
).pipe(share())
this.#subscribeResizeTerminal(dims)
this.#subscribeSetTerminalRows(dims)

this.disposables.push(
onClientMessage(ctx, async (e) => {
Expand Down Expand Up @@ -604,7 +617,6 @@ export class TerminalView extends LitElement {
disconnectedCallback(): void {
super.disconnectedCallback()
this.dispose()
window.removeEventListener('resize', this.#onResizeWindow)
}

protected firstUpdated(props: PropertyValues): void {
Expand Down Expand Up @@ -730,15 +742,18 @@ export class TerminalView extends LitElement {
return getComputedStyle(terminalContainer!).getPropertyValue(variableName) ?? undefined
}

async #onResizeWindow(): Promise<void> {
#getWindowDimensions(): TerminalDimensions | undefined {
if (!this.fitAddon) {
return
}

const { innerWidth } = window
const { innerWidth, innerHeight } = window

// Prevent adjusting the terminal size if the width remains the same
if (Math.abs(this.windowSize.width - innerWidth) <= Number.EPSILON) {
// Prevent adjusting the terminal size if width & height remain the same
if (
Math.abs(this.windowSize.width - innerWidth) <= Number.EPSILON &&
Math.abs(this.windowSize.height - innerHeight) <= Number.EPSILON
) {
return
}

Expand All @@ -747,16 +762,74 @@ export class TerminalView extends LitElement {
const proposedDimensions = this.#resizeTerminal()

if (proposedDimensions) {
return convertXTermDimensions(proposedDimensions)
}
}

async #subscribeResizeTerminal(
proposedDimensions: Observable<TerminalDimensions | undefined>,
): Promise<void> {
const debounced$ = proposedDimensions.pipe(
filter((x) => !!x),
distinctUntilChanged(),
debounceTime(100),
)

const sub = debounced$.subscribe(async (terminalDimensions) => {
const ctx = getContext()
if (!ctx.postMessage) {
return
}

console.log('resize terminal', terminalDimensions)

await postClientMessage(ctx, ClientMessages.terminalResize, {
'runme.dev/id': this.id!,
terminalDimensions: convertXTermDimensions(proposedDimensions),
terminalDimensions,
})
}
})

this.disposables.push({ dispose: () => sub.unsubscribe() })
}

async #subscribeSetTerminalRows(
proposedDimensions: Observable<TerminalDimensions | undefined>,
): Promise<void> {
const debounced$ = proposedDimensions.pipe(
map((x) => x?.rows),
filter((x) => !!x),
distinctUntilChanged(),
debounceTime(100),
)

const sub = debounced$.subscribe(async (terminalRows) => {
const ctx = getContext()
if (!ctx.postMessage) {
return
}

const parseResult = CellAnnotationsSchema.safeParse({
'runme.dev/id': this.id!,
terminalRows,
})

if (!parseResult.success) {
console.error(parseResult.error.errors)
return
}

ctx.postMessage(<ClientMessage<ClientMessages.mutateAnnotations>>{
type: ClientMessages.mutateAnnotations,
output: {
annotations: <Partial<CellAnnotations>>{
'runme.dev/id': this.id!,
terminalRows,
},
},
})
})

this.disposables.push({ dispose: () => sub.unsubscribe() })
}

async #onFocusWindow(focusTerminal = true): Promise<void> {
Expand Down
12 changes: 11 additions & 1 deletion src/extension/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ import {
} from '../constants'
import { API } from '../utils/deno/api'
import { postClientMessage } from '../utils/messaging'
import { getNotebookExecutionOrder, registerExtensionEnvVarsMutation } from '../utils/configuration'
import {
getNotebookExecutionOrder,
getNotebookTerminalConfigurations,
registerExtensionEnvVarsMutation,
} from '../utils/configuration'
import features, { FEATURES_CONTEXT_STATE_KEY } from '../features'

import getLogger from './logger'
Expand Down Expand Up @@ -432,6 +436,12 @@ export class Kernel implements Disposable {
...editCell.metadata,
...payload.output.annotations,
}

const { rows } = getNotebookTerminalConfigurations(editCell.notebook.metadata)
if (rows && newMetadata?.['terminalRows'] === rows) {
delete newMetadata['terminalRows']
}

const notebookEdit = NotebookEdit.updateCellMetadata(editCell.index, newMetadata)

edit.set(editCell.notebook.uri, [notebookEdit])
Expand Down
2 changes: 2 additions & 0 deletions src/extension/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
getServerRunnerVersion,
getTLSDir,
getTLSEnabled,
isInteractiveTerminalDefault,
ServerLifecycleIdentity,
} from '../utils/configuration'

Expand Down Expand Up @@ -95,6 +96,7 @@ export function getAnnotations(raw: unknown): CellAnnotations | undefined {
...metadata,
id: metadata.id || metadata['runme.dev/id'],
name: metadata.name || metadata['runme.dev/name'],
interactive: metadata.interactive || isInteractiveTerminalDefault(),
}

const parseResult = SafeCellAnnotationsSchema.safeParse(schema)
Expand Down
3 changes: 3 additions & 0 deletions src/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ export const AnnotationSchema = {
return numeric
}
}
if (typeof subject === 'number' && Number.isFinite(subject)) {
return subject
}

return undefined
}, z.number().int().positive().optional()),
Expand Down
6 changes: 6 additions & 0 deletions src/utils/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const notebookTerminalSchema = {
backgroundTask: z.boolean().default(true),
nonInteractive: z.boolean().default(false),
interactive: z.boolean().default(true),
interactiveIsDefault: z.boolean().default(true),
fontSize: z.number().default(editorSettings.get<number>('fontSize', 10)),
fontFamily: z
.string()
Expand Down Expand Up @@ -264,6 +265,10 @@ const getServerRunnerVersion = (): ServerRunnerVersion => {
return getServerConfigurationValue<ServerRunnerVersion>('runnerVersion', 'v1')
}

const isInteractiveTerminalDefault = (): boolean => {
return getRunmeTerminalConfigurationValue('interactiveIsDefault', true)
}

const isNotebookTerminalFeatureEnabled = (
featureName: keyof typeof notebookTerminalSchema,
): boolean => {
Expand Down Expand Up @@ -478,6 +483,7 @@ export {
getServerConfigurationValue,
getTLSDir,
getTLSEnabled,
isInteractiveTerminalDefault,
isNotebookTerminalEnabledForCell,
isNotebookTerminalFeatureEnabled,
isPlatformAuthEnabled,
Expand Down
1 change: 1 addition & 0 deletions tests/extension/__snapshots__/configuration.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ exports[`Configuration > should get nullish from font family 1`] = `
"fontFamily": undefined,
"fontSize": undefined,
"interactive": true,
"interactiveIsDefault": true,
"nonInteractive": false,
"rows": 10,
"scrollback": undefined,
Expand Down
16 changes: 16 additions & 0 deletions tests/extension/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,20 @@ import {
} from '../../src/extension/utils'
import { ENV_STORE, DEFAULT_ENV } from '../../src/extension/constants'
import { CellAnnotations } from '../../src/types'
import { isInteractiveTerminalDefault } from '../../src/utils/configuration'

vi.mock('simple-git')
vi.mock('../../src/extension/grpc/client', () => ({}))
vi.mock('../../../src/extension/grpc/runner/v1', () => ({
ResolveProgramRequest_Mode: vi.fn(),
}))
vi.mock('../../src/utils/configuration', async () => {
const actual = (await vi.importActual('../../src/utils/configuration')) as any
return {
...actual,
isInteractiveTerminalDefault: vi.fn(),
}
})

vi.mock('vscode', async () => {
const { ulid } = (await vi.importActual('ulidx')) as typeof import('ulidx')
Expand Down Expand Up @@ -95,6 +103,7 @@ afterAll(() => {
beforeEach(() => {
vi.mocked(workspace.getConfiguration).mockClear()
vi.mocked(commands.executeCommand).mockClear()
vi.mocked(isInteractiveTerminalDefault).mockReturnValue(true)
})

test('isInteractive', () => {
Expand All @@ -103,6 +112,13 @@ test('isInteractive', () => {
expect(getAnnotations({ metadata: {} } as any).interactive).toBe(true)
})

test('interactiveIsDefaultFalse', () => {
vi.mocked(isInteractiveTerminalDefault).mockReturnValue(false)
expect(getAnnotations({ metadata: { interactive: 'true' } } as any).interactive).toBe(true)
expect(getAnnotations({ metadata: { interactive: 'false' } } as any).interactive).toBe(false)
expect(getAnnotations({ metadata: {} } as any).interactive).toBe(false)
})

test('getTerminalByCell', () => {
expect(
getTerminalByCell({
Expand Down

0 comments on commit 0c8fd3d

Please sign in to comment.