From 68d2e5789e3cc7da77442ff92c760946a441cd1d Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 9 Nov 2026 08:57:05 +0100 Subject: [PATCH] refactor: the stock bundle no longer binds the @viewer opener wfpy-ide's own bundle binds its copy of the @viewer node opener (endrix/wfpy-ide#29), under the same id, so the platform's copy goes: viewer-node-opener.ts, its binding in the stock feature module, and the two route test files that moved to wfpy-ide with it. The double-click listener keeps the NodeOpener point and opens a node's definition for every product; double-click-opens-one-thing, which pins that, stays and needs no opener now. The parity baseline loses the one binding; the seam's DI tests pin that the stock diagram binds no opener. The host still turns the wf:viewer* arguments of the opener's actions into an editor, a diff or a command; that half is next. wfpy-ide's branch passes its 154 tests against this tree. Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv --- CHANGELOG.md | 5 +- packages/diagram-client/README.md | 3 +- .../src/stock-features.module.ts | 5 - .../diagram-client/src/viewer-node-opener.ts | 417 ------------------ .../test/container-parity.test.ts | 3 +- .../test/declared-target-resources.test.ts | 145 ------ .../test/double-click-opens-one-thing.test.ts | 3 - .../fixtures/container-parity.baseline.json | 5 - .../test/node-opener-seam.test.ts | 20 +- .../test/viewer-declared-target.test.ts | 135 ------ 10 files changed, 17 insertions(+), 724 deletions(-) delete mode 100644 packages/diagram-client/src/viewer-node-opener.ts delete mode 100644 packages/diagram-client/test/declared-target-resources.test.ts delete mode 100644 packages/diagram-client/test/viewer-declared-target.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eb7663..90c171c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,8 +68,9 @@ version check rather than run against what it no longer has. definition navigation, or leaves the node to it; one per `id`, the last binding winning. The `@viewer` half of the double-click listener (open, diff, open with an editor or run a command on a task's last tokens, or a - path it declares) is the `viewer` opener (`viewer-node-opener.ts`), which - the stock module binds until wfpy-ide binds its own. + path it declares) became the `viewer` opener (#140) and then wfpy-ide's + code, in its own bundle (endrix/wfpy-ide#29); the stock bundle binds no + opener. - **What a product's webview code needs, from the package.** `@dialogram/diagram-client` exports `VscodeUi` (the host's quick picks, diff --git a/packages/diagram-client/README.md b/packages/diagram-client/README.md index 0c38a69..091564b 100644 --- a/packages/diagram-client/README.md +++ b/packages/diagram-client/README.md @@ -278,8 +278,7 @@ of your nodes, bind a `NodeOpener` with an `id`: its `open(node, { sourceUri })` returns the actions to run, or `undefined` to leave the node to the platform. Openers are asked in the order their ids were first bound, and the first that answers wins; a later binding for an id replaces the earlier one. The -stock bundle binds wfpy's `@viewer` opener (id `viewer`) until wfpy-ide binds -its own. +stock bundle binds none; wfpy-ide's binds its `@viewer` opener. **A diagram of your own.** When you compose the container yourself: diff --git a/packages/diagram-client/src/stock-features.module.ts b/packages/diagram-client/src/stock-features.module.ts index 4749957..f60dc8d 100644 --- a/packages/diagram-client/src/stock-features.module.ts +++ b/packages/diagram-client/src/stock-features.module.ts @@ -18,8 +18,6 @@ import { clientBehavior } from './profile'; import { WorkflowNavigationUi } from './navigation-ui'; import { WorkflowNetworkNavigationMouseListener } from './network-navigation-mouse-listener'; import { ViewerMouseListener } from './viewer-mouse-listener'; -import { NodeOpener } from './node-opener'; -import { ViewerNodeOpener } from './viewer-node-opener'; import { EdgeOpenMouseListener } from './edge-open-mouse-listener'; import { WhitespaceRootPropertiesMouseListener } from './whitespace-root-properties-mouse-listener'; import { WorkflowElkLiveDragRouter } from './elk-live-drag-router'; @@ -116,9 +114,6 @@ export const workflowFeaturesModule = new ContainerModule((bind, unbind, isBound // Double-click on a node: what a product's openers open for it, else its // definition (every product relies on the latter). bindAsService(context, TYPES.MouseListener, ViewerMouseListener); - // What a double-click on a `@viewer` task opens (its last tokens), until - // the product that has `@viewer` binds it in its own bundle. - bind(NodeOpener).toConstantValue(new ViewerNodeOpener()); // Double-click on diagram whitespace to show root/workflow properties. bindAsService(context, TYPES.MouseListener, WhitespaceRootPropertiesMouseListener); diff --git a/packages/diagram-client/src/viewer-node-opener.ts b/packages/diagram-client/src/viewer-node-opener.ts deleted file mode 100644 index 520f6a4..0000000 --- a/packages/diagram-client/src/viewer-node-opener.ts +++ /dev/null @@ -1,417 +0,0 @@ -/** - * What a double-click on a task with a `@viewer` annotation opens: the last - * token on one of its inputs (open, open with a named editor, run a command - * on it), the last tokens on two (diff), or a path the annotation declares. - * A task without one is left to the platform, which opens its definition. - * - * The `@viewer` half of the former viewer listener, moved behind the - * `NodeOpener` contribution point: the actions are the same - * `NavigateToExternalTargetAction`s with the `wf:viewer*` arguments the host - * turns into an editor, a diff or a command. - */ -import { Action, Args, GModelElement, NavigateToExternalTargetAction } from '@eclipse-glsp/sprotty'; -import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; -import type { NodeOpener, NodeOpenerContext } from './node-opener'; -import { settingsNamespace } from './profile'; - -const VIEWER_ACTION_ARG = 'wf:viewerAction'; -const VIEWER_VIEW_TYPE_ARG = 'wf:viewerViewType'; -const VIEWER_COMMAND_ARG = 'wf:viewerCommand'; -const VIEWER_COMMAND_ARGS_ARG = 'wf:viewerCommandArgs'; -const VIEWER_LEFT_URI_ARG = 'wf:viewerLeftUri'; -const VIEWER_RIGHT_URI_ARG = 'wf:viewerRightUri'; -const VIEWER_TITLE_ARG = 'wf:viewerTitle'; -const VIEWER_MESSAGE_ARG = 'wf:viewerMessage'; - -type DefinitionAnnotation = { - name: string; - arguments?: Array<{ name: string; value: string }>; -}; - -/** - * Anything with a scheme is already a complete address — a URL, or a URI the - * host knows how to route. It must not be treated as a path and glued onto a - * directory, which is what turned `https://example.com/x` into - * `file:///w/https://example.com/x`. - */ -const ABSOLUTE_URI = /^[A-Za-z][A-Za-z0-9+.-]*:\/\//; - -function parseStringLiteralish(text: string | undefined): string | undefined { - if (typeof text !== 'string') { - return undefined; - } - const trimmed = text.trim(); - if ((trimmed.startsWith('"') && trimmed.endsWith('"')) || (trimmed.startsWith("'") && trimmed.endsWith("'"))) { - return trimmed.slice(1, -1); - } - return trimmed; -} - -function parseStringListLiteralish(text: string | undefined): string[] | undefined { - if (typeof text !== 'string') { - return undefined; - } - - const trimmed = text.trim(); - if (trimmed === '') { - return undefined; - } - - // Try JSON first (preferred: ["In", "Out"]). - if (trimmed.startsWith('[') && trimmed.endsWith(']')) { - try { - const parsed = JSON.parse(trimmed); - if (Array.isArray(parsed) && parsed.every(x => typeof x === 'string')) { - return parsed as string[]; - } - } catch { - // fall through - } - - // Best-effort: extract quoted strings. - const items: string[] = []; - const re = /"([^"]+)"|'([^']+)'/g; - let m: RegExpExecArray | null; - while ((m = re.exec(trimmed))) { - const s = m[1] ?? m[2]; - if (typeof s === 'string' && s.trim() !== '') { - items.push(s); - } - } - return items.length > 0 ? items : undefined; - } - - return undefined; -} - -function resolveViewerCommandArgs(args: string[] | undefined, targetUri: string, tokenPath: string): string[] | undefined { - if (!args || args.length === 0) { - return undefined; - } - return args.map(value => value - .replace(/\{uri\}/g, targetUri) - .replace(/\{path\}/g, tokenPath) - ); -} - -/** - * The target a declared path names, or undefined when it cannot be resolved. - * - * A relative path means "beside the file this was declared in", so with no such - * file there is no answer — better said than guessed. - */ -function resolveDeclaredTarget( - sourceUri: string | undefined, - declaredPath: string -): string | undefined { - if (ABSOLUTE_URI.test(declaredPath) || declaredPath.startsWith('/')) { - return declaredPath; - } - return sourceUri ? resolveAgainstSource(sourceUri, declaredPath) : undefined; -} - -/** - * What to do with a target, given what it is. - * - * The kind is decided by whoever exported the graph, because that is where the - * filesystem is visible — nothing here can tell a folder from a file by looking - * at the string. - */ -function actionForDeclaredTarget( - target: string, - kind: string | undefined, - viewType: string | undefined -): 'open' | 'openWith' | 'reveal' { - // A directory has no editor to open in; revealing it is the operation that - // exists for one. - if (kind === 'folder') { - return 'reveal'; - } - // No editor is registered for a remote scheme, so openWith dead-ends there. - // Plain open hands it to the host, which launches a browser. - const isRemote = ABSOLUTE_URI.test(target) && !target.startsWith('file:'); - if (isRemote || !viewType) { - return 'open'; - } - return 'openWith'; -} - -/** A path declared in the diagram's own source file, resolved beside it. */ -function resolveAgainstSource(sourceUri: string, relativePath: string): string { - const lastSlash = sourceUri.lastIndexOf('/'); - const directory = lastSlash >= 0 ? sourceUri.slice(0, lastSlash) : sourceUri; - return `${directory}/${relativePath}`; -} - -function fileUriFromFsPath(fsPath: string): string { - const p = fsPath.trim(); - if (p.startsWith('file:')) { - return p; - } - // Assume absolute POSIX path (CLI produces absolute paths on Linux/macOS). - const encoded = encodeURI(p); - return encoded.startsWith('/') ? `file://${encoded}` : encoded; -} - -function collectEdges(root: any): any[] { - const out: any[] = []; - const visit = (el: any): void => { - if (!el) { - return; - } - // Sprotty edges typically have type starting with 'edge:' - if (typeof el.type === 'string' && el.type.startsWith('edge:')) { - out.push(el); - } - const children = el.children as any[] | undefined; - if (Array.isArray(children)) { - for (const c of children) { - visit(c); - } - } - }; - visit(root); - return out; -} - -function incomingEdgesForTargetPort(edges: any[], targetPortId: string): any[] { - return edges.filter(e => (e as any).targetId === targetPortId); -} - -function bestIncomingEdgeForViewerInput(edges: any[], targetPortId: string): any | undefined { - const candidates = incomingEdgesForTargetPort(edges, targetPortId); - if (candidates.length === 0) { - return undefined; - } - - const withToken = candidates.filter(e => { - const v = e?.args?.[WorkflowDiagramMetadata.VIEWER_LAST_TOKEN]; - return lastTokenAsPath(v) !== undefined; - }); - - const stableKey = (e: any): string => { - const astPath = e?.args?.[WorkflowDiagramMetadata.AST_PATH]; - return typeof astPath === 'string' ? astPath : String(e?.id ?? ''); - }; - - const pickFrom = (withToken.length > 0 ? withToken : candidates).slice(); - pickFrom.sort((a, b) => stableKey(a).localeCompare(stableKey(b))); - return pickFrom[pickFrom.length - 1]; -} - -function lastTokenAsPath(value: unknown): string | undefined { - if (typeof value === 'string') { - return value; - } - if (Array.isArray(value) && value.length > 0) { - const last = value[value.length - 1]; - return typeof last === 'string' ? last : undefined; - } - return undefined; -} - -export class ViewerNodeOpener implements NodeOpener { - readonly id = 'viewer'; - - open(node: GModelElement, context: NodeOpenerContext): (Action | Promise)[] | undefined { - if (node.type !== WorkflowDiagramTypes.NODE_EXTERNAL_ACTOR) { - return undefined; - } - const args = (node as unknown as { args?: Args }).args; - if (!args) { - return undefined; - } - const sourceUri = context.sourceUri; - - const defAnnotations = args[WorkflowDiagramMetadata.ENTITY_DEFINITION_ANNOTATIONS] as unknown; - const annotations = Array.isArray(defAnnotations) ? (defAnnotations as DefinitionAnnotation[]) : []; - const viewer = annotations.find(a => a && a.name === 'viewer'); - if (!viewer) { - return undefined; - } - - const argByName = new Map(); - for (const a of viewer.arguments ?? []) { - if (a && typeof a.name === 'string' && typeof a.value === 'string') { - argByName.set(a.name, a.value); - } - } - - const action = ( - parseStringLiteralish(argByName.get('action')) - ?? parseStringLiteralish(argByName.get('action_name')) - ?? 'open' - ).trim(); - const inputs = parseStringListLiteralish(argByName.get('inputs')); - const viewType = parseStringLiteralish(argByName.get('viewType')) - ?? parseStringLiteralish(argByName.get('view_type')); - // Where the target comes from. Everything here has always resolved - // through the last token on an input, which is right for a node whose - // target is something it PRODUCED — it cannot exist before a run. A - // node whose target is a path it declares is openable immediately, and - // must not be told to go and run the workflow first. - const source = parseStringLiteralish(argByName.get('source')) ?? 'token'; - const declaredPath = parseStringLiteralish(argByName.get('path')); - // What the target IS — file, folder, http, url — settled by the - // producer, which can see the filesystem. Nothing here can tell a - // folder from a file by looking at the string. - const resourceKind = parseStringLiteralish(argByName.get('kind')); - const command = parseStringLiteralish(argByName.get('command')) - ?? parseStringLiteralish(argByName.get('command_name')); - const commandArgs = parseStringListLiteralish(argByName.get('args')) - ?? parseStringListLiteralish(argByName.get('command_args')); - const title = parseStringLiteralish(argByName.get('title')); - - const fail = (message: string): (Action | Promise)[] => [ - NavigateToExternalTargetAction.create({ - uri: context.sourceUri ?? 'file:///', - args: { - [VIEWER_ACTION_ARG]: 'error', - [VIEWER_MESSAGE_ARG]: message - } - }) - ]; - - if (source === 'declared') { - if (!declaredPath) { - return fail('@viewer(source="declared"): missing path="...".'); - } - // Relative to the file the diagram was opened from, which is what a - // declaration in that file means. - const target = resolveDeclaredTarget(sourceUri, declaredPath); - if (!target) { - return fail( - `@viewer(source="declared"): cannot resolve the relative path ` - + `"${declaredPath}" — the diagram has no source file to resolve it against.` - ); - } - const declaredAction = actionForDeclaredTarget(target, resourceKind, viewType); - return [ - NavigateToExternalTargetAction.create({ - uri: target, - args: { - [VIEWER_ACTION_ARG]: declaredAction, - ...(declaredAction === 'openWith' - ? { [VIEWER_VIEW_TYPE_ARG]: viewType as string } - : {}) - } - }) - ]; - } - - // Below here every route resolves its target from the last token on an - // input, so an input is what it needs. This guard used to sit above the - // declared branch, where it caught nodes that never wanted a token: a - // node that declares its target has no inputs by nature, so it was told - // to run the workflow first and could never be opened at all. - if (!inputs || inputs.length === 0) { - return fail('@viewer: missing inputs=[...] (run the workflow first, then double-click again).'); - } - - const root: any = (node as any).root; - const edges = root ? collectEdges(root) : []; - - const overlayRunId = root?.args?.['wf:viewerOverlayRunId']; - const overlayOutDir = root?.args?.['wf:viewerOverlayOutDir']; - const hasOverlay = typeof overlayRunId === 'string' && overlayRunId.trim() !== ''; - - const tokenByInputName = new Map(); - for (const inputName of inputs) { - const targetPortId = `${node.id}_port_${inputName}`; - const incoming = bestIncomingEdgeForViewerInput(edges, targetPortId); - const lastToken = incoming?.args?.[WorkflowDiagramMetadata.VIEWER_LAST_TOKEN]; - const tokenPath = lastTokenAsPath(lastToken); - if (tokenPath) { - tokenByInputName.set(inputName, tokenPath); - } - } - - const getTokenOrError = (name: string): string | undefined => { - return tokenByInputName.get(name); - }; - - if (action === 'diff') { - if (inputs.length !== 2) { - return fail('@viewer(action="diff"): requires exactly 2 inputs in inputs=[...].'); - } - const leftPath = getTokenOrError(inputs[0]); - const rightPath = getTokenOrError(inputs[1]); - if (!leftPath || !rightPath) { - if (!hasOverlay) { - return fail(`Viewer diff: no run overlay found. Run the workflow successfully (CLI writes overlays under ${settingsNamespace()}.runOutputDir), then reopen/refresh the diagram.`); - } - return fail(`Viewer diff: missing last-token file(s) for inputs ${inputs.join(', ')} (overlay runId=${String(overlayRunId)}).`); - } - const leftUri = fileUriFromFsPath(leftPath); - const rightUri = fileUriFromFsPath(rightPath); - return [ - NavigateToExternalTargetAction.create({ - uri: leftUri, - args: { - [VIEWER_ACTION_ARG]: 'diff', - [VIEWER_LEFT_URI_ARG]: leftUri, - [VIEWER_RIGHT_URI_ARG]: rightUri, - ...(title ? { [VIEWER_TITLE_ARG]: title } : {}) - } - }) - ]; - } - - // open/openWith: use first input. - const firstInput = inputs[0]; - const tokenPath = getTokenOrError(firstInput); - if (!tokenPath) { - if (!hasOverlay) { - return fail(`Viewer open: no run overlay found. Run the workflow successfully (CLI writes overlays under ${settingsNamespace()}.runOutputDir), then reopen/refresh the diagram.`); - } - const outDirInfo = typeof overlayOutDir === 'string' && overlayOutDir.trim() !== '' ? ` (outDir=${overlayOutDir})` : ''; - return fail(`Viewer open: missing last-token file for input '${firstInput}' (overlay runId=${String(overlayRunId)})${outDirInfo}.`); - } - - const targetUri = fileUriFromFsPath(tokenPath); - - if (action === 'openWith') { - if (!viewType) { - return fail('@viewer(action="openWith"): missing viewType="...".'); - } - return [ - NavigateToExternalTargetAction.create({ - uri: targetUri, - args: { - [VIEWER_ACTION_ARG]: 'openWith', - [VIEWER_VIEW_TYPE_ARG]: viewType - } - }) - ]; - } - - if (action === 'command') { - if (!command) { - return fail('@viewer(action="command"): missing command="...".'); - } - const resolvedArgs = resolveViewerCommandArgs(commandArgs, targetUri, tokenPath); - return [ - NavigateToExternalTargetAction.create({ - uri: targetUri, - args: { - [VIEWER_ACTION_ARG]: 'command', - [VIEWER_COMMAND_ARG]: command, - ...(resolvedArgs - ? { [VIEWER_COMMAND_ARGS_ARG]: JSON.stringify(resolvedArgs) } - : {}) - } - }) - ]; - } - - // Default: open - return [ - NavigateToExternalTargetAction.create({ - uri: targetUri, - args: { - [VIEWER_ACTION_ARG]: 'open' - } - }) - ]; - } -} diff --git a/packages/diagram-client/test/container-parity.test.ts b/packages/diagram-client/test/container-parity.test.ts index be1c4c1..d680fda 100644 --- a/packages/diagram-client/test/container-parity.test.ts +++ b/packages/diagram-client/test/container-parity.test.ts @@ -32,8 +32,7 @@ * states, and the read-only view marker (hides the palette's tools while the view shown is * read-only, bound to itself and as a model-root listener), and the selection trace (its listener, * bound to itself and as a selection listener, and the `SelectionTraceFeedbackCommand` that draws - * the path), and the `@viewer` node opener (`ViewerNodeOpener`, bound as a constant under - * `NodeOpener`) — plus one change: `label:boundary:type` is bound to + * the path) — plus one change: `label:boundary:type` is bound to * the non-editable `BoundaryLabel`, because editing it renamed the port. * The oracle still does its job: it fails on any binding this composition gains * or loses, and updating the fixture is the deliberate act of accepting one. diff --git a/packages/diagram-client/test/declared-target-resources.test.ts b/packages/diagram-client/test/declared-target-resources.test.ts deleted file mode 100644 index 6d2ddcf..0000000 --- a/packages/diagram-client/test/declared-target-resources.test.ts +++ /dev/null @@ -1,145 +0,0 @@ -/** - * A declared target is not always a file you open in an editor. - * - * The declared route was written for one shape — a file, opened with a named - * editor — and hardcoded all three of those. So a folder was handed to a - * command that wants an editor, a URL was concatenated onto the diagram's own - * directory into nonsense, and leaving the editor unnamed was an error rather - * than "just open it". - * - * What the target IS gets decided where the filesystem is visible — by whoever - * exported the graph — and travels as `kind`. This turns that into an action. - */ -import { describe, expect, it } from 'vitest'; -import { ViewerMouseListener } from '../src/viewer-mouse-listener'; -import { ViewerNodeOpener } from '../src/viewer-node-opener'; -import { WorkflowDiagramTypes, WorkflowDiagramMetadata } from '@dialogram/shared'; - -const SOURCE_URI = 'file:///w/flow.py'; - -/** - * `sourceUri` is a required parameter deliberately. With a default, the - * no-source cases below would pass `undefined` and silently get the default - * back — JavaScript applies a default to an explicit `undefined` — so those - * tests would assert against the ordinary case and pass for the wrong reason. - */ -function openAt(args: Record, sourceUri: string | undefined): any { - const node: any = { - id: 'n1', - type: WorkflowDiagramTypes.NODE_EXTERNAL_ACTOR, - args: { - [WorkflowDiagramMetadata.IS_EXTERNAL_ACTOR]: true, - [WorkflowDiagramMetadata.ENTITY_DEFINITION_ANNOTATIONS]: [ - { - name: 'viewer', - arguments: Object.entries({ source: 'declared', ...args }) - .map(([name, value]) => ({ name, value: `"${value}"` })) - } - ] - }, - children: [], - root: { args: {}, children: [] } - }; - node.root.children = [node]; - const listener: any = new ViewerMouseListener(); - // What the stock feature module binds. - listener.contributedOpeners = [new ViewerNodeOpener()]; - listener.editorContext = { sourceUri }; - return listener.doubleClick(node, {} as MouseEvent)[0]; -} - -const open = (args: Record): any => openAt(args, SOURCE_URI); - -describe('a declared file', () => { - it('opens with the named editor when one is named', () => { - const action = open({ path: 'out/report.html', viewType: 'product.preview' }); - - expect(action.args['wf:viewerAction']).toBe('openWith'); - expect(action.args['wf:viewerViewType']).toBe('product.preview'); - expect(action.uri).toBe('file:///w/out/report.html'); - }); - - /** - * Naming no editor means the default one, which is what the host already - * does with an empty viewType. It used to be an error, so a plain text file - * could not be declared at all. - */ - it('opens in the default editor when none is named', () => { - const action = open({ path: 'notes.md' }); - - expect(action.args['wf:viewerAction']).toBe('open'); - expect(action.uri).toBe('file:///w/notes.md'); - }); -}); - -describe('a declared web resource', () => { - it('keeps the URL instead of resolving it beside the diagram', () => { - const action = open({ path: 'https://example.com/spec.json', kind: 'http' }); - - expect(action.uri).toBe('https://example.com/spec.json'); - }); - - /** - * `open` and not `openWith`: no editor is registered for an https URI, so - * openWith dead-ends. Plain open hands it to the host, which launches a - * browser. - */ - it('opens it rather than looking for an editor', () => { - const action = open({ - path: 'https://example.com/spec.json', - kind: 'http', - viewType: 'product.preview' - }); - - expect(action.args['wf:viewerAction']).toBe('open'); - }); - - it('leaves any other scheme alone too', () => { - expect(open({ path: 'vscode://extension/page' }).uri).toBe('vscode://extension/page'); - }); -}); - -describe('a declared folder', () => { - /** - * A directory has no editor to open in — every route through `open` or - * `openWith` fails on one. Revealing it is the operation that exists. - */ - it('is revealed rather than opened', () => { - const action = open({ path: 'designs/', kind: 'folder' }); - - expect(action.args['wf:viewerAction']).toBe('reveal'); - expect(action.uri).toBe('file:///w/designs/'); - }); - - it('is revealed even when an editor was named', () => { - const action = open({ path: 'designs/', kind: 'folder', viewType: 'product.preview' }); - - expect(action.args['wf:viewerAction']).toBe('reveal'); - }); -}); - -describe('a declared target with nothing to resolve against', () => { - /** - * `sourceUri` is optional on the editor context. A relative path used to be - * concatenated onto it unchecked, which threw inside the mouse listener — - * caught only because the package does not typecheck strictly. - */ - it('says so instead of throwing', () => { - const action = openAt({ path: 'out/report.html' }, undefined); - - expect(action.args['wf:viewerAction']).toBe('error'); - expect(action.args['wf:viewerMessage']).toMatch(/relative/i); - }); - - it('still opens an absolute path', () => { - const action = openAt({ path: '/abs/report.html' }, undefined); - - expect(action.args['wf:viewerAction']).toBe('open'); - expect(action.uri).toBe('/abs/report.html'); - }); - - it('still opens a URL', () => { - expect(openAt({ path: 'https://example.com/x' }, undefined).uri) - .toBe('https://example.com/x'); - }); -}); diff --git a/packages/diagram-client/test/double-click-opens-one-thing.test.ts b/packages/diagram-client/test/double-click-opens-one-thing.test.ts index 96ed0e4..26e0e1c 100644 --- a/packages/diagram-client/test/double-click-opens-one-thing.test.ts +++ b/packages/diagram-client/test/double-click-opens-one-thing.test.ts @@ -11,7 +11,6 @@ import { afterEach, describe, expect, it } from 'vitest'; import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import { ViewerMouseListener } from '../src/viewer-mouse-listener'; -import { ViewerNodeOpener } from '../src/viewer-node-opener'; const node = (type: string, extra: Record): any => ({ id: 'clause', @@ -26,8 +25,6 @@ const node = (type: string, extra: Record): any => ({ const viewerAnswer = (element: unknown): unknown[] => { const listener: any = new ViewerMouseListener(); - // What the stock feature module binds. - listener.contributedOpeners = [new ViewerNodeOpener()]; listener.editorContext = { sourceUri: 'file:///w/spec/pipeline.md' }; return listener.doubleClick(element, {} as MouseEvent); }; diff --git a/packages/diagram-client/test/fixtures/container-parity.baseline.json b/packages/diagram-client/test/fixtures/container-parity.baseline.json index 504ce8e..4c354ba 100644 --- a/packages/diagram-client/test/fixtures/container-parity.baseline.json +++ b/packages/diagram-client/test/fixtures/container-parity.baseline.json @@ -86,11 +86,6 @@ "op": "rebind", "to": "ViewportPreservingSetModelCommand" }, - { - "id": "Symbol(dialogram.NodeOpener)", - "op": "bind", - "to": "const:ViewerNodeOpener" - }, { "id": "Symbol(stub.ICopyPasteHandler)", "op": "bind", diff --git a/packages/diagram-client/test/node-opener-seam.test.ts b/packages/diagram-client/test/node-opener-seam.test.ts index 30e2ba3..eb1343c 100644 --- a/packages/diagram-client/test/node-opener-seam.test.ts +++ b/packages/diagram-client/test/node-opener-seam.test.ts @@ -2,8 +2,7 @@ * The contribution point for what a double-click on a node opens * (`node-opener.ts`): an opener a product binds answers first, an opener that * leaves the node lets the platform open its definition, and the stock - * module binds the `@viewer` opener until the product that has `@viewer` - * binds it itself. + * module binds none. */ import { describe, expect, it, vi } from 'vitest'; import { Container, ContainerModule } from 'inversify'; @@ -94,15 +93,20 @@ describe('the openers the listener is given', () => { // The stock module binds the listener as a mouse listener; resolve one the way the container builds it. const ids = (c: Container) => (c.resolve(ViewerMouseListener) as any).nodeOpeners().map((o: NodeOpener) => o.id); - it('are the stock @viewer opener, then a product’s', () => { - expect(ids(container())).toEqual(['viewer']); - expect(ids(container(new ContainerModule(bind => { bind(NodeOpener).toConstantValue(opener('mine', undefined)); })))) - .toEqual(['viewer', 'mine']); + it('are none in the stock diagram, and those a product binds, in binding order', () => { + expect(ids(container())).toEqual([]); + expect(ids(container( + new ContainerModule(bind => { bind(NodeOpener).toConstantValue(opener('viewer', undefined)); }), + new ContainerModule(bind => { bind(NodeOpener).toConstantValue(opener('mine', undefined)); }) + ))).toEqual(['viewer', 'mine']); }); - it('take a product’s viewer opener in place of the stock one', () => { + it('take a later module’s opener in place of an earlier one’s of the same id', () => { const mine = opener('viewer', undefined); - const c = container(new ContainerModule(bind => { bind(NodeOpener).toConstantValue(mine); })); + const c = container( + new ContainerModule(bind => { bind(NodeOpener).toConstantValue(opener('viewer', [])); }), + new ContainerModule(bind => { bind(NodeOpener).toConstantValue(mine); }) + ); expect((c.resolve(ViewerMouseListener) as any).nodeOpeners()).toEqual([mine]); }); diff --git a/packages/diagram-client/test/viewer-declared-target.test.ts b/packages/diagram-client/test/viewer-declared-target.test.ts deleted file mode 100644 index 5db1675..0000000 --- a/packages/diagram-client/test/viewer-declared-target.test.ts +++ /dev/null @@ -1,135 +0,0 @@ -/** - * Double-clicking a node whose open target is a path it DECLARES. - * - * Every other route through the viewer listener resolves its target from the - * last token on an input, which is right for a node whose target is something - * it produced — that cannot exist before a run. A node that declares its target - * is openable immediately, and must not be told to go and run the workflow - * first. - * - * The declared route shipped with no test at all, and it did not work: the - * empty-`inputs` guard sat above it, so a node with a declared path and no - * inputs always got "run the workflow first" instead of its file. Producers of - * this annotation have no inputs by nature, so the route was unreachable for - * exactly the nodes it was written for. - */ -import { describe, expect, it } from 'vitest'; -import { ViewerMouseListener } from '../src/viewer-mouse-listener'; -import { ViewerNodeOpener } from '../src/viewer-node-opener'; -import { WorkflowDiagramTypes, WorkflowDiagramMetadata } from '@dialogram/shared'; - -const SOURCE_URI = 'file:///w/flow.py'; - -interface AnnotationArg { - name: string; - value: string; -} - -/** A node carrying one `viewer` annotation, as the exporter emits it. */ -function nodeWith(args: AnnotationArg[], inputPorts: string[] = []) { - const node: any = { - id: 'n1', - type: WorkflowDiagramTypes.NODE_EXTERNAL_ACTOR, - args: { - [WorkflowDiagramMetadata.IS_EXTERNAL_ACTOR]: true, - [WorkflowDiagramMetadata.ENTITY_DEFINITION_ANNOTATIONS]: [ - { name: 'viewer', arguments: args } - ] - }, - children: inputPorts.map(name => ({ id: `n1_port_${name}`, type: 'port' })), - root: { args: {}, children: [] } - }; - node.root.children = [node]; - return node; -} - -function doubleClick(node: unknown, sourceUri: string | undefined = SOURCE_URI): any[] { - const listener: any = new ViewerMouseListener(); - // What the stock feature module binds. - listener.contributedOpeners = [new ViewerNodeOpener()]; - listener.editorContext = { sourceUri }; - return listener.doubleClick(node, {} as MouseEvent); -} - -const declared = (extra: AnnotationArg[] = []): AnnotationArg[] => [ - { name: 'source', value: '"declared"' }, - { name: 'path', value: '"designs/adder.py"' }, - ...extra -]; - -describe('a declared target with no inputs', () => { - it('opens the declared path instead of demanding a run', () => { - const [action] = doubleClick(nodeWith(declared([ - { name: 'action', value: '"openWith"' }, - { name: 'viewType', value: '"product.networkDiagram"' } - ]))); - - expect(action.args['wf:viewerAction']).toBe('openWith'); - expect(action.uri).toBe('file:///w/designs/adder.py'); - expect(action.args['wf:viewerViewType']).toBe('product.networkDiagram'); - }); - - it('resolves the path beside the file the diagram came from', () => { - const [action] = doubleClick( - nodeWith(declared([{ name: 'viewType', value: '"x"' }])), - 'file:///w/nested/deep/flow.py' - ); - - expect(action.uri).toBe('file:///w/nested/deep/designs/adder.py'); - }); - - it('leaves an absolute path alone', () => { - const [action] = doubleClick(nodeWith([ - { name: 'source', value: '"declared"' }, - { name: 'path', value: '"/abs/designs/adder.py"' }, - { name: 'viewType', value: '"x"' } - ])); - - expect(action.uri).toBe('/abs/designs/adder.py'); - }); - - it('still says what is missing when there is no path', () => { - const [action] = doubleClick(nodeWith([ - { name: 'source', value: '"declared"' }, - { name: 'viewType', value: '"x"' } - ])); - - expect(action.args['wf:viewerAction']).toBe('error'); - expect(action.args['wf:viewerMessage']).toMatch(/path/); - }); -}); - -/** - * The guard moved rather than went away. Everything that resolves through a - * token genuinely needs an input to resolve through, and without the guard it - * would fail later with a worse message — one about a missing token rather than - * a missing declaration. - */ -describe('a token target still needs inputs', () => { - it('asks for inputs when the target comes from a token', () => { - const [action] = doubleClick(nodeWith([{ name: 'action', value: '"open"' }])); - - expect(action.args['wf:viewerAction']).toBe('error'); - expect(action.args['wf:viewerMessage']).toMatch(/inputs/); - }); - - it('asks for inputs for an explicit token source too', () => { - const [action] = doubleClick(nodeWith([ - { name: 'source', value: '"token"' }, - { name: 'action', value: '"openWith"' }, - { name: 'viewType', value: '"x"' } - ])); - - expect(action.args['wf:viewerAction']).toBe('error'); - expect(action.args['wf:viewerMessage']).toMatch(/inputs/); - }); - - it('reports a diff needing two inputs, not zero', () => { - const [action] = doubleClick(nodeWith([ - { name: 'action', value: '"diff"' }, - { name: 'inputs', value: '["only"]' } - ])); - - expect(action.args['wf:viewerMessage']).toMatch(/2 inputs/); - }); -});