From 2a0136a85364b50ffa1f22b67d753001b9bbd531 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Sun, 8 Nov 2026 08:32:45 +0100 Subject: [PATCH 1/2] fix: the palette creates in every product, and copy writes what paste reads Two bugs the declutter review turned up. The create-node and entity-port handlers accepted only a `.py` source, so a palette drop in a Verilog, VHDL or NDF diagram did nothing. The gate now asks the profile's source extensions: the runtime config carries `sourceExtensions`, `SidecarRuntimeService` and `SidecarInvoker` answer `handlesSource(uri)`, and the profile fills the list from its input. A config that names no extension accepts every source. Copy wrote the entity's name as `entityName` with its source text, while paste looked for `name` and `typeName`, so a copied node pasted as nothing; cut and duplicate already wrote the paste shape. One payload now lives in `@dialogram/shared` and is written by copy, cut and duplicate and read by paste and the webview's post-paste selection. The server's `clipboard/py-clipboard` module and the client's `clipboard-payload` are gone; the builder is `buildClipboardPayload` in `clipboard/clipboard`. Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv --- CHANGELOG.md | 13 ++ .../diagram-client/src/clipboard-payload.ts | 50 ------ .../src/post-edit-selection-service.ts | 7 +- .../test/post-edit-selection-paste.test.ts | 46 ++++++ .../diagram-server/src/clipboard/clipboard.ts | 156 +++--------------- .../src/clipboard/py-clipboard.ts | 120 -------------- .../request-clipboard-data-action-handler.ts | 4 +- ...uest-clipboard-data-action-handler.test.ts | 88 ++++++++++ packages/shared/src/clipboard-payload.ts | 65 ++++++++ packages/shared/src/index.ts | 1 + .../server/operations/create-node-handler.ts | 2 +- .../src/server/operations/cut-handler.ts | 6 +- .../server/operations/duplicate-handler.ts | 4 +- .../operations/entity-port-crud-handler.ts | 2 +- .../src/server/operations/paste-handler.ts | 6 +- .../src/server/operations/sidecar-invoker.ts | 5 + .../src/server/sidecar-runtime-config.ts | 25 +++ .../src/sidecar-diagram-profile.ts | 3 +- .../test/create-node-handler.test.ts | 3 + .../test/picked-variant-create-wizard.test.ts | 1 + .../test/source-extension-gate.test.ts | 127 ++++++++++++++ .../test/variant-node-create-wizard.test.ts | 6 + 22 files changed, 424 insertions(+), 316 deletions(-) delete mode 100644 packages/diagram-client/src/clipboard-payload.ts create mode 100644 packages/diagram-client/test/post-edit-selection-paste.test.ts delete mode 100644 packages/diagram-server/src/clipboard/py-clipboard.ts create mode 100644 packages/diagram-server/test/request-clipboard-data-action-handler.test.ts create mode 100644 packages/shared/src/clipboard-payload.ts create mode 100644 packages/sidecar-toolkit/test/source-extension-gate.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d93ed18..96c8a6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,19 @@ platform API version. ### Fixed +- **The palette creates nodes, and the property panel adds entity ports, in + every product's diagram.** Both handlers accepted only a `.py` source, so a + Verilog, VHDL or NDF diagram answered a palette drop with nothing. They now + ask the profile's `sourceExtensions` (toolkit runtime config + `sourceExtensions`, `SidecarInvoker.handlesSource`). +- **Copy then paste re-creates the copied elements.** Copy wrote the entity's + name as `entityName` with its source text, while paste looked for `name` and + `typeName`, so a copied node pasted as nothing; cut and duplicate already + wrote the paste shape. One payload now lives in `@dialogram/shared` + (`ClipboardPayload`, `parseClipboardPayload`, `clipboardDataFromPayload`), + written by copy, cut and duplicate and read by paste and the webview's + post-paste selection. The server's `clipboard/py-clipboard` module is gone; + its builder is `buildClipboardPayload` in `clipboard/clipboard`. - A layout change still on its 500 ms debounce timer when the window closes is written out: the host flushes every diagram's pending layout saves on deactivation. diff --git a/packages/diagram-client/src/clipboard-payload.ts b/packages/diagram-client/src/clipboard-payload.ts deleted file mode 100644 index 1cbb1f8..0000000 --- a/packages/diagram-client/src/clipboard-payload.ts +++ /dev/null @@ -1,50 +0,0 @@ -export const WORKFLOW_CLIPBOARD_FORMAT = 'application/json'; - -type SerializedPosition = { line: number; character: number }; -export type SerializedRange = { start: SerializedPosition; end: SerializedPosition }; - -export type WorkflowClipboardItem = - | { - kind: 'entity'; - elementTypeId: string; - entityName: string; - sourceRange: SerializedRange; - text: string; - } - | { - kind: 'boundaryPort'; - elementTypeId: string; - direction: 'input' | 'output'; - portName: string; - sourceRange: SerializedRange; - text: string; - } - | { - kind: 'connection'; - elementTypeId: string; - sourceRange: SerializedRange; - text: string; - }; - -export interface WorkflowClipboardPayload { - version: 1; - sourceUri: string; - items: WorkflowClipboardItem[]; -} - -export function tryParseWorkflowClipboardPayload(clipboardData: Record): WorkflowClipboardPayload | undefined { - const json = clipboardData[WORKFLOW_CLIPBOARD_FORMAT] ?? Object.values(clipboardData)[0]; - if (!json) { - return undefined; - } - - try { - const parsed = JSON.parse(json) as WorkflowClipboardPayload; - if (!parsed || parsed.version !== 1 || typeof parsed.sourceUri !== 'string' || !Array.isArray(parsed.items)) { - return undefined; - } - return parsed; - } catch { - return undefined; - } -} diff --git a/packages/diagram-client/src/post-edit-selection-service.ts b/packages/diagram-client/src/post-edit-selection-service.ts index 04be9eb..95dd7ce 100644 --- a/packages/diagram-client/src/post-edit-selection-service.ts +++ b/packages/diagram-client/src/post-edit-selection-service.ts @@ -1,6 +1,5 @@ import { injectable } from 'inversify'; -import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; -import { tryParseWorkflowClipboardPayload } from './clipboard-payload'; +import { parseClipboardPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; export type PendingSelection = { entityNames: string[]; @@ -103,7 +102,7 @@ export class PostEditSelectionService { // Called before dispatching PasteOperation. // eslint-disable-next-line @typescript-eslint/no-explicit-any prepareForPaste(clipboardData: Record, modelRoot: any): void { - const payload = tryParseWorkflowClipboardPayload(clipboardData); + const payload = parseClipboardPayload(clipboardData); if (!payload) { this.pending = undefined; return; @@ -116,7 +115,7 @@ export class PostEditSelectionService { for (const item of payload.items ?? []) { if (item.kind === 'entity') { - const newName = generateUnique(item.entityName, usedEntities); + const newName = generateUnique(item.name, usedEntities); entityNames.push(newName); } if (item.kind === 'boundaryPort') { diff --git a/packages/diagram-client/test/post-edit-selection-paste.test.ts b/packages/diagram-client/test/post-edit-selection-paste.test.ts new file mode 100644 index 0000000..d389a06 --- /dev/null +++ b/packages/diagram-client/test/post-edit-selection-paste.test.ts @@ -0,0 +1,46 @@ +/** + * After a paste the webview selects the pasted elements by the names the + * sidecar gives them. It reads those names from the clipboard payload copy + * wrote, so it must read the shape copy writes. + */ +import 'reflect-metadata'; +import { describe, expect, it } from 'vitest'; +import { clipboardDataFromPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { PostEditSelectionService } from '../src/post-edit-selection-service'; + +function modelRootWith(elements: Array<{ id: string; type: string; args: Record }>) { + return { type: 'graph', children: elements }; +} + +describe('PostEditSelectionService.prepareForPaste', () => { + it('plans the pasted names from the entity names of the copied payload', () => { + const service = new PostEditSelectionService(); + const root = modelRootWith([ + { id: 'add', type: WorkflowDiagramTypes.NODE_ACTOR, args: { [WorkflowDiagramMetadata.ENTITY_NAME]: 'add' } }, + { id: 'a', type: WorkflowDiagramTypes.NODE_BOUNDARY_INPUT, args: { [WorkflowDiagramMetadata.PORT_NAME]: 'a' } } + ]); + const clipboardData = clipboardDataFromPayload({ + version: 1, + sourceUri: 'file:///hdl/top.sv', + items: [ + { kind: 'entity', elementTypeId: WorkflowDiagramTypes.NODE_ACTOR, name: 'add', typeName: 'Adder' }, + { kind: 'boundaryPort', elementTypeId: WorkflowDiagramTypes.NODE_BOUNDARY_INPUT, direction: 'input', portName: 'a' } + ] + }); + + service.prepareForPaste(clipboardData, root); + + const pending = (service as any).pending; + expect(pending.entityNames).toHaveLength(1); + expect(pending.entityNames[0]).not.toBe('add'); + expect(pending.entityNames[0]).toMatch(/^add/); + expect(pending.boundaryPortNames).toHaveLength(1); + expect(pending.boundaryPortNames[0]).toMatch(/^a/); + }); + + it('plans nothing for clipboard data that is not a diagram payload', () => { + const service = new PostEditSelectionService(); + service.prepareForPaste({ 'text/plain': 'not a payload' }, modelRootWith([])); + expect((service as any).pending).toBeUndefined(); + }); +}); diff --git a/packages/diagram-server/src/clipboard/clipboard.ts b/packages/diagram-server/src/clipboard/clipboard.ts index b13d55e..1d99b98 100644 --- a/packages/diagram-server/src/clipboard/clipboard.ts +++ b/packages/diagram-server/src/clipboard/clipboard.ts @@ -1,81 +1,8 @@ import type { Args } from '@eclipse-glsp/protocol'; import type { ModelState } from '@eclipse-glsp/server'; -import * as vscode from 'vscode'; -import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { WorkflowDiagramMetadata, WorkflowDiagramTypes, type ClipboardItem, type ClipboardPayload } from '@dialogram/shared'; -export const WORKFLOW_CLIPBOARD_FORMAT = 'application/json'; - -type SerializedPosition = { line: number; character: number }; -export type SerializedRange = { start: SerializedPosition; end: SerializedPosition }; - -export type WorkflowClipboardItem = - | { - kind: 'entity'; - elementTypeId: string; - entityName: string; - sourceRange: SerializedRange; - text: string; - } - | { - kind: 'boundaryPort'; - elementTypeId: string; - direction: 'input' | 'output'; - portName: string; - sourceRange: SerializedRange; - text: string; - } - | { - kind: 'connection'; - elementTypeId: string; - sourceRange: SerializedRange; - text: string; - }; - -export interface WorkflowClipboardPayload { - version: 1; - sourceUri: string; - items: WorkflowClipboardItem[]; -} - -function isSerializedPosition(value: unknown): value is SerializedPosition { - if (!value || typeof value !== 'object') { - return false; - } - const v = value as Record; - return typeof v.line === 'number' && typeof v.character === 'number'; -} - -export function isSerializedRange(value: unknown): value is SerializedRange { - if (!value || typeof value !== 'object') { - return false; - } - const v = value as Record; - return isSerializedPosition(v.start) && isSerializedPosition(v.end); -} - -function findNearestSourceRange(element: unknown): SerializedRange | undefined { - let current: any = element; - while (current) { - const args: Args | undefined = current.args; - const range = args?.[WorkflowDiagramMetadata.SOURCE_RANGE]; - if (isSerializedRange(range)) { - return range; - } - current = current.parent; - } - return undefined; -} - -function getTextForRange(document: vscode.TextDocument, range: SerializedRange): string { - return document.getText( - new vscode.Range( - range.start.line, - range.start.character, - range.end.line, - range.end.character - ) - ); -} +export { clipboardDataFromPayload, parseClipboardPayload, type ClipboardItem, type ClipboardPayload } from '@dialogram/shared'; function isReadOnly(element: any): boolean { const args: Args | undefined = element?.args; @@ -85,65 +12,46 @@ function isReadOnly(element: any): boolean { return args[WorkflowDiagramMetadata.IS_READ_ONLY] === true || args[WorkflowDiagramMetadata.IS_VIRTUAL] === true; } -export async function buildWorkflowClipboardPayload( - modelState: ModelState, - sourceUri: string, - selectedElementIds: string[] -): Promise { - const doc = await vscode.workspace.openTextDocument(vscode.Uri.parse(sourceUri)); - - const items: WorkflowClipboardItem[] = []; +/** + * The clipboard payload for the selected elements: every editable entity, + * boundary port and connection among them, by the names a paste re-creates + * it from. Read-only and virtual elements are left out. + */ +export function buildClipboardPayload(modelState: ModelState, sourceUri: string, selectedElementIds: string[]): ClipboardPayload { + const items: ClipboardItem[] = []; for (const id of selectedElementIds) { const element: any = modelState.index.get(id); if (!element || isReadOnly(element)) { continue; } - const elementTypeId: string | undefined = element.type; const args: Args | undefined = element.args; - const range = findNearestSourceRange(element); - if (!range) { - continue; - } - - const text = getTextForRange(doc, range); - if (!text || text.trim() === '') { - continue; - } - if ( elementTypeId === WorkflowDiagramTypes.NODE_ACTOR || elementTypeId === WorkflowDiagramTypes.NODE_NETWORK || elementTypeId === WorkflowDiagramTypes.NODE_EXTERNAL_ACTOR ) { - const entityName = args?.[WorkflowDiagramMetadata.ENTITY_NAME]; - if (typeof entityName !== 'string' || entityName.trim() === '') { - continue; + const name = args?.[WorkflowDiagramMetadata.ENTITY_NAME]; + const typeName = args?.[WorkflowDiagramMetadata.ENTITY_TYPE]; + if (typeof name === 'string' && typeof typeName === 'string') { + items.push({ kind: 'entity', elementTypeId, name, typeName }); } - items.push({ - kind: 'entity', - elementTypeId, - entityName, - sourceRange: range, - text - }); continue; } if (elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT || elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT) { const portName = args?.[WorkflowDiagramMetadata.PORT_NAME]; - if (typeof portName !== 'string' || portName.trim() === '') { - continue; + const portType = args?.[WorkflowDiagramMetadata.PORT_TYPE]; + if (typeof portName === 'string') { + items.push({ + kind: 'boundaryPort', + elementTypeId, + direction: elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT ? 'input' : 'output', + portName, + portType: typeof portType === 'string' ? portType : undefined + }); } - items.push({ - kind: 'boundaryPort', - elementTypeId, - direction: elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT ? 'input' : 'output', - portName, - sourceRange: range, - text - }); continue; } @@ -151,23 +59,13 @@ export async function buildWorkflowClipboardPayload( items.push({ kind: 'connection', elementTypeId, - sourceRange: range, - text + from: args?.['wf:from'] as string | undefined, + to: args?.['wf:to'] as string | undefined, + outPort: args?.['wf:outPort'] as string | undefined, + inPort: args?.['wf:inPort'] as string | undefined }); continue; } } - - return { - version: 1, - sourceUri, - items - }; -} - -export function clipboardDataFromPayload(payload: WorkflowClipboardPayload): Record { - return { - [WORKFLOW_CLIPBOARD_FORMAT]: JSON.stringify(payload) - }; + return { version: 1, sourceUri, items }; } - diff --git a/packages/diagram-server/src/clipboard/py-clipboard.ts b/packages/diagram-server/src/clipboard/py-clipboard.ts deleted file mode 100644 index edf6d67..0000000 --- a/packages/diagram-server/src/clipboard/py-clipboard.ts +++ /dev/null @@ -1,120 +0,0 @@ -import type { Args } from '@eclipse-glsp/protocol'; -import type { ModelState } from '@eclipse-glsp/server'; -import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; - -export const PY_CLIPBOARD_FORMAT = 'application/json'; - -export type PyClipboardItem = - | { - kind: 'entity'; - elementTypeId: string; - name: string; - typeName: string; - } - | { - kind: 'boundaryPort'; - elementTypeId: string; - direction: 'input' | 'output'; - portName: string; - portType?: string; - } - | { - kind: 'connection'; - elementTypeId: string; - from?: string; - to?: string; - outPort?: string; - inPort?: string; - }; - -export interface PyClipboardPayload { - version: 1; - sourceUri: string; - items: PyClipboardItem[]; -} - -function isReadOnly(element: any): boolean { - const args: Args | undefined = element?.args; - if (!args) { - return false; - } - return args[WorkflowDiagramMetadata.IS_READ_ONLY] === true || args[WorkflowDiagramMetadata.IS_VIRTUAL] === true; -} - -export async function buildPyClipboardPayload( - modelState: ModelState, - sourceUri: string, - selectedElementIds: string[] -): Promise { - const items: PyClipboardItem[] = []; - for (const id of selectedElementIds) { - const element: any = modelState.index.get(id); - if (!element || isReadOnly(element)) { - continue; - } - const elementTypeId: string | undefined = element.type; - const args: Args | undefined = element.args; - - if ( - elementTypeId === WorkflowDiagramTypes.NODE_ACTOR || - elementTypeId === WorkflowDiagramTypes.NODE_NETWORK || - elementTypeId === WorkflowDiagramTypes.NODE_EXTERNAL_ACTOR - ) { - const name = args?.[WorkflowDiagramMetadata.ENTITY_NAME]; - const typeName = args?.[WorkflowDiagramMetadata.ENTITY_TYPE]; - if (typeof name === 'string' && typeof typeName === 'string') { - items.push({ kind: 'entity', elementTypeId, name, typeName }); - } - continue; - } - - if (elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT || elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT) { - const portName = args?.[WorkflowDiagramMetadata.PORT_NAME]; - const portType = args?.[WorkflowDiagramMetadata.PORT_TYPE]; - if (typeof portName === 'string') { - items.push({ - kind: 'boundaryPort', - elementTypeId, - direction: elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT ? 'input' : 'output', - portName, - portType: typeof portType === 'string' ? portType : undefined - }); - } - continue; - } - - if (elementTypeId === WorkflowDiagramTypes.EDGE_CONNECTION || elementTypeId === WorkflowDiagramTypes.EDGE_CONNECTION_NO_ARROW) { - items.push({ - kind: 'connection', - elementTypeId, - from: args?.['wf:from'] as string | undefined, - to: args?.['wf:to'] as string | undefined, - outPort: args?.['wf:outPort'] as string | undefined, - inPort: args?.['wf:inPort'] as string | undefined - }); - continue; - } - } - - return { version: 1, sourceUri, items }; -} - -export function pyClipboardDataFromPayload(payload: PyClipboardPayload): Record { - return { [PY_CLIPBOARD_FORMAT]: JSON.stringify(payload) }; -} - -export function parsePyClipboardPayload(clipboardData: Record): PyClipboardPayload | undefined { - const json = clipboardData[PY_CLIPBOARD_FORMAT] ?? Object.values(clipboardData)[0]; - if (!json) { - return undefined; - } - try { - const parsed = JSON.parse(json) as PyClipboardPayload; - if (!parsed || parsed.version !== 1 || !Array.isArray(parsed.items) || typeof parsed.sourceUri !== 'string') { - return undefined; - } - return parsed; - } catch { - return undefined; - } -} diff --git a/packages/diagram-server/src/server/request-clipboard-data-action-handler.ts b/packages/diagram-server/src/server/request-clipboard-data-action-handler.ts index 59d71c8..abe714a 100644 --- a/packages/diagram-server/src/server/request-clipboard-data-action-handler.ts +++ b/packages/diagram-server/src/server/request-clipboard-data-action-handler.ts @@ -3,7 +3,7 @@ import { ActionHandler, ModelState, type MaybePromise } from '@eclipse-glsp/serv import { inject, injectable } from 'inversify'; import { WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; -import { buildWorkflowClipboardPayload, clipboardDataFromPayload } from '../clipboard/clipboard'; +import { buildClipboardPayload, clipboardDataFromPayload } from '../clipboard/clipboard'; @injectable() export class WorkflowRequestClipboardDataActionHandler implements ActionHandler { @@ -20,7 +20,7 @@ export class WorkflowRequestClipboardDataActionHandler implements ActionHandler return [SetClipboardDataAction.create({}, { responseId: action.requestId })]; } - const payload = await buildWorkflowClipboardPayload(this.modelState, sourceUri, action.editorContext.selectedElementIds); + const payload = buildClipboardPayload(this.modelState, sourceUri, action.editorContext.selectedElementIds); const clipboardData = clipboardDataFromPayload(payload); return [SetClipboardDataAction.create(clipboardData, { responseId: action.requestId })]; diff --git a/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts b/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts new file mode 100644 index 0000000..378f7b6 --- /dev/null +++ b/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts @@ -0,0 +1,88 @@ +/** + * Copy writes the clipboard shape paste reads. The two used to differ: copy + * wrote the entity's name under `entityName` with its source text, while the + * sidecar toolkit's paste looked for `name` and `typeName`, so a copied node + * pasted as nothing. Cut and duplicate already wrote the paste shape. + */ +import { RequestClipboardDataAction } from '@eclipse-glsp/protocol'; +import { describe, expect, it } from 'vitest'; +import { parseClipboardPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; +import { WorkflowRequestClipboardDataActionHandler } from '../src/server/request-clipboard-data-action-handler'; + +function modelStateWith(elements: Record, sourceUri?: string) { + return { + sourceUri, + get: (key: string) => (key === WORKFLOW_NETWORK_MODEL_KEY ? { documentUri: sourceUri } : undefined), + index: { get: (id: string) => elements[id] } + }; +} + +function copyOf(handler: WorkflowRequestClipboardDataActionHandler, selectedElementIds: string[]) { + return handler.execute(RequestClipboardDataAction.create({ selectedElementIds }, { requestId: 'req-1' })); +} + +describe('WorkflowRequestClipboardDataActionHandler', () => { + it('writes the payload the toolkit paste reads: names, not source text', async () => { + const handler = new WorkflowRequestClipboardDataActionHandler(); + (handler as any).modelState = modelStateWith({ + add: { + type: WorkflowDiagramTypes.NODE_ACTOR, + args: { [WorkflowDiagramMetadata.ENTITY_NAME]: 'add', [WorkflowDiagramMetadata.ENTITY_TYPE]: 'Adder' } + }, + inPort: { + type: WorkflowDiagramTypes.NODE_BOUNDARY_INPUT, + args: { [WorkflowDiagramMetadata.PORT_NAME]: 'a', [WorkflowDiagramMetadata.PORT_TYPE]: 'int' } + }, + wire: { + type: WorkflowDiagramTypes.EDGE_CONNECTION, + args: { 'wf:from': 'src', 'wf:to': 'add', 'wf:outPort': 'out', 'wf:inPort': 'a' } + } + }, 'file:///hdl/top.sv'); + + const [action] = await copyOf(handler, ['add', 'inPort', 'wire']); + const payload = parseClipboardPayload((action as any).clipboardData); + + expect(payload).toEqual({ + version: 1, + sourceUri: 'file:///hdl/top.sv', + items: [ + { kind: 'entity', elementTypeId: WorkflowDiagramTypes.NODE_ACTOR, name: 'add', typeName: 'Adder' }, + { kind: 'boundaryPort', elementTypeId: WorkflowDiagramTypes.NODE_BOUNDARY_INPUT, direction: 'input', portName: 'a', portType: 'int' }, + { kind: 'connection', elementTypeId: WorkflowDiagramTypes.EDGE_CONNECTION, from: 'src', to: 'add', outPort: 'out', inPort: 'a' } + ] + }); + expect((action as any).responseId).toBe('req-1'); + }); + + it('leaves out read-only elements and elements with no name', async () => { + const handler = new WorkflowRequestClipboardDataActionHandler(); + (handler as any).modelState = modelStateWith({ + locked: { + type: WorkflowDiagramTypes.NODE_ACTOR, + args: { [WorkflowDiagramMetadata.ENTITY_NAME]: 'x', [WorkflowDiagramMetadata.ENTITY_TYPE]: 'X', [WorkflowDiagramMetadata.IS_READ_ONLY]: true } + }, + nameless: { type: WorkflowDiagramTypes.NODE_ACTOR, args: {} } + }, 'file:///hdl/top.sv'); + + const [action] = await copyOf(handler, ['locked', 'nameless', 'missing']); + expect(parseClipboardPayload((action as any).clipboardData)?.items).toEqual([]); + }); + + it('answers with empty clipboard data when the model has no source', async () => { + const handler = new WorkflowRequestClipboardDataActionHandler(); + (handler as any).modelState = modelStateWith({}, undefined); + + const [action] = await copyOf(handler, ['anything']); + expect((action as any).clipboardData).toEqual({}); + expect(parseClipboardPayload((action as any).clipboardData)).toBeUndefined(); + }); +}); + +describe('parseClipboardPayload', () => { + it('rejects data that is not a diagram payload', () => { + expect(parseClipboardPayload(undefined)).toBeUndefined(); + expect(parseClipboardPayload({})).toBeUndefined(); + expect(parseClipboardPayload({ 'text/plain': 'hello' })).toBeUndefined(); + expect(parseClipboardPayload({ 'application/json': JSON.stringify({ version: 2, sourceUri: 'x', items: [] }) })).toBeUndefined(); + }); +}); diff --git a/packages/shared/src/clipboard-payload.ts b/packages/shared/src/clipboard-payload.ts new file mode 100644 index 0000000..6a56ec9 --- /dev/null +++ b/packages/shared/src/clipboard-payload.ts @@ -0,0 +1,65 @@ +/** + * The one clipboard shape of a diagram. The server writes it on copy (and the + * sidecar toolkit on cut and duplicate), the toolkit's paste reads it, and the + * webview reads it to pick the pasted elements once the model is back. + * + * An item carries the names the paste re-creates the element from, never the + * source text: the pasted entity gets a fresh name in the target diagram, and + * the product's sidecar writes it in the product's own syntax. + */ + +export const CLIPBOARD_FORMAT = 'application/json'; + +export type ClipboardItem = + | { + kind: 'entity'; + elementTypeId: string; + name: string; + typeName: string; + } + | { + kind: 'boundaryPort'; + elementTypeId: string; + direction: 'input' | 'output'; + portName: string; + portType?: string; + } + | { + kind: 'connection'; + elementTypeId: string; + from?: string; + to?: string; + outPort?: string; + inPort?: string; + }; + +export interface ClipboardPayload { + version: 1; + sourceUri: string; + items: ClipboardItem[]; +} + +/** The GLSP clipboard-data record carrying `payload`. */ +export function clipboardDataFromPayload(payload: ClipboardPayload): Record { + return { [CLIPBOARD_FORMAT]: JSON.stringify(payload) }; +} + +/** The payload in a GLSP clipboard-data record, or `undefined` when it carries none. */ +export function parseClipboardPayload(clipboardData: Record | undefined): ClipboardPayload | undefined { + if (!clipboardData) { + return undefined; + } + const json = clipboardData[CLIPBOARD_FORMAT] ?? Object.values(clipboardData)[0]; + if (!json) { + return undefined; + } + try { + const parsed = JSON.parse(json) as ClipboardPayload; + if (!parsed || parsed.version !== 1 || !Array.isArray(parsed.items) || typeof parsed.sourceUri !== 'string') { + return undefined; + } + return parsed; + } catch { + return undefined; + } +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 42311b7..adae842 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -15,3 +15,4 @@ export * from './diagram-seams'; export * from './hierarchy'; export * from './chat-seams'; export * from './create-task-type-operation'; +export * from './clipboard-payload'; diff --git a/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts b/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts index 25b5e90..ee70b0d 100644 --- a/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts @@ -220,7 +220,7 @@ export class CreateNodeOperationHandler extends OperationHandler { createCommand(operation: CreateNodeOperation): MaybePromise { const sourceUri = this.modelState.root.args?.['sourceUri'] as string | undefined; - if (!sourceUri || !sourceUri.endsWith('.py')) { + if (!sourceUri || !this.sidecar.handlesSource(sourceUri)) { return undefined; } return this.createPythonCommand(operation, sourceUri); diff --git a/packages/sidecar-toolkit/src/server/operations/cut-handler.ts b/packages/sidecar-toolkit/src/server/operations/cut-handler.ts index 38427b7..55adf2f 100644 --- a/packages/sidecar-toolkit/src/server/operations/cut-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/cut-handler.ts @@ -3,7 +3,7 @@ import { OperationHandler, ModelState, type Command, type MaybePromise } from '@ import { inject, injectable } from 'inversify'; import * as vscode from 'vscode'; import { readAuthoritativeSourceText } from './authoritative-source-text'; -import { buildPyClipboardPayload, pyClipboardDataFromPayload } from '@dialogram/diagram-server/clipboard/py-clipboard'; +import { buildClipboardPayload, clipboardDataFromPayload } from '@dialogram/diagram-server/clipboard/clipboard'; import { WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { WorkflowDiagramTypeGuards } from '@dialogram/shared'; @@ -44,11 +44,11 @@ export class CutOperationHandler extends OperationHandler { uri: vscodeUri, computeEdits: async () => { const beforeText = (await vscode.workspace.openTextDocument(vscodeUri)).getText(); - const payload = await buildPyClipboardPayload(this.modelState, sourceUri, elementIds); + const payload = await buildClipboardPayload(this.modelState, sourceUri, elementIds); if (payload.items.length === 0) { return undefined; } - await vscode.env.clipboard.writeText(JSON.stringify(pyClipboardDataFromPayload(payload))); + await vscode.env.clipboard.writeText(JSON.stringify(clipboardDataFromPayload(payload))); for (const item of payload.items) { if (item.kind === 'entity') { await this.sendSidecarOp(sourceUri, { diff --git a/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts b/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts index af139c8..162f73c 100644 --- a/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts @@ -6,7 +6,7 @@ import { readAuthoritativeSourceText } from './authoritative-source-text'; import { WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { ReversibleWorkspaceEditCommand } from '@dialogram/diagram-server/operations/reversible-workspace-edit-command'; -import { buildPyClipboardPayload } from '@dialogram/diagram-server/clipboard/py-clipboard'; +import { buildClipboardPayload } from '@dialogram/diagram-server/clipboard/clipboard'; import { SidecarInvoker } from './sidecar-invoker'; const WORKFLOW_DUPLICATE_OPERATION_KIND = 'dialogram.duplicate' as const; @@ -42,7 +42,7 @@ export class DuplicateOperationHandler extends OperationHandler { uri: vscodeUri, computeEdits: async () => { const beforeText = (await vscode.workspace.openTextDocument(vscodeUri)).getText(); - const payload = await buildPyClipboardPayload(this.modelState, sourceUri, operation.elementIds ?? []); + const payload = await buildClipboardPayload(this.modelState, sourceUri, operation.elementIds ?? []); if (payload.items.length === 0) return undefined; const workflowName = this.modelState.root.args?.['wf:workflowName'] as string | undefined; const takenResp = await this.sendSidecarList(sourceUri, { diff --git a/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts b/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts index a4eb74d..fbd0de7 100644 --- a/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts @@ -49,7 +49,7 @@ class EntityPortCrudHandlerBase extends OperationHandler { createCommand(action: Action): Command | undefined { const sourceUri = this.modelState.sourceUri; - if (!sourceUri || !sourceUri.endsWith('.py')) { + if (!sourceUri || !this.sidecar.handlesSource(sourceUri)) { return undefined; } const createKind = this.sidecar.operationKindCreateEntityPort(); diff --git a/packages/sidecar-toolkit/src/server/operations/paste-handler.ts b/packages/sidecar-toolkit/src/server/operations/paste-handler.ts index 31eb54a..38f0f18 100644 --- a/packages/sidecar-toolkit/src/server/operations/paste-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/paste-handler.ts @@ -6,7 +6,7 @@ import { readAuthoritativeSourceText } from './authoritative-source-text'; import { WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { ReversibleWorkspaceEditCommand } from '@dialogram/diagram-server/operations/reversible-workspace-edit-command'; -import { parsePyClipboardPayload } from '@dialogram/diagram-server/clipboard/py-clipboard'; +import { parseClipboardPayload } from '@dialogram/diagram-server/clipboard/clipboard'; import { SidecarInvoker } from './sidecar-invoker'; @injectable() @@ -42,9 +42,9 @@ export class PasteOperationHandler extends OperationHandler { command: ReversibleWorkspaceEditCommand, sourceUri: string, ): Promise { - const payload = parsePyClipboardPayload(operation.clipboardData as any); + const payload = parseClipboardPayload(operation.clipboardData as Record | undefined); if (!payload) { - void vscode.window.showWarningMessage('Clipboard does not contain Python diagram data.'); + void vscode.window.showWarningMessage('The clipboard holds no diagram elements.'); return undefined; } const workflowName = this.modelState.root.args?.['wf:workflowName'] as string | undefined; diff --git a/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts b/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts index 52ba49a..2112e67 100644 --- a/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts +++ b/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts @@ -47,6 +47,11 @@ export class SidecarInvoker { return this.runtimeProfile.undoLabelSuffix; } + /** Whether `sourceUri` names a file this product's sidecar edits. */ + handlesSource(sourceUri: string): boolean { + return this.runtimeProfile.handlesSource(sourceUri); + } + createNodeStrings(): CreateNodeStrings { return this.runtimeProfile.createNodeStrings; } diff --git a/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts b/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts index f0744d7..b4eb12d 100644 --- a/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts +++ b/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts @@ -220,6 +220,13 @@ export interface SidecarRuntimeConfig { * errors stay in the diagram's overlay, as they always have. */ documentErrorsToProblems?: boolean; + /** + * The extensions, dot included, of the source files this runtime edits + * (`['.sv', '.v']` for a Verilog product). An operation handler that + * writes to the model's source refuses a `sourceUri` carrying none of + * them. Absent: no gate, every source is accepted. + */ + sourceExtensions?: string[]; operationKinds: { createEntityPort: string; deleteEntityPort: string }; /** Op prefixes this runtime accepts when rewriting foreign-prefixed ops * (replaces the hardcoded product-prefix literal check). */ @@ -261,6 +268,19 @@ function isPythonInterpreterCommand(command: string): boolean { return /(^|\/)python(?:\d+(?:\.\d+)*)?$/i.test(command.trim()); } +/** + * Whether `sourceUri` ends in one of the config's `sourceExtensions`, case + * ignored. A config that names none accepts every source. + */ +export function handlesSource(cfg: Pick, sourceUri: string): boolean { + const extensions = cfg.sourceExtensions ?? []; + if (extensions.length === 0) { + return true; + } + const lower = sourceUri.toLowerCase(); + return extensions.some(ext => lower.endsWith(ext.toLowerCase())); +} + /** `.` — the fully-qualified sidecar op name for this runtime. */ export function sidecarOp(cfg: SidecarRuntimeConfig, opName: string): string { return `${cfg.sidecarOperationPrefix}.${opName}`; @@ -400,6 +420,11 @@ export class SidecarRuntimeService { return sidecarOp(this.cfg, opName); } + /** Whether `sourceUri` names a file this runtime edits (see `SidecarRuntimeConfig.sourceExtensions`). */ + handlesSource(sourceUri: string): boolean { + return handlesSource(this.cfg, sourceUri); + } + rewriteSidecarOperation(operation: string): string { const dot = operation.indexOf('.'); if (dot <= 0) { diff --git a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts index e6ae486..28260d9 100644 --- a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts +++ b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts @@ -400,7 +400,8 @@ function sidecarRuntimeConfig(input: SidecarProfileInput): SidecarRuntimeConfig createNodeBehavior: input.createNodeBehavior, createNodeVariants: input.createNodeVariants, navigation: input.navigation, - documentErrorsToProblems: input.documentErrorsToProblems + documentErrorsToProblems: input.documentErrorsToProblems, + sourceExtensions: sourceExtensionsOf(input) }; } diff --git a/packages/sidecar-toolkit/test/create-node-handler.test.ts b/packages/sidecar-toolkit/test/create-node-handler.test.ts index ce37582..14e4f35 100644 --- a/packages/sidecar-toolkit/test/create-node-handler.test.ts +++ b/packages/sidecar-toolkit/test/create-node-handler.test.ts @@ -66,6 +66,7 @@ function calpySidecar() { settingsNamespace: () => 'calLang', sidecarOp: (opName: string) => `calpy.${opName}`, undoLabelSuffix: () => ' (wfpy)', + handlesSource: () => true, createNodeStrings: () => CALPY_CREATE_NODE_STRINGS, createNodeBehavior: () => CALPY_CREATE_NODE_BEHAVIOR, // This product contributes no variant entries. @@ -78,6 +79,7 @@ function wfpySidecar() { settingsNamespace: () => 'wfLang', sidecarOp: (opName: string) => `wfpy.${opName}`, undoLabelSuffix: () => ' (wfpy)', + handlesSource: () => true, createNodeStrings: () => WFPY_CREATE_NODE_STRINGS, createNodeBehavior: () => WFPY_CREATE_NODE_BEHAVIOR, createNodeVariants: () => [] @@ -873,6 +875,7 @@ describe('workflow create node handler', () => { settingsNamespace: () => 'anyLang', sidecarOp: (opName: string) => `any.${opName}`, undoLabelSuffix: () => ' (any)', + handlesSource: () => true, createNodeStrings: () => ({ newTypeNamePrompt: () => 'New gizmo name', typeLabel: () => 'gizmo', diff --git a/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts b/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts index 86e4721..184aab0 100644 --- a/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts +++ b/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts @@ -108,6 +108,7 @@ async function runCreate(options: { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS diff --git a/packages/sidecar-toolkit/test/source-extension-gate.test.ts b/packages/sidecar-toolkit/test/source-extension-gate.test.ts new file mode 100644 index 0000000..72622e0 --- /dev/null +++ b/packages/sidecar-toolkit/test/source-extension-gate.test.ts @@ -0,0 +1,127 @@ +/** + * The operation handlers that write to a diagram's source used to accept only a + * `.py` source, so the palette never created a node, and the property panel never + * added an entity port, in a Verilog, VHDL or NDF diagram. The gate now reads the + * profile's `sourceExtensions`. + */ +import 'reflect-metadata'; +import { describe, expect, it } from 'vitest'; +import { Container } from 'inversify'; +import { ModelState } from '@eclipse-glsp/server'; +import { LayoutPersistenceService } from '@dialogram/diagram-server/services/layout-persistence-service'; +import { WorkflowDiagramTypes } from '@dialogram/shared'; +import { createSidecarServerModule } from '../src/server/sidecar-server-module.js'; +import { SidecarInvoker } from '../src/server/operations/sidecar-invoker.js'; +import { CreateNodeOperationHandler } from '../src/server/operations/create-node-handler.js'; +import { createEntityPortCrudHandlers } from '../src/server/operations/entity-port-crud-handler.js'; +import { handlesSource, type SidecarRuntimeConfig } from '../src/server/sidecar-runtime-config.js'; +import { createSidecarDiagramProfile, type SidecarProfileInput } from '../src/sidecar-diagram-profile.js'; +import { NEUTRAL_CREATE_NODE_CONFIG } from './fixtures/create-node-config.js'; + +function verilogConfig(overrides: Partial = {}): SidecarRuntimeConfig { + return { + settingsNamespace: 'hdl', + sidecarOperationPrefix: 'hdl', + sidecarCommandSettingKey: 'sidecarCommand', + sidecarCommandDefault: 'hdl-sidecar', + cliCommandSettingKey: 'cliCommand', + cliCommandDefault: 'hdl', + operationKinds: { createEntityPort: 'hdl.createEntityPort', deleteEntityPort: 'hdl.deleteEntityPort' }, + acceptedOperationPrefixes: ['hdl'], + graphAcquisition: 'cli-plan', + sourceExtensions: ['.sv', '.v'], + ...NEUTRAL_CREATE_NODE_CONFIG, + ...overrides + }; +} + +function containerFor(cfg: SidecarRuntimeConfig, sourceUri: string): Container { + const container = new Container(); + container.load(createSidecarServerModule(cfg)); + container.bind(ModelState).toConstantValue({ sourceUri, root: { args: { sourceUri } } } as any); + container.bind(LayoutPersistenceService).toConstantValue(new LayoutPersistenceService()); + return container; +} + +describe('handlesSource', () => { + it('accepts a source with one of the configured extensions, whatever its case', () => { + const cfg = { sourceExtensions: ['.sv', '.v'] }; + expect(handlesSource(cfg, 'file:///hdl/top.sv')).toBe(true); + expect(handlesSource(cfg, 'file:///hdl/TOP.V')).toBe(true); + expect(handlesSource(cfg, 'file:///hdl/top.py')).toBe(false); + expect(handlesSource(cfg, 'file:///hdl/top.svh')).toBe(false); + }); + + it('accepts every source when the config names no extension', () => { + expect(handlesSource({}, 'file:///any/file.xyz')).toBe(true); + expect(handlesSource({ sourceExtensions: [] }, 'file:///any/file.xyz')).toBe(true); + }); +}); + +describe('the source gate of the editing handlers', () => { + const createNode = { kind: 'createNode', elementTypeId: WorkflowDiagramTypes.NODE_TASK, location: { x: 0, y: 0 } } as any; + + it('lets the palette create a node in a Verilog diagram', () => { + const container = containerFor(verilogConfig(), 'file:///hdl/top.sv'); + const handler = container.resolve(CreateNodeOperationHandler); + expect(handler.createCommand(createNode)).toBeDefined(); + }); + + it('still refuses a source the product does not edit', () => { + const container = containerFor(verilogConfig(), 'file:///hdl/top.py'); + const handler = container.resolve(CreateNodeOperationHandler); + expect(handler.createCommand(createNode)).toBeUndefined(); + }); + + it('keeps the Python product on its own sources', () => { + const python = verilogConfig({ sourceExtensions: ['.py'] }); + expect(containerFor(python, 'file:///wf/flow.py').resolve(CreateNodeOperationHandler).createCommand(createNode)).toBeDefined(); + expect(containerFor(python, 'file:///wf/flow.sv').resolve(CreateNodeOperationHandler).createCommand(createNode)).toBeUndefined(); + }); + + it('lets the property panel add an entity port in a VHDL diagram', () => { + const cfg = verilogConfig({ sourceExtensions: ['.vhd', '.vhdl'] }); + const [CreateHandler] = createEntityPortCrudHandlers(cfg.operationKinds); + const container = containerFor(cfg, 'file:///hdl/top.vhd'); + const handler = container.resolve(CreateHandler) as any; + const op = { kind: cfg.operationKinds.createEntityPort, entityType: 'Counter', portDirection: 'input', portName: 'clk', portType: 'std_logic' }; + expect(handler.createCommand(op)).toBeDefined(); + expect((containerFor(cfg, 'file:///hdl/top.py').resolve(CreateHandler) as any).createCommand(op)).toBeUndefined(); + }); +}); + +describe('the profile carries its source extensions to the server', () => { + const input = { + key: 'sv', + displayName: 'SystemVerilog', + settingsNamespace: 'sv', + customEditorViewType: 'sv.diagram', + glspClientId: 'sv.client', + glspClientName: 'sv', + commands: {} as SidecarProfileInput['commands'], + operationKinds: { createEntityPort: 'sv.createEntityPort', deleteEntityPort: 'sv.deleteEntityPort' }, + sidecarOperationPrefix: 'sv', + sidecarCommandSettingKey: 'sidecarCommand', + sidecarCommandDefault: 'hdl-sidecar', + cliCommandSettingKey: 'cliCommand', + cliCommandDefault: 'hdl', + acceptedOperationPrefixes: ['sv'], + graphAcquisition: 'cli-plan', + undoLabelSuffix: ' (sv)', + sourceExtension: '.sv', + sourceExtensions: ['.sv', '.v'], + chat: { name: 'sv', fullName: 'SystemVerilog', skill: 'sv' }, + ...NEUTRAL_CREATE_NODE_CONFIG + } as unknown as SidecarProfileInput; + + it('so the invoker the handlers inject answers for the product, not for Python', () => { + const profile = createSidecarDiagramProfile(input); + const container = new Container(); + for (const module of profile.serverModules ?? []) { + container.load(module); + } + const invoker = container.get(SidecarInvoker); + expect(invoker.handlesSource('file:///hdl/top.v')).toBe(true); + expect(invoker.handlesSource('file:///hdl/top.py')).toBe(false); + }); +}); diff --git a/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts b/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts index a8a8a90..d112938 100644 --- a/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts +++ b/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts @@ -181,6 +181,7 @@ async function runCreate( settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -297,6 +298,7 @@ describe('a variant entry, driven to the end', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -394,6 +396,7 @@ describe('a source palette entry', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -599,6 +602,7 @@ describe('a variant type already in the project', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -722,6 +726,7 @@ describe('an answer that belongs to the node, not the type', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -899,6 +904,7 @@ async function createWithMixed(existingTypes: string[]) { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', + handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS From 2405c5754bdc3a5a6787d0485cf8d5b6cf7dfc63 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Sun, 8 Nov 2026 08:41:00 +0100 Subject: [PATCH 2/2] fix: what the review of the create gate and the clipboard turned up The `.py` gate goes instead of growing: the other handlers that write to the source ask only that the model has one, so create-node and entity-port do the same. The `sourceExtensions` runtime field and `handlesSource` are gone again. A pasted or duplicated connection now joins the pasted copies, not the entities it was copied from. The pasted name follows one rule, `nextFreeName` in shared, on the sidecar side and in the webview, which predicted `add1` where the toolkit wrote `add2` and so never selected the pasted node. A paste with nothing to paste says so. The shared types are `DiagramClipboardItem` and `DiagramClipboardPayload`, clear of the DOM's `ClipboardItem`. Copy drops a blank name as it did before; cut and duplicate no longer await a synchronous builder. Claude-Session: https://claude.ai/code/session_01EToj6RJRMSSHQiqGNMU9wv --- CHANGELOG.md | 21 ++- .../src/post-edit-selection-service.ts | 13 +- .../test/post-edit-selection-paste.test.ts | 8 +- .../diagram-server/src/clipboard/clipboard.ts | 12 +- ...uest-clipboard-data-action-handler.test.ts | 15 ++- packages/shared/src/clipboard-payload.ts | 28 +++- .../server/operations/create-node-handler.ts | 2 +- .../src/server/operations/cut-handler.ts | 2 +- .../server/operations/duplicate-handler.ts | 19 ++- .../operations/entity-port-crud-handler.ts | 2 +- .../src/server/operations/paste-handler.ts | 21 ++- .../src/server/operations/sidecar-invoker.ts | 5 - .../src/server/sidecar-runtime-config.ts | 25 ---- .../src/sidecar-diagram-profile.ts | 3 +- .../test/create-in-every-product.test.ts | 70 ++++++++++ .../test/create-node-handler.test.ts | 3 - .../test/paste-handler.test.ts | 112 +++++++++++++++ .../test/picked-variant-create-wizard.test.ts | 1 - .../test/source-extension-gate.test.ts | 127 ------------------ .../test/variant-node-create-wizard.test.ts | 6 - 20 files changed, 262 insertions(+), 233 deletions(-) create mode 100644 packages/sidecar-toolkit/test/create-in-every-product.test.ts create mode 100644 packages/sidecar-toolkit/test/paste-handler.test.ts delete mode 100644 packages/sidecar-toolkit/test/source-extension-gate.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 96c8a6e..d6ff007 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,17 +92,24 @@ platform API version. - **The palette creates nodes, and the property panel adds entity ports, in every product's diagram.** Both handlers accepted only a `.py` source, so a - Verilog, VHDL or NDF diagram answered a palette drop with nothing. They now - ask the profile's `sourceExtensions` (toolkit runtime config - `sourceExtensions`, `SidecarInvoker.handlesSource`). + Verilog, VHDL or NDF diagram answered a palette drop with nothing. Like every + other handler that writes to the source, they now ask only that the model + has one. - **Copy then paste re-creates the copied elements.** Copy wrote the entity's name as `entityName` with its source text, while paste looked for `name` and `typeName`, so a copied node pasted as nothing; cut and duplicate already wrote the paste shape. One payload now lives in `@dialogram/shared` - (`ClipboardPayload`, `parseClipboardPayload`, `clipboardDataFromPayload`), - written by copy, cut and duplicate and read by paste and the webview's - post-paste selection. The server's `clipboard/py-clipboard` module is gone; - its builder is `buildClipboardPayload` in `clipboard/clipboard`. + (`DiagramClipboardPayload`, `parseClipboardPayload`, + `clipboardDataFromPayload`), written by copy, cut and duplicate and read by + paste and the webview's post-paste selection. The server's + `clipboard/py-clipboard` module is gone; its builder is + `buildClipboardPayload` in `clipboard/clipboard`. +- **A pasted or duplicated connection joins the pasted copies**, not the + entities it was copied from, which used to get a second wire while the copy + stayed unwired. The pasted name follows one rule, `nextFreeName` in + `@dialogram/shared` (`add`, then `add2`, `add3`), on the sidecar side and in + the webview, which predicted `add1` and so never selected the pasted node. + A paste with nothing to paste says so instead of doing nothing. - A layout change still on its 500 ms debounce timer when the window closes is written out: the host flushes every diagram's pending layout saves on deactivation. diff --git a/packages/diagram-client/src/post-edit-selection-service.ts b/packages/diagram-client/src/post-edit-selection-service.ts index 95dd7ce..5fe08cc 100644 --- a/packages/diagram-client/src/post-edit-selection-service.ts +++ b/packages/diagram-client/src/post-edit-selection-service.ts @@ -1,21 +1,14 @@ import { injectable } from 'inversify'; -import { parseClipboardPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; +import { nextFreeName, parseClipboardPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; export type PendingSelection = { entityNames: string[]; boundaryPortNames: string[]; }; +/** The name the pasted element will get, by the rule the edit backend applies. */ function generateUnique(base: string, used: Set): string { - if (!used.has(base)) { - used.add(base); - return base; - } - let i = 1; - while (used.has(`${base}${i}`)) { - i++; - } - const name = `${base}${i}`; + const name = nextFreeName(base, used); used.add(name); return name; } diff --git a/packages/diagram-client/test/post-edit-selection-paste.test.ts b/packages/diagram-client/test/post-edit-selection-paste.test.ts index d389a06..7aff6d5 100644 --- a/packages/diagram-client/test/post-edit-selection-paste.test.ts +++ b/packages/diagram-client/test/post-edit-selection-paste.test.ts @@ -30,12 +30,8 @@ describe('PostEditSelectionService.prepareForPaste', () => { service.prepareForPaste(clipboardData, root); - const pending = (service as any).pending; - expect(pending.entityNames).toHaveLength(1); - expect(pending.entityNames[0]).not.toBe('add'); - expect(pending.entityNames[0]).toMatch(/^add/); - expect(pending.boundaryPortNames).toHaveLength(1); - expect(pending.boundaryPortNames[0]).toMatch(/^a/); + // The same names the toolkit's paste writes (see `nextFreeName`). + expect((service as any).pending).toEqual({ entityNames: ['add2'], boundaryPortNames: ['a2'] }); }); it('plans nothing for clipboard data that is not a diagram payload', () => { diff --git a/packages/diagram-server/src/clipboard/clipboard.ts b/packages/diagram-server/src/clipboard/clipboard.ts index 1d99b98..ef5bdfc 100644 --- a/packages/diagram-server/src/clipboard/clipboard.ts +++ b/packages/diagram-server/src/clipboard/clipboard.ts @@ -1,8 +1,8 @@ import type { Args } from '@eclipse-glsp/protocol'; import type { ModelState } from '@eclipse-glsp/server'; -import { WorkflowDiagramMetadata, WorkflowDiagramTypes, type ClipboardItem, type ClipboardPayload } from '@dialogram/shared'; +import { WorkflowDiagramMetadata, WorkflowDiagramTypes, type DiagramClipboardItem, type DiagramClipboardPayload } from '@dialogram/shared'; -export { clipboardDataFromPayload, parseClipboardPayload, type ClipboardItem, type ClipboardPayload } from '@dialogram/shared'; +export { clipboardDataFromPayload, parseClipboardPayload, type DiagramClipboardItem, type DiagramClipboardPayload } from '@dialogram/shared'; function isReadOnly(element: any): boolean { const args: Args | undefined = element?.args; @@ -17,8 +17,8 @@ function isReadOnly(element: any): boolean { * boundary port and connection among them, by the names a paste re-creates * it from. Read-only and virtual elements are left out. */ -export function buildClipboardPayload(modelState: ModelState, sourceUri: string, selectedElementIds: string[]): ClipboardPayload { - const items: ClipboardItem[] = []; +export function buildClipboardPayload(modelState: ModelState, sourceUri: string, selectedElementIds: string[]): DiagramClipboardPayload { + const items: DiagramClipboardItem[] = []; for (const id of selectedElementIds) { const element: any = modelState.index.get(id); if (!element || isReadOnly(element)) { @@ -34,7 +34,7 @@ export function buildClipboardPayload(modelState: ModelState, sourceUri: string, ) { const name = args?.[WorkflowDiagramMetadata.ENTITY_NAME]; const typeName = args?.[WorkflowDiagramMetadata.ENTITY_TYPE]; - if (typeof name === 'string' && typeof typeName === 'string') { + if (typeof name === 'string' && name.trim() !== '' && typeof typeName === 'string' && typeName.trim() !== '') { items.push({ kind: 'entity', elementTypeId, name, typeName }); } continue; @@ -43,7 +43,7 @@ export function buildClipboardPayload(modelState: ModelState, sourceUri: string, if (elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_INPUT || elementTypeId === WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT) { const portName = args?.[WorkflowDiagramMetadata.PORT_NAME]; const portType = args?.[WorkflowDiagramMetadata.PORT_TYPE]; - if (typeof portName === 'string') { + if (typeof portName === 'string' && portName.trim() !== '') { items.push({ kind: 'boundaryPort', elementTypeId, diff --git a/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts b/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts index 378f7b6..7e31bf5 100644 --- a/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts +++ b/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts @@ -6,7 +6,7 @@ */ import { RequestClipboardDataAction } from '@eclipse-glsp/protocol'; import { describe, expect, it } from 'vitest'; -import { parseClipboardPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; +import { nextFreeName, parseClipboardPayload, WorkflowDiagramMetadata, WorkflowDiagramTypes, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import { WorkflowRequestClipboardDataActionHandler } from '../src/server/request-clipboard-data-action-handler'; function modelStateWith(elements: Record, sourceUri?: string) { @@ -61,10 +61,11 @@ describe('WorkflowRequestClipboardDataActionHandler', () => { type: WorkflowDiagramTypes.NODE_ACTOR, args: { [WorkflowDiagramMetadata.ENTITY_NAME]: 'x', [WorkflowDiagramMetadata.ENTITY_TYPE]: 'X', [WorkflowDiagramMetadata.IS_READ_ONLY]: true } }, - nameless: { type: WorkflowDiagramTypes.NODE_ACTOR, args: {} } + nameless: { type: WorkflowDiagramTypes.NODE_ACTOR, args: {} }, + blank: { type: WorkflowDiagramTypes.NODE_ACTOR, args: { [WorkflowDiagramMetadata.ENTITY_NAME]: ' ', [WorkflowDiagramMetadata.ENTITY_TYPE]: 'X' } } }, 'file:///hdl/top.sv'); - const [action] = await copyOf(handler, ['locked', 'nameless', 'missing']); + const [action] = await copyOf(handler, ['locked', 'nameless', 'blank', 'missing']); expect(parseClipboardPayload((action as any).clipboardData)?.items).toEqual([]); }); @@ -78,6 +79,14 @@ describe('WorkflowRequestClipboardDataActionHandler', () => { }); }); +describe('nextFreeName', () => { + it('keeps a free name and numbers a taken one from 2', () => { + expect(nextFreeName('add', new Set())).toBe('add'); + expect(nextFreeName('add', new Set(['add']))).toBe('add2'); + expect(nextFreeName('add', new Set(['add', 'add2', 'add3']))).toBe('add4'); + }); +}); + describe('parseClipboardPayload', () => { it('rejects data that is not a diagram payload', () => { expect(parseClipboardPayload(undefined)).toBeUndefined(); diff --git a/packages/shared/src/clipboard-payload.ts b/packages/shared/src/clipboard-payload.ts index 6a56ec9..0dcb518 100644 --- a/packages/shared/src/clipboard-payload.ts +++ b/packages/shared/src/clipboard-payload.ts @@ -10,7 +10,7 @@ export const CLIPBOARD_FORMAT = 'application/json'; -export type ClipboardItem = +export type DiagramClipboardItem = | { kind: 'entity'; elementTypeId: string; @@ -33,19 +33,35 @@ export type ClipboardItem = inPort?: string; }; -export interface ClipboardPayload { +export interface DiagramClipboardPayload { version: 1; sourceUri: string; - items: ClipboardItem[]; + items: DiagramClipboardItem[]; +} + +/** + * The name a pasted or duplicated element gets: `base` when free, else + * `base2`, `base3`, ... The sidecar writes this name, and the webview + * predicts it to select the new element, so both apply this one rule. + */ +export function nextFreeName(base: string, taken: ReadonlySet): string { + if (!taken.has(base)) { + return base; + } + let index = 2; + while (taken.has(`${base}${index}`)) { + index += 1; + } + return `${base}${index}`; } /** The GLSP clipboard-data record carrying `payload`. */ -export function clipboardDataFromPayload(payload: ClipboardPayload): Record { +export function clipboardDataFromPayload(payload: DiagramClipboardPayload): Record { return { [CLIPBOARD_FORMAT]: JSON.stringify(payload) }; } /** The payload in a GLSP clipboard-data record, or `undefined` when it carries none. */ -export function parseClipboardPayload(clipboardData: Record | undefined): ClipboardPayload | undefined { +export function parseClipboardPayload(clipboardData: Record | undefined): DiagramClipboardPayload | undefined { if (!clipboardData) { return undefined; } @@ -54,7 +70,7 @@ export function parseClipboardPayload(clipboardData: Record | un return undefined; } try { - const parsed = JSON.parse(json) as ClipboardPayload; + const parsed = JSON.parse(json) as DiagramClipboardPayload; if (!parsed || parsed.version !== 1 || !Array.isArray(parsed.items) || typeof parsed.sourceUri !== 'string') { return undefined; } diff --git a/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts b/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts index ee70b0d..308f86e 100644 --- a/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/create-node-handler.ts @@ -220,7 +220,7 @@ export class CreateNodeOperationHandler extends OperationHandler { createCommand(operation: CreateNodeOperation): MaybePromise { const sourceUri = this.modelState.root.args?.['sourceUri'] as string | undefined; - if (!sourceUri || !this.sidecar.handlesSource(sourceUri)) { + if (!sourceUri) { return undefined; } return this.createPythonCommand(operation, sourceUri); diff --git a/packages/sidecar-toolkit/src/server/operations/cut-handler.ts b/packages/sidecar-toolkit/src/server/operations/cut-handler.ts index 55adf2f..e4995c7 100644 --- a/packages/sidecar-toolkit/src/server/operations/cut-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/cut-handler.ts @@ -44,7 +44,7 @@ export class CutOperationHandler extends OperationHandler { uri: vscodeUri, computeEdits: async () => { const beforeText = (await vscode.workspace.openTextDocument(vscodeUri)).getText(); - const payload = await buildClipboardPayload(this.modelState, sourceUri, elementIds); + const payload = buildClipboardPayload(this.modelState, sourceUri, elementIds); if (payload.items.length === 0) { return undefined; } diff --git a/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts b/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts index 162f73c..724460e 100644 --- a/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts @@ -3,7 +3,7 @@ import { OperationHandler, ModelState, type Command, type MaybePromise } from '@ import { inject, injectable } from 'inversify'; import * as vscode from 'vscode'; import { readAuthoritativeSourceText } from './authoritative-source-text'; -import { WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; +import { nextFreeName, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { ReversibleWorkspaceEditCommand } from '@dialogram/diagram-server/operations/reversible-workspace-edit-command'; import { buildClipboardPayload } from '@dialogram/diagram-server/clipboard/clipboard'; @@ -42,7 +42,7 @@ export class DuplicateOperationHandler extends OperationHandler { uri: vscodeUri, computeEdits: async () => { const beforeText = (await vscode.workspace.openTextDocument(vscodeUri)).getText(); - const payload = await buildClipboardPayload(this.modelState, sourceUri, operation.elementIds ?? []); + const payload = buildClipboardPayload(this.modelState, sourceUri, operation.elementIds ?? []); if (payload.items.length === 0) return undefined; const workflowName = this.modelState.root.args?.['wf:workflowName'] as string | undefined; const takenResp = await this.sendSidecarList(sourceUri, { @@ -50,16 +50,13 @@ export class DuplicateOperationHandler extends OperationHandler { args: { workflow: workflowName } }); const taken = new Set((takenResp?.diagnostic?.names as string[] | undefined) ?? []); + // A duplicated connection joins the copies, not the entities it was copied from. + const renamed = new Map(); for (const item of payload.items) { if (item.kind === 'entity') { - const base = item.name; - let name = base; - let idx = 2; - while (taken.has(name)) { - name = `${base}${idx}`; - idx += 1; - } + const name = nextFreeName(item.name, taken); taken.add(name); + renamed.set(item.name, name); await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('createNode'), args: { workflow: workflowName, type: item.typeName, name } @@ -72,10 +69,10 @@ export class DuplicateOperationHandler extends OperationHandler { } else if (item.kind === 'connection') { if (!item.outPort || !item.inPort) continue; const fromPort = item.from - ? { type: 'actor', actor: item.from, port: item.outPort } + ? { type: 'actor', actor: renamed.get(item.from) ?? item.from, port: item.outPort } : { type: 'workflow', name: item.outPort }; const toPort = item.to - ? { type: 'actor', actor: item.to, port: item.inPort } + ? { type: 'actor', actor: renamed.get(item.to) ?? item.to, port: item.inPort } : { type: 'workflow', name: item.inPort }; await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('connect'), diff --git a/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts b/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts index fbd0de7..5bb5595 100644 --- a/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/entity-port-crud-handler.ts @@ -49,7 +49,7 @@ class EntityPortCrudHandlerBase extends OperationHandler { createCommand(action: Action): Command | undefined { const sourceUri = this.modelState.sourceUri; - if (!sourceUri || !this.sidecar.handlesSource(sourceUri)) { + if (!sourceUri) { return undefined; } const createKind = this.sidecar.operationKindCreateEntityPort(); diff --git a/packages/sidecar-toolkit/src/server/operations/paste-handler.ts b/packages/sidecar-toolkit/src/server/operations/paste-handler.ts index 38f0f18..03ae100 100644 --- a/packages/sidecar-toolkit/src/server/operations/paste-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/paste-handler.ts @@ -3,7 +3,7 @@ import { OperationHandler, ModelState, type Command, type MaybePromise } from '@ import { inject, injectable } from 'inversify'; import * as vscode from 'vscode'; import { readAuthoritativeSourceText } from './authoritative-source-text'; -import { WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; +import { nextFreeName, WORKFLOW_NETWORK_MODEL_KEY } from '@dialogram/shared'; import type { WorkflowDiagramModel } from '@dialogram/shared'; import { ReversibleWorkspaceEditCommand } from '@dialogram/diagram-server/operations/reversible-workspace-edit-command'; import { parseClipboardPayload } from '@dialogram/diagram-server/clipboard/clipboard'; @@ -43,8 +43,8 @@ export class PasteOperationHandler extends OperationHandler { sourceUri: string, ): Promise { const payload = parseClipboardPayload(operation.clipboardData as Record | undefined); - if (!payload) { - void vscode.window.showWarningMessage('The clipboard holds no diagram elements.'); + if (!payload || payload.items.length === 0) { + void vscode.window.showWarningMessage('The clipboard holds no diagram elements to paste.'); return undefined; } const workflowName = this.modelState.root.args?.['wf:workflowName'] as string | undefined; @@ -54,16 +54,13 @@ export class PasteOperationHandler extends OperationHandler { }); const taken = new Set((takenResp?.diagnostic?.names as string[] | undefined) ?? []); const beforeText = (await vscode.workspace.openTextDocument(vscode.Uri.parse(sourceUri))).getText(); + // A pasted connection joins the pasted copies, not the entities it was copied from. + const renamed = new Map(); for (const item of payload.items) { if (item.kind === 'entity') { - const base = item.name; - let name = base; - let idx = 2; - while (taken.has(name)) { - name = `${base}${idx}`; - idx += 1; - } + const name = nextFreeName(item.name, taken); taken.add(name); + renamed.set(item.name, name); await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('createNode'), args: { workflow: workflowName, type: item.typeName, name } @@ -76,10 +73,10 @@ export class PasteOperationHandler extends OperationHandler { } else if (item.kind === 'connection') { if (!item.outPort || !item.inPort) continue; const fromPort = item.from - ? { type: 'actor', actor: item.from, port: item.outPort } + ? { type: 'actor', actor: renamed.get(item.from) ?? item.from, port: item.outPort } : { type: 'workflow', name: item.outPort }; const toPort = item.to - ? { type: 'actor', actor: item.to, port: item.inPort } + ? { type: 'actor', actor: renamed.get(item.to) ?? item.to, port: item.inPort } : { type: 'workflow', name: item.inPort }; await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('connect'), diff --git a/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts b/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts index 2112e67..52ba49a 100644 --- a/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts +++ b/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts @@ -47,11 +47,6 @@ export class SidecarInvoker { return this.runtimeProfile.undoLabelSuffix; } - /** Whether `sourceUri` names a file this product's sidecar edits. */ - handlesSource(sourceUri: string): boolean { - return this.runtimeProfile.handlesSource(sourceUri); - } - createNodeStrings(): CreateNodeStrings { return this.runtimeProfile.createNodeStrings; } diff --git a/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts b/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts index b4eb12d..f0744d7 100644 --- a/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts +++ b/packages/sidecar-toolkit/src/server/sidecar-runtime-config.ts @@ -220,13 +220,6 @@ export interface SidecarRuntimeConfig { * errors stay in the diagram's overlay, as they always have. */ documentErrorsToProblems?: boolean; - /** - * The extensions, dot included, of the source files this runtime edits - * (`['.sv', '.v']` for a Verilog product). An operation handler that - * writes to the model's source refuses a `sourceUri` carrying none of - * them. Absent: no gate, every source is accepted. - */ - sourceExtensions?: string[]; operationKinds: { createEntityPort: string; deleteEntityPort: string }; /** Op prefixes this runtime accepts when rewriting foreign-prefixed ops * (replaces the hardcoded product-prefix literal check). */ @@ -268,19 +261,6 @@ function isPythonInterpreterCommand(command: string): boolean { return /(^|\/)python(?:\d+(?:\.\d+)*)?$/i.test(command.trim()); } -/** - * Whether `sourceUri` ends in one of the config's `sourceExtensions`, case - * ignored. A config that names none accepts every source. - */ -export function handlesSource(cfg: Pick, sourceUri: string): boolean { - const extensions = cfg.sourceExtensions ?? []; - if (extensions.length === 0) { - return true; - } - const lower = sourceUri.toLowerCase(); - return extensions.some(ext => lower.endsWith(ext.toLowerCase())); -} - /** `.` — the fully-qualified sidecar op name for this runtime. */ export function sidecarOp(cfg: SidecarRuntimeConfig, opName: string): string { return `${cfg.sidecarOperationPrefix}.${opName}`; @@ -420,11 +400,6 @@ export class SidecarRuntimeService { return sidecarOp(this.cfg, opName); } - /** Whether `sourceUri` names a file this runtime edits (see `SidecarRuntimeConfig.sourceExtensions`). */ - handlesSource(sourceUri: string): boolean { - return handlesSource(this.cfg, sourceUri); - } - rewriteSidecarOperation(operation: string): string { const dot = operation.indexOf('.'); if (dot <= 0) { diff --git a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts index 28260d9..e6ae486 100644 --- a/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts +++ b/packages/sidecar-toolkit/src/sidecar-diagram-profile.ts @@ -400,8 +400,7 @@ function sidecarRuntimeConfig(input: SidecarProfileInput): SidecarRuntimeConfig createNodeBehavior: input.createNodeBehavior, createNodeVariants: input.createNodeVariants, navigation: input.navigation, - documentErrorsToProblems: input.documentErrorsToProblems, - sourceExtensions: sourceExtensionsOf(input) + documentErrorsToProblems: input.documentErrorsToProblems }; } diff --git a/packages/sidecar-toolkit/test/create-in-every-product.test.ts b/packages/sidecar-toolkit/test/create-in-every-product.test.ts new file mode 100644 index 0000000..0fe07d3 --- /dev/null +++ b/packages/sidecar-toolkit/test/create-in-every-product.test.ts @@ -0,0 +1,70 @@ +/** + * The create-node and entity-port handlers used to accept only a `.py` source, + * so the palette never created a node, and the property panel never added an + * entity port, in a Verilog, VHDL or NDF diagram. Like every other handler + * that writes to the source, they now ask only that the model has one. + */ +import 'reflect-metadata'; +import { describe, expect, it } from 'vitest'; +import { Container } from 'inversify'; +import { ModelState } from '@eclipse-glsp/server'; +import { LayoutPersistenceService } from '@dialogram/diagram-server/services/layout-persistence-service'; +import { WorkflowDiagramTypes } from '@dialogram/shared'; +import { createSidecarServerModule } from '../src/server/sidecar-server-module.js'; +import { CreateNodeOperationHandler } from '../src/server/operations/create-node-handler.js'; +import { createEntityPortCrudHandlers } from '../src/server/operations/entity-port-crud-handler.js'; +import type { SidecarRuntimeConfig } from '../src/server/sidecar-runtime-config.js'; +import { NEUTRAL_CREATE_NODE_CONFIG } from './fixtures/create-node-config.js'; + +const HDL_CONFIG: SidecarRuntimeConfig = { + settingsNamespace: 'hdl', + sidecarOperationPrefix: 'hdl', + sidecarCommandSettingKey: 'sidecarCommand', + sidecarCommandDefault: 'hdl-sidecar', + cliCommandSettingKey: 'cliCommand', + cliCommandDefault: 'hdl', + operationKinds: { createEntityPort: 'hdl.createEntityPort', deleteEntityPort: 'hdl.deleteEntityPort' }, + acceptedOperationPrefixes: ['hdl'], + graphAcquisition: 'cli-plan', + ...NEUTRAL_CREATE_NODE_CONFIG +}; + +function containerFor(sourceUri: string | undefined): Container { + const container = new Container(); + container.load(createSidecarServerModule(HDL_CONFIG)); + container.bind(ModelState).toConstantValue({ sourceUri, root: { args: { sourceUri } } } as any); + container.bind(LayoutPersistenceService).toConstantValue(new LayoutPersistenceService()); + return container; +} + +const createNode = { kind: 'createNode', elementTypeId: WorkflowDiagramTypes.NODE_TASK, location: { x: 0, y: 0 } } as any; + +describe('the palette creates a node in every product', () => { + it.each(['file:///hdl/top.sv', 'file:///hdl/top.vhd', 'file:///spec/docs/forwarding.md', 'file:///wf/flow.py'])( + 'for the source %s', + sourceUri => { + const handler = containerFor(sourceUri).resolve(CreateNodeOperationHandler); + expect(handler.createCommand(createNode)).toBeDefined(); + } + ); + + it('but not for a model without a source', () => { + const handler = containerFor(undefined).resolve(CreateNodeOperationHandler); + expect(handler.createCommand(createNode)).toBeUndefined(); + }); +}); + +describe('the property panel adds an entity port in every product', () => { + const [CreateHandler] = createEntityPortCrudHandlers(HDL_CONFIG.operationKinds); + const op = { kind: HDL_CONFIG.operationKinds.createEntityPort, entityType: 'Counter', portDirection: 'input', portName: 'clk', portType: 'std_logic' }; + + it.each(['file:///hdl/top.vhd', 'file:///hdl/top.sv'])('for the source %s', sourceUri => { + const handler = containerFor(sourceUri).resolve(CreateHandler) as any; + expect(handler.createCommand(op)).toBeDefined(); + }); + + it('but not for a model without a source', () => { + const handler = containerFor(undefined).resolve(CreateHandler) as any; + expect(handler.createCommand(op)).toBeUndefined(); + }); +}); diff --git a/packages/sidecar-toolkit/test/create-node-handler.test.ts b/packages/sidecar-toolkit/test/create-node-handler.test.ts index 14e4f35..ce37582 100644 --- a/packages/sidecar-toolkit/test/create-node-handler.test.ts +++ b/packages/sidecar-toolkit/test/create-node-handler.test.ts @@ -66,7 +66,6 @@ function calpySidecar() { settingsNamespace: () => 'calLang', sidecarOp: (opName: string) => `calpy.${opName}`, undoLabelSuffix: () => ' (wfpy)', - handlesSource: () => true, createNodeStrings: () => CALPY_CREATE_NODE_STRINGS, createNodeBehavior: () => CALPY_CREATE_NODE_BEHAVIOR, // This product contributes no variant entries. @@ -79,7 +78,6 @@ function wfpySidecar() { settingsNamespace: () => 'wfLang', sidecarOp: (opName: string) => `wfpy.${opName}`, undoLabelSuffix: () => ' (wfpy)', - handlesSource: () => true, createNodeStrings: () => WFPY_CREATE_NODE_STRINGS, createNodeBehavior: () => WFPY_CREATE_NODE_BEHAVIOR, createNodeVariants: () => [] @@ -875,7 +873,6 @@ describe('workflow create node handler', () => { settingsNamespace: () => 'anyLang', sidecarOp: (opName: string) => `any.${opName}`, undoLabelSuffix: () => ' (any)', - handlesSource: () => true, createNodeStrings: () => ({ newTypeNamePrompt: () => 'New gizmo name', typeLabel: () => 'gizmo', diff --git a/packages/sidecar-toolkit/test/paste-handler.test.ts b/packages/sidecar-toolkit/test/paste-handler.test.ts new file mode 100644 index 0000000..4a0b032 --- /dev/null +++ b/packages/sidecar-toolkit/test/paste-handler.test.ts @@ -0,0 +1,112 @@ +/** + * Paste re-creates the clipboard's elements under free names and wires the + * pasted connections to the pasted copies, not to the entities they were + * copied from. + */ +import 'reflect-metadata'; +import { promises as fs } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { PasteOperation } from '@eclipse-glsp/protocol'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import * as vscode from 'vscode'; +import { clipboardDataFromPayload, WorkflowDiagramTypes, WORKFLOW_NETWORK_MODEL_KEY, type DiagramClipboardPayload } from '@dialogram/shared'; +import { PasteOperationHandler } from '../src/server/operations/paste-handler.js'; + +let dir: string; +let sourcePath: string; +let sourceUri: string; +const originalOpenTextDocument = (vscode.workspace as any).openTextDocument; +const originalShowWarningMessage = (vscode.window as any).showWarningMessage; + +beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'dialogram-paste-')); + sourcePath = path.join(dir, 'top.sv'); + await fs.writeFile(sourcePath, 'module top; endmodule\n'); + sourceUri = vscode.Uri.file(sourcePath).toString(); + (vscode.workspace as any).openTextDocument = async () => ({ getText: () => 'module top; endmodule\n' }); +}); + +afterEach(async () => { + (vscode.workspace as any).openTextDocument = originalOpenTextDocument; + (vscode.window as any).showWarningMessage = originalShowWarningMessage; + await fs.rm(dir, { recursive: true, force: true }); +}); + +function handlerWith(taken: string[], sent: Array<{ op: string; args: Record }>): PasteOperationHandler { + const handler = new PasteOperationHandler(); + (handler as any).modelState = { + sourceUri, + root: { args: { 'wf:workflowName': 'top' } }, + get: (key: string) => (key === WORKFLOW_NETWORK_MODEL_KEY ? { documentUri: sourceUri } : undefined) + }; + (handler as any).sidecar = { + sidecarOp: (name: string) => `hdl.${name}`, + sendSidecarOp: async (_uri: string, payload: { op: string; args: Record }) => { + sent.push(payload); + return true; + }, + sendSidecarList: async () => ({ diagnostic: { names: taken } }) + }; + return handler; +} + +async function paste(handler: PasteOperationHandler, payload: DiagramClipboardPayload) { + const operation = PasteOperation.create({ clipboardData: clipboardDataFromPayload(payload), editorContext: { selectedElementIds: [] } }); + const command = handler.createCommand(operation) as any; + expect(command).toBeDefined(); + return (await command.computeEdits?.()) ?? (await command.options?.computeEdits?.()); +} + +describe('PasteOperationHandler', () => { + it('names the pasted entity by the shared rule and wires the pasted connection to it', async () => { + const sent: Array<{ op: string; args: Record }> = []; + const handler = handlerWith(['src', 'add'], sent); + + await paste(handler, { + version: 1, + sourceUri, + items: [ + { kind: 'entity', elementTypeId: WorkflowDiagramTypes.NODE_ACTOR, name: 'add', typeName: 'Adder' }, + { kind: 'connection', elementTypeId: WorkflowDiagramTypes.EDGE_CONNECTION, from: 'src', to: 'add', outPort: 'out', inPort: 'a' } + ] + }); + + expect(sent).toEqual([ + { op: 'hdl.createNode', args: { workflow: 'top', type: 'Adder', name: 'add2' } }, + { + op: 'hdl.connect', + args: { + workflow: 'top', + from_port: { type: 'actor', actor: 'src', port: 'out' }, + to_port: { type: 'actor', actor: 'add2', port: 'a' } + } + } + ]); + }); + + it('keeps the copied name when the target diagram does not use it', async () => { + const sent: Array<{ op: string; args: Record }> = []; + await paste(handlerWith(['other'], sent), { + version: 1, + sourceUri, + items: [{ kind: 'entity', elementTypeId: WorkflowDiagramTypes.NODE_ACTOR, name: 'add', typeName: 'Adder' }] + }); + expect(sent).toEqual([{ op: 'hdl.createNode', args: { workflow: 'top', type: 'Adder', name: 'add' } }]); + }); + + it('says so, and sends nothing, when the clipboard holds no element to paste', async () => { + const warnings: string[] = []; + (vscode.window as any).showWarningMessage = async (message: string) => { + warnings.push(message); + }; + const sent: Array<{ op: string; args: Record }> = []; + const handler = handlerWith([], sent); + + const edits = await paste(handler, { version: 1, sourceUri, items: [] }); + + expect(edits).toBeUndefined(); + expect(sent).toEqual([]); + expect(warnings).toHaveLength(1); + }); +}); diff --git a/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts b/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts index 184aab0..86e4721 100644 --- a/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts +++ b/packages/sidecar-toolkit/test/picked-variant-create-wizard.test.ts @@ -108,7 +108,6 @@ async function runCreate(options: { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS diff --git a/packages/sidecar-toolkit/test/source-extension-gate.test.ts b/packages/sidecar-toolkit/test/source-extension-gate.test.ts deleted file mode 100644 index 72622e0..0000000 --- a/packages/sidecar-toolkit/test/source-extension-gate.test.ts +++ /dev/null @@ -1,127 +0,0 @@ -/** - * The operation handlers that write to a diagram's source used to accept only a - * `.py` source, so the palette never created a node, and the property panel never - * added an entity port, in a Verilog, VHDL or NDF diagram. The gate now reads the - * profile's `sourceExtensions`. - */ -import 'reflect-metadata'; -import { describe, expect, it } from 'vitest'; -import { Container } from 'inversify'; -import { ModelState } from '@eclipse-glsp/server'; -import { LayoutPersistenceService } from '@dialogram/diagram-server/services/layout-persistence-service'; -import { WorkflowDiagramTypes } from '@dialogram/shared'; -import { createSidecarServerModule } from '../src/server/sidecar-server-module.js'; -import { SidecarInvoker } from '../src/server/operations/sidecar-invoker.js'; -import { CreateNodeOperationHandler } from '../src/server/operations/create-node-handler.js'; -import { createEntityPortCrudHandlers } from '../src/server/operations/entity-port-crud-handler.js'; -import { handlesSource, type SidecarRuntimeConfig } from '../src/server/sidecar-runtime-config.js'; -import { createSidecarDiagramProfile, type SidecarProfileInput } from '../src/sidecar-diagram-profile.js'; -import { NEUTRAL_CREATE_NODE_CONFIG } from './fixtures/create-node-config.js'; - -function verilogConfig(overrides: Partial = {}): SidecarRuntimeConfig { - return { - settingsNamespace: 'hdl', - sidecarOperationPrefix: 'hdl', - sidecarCommandSettingKey: 'sidecarCommand', - sidecarCommandDefault: 'hdl-sidecar', - cliCommandSettingKey: 'cliCommand', - cliCommandDefault: 'hdl', - operationKinds: { createEntityPort: 'hdl.createEntityPort', deleteEntityPort: 'hdl.deleteEntityPort' }, - acceptedOperationPrefixes: ['hdl'], - graphAcquisition: 'cli-plan', - sourceExtensions: ['.sv', '.v'], - ...NEUTRAL_CREATE_NODE_CONFIG, - ...overrides - }; -} - -function containerFor(cfg: SidecarRuntimeConfig, sourceUri: string): Container { - const container = new Container(); - container.load(createSidecarServerModule(cfg)); - container.bind(ModelState).toConstantValue({ sourceUri, root: { args: { sourceUri } } } as any); - container.bind(LayoutPersistenceService).toConstantValue(new LayoutPersistenceService()); - return container; -} - -describe('handlesSource', () => { - it('accepts a source with one of the configured extensions, whatever its case', () => { - const cfg = { sourceExtensions: ['.sv', '.v'] }; - expect(handlesSource(cfg, 'file:///hdl/top.sv')).toBe(true); - expect(handlesSource(cfg, 'file:///hdl/TOP.V')).toBe(true); - expect(handlesSource(cfg, 'file:///hdl/top.py')).toBe(false); - expect(handlesSource(cfg, 'file:///hdl/top.svh')).toBe(false); - }); - - it('accepts every source when the config names no extension', () => { - expect(handlesSource({}, 'file:///any/file.xyz')).toBe(true); - expect(handlesSource({ sourceExtensions: [] }, 'file:///any/file.xyz')).toBe(true); - }); -}); - -describe('the source gate of the editing handlers', () => { - const createNode = { kind: 'createNode', elementTypeId: WorkflowDiagramTypes.NODE_TASK, location: { x: 0, y: 0 } } as any; - - it('lets the palette create a node in a Verilog diagram', () => { - const container = containerFor(verilogConfig(), 'file:///hdl/top.sv'); - const handler = container.resolve(CreateNodeOperationHandler); - expect(handler.createCommand(createNode)).toBeDefined(); - }); - - it('still refuses a source the product does not edit', () => { - const container = containerFor(verilogConfig(), 'file:///hdl/top.py'); - const handler = container.resolve(CreateNodeOperationHandler); - expect(handler.createCommand(createNode)).toBeUndefined(); - }); - - it('keeps the Python product on its own sources', () => { - const python = verilogConfig({ sourceExtensions: ['.py'] }); - expect(containerFor(python, 'file:///wf/flow.py').resolve(CreateNodeOperationHandler).createCommand(createNode)).toBeDefined(); - expect(containerFor(python, 'file:///wf/flow.sv').resolve(CreateNodeOperationHandler).createCommand(createNode)).toBeUndefined(); - }); - - it('lets the property panel add an entity port in a VHDL diagram', () => { - const cfg = verilogConfig({ sourceExtensions: ['.vhd', '.vhdl'] }); - const [CreateHandler] = createEntityPortCrudHandlers(cfg.operationKinds); - const container = containerFor(cfg, 'file:///hdl/top.vhd'); - const handler = container.resolve(CreateHandler) as any; - const op = { kind: cfg.operationKinds.createEntityPort, entityType: 'Counter', portDirection: 'input', portName: 'clk', portType: 'std_logic' }; - expect(handler.createCommand(op)).toBeDefined(); - expect((containerFor(cfg, 'file:///hdl/top.py').resolve(CreateHandler) as any).createCommand(op)).toBeUndefined(); - }); -}); - -describe('the profile carries its source extensions to the server', () => { - const input = { - key: 'sv', - displayName: 'SystemVerilog', - settingsNamespace: 'sv', - customEditorViewType: 'sv.diagram', - glspClientId: 'sv.client', - glspClientName: 'sv', - commands: {} as SidecarProfileInput['commands'], - operationKinds: { createEntityPort: 'sv.createEntityPort', deleteEntityPort: 'sv.deleteEntityPort' }, - sidecarOperationPrefix: 'sv', - sidecarCommandSettingKey: 'sidecarCommand', - sidecarCommandDefault: 'hdl-sidecar', - cliCommandSettingKey: 'cliCommand', - cliCommandDefault: 'hdl', - acceptedOperationPrefixes: ['sv'], - graphAcquisition: 'cli-plan', - undoLabelSuffix: ' (sv)', - sourceExtension: '.sv', - sourceExtensions: ['.sv', '.v'], - chat: { name: 'sv', fullName: 'SystemVerilog', skill: 'sv' }, - ...NEUTRAL_CREATE_NODE_CONFIG - } as unknown as SidecarProfileInput; - - it('so the invoker the handlers inject answers for the product, not for Python', () => { - const profile = createSidecarDiagramProfile(input); - const container = new Container(); - for (const module of profile.serverModules ?? []) { - container.load(module); - } - const invoker = container.get(SidecarInvoker); - expect(invoker.handlesSource('file:///hdl/top.v')).toBe(true); - expect(invoker.handlesSource('file:///hdl/top.py')).toBe(false); - }); -}); diff --git a/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts b/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts index d112938..a8a8a90 100644 --- a/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts +++ b/packages/sidecar-toolkit/test/variant-node-create-wizard.test.ts @@ -181,7 +181,6 @@ async function runCreate( settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -298,7 +297,6 @@ describe('a variant entry, driven to the end', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -396,7 +394,6 @@ describe('a source palette entry', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -602,7 +599,6 @@ describe('a variant type already in the project', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -726,7 +722,6 @@ describe('an answer that belongs to the node, not the type', () => { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS @@ -904,7 +899,6 @@ async function createWithMixed(existingTypes: string[]) { settingsNamespace: () => 'wfLang', sidecarOp: (op: string) => `wfpy.${op}`, undoLabelSuffix: () => ' (wf)', - handlesSource: () => true, createNodeStrings: () => STRINGS, createNodeBehavior: () => BEHAVIOR, createNodeVariants: () => VARIANTS