From 4cdf5470d10b04125439a56875487349b0526a70 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 22 Oct 2026 15:31:42 -0400 Subject: [PATCH] Keep the Property Inspector read-only for MAT-files, as it is for dictionaries and models The inspector took edits for a MAT-file's variables, and for nothing else. What a file type can edit is core's to decide and say; how the extension presents it is the extension's, and one panel should behave the same for every file. Until every property setter the panel would reach is qualified, the panel is read-only for all of them. So the MAT-file editor gives the inspector no edit channel any more. piBuilder marks every row read-only again, whatever core's property sheet allows, and no row carries an edit key; the webview sends no edit and shows no refusal line; PropertiesViewProvider routes no edit; the editor has no inspector-edit gesture. Their tests go with them, and the change log no longer says MAT-file object properties can be edited in the Properties panel. The table is unchanged: its cells still take every edit they took, and the Add gallery, Bus element rows, struct field renames and the reasons core gives for a refused Add are as they were. A new test holds every row of every class the MAT-file gallery creates, a Bus's element included, read-only in the panel, while core itself offers edits of many of them. --- CHANGELOG.md | 1 - src/common/protocol.ts | 25 +-- src/extension.ts | 20 +-- src/host/MatEditorProvider.ts | 64 -------- src/host/PropertiesViewProvider.ts | 42 +---- src/host/piBuilder.ts | 17 +- .../components/dex-property-inspector.ts | 17 +- src/webview/pi-main.ts | 17 -- test-integration/suite/matAddGallery.test.ts | 145 ------------------ test-integration/suite/propertiesView.test.ts | 53 ------- test/inspectorEdit.test.ts | 118 -------------- test/piBuilder.test.ts | 30 ++++ 12 files changed, 46 insertions(+), 503 deletions(-) delete mode 100644 test/inspectorEdit.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c110c74..e8fed0ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,6 @@ All notable changes to the Simulink Data Explorer extension. - You can now add variables, strings and Simulink objects to MAT-files from the Add gallery. - Bus elements in MAT-files can be added and removed. -- Properties of Simulink objects in MAT-files can be edited in the Properties panel. - Struct fields in MAT-files can be renamed. ## [1.43.0] — 2026-10-21 diff --git a/src/common/protocol.ts b/src/common/protocol.ts index d5cfbf9f..049fa163 100644 --- a/src/common/protocol.ts +++ b/src/common/protocol.ts @@ -231,15 +231,6 @@ export interface ValidationErrorMessage { export interface ShowPropsMessage { type: 'showProps'; groups: any[]; - // The node the groups are of, which an edit made in the panel names (PropertyEditMessage). - nodeId?: string; -} - -/** The panel's last edit was refused: why, in core's words. Follows the showProps that reverts it. */ -export interface PropertyRefusedMessage { - type: 'propertyRefused'; - propName: string; - reason: string; } /** Clear the inspector (nothing selected). */ @@ -360,7 +351,7 @@ export type HostToTableMessage = | ChildRowsMessage; /** Every message the property-inspector webview can receive from the host. */ -export type HostToPropsMessage = ShowPropsMessage | EmptyMessage | MatrixCellsMessage | PropertyRefusedMessage; +export type HostToPropsMessage = ShowPropsMessage | EmptyMessage | MatrixCellsMessage; // --- Webview -> Host (table view -> providers) -------------------------------- @@ -565,16 +556,4 @@ export type ProjectToHostMessage = ReadyMessage | OpenFileMessage; // same thing from either webview, and the host answers both with the same closure. The // same goes for `RequestMatrixMessage`: the inspector's Value row opens the same Variable // Editor the table's does, and it is answered by the same function. -/** - * An edit committed in the Property Inspector: property `propName` (core's key) of node - * `nodeId`, the node the panel shows. Taken only where the host gave the panel an edit channel. - */ -export interface PropertyEditMessage { - type: 'editProperty'; - nodeId: string; - propName: string; - newValue: string; - oldValue: string; -} - -export type PropsToHostMessage = ReadyMessage | NavigateMessage | RequestMatrixMessage | PropertyEditMessage; +export type PropsToHostMessage = ReadyMessage | NavigateMessage | RequestMatrixMessage; diff --git a/src/extension.ts b/src/extension.ts index bd84d3ab..71f38856 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,7 +1,7 @@ // Copyright 2026 The MathWorks, Inc. import * as vscode from 'vscode'; import { SectionsTreeProvider } from './host/SectionsTreeProvider.js'; -import { PropertiesViewProvider, type PropertyEditHandler } from './host/PropertiesViewProvider.js'; +import { PropertiesViewProvider } from './host/PropertiesViewProvider.js'; import { BinaryEditorProvider } from './host/BinaryEditorProvider.js'; import { SlddTextEditorProvider } from './host/SlddTextEditorProvider.js'; import { BinarySlddEditorProvider } from './host/BinarySlddEditorProvider.js'; @@ -96,10 +96,8 @@ export function activate(context: vscode.ExtensionContext): void { // Section rows (id `section:`) have no properties, so clear the PI. // The selection the inspector was last asked to show, so a view whose tree is replaced (a // MAT-file reverted or reloaded from disk) can have its node shown again from the new tree. - // `editor` is the edit channel of a document whose host takes edits made in the inspector (a - // MAT-file's); the inspector is read-only for every other. let lastSelection: { uriString: string; rowIds: string[] } | undefined; - const showSelection = (uriString: string, rowIds: string[], editor?: PropertyEditHandler): void => { + const showSelection = (uriString: string, rowIds: string[]): void => { lastSelection = { uriString, rowIds }; const first = rowIds && rowIds.length > 0 ? rowIds[0] : undefined; if (!first || isSectionRowId(first)) { @@ -107,7 +105,7 @@ export function activate(context: vscode.ExtensionContext): void { return; } const node = findNode(uriString, first); - if (node) piProvider.showNode(node, uriString, editor); + if (node) piProvider.showNode(node, uriString); else piProvider.clear(); }; @@ -141,20 +139,12 @@ export function activate(context: vscode.ExtensionContext): void { // A MAT-file opens in this writable provider (edits through core's session, save // through core's writer). Shares the selection→PI and navigate wiring. const matProvider = new MatEditorProvider(context); - // Its inspector takes edits: each is a gesture of the MAT-file editor's, after which the node is - // shown again (a mode edit of a NumericType derives its other properties), with the reason - // core gave where it refused. - const matInspector = (uriString: string): PropertyEditHandler => (nodeId, propName, newValue, oldValue) => { - const result = matProvider.editFromInspector(uriString, nodeId, propName, newValue, oldValue); - showSelection(uriString, [result.nodeId ?? nodeId], matInspector(uriString)); - if (result.refused) piProvider.refuse(propName, result.refused); - }; - matProvider.onSelect = (uriString, rowIds) => showSelection(uriString, rowIds, matInspector(uriString)); + matProvider.onSelect = (uriString, rowIds) => showSelection(uriString, rowIds); matProvider.onNavigate = navigate; // Node ids are paths, so the node the inspector shows is found again in the new tree, or the // inspector is cleared if it is gone; it never goes on showing a node of the tree that left. matProvider.onReplaced = (uriString) => { - if (lastSelection?.uriString === uriString) showSelection(uriString, lastSelection.rowIds, matInspector(uriString)); + if (lastSelection?.uriString === uriString) showSelection(uriString, lastSelection.rowIds); }; // Health badges/colors on tree rows (missing/cycle/modified). The tree diff --git a/src/host/MatEditorProvider.ts b/src/host/MatEditorProvider.ts index 6f5493a6..57e6799e 100644 --- a/src/host/MatEditorProvider.ts +++ b/src/host/MatEditorProvider.ts @@ -485,13 +485,6 @@ function backupSource(id: string): { uri: vscode.Uri; stamp: DiskStamp | undefin * plain variable and a struct, and the classes of MATLAB's own default objects where the file's * objects can be added to. A core without the list adds a plain variable only, as it always has. */ -/** What an edit from the Property Inspector came to: the node's id after it, or why it was refused. */ -export interface InspectorResult { - nodeId?: string; - refused?: string; -} -type InspectorEdit = (nodeId: string, propName: string, newValue: string, oldValue: string) => InspectorResult; - function allowedClasses(mat: any): readonly string[] { if (typeof mat?.getAllowedTypes !== 'function') return ['MatlabVariable']; const classes = mat.getAllowedTypes(); @@ -503,16 +496,6 @@ function tileLabelOf(className: string): string { return matCatalog([className]).flatMap((c) => c.tiles).find((t) => t.className === className)?.label ?? className; } -/** The label the inspector shows property `key` of `node` by, or the key where it shows none. */ -function inspectorLabelOf(node: any, key: string): string { - try { - const prop = node?.toPIObject?.()?.propertySheet?.properties?.find((p: any) => p.name === key); - return typeof prop?.displayName === 'string' && prop.displayName !== '' ? prop.displayName : key; - } catch { - return key; - } -} - const sameList = (a: readonly string[], b: readonly string[]): boolean => a.length === b.length && a.every((x, i) => x === b[i]); export class MatEditorProvider implements vscode.CustomEditorProvider { @@ -526,9 +509,6 @@ export class MatEditorProvider implements vscode.CustomEditorProvider void; - // Each open view's edit of a node from the Property Inspector, by document (editFromInspector). - private readonly inspectorEdits = new Map(); - private readonly _onDidChangeCustomDocument = new vscode.EventEmitter>(); readonly onDidChangeCustomDocument = this._onDidChangeCustomDocument.event; @@ -745,35 +725,6 @@ export class MatEditorProvider implements vscode.CustomEditorProvider { - const node = findNode(nodeId); - if (!node) return { refused: 'Could not locate the item in the model.' }; - // What the user knows the property as: the inspector's label for it ('Word Length'), not its key. - const label = inspectorLabelOf(node, propName); - let result: unknown; - const observed = gesture('Edit ' + label, (mat) => { - DataModel.setActive(mat, node); - result = DataModel.editProperty(node.id, propName, newValue, oldValue); - }); - if (!observed) return { refused: `${name} cannot be edited.` }; - const refused = result as { error?: boolean; reason?: string } | boolean; - if (refused && typeof refused === 'object' && refused.error) { - for (let v = node; v; v = v.parent) { - if (v.parent === document.model()) { - repaintVariables([v]); - break; - } - } - return { refused: document.describe(refused.reason) }; - } - if (refused === false) return { refused: `Could not edit ${label} of this item.` }; - if (propName === 'Name') webview.postMessage({ type: 'selectRows', rowIds: [node.id] }); - return { nodeId: node.id }; - }; - // Delete the rows the gesture named, as ONE edit: the variables in one batch (one step), and // each row under a variable on its own (one step each). Nodes are resolved first and each is // deleted by the id it has at that moment: an element's id is its index, which the removal of @@ -901,11 +852,8 @@ export class MatEditorProvider implements vscode.CustomEditorProvider { document.views.delete(view); - if (this.inspectorEdits.get(uriString) === applyInspectorEdit) this.inspectorEdits.delete(uriString); unregisterAddGalleryView(webviewPanel); sub.dispose(); navSub.dispose(); @@ -913,18 +861,6 @@ export class MatEditorProvider implements vscode.CustomEditorProvider { diff --git a/src/host/PropertiesViewProvider.ts b/src/host/PropertiesViewProvider.ts index a9d74e72..f60ad662 100644 --- a/src/host/PropertiesViewProvider.ts +++ b/src/host/PropertiesViewProvider.ts @@ -5,20 +5,11 @@ import { matrixCellsMessage } from './matrixRequest.js'; import { renderWebviewHtml } from './webviewHtml.js'; import type { PropsToHostMessage } from '../common/protocol.js'; -/** - * Where an edit made in the panel goes: the host of the node's document, which makes it a - * gesture of its own (one undo step, core's refusal reported back through `refuse`). Handed to - * showNode by a host that takes such edits; the panel is read-only for the others. - */ -export type PropertyEditHandler = (nodeId: string, propName: string, newValue: string, oldValue: string) => void; - export class PropertiesViewProvider implements vscode.WebviewViewProvider { public static readonly viewType = 'dataExplorer.properties'; private view: vscode.WebviewView | null = null; private ready = false; - private pending: { groups: any[]; nodeId?: string } | null = null; // pending groups - // Where an edit of the node on display goes, or null: the panel is then read-only. - private editor: PropertyEditHandler | null = null; + private pending: any[] | null = null; // pending groups // The node currently on display, kept for `requestMatrix` only: its Value row may // carry a MatrixDescriptor, and the cells behind one are fetched when the panel // opens rather than stamped onto the row (see matrixRequest.ts for why). @@ -46,7 +37,7 @@ export class PropertiesViewProvider implements vscode.WebviewViewProvider { if (msg?.type === 'ready') { this.ready = true; if (this.pending) { - view.webview.postMessage({ type: 'showProps', ...this.pending }); + view.webview.postMessage({ type: 'showProps', groups: this.pending }); this.pending = null; } } else if (msg?.type === 'navigate') { @@ -55,12 +46,6 @@ export class PropertiesViewProvider implements vscode.WebviewViewProvider { this.onNavigate?.(msg.target, this.shownFrom); } else if (msg?.type === 'requestMatrix') { this.answerMatrix(view.webview, msg.nodeId); - } else if (msg?.type === 'editProperty') { - // Only an edit of the node on display, and only where its host takes one: a stale - // edit (the selection moved while it was typed) or one no host takes changes nothing. - if (this.editor && this.shown && this.shown.id === msg.nodeId) { - this.editor(msg.nodeId, msg.propName, msg.newValue, msg.oldValue); - } } }); view.onDidDispose(() => { @@ -68,39 +53,24 @@ export class PropertiesViewProvider implements vscode.WebviewViewProvider { this.ready = false; this.shown = null; this.shownFrom = undefined; - this.editor = null; }); view.webview.html = this.getHtml(view.webview, distRoot); } - /** - * Show `node`'s properties. With `editor`, the rows core says are editable are, and an edit - * of one goes to it; without, the panel is read-only. - */ - showNode(node: any, fromUri?: string, editor?: PropertyEditHandler): void { - const groups = buildPropertyGroups(node, { editable: !!editor }); + showNode(node: any, fromUri?: string): void { + const groups = buildPropertyGroups(node); this.shown = node; this.shownFrom = fromUri; - this.editor = editor ?? null; - const payload = { groups, ...(typeof node?.id === 'string' ? { nodeId: node.id } : {}) }; if (this.view && this.ready) { - this.view.webview.postMessage({ type: 'showProps', ...payload }); + this.view.webview.postMessage({ type: 'showProps', groups }); } else { - this.pending = payload; - } - } - - /** Say why the panel's last edit was refused. Sent after the showProps that put the row back. */ - refuse(propName: string, reason: string): void { - if (this.view && this.ready) { - this.view.webview.postMessage({ type: 'propertyRefused', propName, reason }); + this.pending = groups; } } clear(): void { this.shown = null; this.shownFrom = undefined; - this.editor = null; if (this.view && this.ready) { this.view.webview.postMessage({ type: 'empty' }); } else { diff --git a/src/host/piBuilder.ts b/src/host/piBuilder.ts index 1bcc4688..8346ebe0 100644 --- a/src/host/piBuilder.ts +++ b/src/host/piBuilder.ts @@ -5,9 +5,6 @@ export interface PIPropertyRow { name: string; value: string; editable: boolean; - // On an editable row: the property's key, as core's editProperty takes it ('wordLength'), - // where `name` is the label the panel shows ('Word Length'). - key?: string; type: 'text' | 'link'; linkTarget?: string; // A link on the VALUE, set by core on the Data Type property when the type names another @@ -27,17 +24,9 @@ export interface PIPropertyGroup { properties: PIPropertyRow[]; } -export interface PIBuildOptions { - // Whether the panel can write an edit back: its host handed it an edit channel for this - // node's document (PropertiesViewProvider.showNode). Then a row is editable exactly where core - // says it is (toPIObject's `editable`), and carries the key core's editProperty takes. - // Without one every row is read-only, as the dictionary and model views' panels are. - editable?: boolean; -} - // Replicates the vendored two-step transform (PIController.convertToPIObject + // LitRenderer.setPIContent). graphture/usedBy extras are intentionally omitted. -export function buildPropertyGroups(node: any, options: PIBuildOptions = {}): PIPropertyGroup[] { +export function buildPropertyGroups(node: any): PIPropertyGroup[] { if (!node || typeof node.toPIObject !== 'function') return []; const raw = node.toPIObject(); if (!raw || !raw.propertySheet) return []; @@ -56,12 +45,10 @@ export function buildPropertyGroups(node: any, options: PIBuildOptions = {}): PI // folded into `link`: that one would make this a `type: 'link'` row, whose template // anchors the property name and mutes the value. const valueLink = (propDef as any).valueLink; - const editable = !!options.editable && propDef.editable === true && !link; const row: PIPropertyRow = { name: propDef.displayName || propDef.name, value: String(obj[propDef.name] ?? ''), - editable, - ...(editable ? { key: propDef.name } : {}), + editable: false, // read-only V1 type: link ? 'link' : 'text', linkTarget: link || undefined, ...(typeof valueLink === 'string' && valueLink !== '' ? { valueLink } : {}), diff --git a/src/webview/components/dex-property-inspector.ts b/src/webview/components/dex-property-inspector.ts index 633720e5..967a8956 100644 --- a/src/webview/components/dex-property-inspector.ts +++ b/src/webview/components/dex-property-inspector.ts @@ -15,8 +15,6 @@ export interface PropertyRow { name: string; value: string; editable?: boolean; - // The key an edit of an editable row names (piBuilder): core's, where `name` is its label. - key?: string; type?: 'text' | 'link'; linkTarget?: string; // A link on the VALUE (piBuilder, from core's toPIObject): the Data Type of an entry @@ -135,22 +133,10 @@ export class DexPropertyInspector extends LitElement { font-size: 11px; color: var(--dex-color-text-secondary, #888); } - - .prop-refusal { - margin: 4px 8px; - padding: 4px 6px; - font-size: 12px; - border-left: 2px solid var(--vscode-inputValidation-errorBorder, #be1100); - background: var(--vscode-inputValidation-errorBackground, transparent); - color: var(--vscode-errorForeground, inherit); - } `; @property({ type: Array }) groups: PropertyGroup[] = []; - // Why the last edit made here was refused, in core's words, until the next node is shown. - @property({ attribute: false }) refusal: string | null = null; - private _onPropertyChange(propName: string, newValue: string, oldValue: string): void { this.dispatchEvent(new CustomEvent('dex-property-changed', { detail: { propName, newValue, oldValue }, @@ -176,7 +162,7 @@ export class DexPropertyInspector extends LitElement { return html` this._onPropertyChange(prop.key ?? prop.name, (e.target as HTMLInputElement).value, prop.value)} + @change=${(e: Event) => this._onPropertyChange(prop.name, (e.target as HTMLInputElement).value, prop.value)} />`; } // The type this value names, as a link to its definition — the same jump the table's @@ -206,7 +192,6 @@ export class DexPropertyInspector extends LitElement { override render() { return html`
- ${this.refusal ? html`` : ''} ${this.groups.map(group => html`
${group.title}
diff --git a/src/webview/pi-main.ts b/src/webview/pi-main.ts index 7fd5a9f7..ca0754bd 100644 --- a/src/webview/pi-main.ts +++ b/src/webview/pi-main.ts @@ -26,9 +26,6 @@ function setEmpty(show: boolean): void { if (pi) (pi as HTMLElement).style.display = show ? 'none' : 'block'; } -// The node the panel shows, as the last showProps named it: what an edit made here is of. -let shownNodeId: string | null = null; - window.addEventListener('message', (event: MessageEvent) => { const msg = event.data as HostToPropsMessage; // `showProps` and `empty` each replace every row an open grid was anchored to, so @@ -39,28 +36,14 @@ window.addEventListener('message', (event: MessageEvent) => { if (msg.type === 'showProps') { matrixOpen.close(); const groups = msg.groups ?? []; - shownNodeId = typeof msg.nodeId === 'string' ? msg.nodeId : null; - pi.refusal = null; pi.groups = groups; setEmpty(groups.length === 0); } else if (msg.type === 'empty') { matrixOpen.close(); - shownNodeId = null; - pi.refusal = null; setEmpty(true); - } else if (msg.type === 'propertyRefused') { - pi.refusal = typeof msg.reason === 'string' ? msg.reason : null; } }); -// An edit committed in an editable row (the host made rows editable only where it takes them): -// sent with the node it is of, which the host holds to the node it is showing. -pi?.addEventListener('dex-property-changed', (e: Event) => { - const detail = (e as CustomEvent).detail ?? {}; - if (!shownNodeId || typeof detail.propName !== 'string') return; - vscode.postMessage({ type: 'editProperty', nodeId: shownNodeId, propName: detail.propName, newValue: String(detail.newValue ?? ''), oldValue: String(detail.oldValue ?? '') }); -}); - // A cross-reference in the inspector was clicked. This webview has no rows to select in — // the target is a row in a TABLE, which is a different webview — so the host resolves it, // exactly as it does for a cross-tab Usage link (host/navigate.ts). diff --git a/test-integration/suite/matAddGallery.test.ts b/test-integration/suite/matAddGallery.test.ts index 80c005eb..44fdded4 100644 --- a/test-integration/suite/matAddGallery.test.ts +++ b/test-integration/suite/matAddGallery.test.ts @@ -358,151 +358,6 @@ suite('MAT-file editor: the Add gallery', () => { assert.strictEqual(o.doc.edits.dirty, false); }); - // ---- The Property Inspector: the rows a variable's table row has no column for ---------------- - - /** Core's own text of `key` on the live node `id`, as the inspector shows it. */ - const piValue = (id: string, key: string): string => { - const node = (DataModel as any).findNodeById(id); - return String(node.toPIObject().objects[0][key] ?? ''); - }; - - /** `key` of the variable `name` in `bytes`, read back as a reopened file shows it. */ - function savedPiValues(bytes: Uint8Array, path: string[], keys: string[]): Record { - const srcId = `matadd-readback:${Math.random()}`; - const mat: any = DataModel.addMatSource(srcId, bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer, { path: 'r.mat' }); - try { - let node: any = mat; - for (const n of path) node = node.children.find((c: any) => c.name === n); - const obj = node.toPIObject().objects[0]; - return Object.fromEntries(keys.map((k) => [k, String(obj[k] ?? '')])); - } finally { - DataModel.removeDataSource(srcId); - } - } - - // Per class the gallery offers an object of: an edit of each row the inspector alone offers - // (and Data Type, which the table shows in a column of its own and does not edit), in an order - // MATLAB takes them in, and the same edits as MATLAB statements for the live gate. A Bus's - // element is edited on the element. - const PI_EDITS: Record = { - 'Simulink.NumericType': { - edits: [['dataTypeMode', 'Fixed-point: binary point scaling'], ['wordLength', '12'], ['fractionLength', '4'], ['signedness', 'Unsigned'], - ['dataTypeOverride', 'Off'], ['isAlias', 'true'], ['dataScope', 'Exported'], ['headerFile', 'nt.h']], - matlab: "obj.DataTypeMode = 'Fixed-point: binary point scaling'; obj.WordLength = 12; obj.FractionLength = 4; obj.Signedness = 'Unsigned'; " + - "obj.DataTypeOverride = 'Off'; obj.IsAlias = true; obj.DataScope = 'Exported'; obj.HeaderFile = 'nt.h';", - }, - 'Simulink.Signal': { - edits: [['dimensions', '[2 3]'], ['initialValue', '0'], ['sampleTime', '0.1'], ['samplingMode', 'Sample based'], ['DataType', 'single'], - ['storageClass', 'ExportedGlobal'], ['identifier', 'sigX']], - matlab: "obj.Dimensions = [2 3]; obj.InitialValue = '0'; obj.SampleTime = 0.1; obj.SamplingMode = 'Sample based'; obj.DataType = 'single'; " + - "obj.CoderInfo.StorageClass = 'ExportedGlobal'; obj.CoderInfo.Identifier = 'sigX';", - }, - 'Simulink.Bus': { - edits: [['dataScope', 'Exported'], ['headerFile', 'b.h'], ['alignment', '8'], ['preserveElementDimensions', 'true']], - element: [['dimensions', '[1 4]']], - matlab: "obj.DataScope = 'Exported'; obj.HeaderFile = 'b.h'; obj.Alignment = 8; obj.PreserveElementDimensions = true; " + - 'obj.Elements(1) = Simulink.BusElement; obj.Elements(1).Dimensions = [1 4];', - }, - 'Simulink.AliasType': { - edits: [['dataScope', 'Exported'], ['headerFile', 'a.h'], ['BaseType', 'int16']], - matlab: "obj.DataScope = 'Exported'; obj.HeaderFile = 'a.h'; obj.BaseType = 'int16';", - }, - 'Simulink.ValueType': { - edits: [['dimensions', '[2 2]'], ['DataType', 'int8']], - matlab: "obj.Dimensions = [2 2]; obj.DataType = 'int8';", - }, - 'Simulink.Parameter': { - edits: [['DataType', 'int16'], ['Unit', 'm'], ['storageClass', 'ExportedGlobal'], ['identifier', 'pX']], - matlab: "obj.DataType = 'int16'; obj.Unit = 'm'; obj.CoderInfo.StorageClass = 'ExportedGlobal'; obj.CoderInfo.Identifier = 'pX';", - }, - }; - - for (const [className, plan] of Object.entries(PI_EDITS)) { - test(`a new ${className}'s inspector rows each take an edit, which saves and reopens as it was`, async () => { - const o = await open('pv_basic.mat', fixture('pv_basic.mat')); - if (!lastSetRows(o.view.posts).add.classes.includes(className)) return; // core does not offer it - const name = TILES[className].name; - add(o, className, false); - await waitFor(`the ${name} row`, () => !!topRowOrNull(o.view.posts, name)); - const uri = o.uri.toString(); - const editAll = (id: string, edits: [string, string][]) => { - for (const [key, value] of edits) { - const result = provider.editFromInspector(uri, id, key, value, piValue(id, key)); - assert.strictEqual(result.refused, undefined, `${className} ${key} = ${value}: ${result.refused}`); - assert.strictEqual(piValue(result.nodeId!, key), value, `${key} shows what was typed`); - } - }; - const id = topRow(o.view.posts, name).ID; - editAll(id, plan.edits); - let elementPath: string[] | null = null; - if (plan.element) { - o.view.send({ type: 'addChild', rowId: id }); - await waitFor('the element', () => childrenOf(o.view.posts, topRow(o.view.posts, name)).length === 1); - const el = tableOf(o.view.posts).find((r) => r.parent === id)!; - editAll(el.ID, plan.element); - elementPath = [name, nameOf(el)]; - } - const expected = Object.fromEntries(plan.edits.map(([k]) => [k, piValue(id, k)])); - assert.strictEqual(o.events.length, 1 + plan.edits.length + (plan.element ? 1 + plan.element.length : 0), 'one edit per gesture, the add among them'); - await save(o); - keptUntouched(o.original, disk(o), [name]); - forGate(o.original, o.path, { new: [{ var: name, class: className, code: plan.matlab }] }); - assert.deepStrictEqual(savedPiValues(disk(o), [name], Object.keys(expected)), expected, 'the saved file shows every edit'); - if (elementPath) { - assert.deepStrictEqual(savedPiValues(disk(o), elementPath, plan.element!.map(([k]) => k)), Object.fromEntries(plan.element!)); - } - // And in another tab: the row the table shows for it. - const again = await reopen(o.path); - assert.strictEqual(topRow(again.view.posts, name).Class, className); - }); - } - - test("a NumericType's mode edit makes its derived rows editable and shown; its edits undo and redo", async () => { - const o = await open('pv_basic.mat', fixture('pv_basic.mat')); - add(o, 'Simulink.NumericType', false); - await waitFor('the NumericType row', () => !!topRowOrNull(o.view.posts, 'NumericType')); - const uri = o.uri.toString(); - const id = topRow(o.view.posts, 'NumericType').ID; - const editable = (key: string) => (DataModel as any).findNodeById(id).toPIObject().propertySheet.properties.find((p: any) => p.name === key)?.editable; - assert.strictEqual(editable('wordLength'), false, 'derived while the mode is Double'); - provider.editFromInspector(uri, id, 'dataTypeMode', 'Fixed-point: binary point scaling', 'Double'); - assert.strictEqual(editable('wordLength'), true); - // The edit is named as the inspector names the property, not by its key. - const label = (DataModel as any).findNodeById(id).toPIObject().propertySheet.properties.find((p: any) => p.name === 'dataTypeMode').displayName; - assert.strictEqual((o.events[1] as any).label, `Edit ${label}`); - assert.ok(!/dataTypeMode/.test((o.events[1] as any).label)); - assert.strictEqual(piValue(id, 'wordLength'), '16', "the mode's own word length"); - provider.editFromInspector(uri, id, 'fractionLength', '4', '0'); - assert.strictEqual(piValue(id, 'slope'), '0.0625', 'the slope follows the fraction length'); - // The add, the mode, the fraction length: three edits, undone and redone in turn. - assert.strictEqual(o.events.length, 3); - await o.events[2].undo(); - assert.strictEqual(piValue(id, 'fractionLength'), '0'); - assert.strictEqual(piValue(id, 'slope'), '1'); - await o.events[1].undo(); - assert.strictEqual(piValue(id, 'dataTypeMode'), 'Double'); - await o.events[1].redo(); - await o.events[2].redo(); - assert.strictEqual(piValue(id, 'fractionLength'), '4'); - await save(o); - assert.deepStrictEqual(savedPiValues(disk(o), ['NumericType'], ['dataTypeMode', 'fractionLength']), { dataTypeMode: 'Fixed-point: binary point scaling', fractionLength: '4' }); - forGate(o.original, o.path, { new: [{ var: 'NumericType', class: 'Simulink.NumericType', code: "obj.DataTypeMode = 'Fixed-point: binary point scaling'; obj.FractionLength = 4;" }] }); - }); - - test("an inspector edit core refuses comes back with core's reason, and records nothing", async () => { - const o = await open('pv_basic.mat', fixture('pv_basic.mat')); - add(o, 'Simulink.Parameter', false); - await waitFor('the Param row', () => !!topRowOrNull(o.view.posts, 'Param')); - assert.strictEqual(o.events.length, 1); - const id = topRow(o.view.posts, 'Param').ID; - const result = provider.editFromInspector(o.uri.toString(), id, 'identifier', 'pX', ''); - assert.match(result.refused ?? '', /Identifier/); - assert.match(result.refused ?? '', /"Param"/); - assert.ok(!(result.refused ?? '').includes('binedit:'), 'the refusal names the variable, not the srcId'); - assert.strictEqual(o.events.length, 1, 'no edit reached VS Code'); - assert.strictEqual(piValue(id, 'identifier'), ''); - }); - // Only for the live gate, with DEX_GATE_OUT and DEX_GATE_FIXTURES set: one variable of every // class core lists added to each MAT-file of DEX_GATE_FIXTURES, and an element to every Bus it // holds, each saved on its own — so the files MATLAB judges come from originals of the diff --git a/test-integration/suite/propertiesView.test.ts b/test-integration/suite/propertiesView.test.ts index 6974f5c1..b9bde770 100644 --- a/test-integration/suite/propertiesView.test.ts +++ b/test-integration/suite/propertiesView.test.ts @@ -27,25 +27,8 @@ function fakeNode() { }; } -// A node with an id and one property core says is editable, and one it says is not. -function editableNode(id: string) { - return { - id, - toPIObject() { - return { - objects: [{ Name: 'Kp', wordLength: '16' }], - propertySheet: { - groups: [{ name: 'g', displayName: 'General', items: [{ type: 'property', name: 'Name' }, { type: 'property', name: 'wordLength' }] }], - properties: [{ name: 'Name', displayName: 'Name', editable: false }, { name: 'wordLength', displayName: 'Word Length', editable: true }], - }, - }; - }, - }; -} - interface Recorded { posted: any[]; - send: (m: any) => void; fireReady: () => void; fireDispose: () => void; view: vscode.WebviewView; @@ -88,7 +71,6 @@ function fakeView(extensionUri: vscode.Uri): Recorded { return { posted, - send: (m: any) => messageHandler?.(m), fireReady: () => messageHandler?.({ type: 'ready' }), fireDispose: () => disposeHandler?.(), view: view as vscode.WebviewView, @@ -158,41 +140,6 @@ suite('PropertiesViewProvider', () => { assert.strictEqual(rec.posted.length, 0, 'a cleared buffer posts nothing on ready'); }); - test('a node shown with an edit channel has its editable rows, and an edit of it goes to the channel', () => { - const provider = new PropertiesViewProvider(extensionUri()); - const rec = fakeView(extensionUri()); - provider.resolveWebviewView(rec.view); - rec.fireReady(); - const edits: string[][] = []; - provider.showNode(editableNode('m/nt'), 'file:///x.mat', (...a) => edits.push(a)); - const shown = rec.posted.pop(); - assert.strictEqual(shown.nodeId, 'm/nt'); - assert.deepStrictEqual( - shown.groups[0].properties.map((p: any) => [p.name, p.editable, p.key]), - [['Name', false, undefined], ['Word Length', true, 'wordLength']], - ); - rec.send({ type: 'editProperty', nodeId: 'm/nt', propName: 'wordLength', newValue: '8', oldValue: '16' }); - assert.deepStrictEqual(edits, [['m/nt', 'wordLength', '8', '16']]); - // An edit of a node no longer on display (the selection moved while it was typed) is dropped. - rec.send({ type: 'editProperty', nodeId: 'm/other', propName: 'wordLength', newValue: '4', oldValue: '16' }); - assert.strictEqual(edits.length, 1); - provider.refuse('wordLength', 'Cannot edit "nt": no.'); - assert.deepStrictEqual(rec.posted.pop(), { type: 'propertyRefused', propName: 'wordLength', reason: 'Cannot edit "nt": no.' }); - }); - - test('a node shown without one is read-only, and an edit of it goes nowhere', () => { - const provider = new PropertiesViewProvider(extensionUri()); - const rec = fakeView(extensionUri()); - provider.resolveWebviewView(rec.view); - rec.fireReady(); - const edits: string[][] = []; - provider.showNode(editableNode('s/nt'), 'file:///x.sldd', (...a) => edits.push(a)); - provider.showNode(editableNode('s/nt'), 'file:///x.sldd'); - assert.ok(rec.posted.pop().groups[0].properties.every((p: any) => p.editable === false && p.key === undefined)); - rec.send({ type: 'editProperty', nodeId: 's/nt', propName: 'wordLength', newValue: '8', oldValue: '16' }); - assert.deepStrictEqual(edits, [], 'the channel of the node shown before is not this one'); - }); - test('after dispose, showNode buffers again instead of posting to a dead view', () => { const provider = new PropertiesViewProvider(extensionUri()); const rec = fakeView(extensionUri()); diff --git a/test/inspectorEdit.test.ts b/test/inspectorEdit.test.ts deleted file mode 100644 index dce65e50..00000000 --- a/test/inspectorEdit.test.ts +++ /dev/null @@ -1,118 +0,0 @@ -// Copyright 2026 The MathWorks, Inc. -// @vitest-environment happy-dom -// -// The Property Inspector's edit channel, which a MAT-file's editor gives it (the dictionary and -// model views' panels stay read-only): -// -// - piBuilder makes a row editable exactly where core's toPIObject says, and gives it the key -// core's editProperty takes, only when the host has an edit channel; without one, nothing; -// - the inspector reports an edit by that key, and shows a refusal until the next node; -// - pi-main.ts, as it ships, sends the edit with the node the panel shows, and shows the -// reason the host sends back. -// -// The rows are the real ones: core's MAT-file nodes, created as the Add gallery creates them. -import { describe, it, expect, beforeAll, beforeEach } from 'vitest'; -import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; -import { DataModel } from 'data-explorer-core'; -import { buildPropertyGroups } from '../src/host/piBuilder.js'; - -/* eslint-disable @typescript-eslint/no-explicit-any */ - -const read = (p: string) => readFileSync(join(import.meta.dirname, '..', p), 'utf8'); -const PI_BODY = /body: `([\s\S]*?)`,/.exec(read('src/host/PropertiesViewProvider.ts'))![1]; - -/** A fresh MAT-file with one new variable of `className`, as the gallery creates it. */ -function created(className: string): any { - const b = readFileSync(join(import.meta.dirname, '..', 'test-integration', 'fixtures', 'mat', 'pv_basic.mat')); - const srcId = `inspector-edit:${className}:${Math.random()}`; - const mat: any = DataModel.addMatSource(srcId, b.buffer.slice(b.byteOffset, b.byteOffset + b.byteLength) as ArrayBuffer, { path: 'pv.mat' }); - DataModel.setActiveContext(mat); - return DataModel.addEntry('', className); -} - -const rows = (node: any, editable: boolean) => buildPropertyGroups(node, { editable }).flatMap((g) => g.properties); - -describe('piBuilder, given an edit channel', () => { - it('makes a row editable where core says it is, with the key core takes', () => { - const nt = created('Simulink.NumericType'); - const pi = nt.toPIObject(); - const coreEditable = pi.propertySheet.properties.filter((p: any) => p.editable).map((p: any) => p.name); - const editable = rows(nt, true).filter((r) => r.editable); - expect(editable.map((r) => r.key).sort()).toEqual([...coreEditable].sort()); - expect(editable.find((r) => r.key === 'dataTypeMode')).toMatchObject({ editable: true, value: 'Double' }); - // A word length is derived while the mode is Double: core offers no edit of it. - expect(rows(nt, true).find((r) => r.name.replace(/\s/g, '').toLowerCase() === 'wordlength')?.editable).toBe(false); - }); - - it('follows core as an edit changes what is editable: a fixed-point mode makes the word length editable', () => { - const nt = created('Simulink.NumericType'); - DataModel.editProperty(nt.id, 'dataTypeMode', 'Fixed-point: binary point scaling', 'Double'); - const word = rows(nt, true).find((r) => r.key === 'wordLength'); - expect(word).toMatchObject({ editable: true, value: '16' }); - }); - - it('makes nothing editable without one, and gives no row a key', () => { - for (const cls of ['Simulink.NumericType', 'Simulink.Signal', 'Simulink.Bus', 'Simulink.Parameter']) { - expect(rows(created(cls), false).every((r) => r.editable === false && r.key === undefined), cls).toBe(true); - } - }); -}); - -describe('the inspector webview, as it ships', () => { - const posted: any[] = []; - let pi: any; - - const send = (data: any) => window.dispatchEvent(new MessageEvent('message', { data })); - - beforeAll(async () => { - (globalThis as any).acquireVsCodeApi = () => ({ postMessage: (m: any) => posted.push(m) }); - document.body.innerHTML = PI_BODY; - await import('../src/webview/pi-main.js'); - pi = document.querySelector('dex-property-inspector'); - }, 60_000); - - beforeEach(() => { - posted.length = 0; - }); - - const inputOf = async (label: string): Promise => { - await pi.updateComplete; - const row = [...pi.shadowRoot.querySelectorAll('.prop-row')].find((r: any) => r.querySelector('.prop-name')?.textContent === label) as HTMLElement; - expect(row, `a row labelled ${label}`).toBeTruthy(); - return row.querySelector('input') as HTMLInputElement; - }; - - it('sends an edit of an editable row by its key, with the node the panel shows', async () => { - const nt = created('Simulink.NumericType'); - const groups = buildPropertyGroups(nt, { editable: true }); - send({ type: 'showProps', groups, nodeId: nt.id }); - const label = groups.flatMap((g) => g.properties).find((r) => r.key === 'dataTypeMode')!.name; - const input = await inputOf(label); - input.value = 'Single'; - input.dispatchEvent(new Event('change')); - expect(posted).toEqual([{ type: 'editProperty', nodeId: nt.id, propName: 'dataTypeMode', newValue: 'Single', oldValue: 'Double' }]); - }); - - it('shows the reason an edit was refused, until the next node is shown', async () => { - const p = created('Simulink.Parameter'); - send({ type: 'showProps', groups: buildPropertyGroups(p, { editable: true }), nodeId: p.id }); - send({ type: 'propertyRefused', propName: 'identifier', reason: 'Cannot edit "Param": no identifier under Auto.' }); - await pi.updateComplete; - const alert = pi.shadowRoot.querySelector('.prop-refusal'); - expect(alert?.getAttribute('role')).toBe('alert'); - expect(alert?.textContent).toBe('Cannot edit "Param": no identifier under Auto.'); - send({ type: 'showProps', groups: buildPropertyGroups(p, { editable: true }), nodeId: p.id }); - await pi.updateComplete; - expect(pi.shadowRoot.querySelector('.prop-refusal')).toBeNull(); - }); - - it('sends nothing for a panel shown without a node, or emptied', async () => { - send({ type: 'showProps', groups: [{ title: 'G', properties: [{ name: 'Value', value: '1', editable: true, key: 'Value' }] }] }); - const input = await inputOf('Value'); - input.value = '2'; - input.dispatchEvent(new Event('change')); - send({ type: 'empty' }); - expect(posted).toEqual([]); - }); -}); diff --git a/test/piBuilder.test.ts b/test/piBuilder.test.ts index ffe36c06..c360f572 100644 --- a/test/piBuilder.test.ts +++ b/test/piBuilder.test.ts @@ -4,6 +4,7 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { getModel, getModelFromBytes, invalidate } from '../src/host/SlddModel.js'; import { buildPropertyGroups } from '../src/host/piBuilder.js'; +import { DataModel } from 'data-explorer-core'; function fixturePath(name: string): string { return fileURLToPath(new URL(`./fixtures/${name}`, import.meta.url)); @@ -185,6 +186,35 @@ describe('buildPropertyGroups', () => { expect(all.every((p) => p.editable === false)).toBe(true); }); + it("marks every property of a MAT-file's variables read-only too, however much core lets a MAT-file edit", () => { + // Core decides what a file type lets edit, and says so per property (toPIObject's `editable`); + // the panel is read-only in every editor until its edits are qualified, a MAT-file's included. + // Every class the MAT-file Add gallery creates, and a new Bus's element. + const b = readFileSync(fileURLToPath(new URL('../test-integration/fixtures/mat/pv_basic.mat', import.meta.url))); + const srcId = 'pi-readonly-mat'; + const mat: any = DataModel.addMatSource(srcId, b.buffer.slice(b.byteOffset, b.byteOffset + b.byteLength) as ArrayBuffer, { path: 'pv_basic.mat' }); + try { + DataModel.setActiveContext(mat); + const classes: string[] = mat.getAllowedTypes(); + expect(classes).toEqual(expect.arrayContaining(['MatlabVariable', 'MatlabStruct', 'Simulink.Parameter', 'Simulink.Signal', 'Simulink.Bus', 'string'])); + const nodes: any[] = classes.map((c) => DataModel.addEntry('', c)); + const bus = nodes.find((n) => n?.className === 'Simulink.Bus'); + DataModel.addChildTo(bus.id); + nodes.push(bus.children[0], ...mat.children.filter((c: any) => !nodes.includes(c))); + let coreEditable = 0; + for (const node of nodes) { + expect(node, 'core created a variable of each class it lists').toBeTruthy(); + coreEditable += (node.toPIObject?.()?.propertySheet?.properties ?? []).filter((p: any) => p.editable === true).length; + const rows = buildPropertyGroups(node).flatMap((g) => g.properties); + expect(rows.length, node.name).toBeGreaterThan(0); + expect(rows.filter((p) => p.editable !== false).map((p) => p.name), `${node.className} ${node.name}`).toEqual([]); + } + expect(coreEditable, 'core itself offers edits of these rows').toBeGreaterThan(0); + } finally { + DataModel.removeDataSource(srcId); + } + }); + it('places a block in the model, which is what tells two `Gain` rows apart', () => { // The selection's own answer to "which Gain is this?". The table qualifies a block // row's Name with its enclosing systems and hovers the whole path; the panel is