diff --git a/CHANGELOG.md b/CHANGELOG.md index d93ed18..d6ff007 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,26 @@ 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. 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` + (`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/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..5fe08cc 100644 --- a/packages/diagram-client/src/post-edit-selection-service.ts +++ b/packages/diagram-client/src/post-edit-selection-service.ts @@ -1,22 +1,14 @@ import { injectable } from 'inversify'; -import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; -import { tryParseWorkflowClipboardPayload } from './clipboard-payload'; +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; } @@ -103,7 +95,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 +108,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..7aff6d5 --- /dev/null +++ b/packages/diagram-client/test/post-edit-selection-paste.test.ts @@ -0,0 +1,42 @@ +/** + * 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); + + // 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', () => { + 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..ef5bdfc 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 DiagramClipboardItem, type DiagramClipboardPayload } 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 DiagramClipboardItem, type DiagramClipboardPayload } 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[]): DiagramClipboardPayload { + const items: DiagramClipboardItem[] = []; 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' && name.trim() !== '' && typeof typeName === 'string' && typeName.trim() !== '') { + 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' && portName.trim() !== '') { + 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..7e31bf5 --- /dev/null +++ b/packages/diagram-server/test/request-clipboard-data-action-handler.test.ts @@ -0,0 +1,97 @@ +/** + * 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 { 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) { + 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: {} }, + blank: { type: WorkflowDiagramTypes.NODE_ACTOR, args: { [WorkflowDiagramMetadata.ENTITY_NAME]: ' ', [WorkflowDiagramMetadata.ENTITY_TYPE]: 'X' } } + }, 'file:///hdl/top.sv'); + + const [action] = await copyOf(handler, ['locked', 'nameless', 'blank', '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('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(); + 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..0dcb518 --- /dev/null +++ b/packages/shared/src/clipboard-payload.ts @@ -0,0 +1,81 @@ +/** + * 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 DiagramClipboardItem = + | { + 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 DiagramClipboardPayload { + version: 1; + sourceUri: string; + 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: 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): DiagramClipboardPayload | 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 DiagramClipboardPayload; + 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..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 || !sourceUri.endsWith('.py')) { + 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 38427b7..e4995c7 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 = 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..724460e 100644 --- a/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/duplicate-handler.ts @@ -3,10 +3,10 @@ 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 { 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 = 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 a4eb74d..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 || !sourceUri.endsWith('.py')) { + 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 31eb54a..03ae100 100644 --- a/packages/sidecar-toolkit/src/server/operations/paste-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/paste-handler.ts @@ -3,10 +3,10 @@ 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 { 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); - if (!payload) { - void vscode.window.showWarningMessage('Clipboard does not contain Python diagram data.'); + const payload = parseClipboardPayload(operation.clipboardData as Record | undefined); + 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/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/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); + }); +});