From 9d9794aaba38f9934571ed993861a1800efe5caf Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 20:16:18 -0400 Subject: [PATCH 1/5] Expect a Signal's Dimensions Mode to be a dropdown, as core now offers it Core now offers a Simulink.Signal's Complexity and DimensionsMode as dropdowns over MATLAB's own sets: auto, real, complex and auto, Fixed, Variable. This reverses the old "safe as read-only" label. The table's generic select cell already renders and edits such a cell, and the host already routes the edit to node.setProperty by column id, so no source change is needed here. test/dimensionsMode.test.ts pinned exactly the label the ruling removes. It now expects the real codegen.sldd Signal's dimensionsMode to be a select, with the cell { text: 'Fixed', editable: true, editor: 'select', options: ['auto', 'Fixed', 'Variable'] }, and is retitled "editable". Against the pinned core it fails with "expected 'label' to be 'select'". Two comments that described the label are brought up to date. One is in elementProps.test.ts. The other is in rowBuilder.ts's column-group note, which now names ValueType and Signal beside BusElement. --- src/host/rowBuilder.ts | 9 +++++---- test/dimensionsMode.test.ts | 14 +++++++++++--- test/elementProps.test.ts | 4 ++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/host/rowBuilder.ts b/src/host/rowBuilder.ts index 95b4a41..be65904 100644 --- a/src/host/rowBuilder.ts +++ b/src/host/rowBuilder.ts @@ -36,10 +36,11 @@ export const COLUMN_LABELS: Record = { // columns (dimensions/complexity/dimensionsMode → Data Object; // storageClass/headerFile/alignment → Code Generation) all get their picker // header here. Read-only for most classes, but not by definition: on a -// Simulink.BusElement, complexity and dimensionsMode emit an editable select over -// MATLAB's own enum, which the generic cell path renders and edits like any other -// object cell. Labels still come from the schema (schemaColumnLabels), which -// remains the single source of truth for a column's display name. +// Simulink.BusElement, a Simulink.ValueType and a Simulink.Signal, complexity and +// dimensionsMode emit an editable select over MATLAB's own enum for that class, +// which the generic cell path renders and edits like any other object cell. +// Labels still come from the schema (schemaColumnLabels), which remains the +// single source of truth for a column's display name. export const COLUMN_GROUPS: Record = { Min: 'Data Object', Max: 'Data Object', Unit: 'Data Object', dimensions: 'Data Object', complexity: 'Data Object', dimensionsMode: 'Data Object', diff --git a/test/dimensionsMode.test.ts b/test/dimensionsMode.test.ts index 534f8f7..f7f961e 100644 --- a/test/dimensionsMode.test.ts +++ b/test/dimensionsMode.test.ts @@ -36,13 +36,21 @@ describe('Dimensions Mode: real-fixture parity', () => { expect(cgSignal.serial._properties.DimensionsMode).toBe('Fixed'); }); - it('the Signal surfaces a read-only dimensionsMode column from real data', () => { + it('the Signal surfaces an editable dimensionsMode column from real data', () => { + // MATLAB constrains a Signal's DimensionsMode to set(Simulink.Signal,'DimensionsMode'), + // auto|Fixed|Variable, so it is a dropdown over that set, in MATLAB's own casing. It + // reaches the table as the object cell dex-tree-table opens a select from, not a string. const { cgSignal } = loadCodegen(); const prop = cgSignal.getProperties().find((p: any) => p.key === 'dimensionsMode'); expect(prop).toBeDefined(); - expect(prop.editor).toBe('label'); + expect(prop.editor).toBe('select'); const row: any = cgSignal.toRow(); - expect(row.dimensionsMode).toBe('Fixed'); + expect(row.dimensionsMode).toEqual({ + text: 'Fixed', + editable: true, + editor: 'select', + options: ['auto', 'Fixed', 'Variable'], + }); }); it('the Parameter has no DimensionsMode and surfaces no such column', () => { diff --git a/test/elementProps.test.ts b/test/elementProps.test.ts index facecc7..e52e83c 100644 --- a/test/elementProps.test.ts +++ b/test/elementProps.test.ts @@ -62,8 +62,8 @@ for (const variant of ['elem_text', 'elem_binary'] as const) { const props = x.getProperties(); // MATLAB constrains a BusElement's Complexity and DimensionsMode to a // closed enum, so each is a dropdown; Dimensions is a free numeric value - // and stays read-only. A Signal's dimensionsMode column comes from the - // declarative schema and is still a label — see dimensionsMode.test.ts. + // and stays read-only. A Signal's complexity and dimensionsMode are selects + // too, over the Signal's own sets, which add 'auto' — see dimensionsMode.test.ts. for (const [key, editor] of Object.entries({ complexity: 'select', dimensions: 'label', dimensionsMode: 'select' })) { const prop = props.find((p: any) => p.key === key); expect(prop, key).toBeDefined(); From ef578bc356cd8077dfa0f32738c7bafc21cd7e2e Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 20:32:03 -0400 Subject: [PATCH 2/5] Test that a Parameter's or Signal's Alignment can be edited in the table data-explorer-core now lets Alignment (CoderInfo.Alignment) be edited by MATLAB's rule wherever MATLAB takes an edit, which is any StorageClass but 'Auto' (group E ruling, C-086). The vscode side needs no code for it: the generic schema cell renders the text editor, and the text editor provider hands the column to node.setProperty. test-integration/suite/alignmentEdit.test.ts locks the behaviour through the real SlddTextEditorProvider, over core's MATLAB-written classes_r2027a_text.sldd, copied into test-integration/fixtures/classes beside the workspace folder: - The alignment cell is an editable text cell on an ExportedGlobal Parameter or Signal, and plain text under 'Auto'. - 8 on pSC saves pAl8's CoderInfo: a typed int32, first in CoderInfo. Nothing else changes. - -1 removes the key, so pSC is back to what MATLAB wrote. pAl8 set to -1 saves pAlM1's CoderInfo. - 3 is refused in MATLAB's words, and the document is unchanged. So is an Alignment under 'Auto'. - StorageClass set to 'Auto' on pAl8 saves pNo's CoderInfo. All six fail against the pinned core v1.39.0, where the cell is plain text and the edit is accepted and then lost. They pass against core main. A comment in rowBuilder.ts now names alignment among the columns that can be edited. --- src/host/rowBuilder.ts | 4 +- .../fixtures/classes/classes_r2027a_text.sldd | 715 ++++++++++++++++++ test-integration/suite/alignmentEdit.test.ts | 217 ++++++ 3 files changed, 935 insertions(+), 1 deletion(-) create mode 100644 test-integration/fixtures/classes/classes_r2027a_text.sldd create mode 100644 test-integration/suite/alignmentEdit.test.ts diff --git a/src/host/rowBuilder.ts b/src/host/rowBuilder.ts index be65904..398b644 100644 --- a/src/host/rowBuilder.ts +++ b/src/host/rowBuilder.ts @@ -38,7 +38,9 @@ export const COLUMN_LABELS: Record = { // header here. Read-only for most classes, but not by definition: on a // Simulink.BusElement, a Simulink.ValueType and a Simulink.Signal, complexity and // dimensionsMode emit an editable select over MATLAB's own enum for that class, -// which the generic cell path renders and edits like any other object cell. +// and on a Simulink.Parameter or Simulink.Signal whose StorageClass is not 'Auto', +// alignment emits an editable text cell. The generic cell path renders and edits +// each like any other object cell. // Labels still come from the schema (schemaColumnLabels), which remains the // single source of truth for a column's display name. export const COLUMN_GROUPS: Record = { diff --git a/test-integration/fixtures/classes/classes_r2027a_text.sldd b/test-integration/fixtures/classes/classes_r2027a_text.sldd new file mode 100644 index 0000000..a66baae --- /dev/null +++ b/test-integration/fixtures/classes/classes_r2027a_text.sldd @@ -0,0 +1,715 @@ +{ + "__MW_TEXT_COREPROPERTIES__": { + "release": "R2027a" + }, + "__MW_TEXT_PARTS__": { + "__MW_TEXT_PART__/data/chunk0": { + "__MW_TEXT_content": { + "entries": [ + { + "name": "bpD", + "metadata": { + "uuid": "9465308b-bf22-40bb-addd-808bd65f72a1", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.001240", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Breakpoint", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "Breakpoints": { + "_id": "2", + "_object_class": "Simulink.lookuptable.Breakpoint", + "_properties": { + "DataType": "auto", + "Description": "", + "Dimensions": [ + 0, + 0 + ], + "FieldName": "BP", + "TunableSizeName": "N", + "TunableSizeValue": -1, + "Unit": "" + } + }, + "CoderInfo": { + "_id": "3", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "4", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Parameter", + "StorageClass": "Auto" + } + }, + "StructTypeInfo": { + "_id": "5", + "_object_class": "Simulink.lookuptable.StructTypeInfo", + "_properties": { + "DataScope": "Auto", + "HeaderFileName": "", + "Name": "" + } + }, + "SupportTunableSize": false + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "busAl8", + "metadata": { + "uuid": "2a268469-ee64-4fb5-bb96-856f31077990", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.009331", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Bus", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "Alignment": 8, + "DataScope": "Auto", + "Description": "", + "Elements_internal": [], + "HeaderFile": "", + "PreserveElementDimensions": false + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "lutD", + "metadata": { + "uuid": "6b74d389-e89c-482e-b7f8-1196f25ea9c8", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191455.996084", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.LookupTable", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "AllowMultipleInstancesOfTypeToHaveDifferentTableBreakpointSizes": false, + "Breakpoints": { + "_id": "3", + "_object_class": "Simulink.lookuptable.Breakpoint", + "_properties": { + "DataType": "auto", + "Description": "", + "Dimensions": [ + 0, + 0 + ], + "FieldName": "BP1", + "TunableSizeName": "N1", + "TunableSizeValue": -1, + "Unit": "" + } + }, + "CoderInfo": { + "_id": "4", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "5", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Parameter", + "StorageClass": "Auto" + } + }, + "StructTypeInfo": { + "_id": "6", + "_object_class": "Simulink.lookuptable.StructTypeInfo", + "_properties": { + "DataScope": "Auto", + "HeaderFileName": "", + "Name": "" + } + }, + "SupportTunableSize": false, + "Table": { + "_id": "2", + "_object_class": "Simulink.lookuptable.Table", + "_properties": { + "DataType": "auto", + "Description": "", + "Dimensions": [ + 0, + 0 + ], + "FieldName": "Table", + "Unit": "" + } + } + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "pAl8", + "metadata": { + "uuid": "ff951763-b858-4398-96c3-284c2ecd7059", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.006202", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Parameter", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "2", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "Alignment": { + "_type": "int32", + "_value": "8" + }, + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "3", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Parameter", + "StorageClass": "ExportedGlobal" + } + }, + "Complexity": "real", + "Dimensions": [ + 1, + 1 + ], + "Value": 1 + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "pAlM1", + "metadata": { + "uuid": "cef41108-7d32-4a4b-88aa-2ac906315fe5", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.007650", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Parameter", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "2", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "3", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Parameter", + "StorageClass": "ExportedGlobal" + } + }, + "Complexity": "real", + "Dimensions": [ + 1, + 1 + ], + "Value": 1 + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "pNo", + "metadata": { + "uuid": "6b79f286-7cb5-4c37-bc33-72bb8614e4af", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.008218", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Parameter", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "2", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "3", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Parameter", + "StorageClass": "Auto" + } + }, + "Complexity": "real", + "Dimensions": [ + 1, + 1 + ], + "Value": 1 + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "pSC", + "metadata": { + "uuid": "f296894c-f820-4684-9d94-03561e8e3b38", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.007410", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Parameter", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "2", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "3", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Parameter", + "StorageClass": "ExportedGlobal" + } + }, + "Complexity": "real", + "Dimensions": [ + 1, + 1 + ], + "Value": 1 + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "sAl16", + "metadata": { + "uuid": "23ec5395-cc10-42b7-a493-a700ac0b6f59", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.008919", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Signal", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "3", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "Alignment": { + "_type": "int32", + "_value": "16" + }, + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "4", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Signal", + "StorageClass": "ExportedGlobal" + } + }, + "LoggingInfo": { + "_id": "2", + "_object_class": "Simulink.LoggingInfo", + "_properties": {} + } + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "sigA", + "metadata": { + "uuid": "d1b3fcc1-fc53-43d7-a3c9-a38838c7eef9", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.004163", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Signal", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "3", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "4", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Signal", + "StorageClass": "Auto" + } + }, + "LoggingInfo": { + "_id": "2", + "_object_class": "Simulink.LoggingInfo", + "_properties": {} + } + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "sigC", + "metadata": { + "uuid": "f2e39f44-f6ec-455c-a5e1-a1b3698e0c6f", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.002924", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Signal", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "3", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "4", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Signal", + "StorageClass": "Auto" + } + }, + "Complexity": "complex", + "DimensionsMode": "Variable", + "LoggingInfo": { + "_id": "2", + "_object_class": "Simulink.LoggingInfo", + "_properties": {} + } + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "sigNo", + "metadata": { + "uuid": "165a4559-8f23-4d32-99cb-d8de6f2837b8", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.004707", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Signal", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "3", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "4", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Signal", + "StorageClass": "Auto" + } + }, + "LoggingInfo": { + "_id": "2", + "_object_class": "Simulink.LoggingInfo", + "_properties": {} + } + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "sigR", + "metadata": { + "uuid": "8d072d25-781d-42f5-b017-4d772b671dfb", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.003566", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.Signal", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_id": "1", + "_properties": { + "CoderInfo": { + "_id": "3", + "_object_class": "Simulink.CoderInfo", + "_properties": { + "CSCPackageName": "Simulink", + "CustomAttributes": { + "_id": "4", + "_object_class": "SimulinkCSC.AttribClass_Simulink_Default", + "_properties": {} + }, + "CustomStorageClass": "Default", + "ParameterOrSignal": "Signal", + "StorageClass": "Auto" + } + }, + "Complexity": "real", + "DimensionsMode": "Fixed", + "LoggingInfo": { + "_id": "2", + "_object_class": "Simulink.LoggingInfo", + "_properties": {} + } + } + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "vbD", + "metadata": { + "uuid": "a2583ad4-4847-45b4-bad9-83c9830a0f3b", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.011733", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.VariantBank", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_custom_save": { + "_array_type": "Struct", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "ActiveChoiceCoderInfo": "", + "AllChoicesCoderInfo": "", + "BankCoderInfo": "", + "Description": "", + "Name": "", + "VariantConditions": { + "_array_type": "Cell", + "_dimensions": [ + 1, + 0 + ], + "_elements": [] + } + } + ], + "_fields": [ + "Name", + "Description", + "VariantConditions", + "AllChoicesCoderInfo", + "ActiveChoiceCoderInfo", + "BankCoderInfo" + ] + }, + "_id": "1" + } + ], + "_mw_element_type": "MATLABArray" + } + }, + { + "name": "vbciD", + "metadata": { + "uuid": "43c8b8f7-361e-48ba-bd2b-693a8ae93745", + "namespace": "dacaf35e-55a5-454d-a7c1-93db038a210e", + "lastmod": "20261016T191456.013688", + "modifiedby": "weiwang", + "isderived": "0" + }, + "value": { + "_array_class": "Simulink.VariantBankCoderInfo", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "_custom_save": { + "_array_type": "Struct", + "_dimensions": [ + 1, + 1 + ], + "_elements": [ + { + "DefinitionFile": "", + "HeaderFile": "", + "PostStatement": "", + "PreStatement": "", + "Qualifier": "None" + } + ], + "_fields": [ + "HeaderFile", + "DefinitionFile", + "PreStatement", + "PostStatement", + "Qualifier" + ] + }, + "_id": "1" + } + ], + "_mw_element_type": "MATLABArray" + } + } + ], + "Dictionary References": [], + "AllowAccessBWS": false + } + } + } +} \ No newline at end of file diff --git a/test-integration/suite/alignmentEdit.test.ts b/test-integration/suite/alignmentEdit.test.ts new file mode 100644 index 0000000..5b23ff1 --- /dev/null +++ b/test-integration/suite/alignmentEdit.test.ts @@ -0,0 +1,217 @@ +// Copyright 2026 The MathWorks, Inc. +// A Simulink.Parameter's or Simulink.Signal's Alignment (CoderInfo.Alignment) is edited in its +// table cell, by MATLAB's rule, and the text editor saves it the way MATLAB saves it. +// +// The ruling (2026-10-16, group E, theme 2: follow MATLAB; C-086) turned the read-only label +// into an editable text cell wherever MATLAB takes an edit, which is any StorageClass but +// 'Auto'. The rule and the stored form are data-explorer-core's (its test/alignmentEdit.test.ts +// pins them on the node). vscode adds no code for it: the generic schema cell renders the +// editor and both providers hand the column to `node.setProperty`. So this suite is the lock on +// the part only the host can show, through the real SlddTextEditorProvider: the row it paints, +// the bytes its edit writes, and the refusal it relays. +// +// The fixture is core's MATLAB-written test/fixtures/classes_r2027a_text.sldd (R2027a, made by +// make_classes_r2027a.m there), kept beside the workspace folder so findFiles never returns it. +// Its entries were set up in MATLAB: pSC is ExportedGlobal with the default Alignment, pAl8 is +// ExportedGlobal with 8, pAlM1 is ExportedGlobal set to 8 and back to -1, pNo is Auto, and +// sAl16 is a Simulink.Signal, ExportedGlobal, with 16. So what MATLAB saves is in the file +// itself: pSC set to 8 has to save pAl8's CoderInfo, and pAl8 set back to -1 has to save pSC's. +// +// Each test edits its own copy, made in a temporary directory, so no test sees another's edit. +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { SlddTextEditorProvider } from '../../src/host/SlddTextEditorProvider'; +import { chunkOf, ctx, fakePanel, rowNamed, settle, token, waitFor, ws } from './tools/hostHarness'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +const AUTO = "Cannot set Alignment when StorageClass is 'Auto'."; +const invalid = (n: number) => + `${n} is not a valid alignment boundary. The alignment boundary must be either -1 or a power of two.`; + +const FIXTURE = 'classes_r2027a_text.sldd'; + +/** An entry's `value`, as the text spells it. */ +function valueOf(text: string, name: string): any { + const entry = ((chunkOf(text).entries ?? []) as any[]).find((e) => e.name === name); + assert.ok(entry, `the text holds the entry "${name}"`); + return entry.value; +} + +/** The CoderInfo object of a Parameter's or Signal's single element. */ +const coderInfoOf = (text: string, name: string): any => valueOf(text, name)._elements[0]._properties.CoderInfo; + +/** Every entry but the ones named, as the text spells it, for "nothing else changed". */ +const othersOf = (text: string, names: string[]): any[] => + ((chunkOf(text).entries ?? []) as any[]).filter((e) => !names.includes(e.name)); + +interface Opened { + doc: vscode.TextDocument; + view: ReturnType; + rows: any[]; +} + +suite('a Parameter or Signal Alignment is edited in its table cell, as MATLAB edits it', () => { + let dir: string | undefined; + let view: ReturnType | undefined; + let copies = 0; + + suiteSetup(async () => { + await vscode.extensions.getExtension('mathworks.simulink-data-explorer')?.activate(); + }); + + teardown(() => { + view?.close(); + view = undefined; + if (dir) rmSync(dir, { recursive: true, force: true }); + dir = undefined; + }); + + /** A fresh copy of the fixture in the text editor, painted the way the webview asks for it. */ + async function open(): Promise { + dir = mkdtempSync(join(tmpdir(), 'dex-alignment-')); + // A name of its own per test: VS Code keeps a TextDocument by URI, and a copy at a path an + // earlier test edited could come back as that test's dirty document. + const uri = vscode.Uri.file(join(dir, `alignment_${++copies}.sldd`)); + await vscode.workspace.fs.copy(vscode.Uri.joinPath(ws(), '..', 'classes', FIXTURE), uri); + const doc = await vscode.workspace.openTextDocument(uri); + view = fakePanel(); + await new SlddTextEditorProvider(ctx()).resolveCustomTextEditor(doc, view.panel, token()); + view.send({ type: 'ready' }); + await waitFor('the table paint', () => view!.posts.some((p) => p.type === 'setRows' || p.type === 'error')); + const failed = view.posts.find((p) => p.type === 'error'); + assert.ok(!failed, `the tab posted an error: ${failed?.message}`); + return { doc, view, rows: view.posts.find((p) => p.type === 'setRows').rows }; + } + + /** + * One cell edit, exactly as the table sends it, and what came of it: the text after it, and + * the refusal if there was one. Waits for the edit to land in the document or be refused, + * and gives up quietly after a while, so that an edit that does neither fails on the + * assertion that says so rather than as a timeout. + */ + async function edit( + opened: Opened, + entry: string, + columnId: string, + oldValue: string, + newValue: string, + ): Promise<{ before: string; after: string; refusal: any; repaint: any }> { + const before = opened.doc.getText(); + const from = opened.view.posts.length; + const rowId = rowNamed(opened.rows, entry).ID; + opened.view.send({ type: 'edit', rowId, columnId, oldValue, newValue }); + const since = () => opened.view.posts.slice(from); + const deadline = Date.now() + 8000; + while ( + opened.doc.getText() === before && + !since().some((p) => p.type === 'validationError' || p.type === 'error') && + Date.now() < deadline + ) { + await settle(20); + } + // The repaint is posted beside the write or the refusal; give it a moment to arrive. + await settle(300); + const failed = since().find((p) => p.type === 'error'); + assert.ok(!failed, `the edit posted an error: ${failed?.message}`); + const repaints = since().filter((p) => p.type === 'updateEntryRows' && p.entryRowId === rowId); + const repaint = repaints.length ? repaints[repaints.length - 1].rows.find((r: any) => r.ID === rowId) : undefined; + return { + before, + after: opened.doc.getText(), + refusal: since().find((p) => p.type === 'validationError'), + repaint, + }; + } + + test('the cell is a text editor under any StorageClass but Auto, and plain text under Auto', async () => { + const { rows } = await open(); + for (const [name, text] of [['pSC', '-1'], ['pAl8', '8'], ['pAlM1', '-1'], ['sAl16', '16']]) { + const cell = rowNamed(rows, name).alignment; + assert.strictEqual(typeof cell, 'object', `${name}'s Alignment is an editable cell (${JSON.stringify(cell)})`); + assert.strictEqual(cell.editable, true, `${name}'s Alignment cell is editable`); + assert.strictEqual(cell.editor, 'text', `${name}'s Alignment is typed into, not chosen from a list`); + assert.strictEqual(cell.options, undefined, `${name}'s Alignment offers no list`); + assert.strictEqual(cell.text, text, `${name}'s Alignment shows the value MATLAB saved`); + } + // StorageClass Auto: MATLAB refuses every Alignment but -1, so the table offers no editor. + assert.strictEqual(rowNamed(rows, 'pNo').alignment, '-1', 'a Parameter under Auto shows -1, read-only'); + assert.strictEqual(rowNamed(rows, 'sigA').alignment, '-1', 'a Signal under Auto shows -1, read-only'); + }); + + test('8 on pSC saves the CoderInfo MATLAB saved for pAl8, and changes nothing else', async () => { + const opened = await open(); + const { before, after, refusal, repaint } = await edit(opened, 'pSC', 'alignment', '-1', '8'); + assert.strictEqual(refusal, undefined, `the edit was accepted (refused: ${refusal?.reason})`); + assert.notStrictEqual(after, before, 'the edit reached the document'); + + const info = coderInfoOf(after, 'pSC'); + assert.deepStrictEqual(info._properties.Alignment, { _type: 'int32', _value: '8' }, 'a typed int32, never a string'); + assert.strictEqual(Object.keys(info._properties)[0], 'Alignment', 'first in CoderInfo, where MATLAB puts it'); + assert.deepStrictEqual(info, coderInfoOf(before, 'pAl8'), "pAl8's CoderInfo, as MATLAB saved it"); + + // The rest of pSC's value, and every other entry, exactly as read. + const expected = valueOf(before, 'pSC'); + expected._elements[0]._properties.CoderInfo = coderInfoOf(before, 'pAl8'); + assert.deepStrictEqual(valueOf(after, 'pSC'), expected, 'nothing else in pSC changed'); + assert.deepStrictEqual(othersOf(after, ['pSC']), othersOf(before, ['pSC']), 'no other entry changed'); + + assert.ok(repaint, 'the entry was repainted'); + assert.strictEqual(repaint.alignment?.text, '8', 'and its cell shows the new Alignment'); + assert.strictEqual(repaint.alignment?.editable, true, 'still editable'); + }); + + test('-1 takes the key back out, which is how MATLAB saves the default', async () => { + const opened = await open(); + const original = opened.doc.getText(); + const set = await edit(opened, 'pSC', 'alignment', '-1', '8'); + assert.strictEqual(set.refusal, undefined, `8 was accepted (refused: ${set.refusal?.reason})`); + assert.deepStrictEqual( + coderInfoOf(set.after, 'pSC')._properties.Alignment, + { _type: 'int32', _value: '8' }, + '8 reached the document', + ); + + const back = await edit(opened, 'pSC', 'alignment', '8', '-1'); + assert.strictEqual(back.refusal, undefined, `-1 was accepted (refused: ${back.refusal?.reason})`); + assert.ok(!('Alignment' in coderInfoOf(back.after, 'pSC')._properties), 'no Alignment key at -1'); + assert.deepStrictEqual(valueOf(back.after, 'pSC'), valueOf(original, 'pSC'), 'pSC is back to what MATLAB wrote'); + assert.deepStrictEqual(othersOf(back.after, ['pSC']), othersOf(original, ['pSC']), 'no other entry changed'); + assert.strictEqual(back.repaint?.alignment?.text, '-1', 'and its cell shows -1'); + + // The same on an entry MATLAB itself saved at 8: pAl8 set to -1 saves pAlM1's CoderInfo. + const pAl8 = await edit(opened, 'pAl8', 'alignment', '8', '-1'); + assert.strictEqual(pAl8.refusal, undefined, `-1 on pAl8 was accepted (refused: ${pAl8.refusal?.reason})`); + assert.deepStrictEqual(coderInfoOf(pAl8.after, 'pAl8'), coderInfoOf(original, 'pAlM1'), "pAlM1's CoderInfo"); + }); + + test('a value MATLAB refuses is refused in its words, and the document is unchanged', async () => { + const opened = await open(); + const { before, after, refusal, repaint } = await edit(opened, 'pSC', 'alignment', '-1', '3'); + assert.ok(refusal, 'the edit was refused'); + assert.strictEqual(refusal.reason, invalid(3), "MATLAB's reason, without its inaccurate 128 bound"); + assert.strictEqual(refusal.invalidValue, '3', 'naming what was typed'); + assert.strictEqual(after, before, 'the document is unchanged'); + assert.strictEqual(repaint?.alignment?.text, '-1', 'and the cell is repainted back to -1'); + }); + + test('under StorageClass Auto an Alignment is refused, as MATLAB refuses it', async () => { + const opened = await open(); + const { before, after, refusal } = await edit(opened, 'pNo', 'alignment', '-1', '8'); + assert.ok(refusal, 'the edit was refused'); + assert.strictEqual(refusal.reason, AUTO, "MATLAB's reason"); + assert.strictEqual(after, before, 'the document is unchanged'); + }); + + test('StorageClass set to Auto resets Alignment, saving the CoderInfo MATLAB saved for pNo', async () => { + const opened = await open(); + const { before, after, refusal, repaint } = await edit(opened, 'pAl8', 'storageClass', 'ExportedGlobal', 'Auto'); + assert.strictEqual(refusal, undefined, `the edit was accepted (refused: ${refusal?.reason})`); + assert.notStrictEqual(after, before, 'the edit reached the document'); + assert.deepStrictEqual(coderInfoOf(after, 'pAl8'), coderInfoOf(before, 'pNo'), "pNo's CoderInfo: Auto, and no Alignment"); + assert.strictEqual(repaint?.alignment, '-1', 'and the cell is -1 and read-only again'); + }); +}); From 3c9dfa71357eddef5d9635efa6fccf1a4716a1ed Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 20:50:08 -0400 Subject: [PATCH 3/5] Let the table paste or drop an mpt entry into Design Data, as core now admits Core now admits a paste or a drop of an mpt.Parameter or mpt.Signal entry into Design Data (SectionNode.allowsType), while creating one stays refused (getAllowedTypes). The section rules the host posts to the table held the creation list, so the drop cursor and the Paste item refused an mpt entry the host would have accepted, and Ctrl+V, which goes through the menu's decision, never reached the host. sectionRules now posts getAdmittedTypes, and falls back to getAllowedTypes on a core that does not have it. The Add gallery keeps the creation list. Tests: the posted rules agree with allowsType for every class, the cursor and the Paste item admit an mpt entry into Design Data and refuse it elsewhere, and both paste paths keep the class. A new integration test runs the paste and the drop through both real providers. --- src/host/sectionRules.ts | 21 +- test-integration/suite/mptPasteDrop.test.ts | 254 ++++++++++++++++++++ test/addCatalog.test.ts | 19 ++ test/dropDecision.test.ts | 43 +++- test/menuItems.test.ts | 46 ++++ test/sectionRules.test.ts | 56 ++++- test/structuralEdit.test.ts | 60 +++++ test/xmlStructuralEdit.test.ts | 52 ++++ 8 files changed, 545 insertions(+), 6 deletions(-) create mode 100644 test-integration/suite/mptPasteDrop.test.ts diff --git a/src/host/sectionRules.ts b/src/host/sectionRules.ts index 9627f13..8951a00 100644 --- a/src/host/sectionRules.ts +++ b/src/host/sectionRules.ts @@ -4,9 +4,17 @@ // needs to PREDICT a drop locally (dropDecision) without a host round-trip on // every dragover: the section name, its human label, whether it is derived // (architectural), and the classes it accepts. It is posted to each webview -// alongside its rows, and mirrors SectionNode's allow-list + the section→ -// isderived mapping so the webview's prediction matches what the host paste -// would actually do. +// alongside its rows, and mirrors the section→isderived mapping and the classes +// SectionNode.allowsType admits, so the webview's prediction matches what the +// host paste would actually do. +// +// `allowedTypes` feeds only that prediction: the drop cursor and the Paste item. +// It is what a paste ADMITS (getAdmittedTypes), not what may be CREATED +// (getAllowedTypes). The two differ by the registered subclasses: a paste or drop +// of an mpt.Parameter or mpt.Signal the dictionary already holds lands in Design +// Data, but none can be created there, so the Add gallery keeps reading +// getAllowedTypes. A core without getAdmittedTypes admits exactly its creation +// list, which is the fallback. import { getSectionMetadata } from 'data-explorer-core'; export interface SectionRule { @@ -22,6 +30,11 @@ export function sectionRules(model: any): SectionRule[] { sectionName: s.name, sectionLabel: s.displayName ?? s.name, isDerived: getSectionMetadata(s.name).isderived === '1', - allowedTypes: typeof s.getAllowedTypes === 'function' ? s.getAllowedTypes() : [], + allowedTypes: + typeof s.getAdmittedTypes === 'function' + ? s.getAdmittedTypes() + : typeof s.getAllowedTypes === 'function' + ? s.getAllowedTypes() + : [], })); } diff --git a/test-integration/suite/mptPasteDrop.test.ts b/test-integration/suite/mptPasteDrop.test.ts new file mode 100644 index 0000000..6144fb5 --- /dev/null +++ b/test-integration/suite/mptPasteDrop.test.ts @@ -0,0 +1,254 @@ +// Copyright 2026 The MathWorks, Inc. +// A paste or a drop of an mpt entry, through the real providers (C-180). +// +// The ruling: a paste or a drop of an mpt.Parameter or mpt.Signal entry the dictionary already +// holds is admitted into Design Data, while creating one from scratch stays refused. Core answers +// admission with SectionNode.allowsType, which the host's paste asks. The table predicts the same +// answer locally from the `sectionRules` the provider posts: the drop cursor through dropDecision, +// and the Paste item through buildContextMenuItems. Those rules held the creation list, +// getAllowedTypes, which names no mpt class. So the table refused an entry the host would accept, +// and Ctrl+V, which goes through the menu's decision, never reached the host. +// +// So each case reads what a real provider puts on the wire: the `sectionRules` of the first paint, +// the `clipboardState` a copy broadcasts, and the `dragState` a drag broadcasts. It asks the +// table's own functions what they would show over a Design Data row and an Architectural Data row. +// Then it sends the paste and the drop, and reads the file. +// +// Both .sldd formats, because the JSON provider and the binary one are separate editors. The +// dictionary is params.sldd in both, with gravity re-classed as an mpt.Parameter and sig1 as an +// mpt.Signal. MyBus is moved into Architectural Data by its derived flag, so that section has a +// row to paste over (the two sections share one namespace, so the flag is the whole split). +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { unzipSync, zipSync, strFromU8, strToU8 } from 'fflate'; +import { SlddTextEditorProvider } from '../../src/host/SlddTextEditorProvider'; +import { BinarySlddEditorProvider } from '../../src/host/BinarySlddEditorProvider'; +import { clearClipboard } from '../../src/host/clipboard'; +import { clearDrag } from '../../src/host/dragState'; +import { dropDecision } from '../../src/webview/dropDecision'; +import { buildContextMenuItems } from '../../src/webview/menuItems'; +import { chunkOf, ctx, fakePanel, isEntryRow, nameOf, settle, token, waitFor, ws, wsUri } from './tools/hostHarness'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +const PARAM = 'gravity'; +const SIGNAL = 'sig1'; +const ARCH_ENTRY = 'MyBus'; +const REFUSED_IN_ARCH = 'Simulink Parameter cannot be in Architectural Data'; + +/** `text` with the first `from` after `marker` replaced by `to`. */ +function replaceAfter(text: string, marker: string, from: string, to: string): string { + const at = text.indexOf(marker); + assert.ok(at >= 0, `the fixture spells ${marker}`); + const hit = text.indexOf(from, at); + assert.ok(hit >= 0, `the fixture spells ${from} after ${marker}`); + return text.slice(0, hit) + to + text.slice(hit + from.length); +} + +const rowOf = (rows: any[], name: string): any => { + const row = rows.find((r) => isEntryRow(r) && nameOf(r) === name); + assert.ok(row, `the table has an entry row for "${name}"`); + return row; +}; + +const errorsIn = (posts: any[]): string[] => posts.filter((m) => m.type === 'error').map((m) => String(m.message)); + +interface Tab { + view: ReturnType; + rows: any[]; + /** The `sectionRules` message of the first paint. */ + painted: any; + /** What the document holds now: the file's text, or the binary chunk. */ + snapshot: () => string; + /** `name class isderived` for each mpt entry the document holds, sorted. */ + mpt: () => string[]; +} + +const ruleOf = (tab: Tab, sectionName: string): any => { + const rule = (tab.painted.rules as any[]).find((r) => r.sectionName === sectionName); + assert.ok(rule, `the dictionary has a ${sectionName} section`); + return rule; +}; + +/** The Paste item the table would show over `row`: table-main's menuFor, on the posted facts. */ +function pasteItemOver(tab: Tab, clip: any, row: any, sectionName: string): any { + const rule = ruleOf(tab, sectionName); + const items = buildContextMenuItems({ + rows: tab.rows, + selectedRowIds: [row.ID], + anchorRowId: row.ID, + clipboard: { canPaste: clip.canPaste, mode: clip.mode, items: clip.items }, + editable: true, + hasTextView: true, + pasteTarget: { sectionLabel: rule.sectionLabel, isDerived: rule.isDerived, allowedTypes: rule.allowedTypes }, + }); + const paste = items.find((i) => i.id === 'paste'); + assert.ok(paste, 'the menu has a Paste item'); + return paste; +} + +/** Send `msg`, and wait for the host to refuse it or to change the document. */ +async function answered(tab: Tab, msg: any): Promise { + const before = tab.snapshot(); + tab.view.posts.length = 0; + tab.view.send(msg); + await waitFor(`an answer to ${msg.type}`, () => errorsIn(tab.view.posts).length > 0 || tab.snapshot() !== before); + await settle(100); +} + +async function pasteAndDrop(tab: Tab): Promise { + const param = rowOf(tab.rows, PARAM); + const signal = rowOf(tab.rows, SIGNAL); + const archRow = rowOf(tab.rows, ARCH_ENTRY); + assert.deepStrictEqual(tab.mpt(), [`${PARAM} mpt.Parameter 0`, `${SIGNAL} mpt.Signal 0`], 'the two mpt entries'); + + // The rules the provider posted say what a paste admits: the mpt classes into design only. + for (const cls of ['mpt.Parameter', 'mpt.Signal']) { + assert.ok(ruleOf(tab, 'design').allowedTypes.includes(cls), `Design Data's posted rule admits ${cls}`); + assert.ok(!ruleOf(tab, 'arch').allowedTypes.includes(cls), `Architectural Data's posted rule does not admit ${cls}`); + } + + // Copy both. The Paste item over a design row is offered, and over an arch row it says why not. + tab.view.posts.length = 0; + tab.view.send({ type: 'copy', rowIds: [param.ID, signal.ID] }); + await waitFor('the clipboard state', () => tab.view.posts.some((m) => m.type === 'clipboardState' && m.items?.length === 2)); + const clip = tab.view.posts.filter((m) => m.type === 'clipboardState').pop(); + const overDesign = pasteItemOver(tab, clip, param, 'design'); + assert.ok(!overDesign.disabled, `Paste over a design row is offered (reason: ${overDesign.reason})`); + const overArch = pasteItemOver(tab, clip, archRow, 'arch'); + assert.strictEqual(overArch.disabled, true, 'Paste over an arch row is not offered'); + assert.strictEqual(overArch.reason, REFUSED_IN_ARCH); + + // The paste the menu now offers lands, and each copy keeps its class. + await answered(tab, { type: 'paste', rowId: param.ID }); + assert.deepStrictEqual(errorsIn(tab.view.posts), [], 'the paste into design is applied'); + const afterPaste = tab.mpt(); + assert.strictEqual(afterPaste.length, 4, `two mpt copies were added: ${afterPaste.join(', ')}`); + assert.deepStrictEqual( + afterPaste.map((e) => e.split(' ').slice(1).join(' ')).sort(), + ['mpt.Parameter 0', 'mpt.Parameter 0', 'mpt.Signal 0', 'mpt.Signal 0'], + 'both copies are in design, with their classes', + ); + + // The host still refuses the same paste over the arch row, which the menu does not offer. + const beforeArch = tab.snapshot(); + await answered(tab, { type: 'paste', rowId: archRow.ID }); + const refusal = errorsIn(tab.view.posts); + assert.strictEqual(refusal.length, 1, `one refusal: ${refusal.join(' | ')}`); + assert.ok( + refusal[0].includes('A "mpt.Parameter" entry is not allowed in Architectural Data.'), + `the refusal names the class and the section: ${refusal[0]}`, + ); + assert.strictEqual(tab.snapshot(), beforeArch, 'and the document is untouched'); + + // A drag of the same two rows: the cursor allows a copy onto design, and refuses arch. + tab.view.posts.length = 0; + tab.view.send({ type: 'dragStart', rowIds: [param.ID, signal.ID] }); + await waitFor('the drag descriptor', () => tab.view.posts.some((m) => m.type === 'dragState' && m.descriptor)); + const drag = tab.view.posts.filter((m) => m.type === 'dragState').pop().descriptor; + const ontoDesign = dropDecision(drag, { docUri: tab.painted.docUri, ...ruleOf(tab, 'design') }, 'copy'); + assert.strictEqual(ontoDesign.canDrop, true, `a copy onto design is allowed (${ontoDesign.tooltip})`); + const ontoArch = dropDecision(drag, { docUri: tab.painted.docUri, ...ruleOf(tab, 'arch') }, 'copy'); + assert.strictEqual(ontoArch.canDrop, false, 'a copy onto arch is refused'); + assert.strictEqual(ontoArch.tooltip, REFUSED_IN_ARCH); + + // And the drop the cursor allows lands too. + await answered(tab, { type: 'drop', rowId: 'section:design', mode: 'copy' }); + assert.deepStrictEqual(errorsIn(tab.view.posts), [], 'the drop onto design is applied'); + const afterDrop = tab.mpt(); + assert.strictEqual(afterDrop.length, 6, `two more mpt copies were added: ${afterDrop.join(', ')}`); + assert.ok(afterDrop.every((e) => e.endsWith(' 0')), 'every copy is in design'); +} + +/** The params.sldd text with the two entries re-classed and MyBus moved into arch. */ +function mptJson(original: string): string { + let text = replaceAfter(original, `"name": "${PARAM}"`, '"Simulink.Parameter"', '"mpt.Parameter"'); + text = replaceAfter(text, `"name": "${SIGNAL}"`, '"Simulink.Signal"', '"mpt.Signal"'); + return replaceAfter(text, `"name": "${ARCH_ENTRY}"`, '"isderived": "0"', '"isderived": "1"'); +} + +suite('A paste and a drop of an mpt entry, on a JSON .sldd', () => { + test('are offered and applied in Design Data, and refused in Architectural Data', async () => { + const original = strFromU8(await vscode.workspace.fs.readFile(wsUri('params.sldd'))); + const uri = wsUri('mpt_paste_drop.sldd'); + await vscode.workspace.fs.writeFile(uri, strToU8(mptJson(original))); + const view = fakePanel(); + try { + const doc = await vscode.workspace.openTextDocument(uri); + await new SlddTextEditorProvider(ctx()).resolveCustomTextEditor(doc, view.panel, token()); + view.send({ type: 'ready' }); + await waitFor('the first paint', () => view.posts.some((m) => m.type === 'sectionRules')); + await pasteAndDrop({ + view, + rows: view.posts.filter((m) => m.type === 'setRows').pop().rows as any[], + painted: view.posts.filter((m) => m.type === 'sectionRules').pop(), + snapshot: () => doc.getText(), + mpt: () => + (chunkOf(doc.getText()).entries as any[]) + .filter((e) => String(e.value?._array_class ?? '').startsWith('mpt.')) + .map((e) => `${e.name} ${e.value._array_class} ${e.metadata?.isderived}`) + .sort(), + }); + } finally { + view.close(); + clearClipboard(); + clearDrag(); + await vscode.workspace.fs.delete(uri); + } + }).timeout(60000); +}); + +/** `name class isderived` for each mpt entry of a binary .sldd's data/chunk0.xml, sorted. */ +function mptOfXml(xml: string): string[] { + const out: string[] = []; + for (const obj of xml.split('').slice(1)) { + const name = /

([^<]*)<\/P>/.exec(obj)?.[1]; + const derived = /

([^<]*)<\/P>/.exec(obj)?.[1] ?? '0'; + const cls = //.exec(obj)?.[1] ?? ''; + if (cls.startsWith('mpt.')) out.push(`${name} ${cls} ${derived}`); + } + return out.sort(); +} + +suite('A paste and a drop of an mpt entry, on a compressed-binary .sldd', () => { + test('are offered and applied in Design Data, and refused in Architectural Data', async () => { + // The real MATLAB-written file, with only the chunk changed: a paste inserts before the + // chunk's trailing `DD.Dictionary` object, which the hand-written binary fixtures lack. + const binary = vscode.Uri.joinPath(ws(), '..', '..', '..', 'test', 'parity', 'artifacts', 'binary', 'params.sldd'); + const parts = unzipSync(await vscode.workspace.fs.readFile(binary)); + let xml = strFromU8(parts['data/chunk0.xml']); + xml = replaceAfter(xml, `

${PARAM}

`, '', ''); + xml = replaceAfter(xml, `

${SIGNAL}

`, '', ''); + xml = replaceAfter( + xml, + `

${ARCH_ENTRY}

`, + '

0

', + '

1

', + ); + parts['data/chunk0.xml'] = strToU8(xml); + const uri = vscode.Uri.joinPath(ws(), '..', 'mpt_paste_drop_bin.sldd'); + await vscode.workspace.fs.writeFile(uri, zipSync(parts)); + + const provider = new BinarySlddEditorProvider(ctx()); + const doc: any = await provider.openCustomDocument(uri, {} as vscode.CustomDocumentOpenContext, token()); + const view = fakePanel(); + try { + await provider.resolveCustomEditor(doc, view.panel, token()); + view.send({ type: 'ready' }); + await waitFor('the first paint', () => view.posts.some((m) => m.type === 'sectionRules')); + await pasteAndDrop({ + view, + rows: view.posts.filter((m) => m.type === 'setRows').pop().rows as any[], + painted: view.posts.filter((m) => m.type === 'sectionRules').pop(), + snapshot: () => doc.chunkXml as string, + mpt: () => mptOfXml(doc.chunkXml as string), + }); + } finally { + view.close(); + doc.dispose(); + clearClipboard(); + clearDrag(); + await vscode.workspace.fs.delete(uri); + } + }).timeout(60000); +}); diff --git a/test/addCatalog.test.ts b/test/addCatalog.test.ts index 8a79f96..0efac4e 100644 --- a/test/addCatalog.test.ts +++ b/test/addCatalog.test.ts @@ -196,6 +196,25 @@ describe('the catalog partitions what core allows', () => { it('never offers anything in Other Data', () => { expect(allTiles().filter((t) => (t.section as string) === 'other')).toEqual([]); }); + + // C-180. Core admits a paste of an mpt.Parameter or mpt.Signal into Design Data + // (allowsType, getAdmittedTypes) but refuses to create one (getAllowedTypes, which + // corePairs reads). A tile is a creation, so no tile may name a class that a section + // admits only to a paste. The set is pinned, so a class core starts admitting this way + // is a decision about the gallery here too. + it('offers no class a section admits only to a paste, such as an mpt class', () => { + const sections = sectionsOf(); + const pasteOnly = new Set(); + for (const key of SECTION_KEYS) { + const allowed = new Set(sections[key].getAllowedTypes()); + for (const className of sections[key].getAdmittedTypes?.() ?? []) { + if (!allowed.has(className)) pasteOnly.add(pairKey(className, key)); + } + } + expect([...pasteOnly].sort()).toEqual([pairKey('mpt.Parameter', 'design'), pairKey('mpt.Signal', 'design')]); + expect(allTiles().filter((t) => pasteOnly.has(pairKey(t.className, t.section))).map(tileName)).toEqual([]); + expect(allTiles().filter((t) => t.className.startsWith('mpt.')).map(tileName)).toEqual([]); + }); }); describe('how the catalog states a destination', () => { diff --git a/test/dropDecision.test.ts b/test/dropDecision.test.ts index 91f7273..b22b5ef 100644 --- a/test/dropDecision.test.ts +++ b/test/dropDecision.test.ts @@ -7,7 +7,11 @@ // mirror the Kind an entry shows AFTER the paste (a pasted entry loses its // SystemComposer classification, so a derived Bus becomes a "Data Interface"). import { describe, it, expect } from 'vitest'; -import { dropDecision } from '../src/webview/dropDecision.js'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dropDecision, rejectReason } from '../src/webview/dropDecision.js'; +import { getModel, invalidate } from '../src/host/SlddModel.js'; +import { sectionRules } from '../src/host/sectionRules.js'; // Allow-lists mirror SectionNode.ALLOWED_TYPES (only the classes these tests use). // 'MatlabVariable' and 'Constant' are in the real lists too, and they matter here: @@ -340,3 +344,40 @@ describe('dropDecision — a target with no allow-list is unrestricted', () => { expect(d.tooltip).toBe('MATLAB Variable must be scalar and numeric to be a Constant'); }); }); + +// C-180. Core admits a paste of an mpt.Parameter or mpt.Signal entry into Design Data, and +// nowhere else. The targets here are built by sectionRules from a real model, not from a list +// written in this file, so they say what the host's paste would say. +describe('dropDecision — an mpt entry, against the rules a real model posts', () => { + const archText = readFileSync(fileURLToPath(new URL('./fixtures/arch.sldd', import.meta.url)), 'utf8'); + function realTarget(sectionName: string) { + const uri = 'test://drop-mpt.sldd'; + invalidate(uri); + const rule = sectionRules(getModel(uri, 'arch.sldd', archText)).find((r) => r.sectionName === sectionName)!; + return { docUri: 'a.sldd', ...rule }; + } + // The facts a drag of a MATLAB-written mpt entry carries: its node is a ParameterNode or + // SignalNode, so the Kind is the superclass's. + const mptParam = () => ({ ...param(), className: 'mpt.Parameter', arrayClass: 'mpt.Parameter' }); + const mptSignal = () => ({ + className: 'mpt.Signal', + arrayClass: 'mpt.Signal', + kind: 'Simulink Signal', + isMatlabVariable: false, + isScalarNumeric: false, + }); + + it('lets an mpt.Parameter or mpt.Signal into Design Data', () => { + expect(rejectReason(realTarget('design'), mptParam())).toBeNull(); + expect(rejectReason(realTarget('design'), mptSignal())).toBeNull(); + const d = dropDecision(designSource([mptParam(), mptSignal()], 'b.sldd'), realTarget('design'), 'copy'); + expect(d.canDrop).toBe(true); + expect(d.tooltip).toBe('Copy 2 items'); + }); + + it('still refuses them in Architectural Data and Configurations', () => { + expect(rejectReason(realTarget('arch'), mptParam())).toBe('Simulink Parameter cannot be in Architectural Data'); + expect(rejectReason(realTarget('arch'), mptSignal())).toBe('Simulink Signal cannot be in Architectural Data'); + expect(rejectReason(realTarget('config'), mptParam())).toBe('Simulink Parameter cannot be in Configurations'); + }); +}); diff --git a/test/menuItems.test.ts b/test/menuItems.test.ts index b4d1ced..6d6f2b0 100644 --- a/test/menuItems.test.ts +++ b/test/menuItems.test.ts @@ -2,6 +2,10 @@ import { describe, it, expect, afterEach } from 'vitest'; import { buildContextMenuItems, shouldShowContextMenu, shouldOpenCellEditor, resolveShortcutAction, type MenuRow, type ClipboardState, type MenuInput, type PasteTarget } from '../src/webview/menuItems.js'; import type { ContextMenuItem } from '../src/webview/components/dex-context-menu.js'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { getModel, invalidate } from '../src/host/SlddModel.js'; +import { sectionRules } from '../src/host/sectionRules.js'; // The rows a menu is built over: one section holding an entry A with a child x, plus a // second entry B, plus a second section holding V. Enough to state every shape the spec @@ -421,3 +425,45 @@ describe('document-level readonly gates (shouldShowContextMenu / shouldOpenCellE expect(shouldOpenCellEditor(false)).toBe(false); }); }); + +// C-180. Core admits a paste of an mpt.Parameter or mpt.Signal entry into Design Data. The +// Paste item asks the target section's posted rules, so it is built here the way table-main +// builds it, from what sectionRules posts for a real model. +describe('Paste of an mpt entry, against the rules a real model posts', () => { + const archText = readFileSync(fileURLToPath(new URL('./fixtures/arch.sldd', import.meta.url)), 'utf8'); + function realPasteTarget(sectionName: string): PasteTarget { + const uri = 'test://menu-mpt.sldd'; + invalidate(uri); + const rule = sectionRules(getModel(uri, 'arch.sldd', archText)).find((r) => r.sectionName === sectionName)!; + return { sectionLabel: rule.sectionLabel, isDerived: rule.isDerived, allowedTypes: rule.allowedTypes }; + } + const MPT_CLIP: ClipboardState = { + canPaste: true, + mode: 'copy', + items: [ + { className: 'mpt.Parameter', arrayClass: 'mpt.Parameter', kind: 'Simulink Parameter', isMatlabVariable: false, isScalarNumeric: false }, + { className: 'mpt.Signal', arrayClass: 'mpt.Signal', kind: 'Simulink Signal', isMatlabVariable: false, isScalarNumeric: false }, + ], + }; + + it('offers Paste over a Design Data row', () => { + const paste = byId(menu({ clipboard: MPT_CLIP, pasteTarget: realPasteTarget('design') }), 'paste')!; + expect(paste.label).toBe('Paste 2 Entries'); + expect(paste.disabled).toBeFalsy(); + expect(paste.reason).toBeUndefined(); + }); + + it('still refuses it over an Architectural Data row, and says why', () => { + const paste = byId( + menu({ + clipboard: MPT_CLIP, + selectedRowIds: ['u/arch/V'], + anchorRowId: 'u/arch/V', + pasteTarget: realPasteTarget('arch'), + }), + 'paste', + )!; + expect(paste.disabled).toBe(true); + expect(paste.reason).toBe('Simulink Parameter cannot be in Architectural Data'); + }); +}); diff --git a/test/sectionRules.test.ts b/test/sectionRules.test.ts index c4887d5..09d7134 100644 --- a/test/sectionRules.test.ts +++ b/test/sectionRules.test.ts @@ -27,7 +27,7 @@ describe('sectionRules', () => { expect(design!.isDerived).toBe(false); expect(arch!.isDerived).toBe(true); - // Allow-lists mirror SectionNode.ALLOWED_TYPES. + // Allow-lists hold what each section admits to a paste (SectionNode.allowsType). expect(design!.allowedTypes).toContain('Simulink.Parameter'); expect(arch!.allowedTypes).not.toContain('Simulink.Parameter'); expect(arch!.allowedTypes).toContain('Simulink.ServiceBus'); @@ -37,6 +37,60 @@ describe('sectionRules', () => { expect(arch!.sectionLabel).toBe('Architectural Data'); }); + // C-180. Core admits a paste or drop of an mpt.Parameter or mpt.Signal entry into Design + // Data (SectionNode.allowsType, which the host's paste checks), while creating one stays + // refused (getAllowedTypes, which the Add gallery reads). These lists feed the drop cursor + // and the Paste item, so they have to say what a paste admits. Posting the creation list + // made both refuse an mpt entry the host would have accepted. + it('lists the mpt classes Design Data admits to a paste, and no other section does', () => { + const uri = 'test://rules-mpt.sldd'; + invalidate(uri); + const rules = sectionRules(getModel(uri, 'arch.sldd', archText)); + const listOf = (name: string) => rules.find((r) => r.sectionName === name)!.allowedTypes; + + expect(listOf('design')).toContain('mpt.Parameter'); + expect(listOf('design')).toContain('mpt.Signal'); + for (const name of ['arch', 'config', 'other']) { + expect(listOf(name), name).not.toContain('mpt.Parameter'); + expect(listOf(name), name).not.toContain('mpt.Signal'); + } + }); + + it('agrees with the section’s own allowsType on every class any section names', () => { + // The prediction is only right if it is the host's answer. allowsType is what the + // host's paste asks, so for each section and each class, being on the posted list + // must be the same as allowsType saying yes. All four lists are non-empty, so the + // "empty list means no restriction" rule never stands in for an answer here. + const uri = 'test://rules-agree.sldd'; + invalidate(uri); + const model = getModel(uri, 'arch.sldd', archText); + const rules = sectionRules(model); + const sections = model.children as any[]; + const classes = new Set(['mpt.Parameter', 'mpt.Signal', 'NoSuch.Class']); + for (const s of sections) for (const c of s.getAllowedTypes()) classes.add(c); + + for (const s of sections) { + const listed = rules.find((r) => r.sectionName === s.name)!.allowedTypes; + expect(listed.length, s.name).toBeGreaterThan(0); + for (const c of classes) { + expect(listed.includes(c), `${s.name}: ${c}`).toBe(s.allowsType(c)); + } + } + }); + + it('prefers the admitted list to the creation list when a section has both', () => { + const rules = sectionRules({ + children: [{ name: 'design', getAllowedTypes: () => ['A'], getAdmittedTypes: () => ['A', 'B'] }], + }); + expect(rules[0].allowedTypes).toEqual(['A', 'B']); + }); + + it('falls back to the creation list on a core whose sections have no admitted list', () => { + // A core older than C-180 has getAllowedTypes only, and a paste there admits exactly it. + const rules = sectionRules({ children: [{ name: 'design', getAllowedTypes: () => ['A'] }] }); + expect(rules[0].allowedTypes).toEqual(['A']); + }); + it('covers all four .sldd sections so no section is left unpredictable', () => { // sectionRuleForRow returns null for a section with no rule, and predictDrop // then bails — dragging onto that section would silently fall back to the diff --git a/test/structuralEdit.test.ts b/test/structuralEdit.test.ts index aa28d45..3b1b20d 100644 --- a/test/structuralEdit.test.ts +++ b/test/structuralEdit.test.ts @@ -418,6 +418,66 @@ describe('pasteEntry gates a classless payload against the allow-list too', () = }); }); +// C-180. A paste copies an mpt object MATLAB already wrote, so core admits an mpt.Parameter +// or mpt.Signal into Design Data, while creating one from scratch stays refused. The source is +// params.sldd's MATLAB-written gravity and sig1, re-classed in the text, so the payload is what +// a copy of a real mpt entry carries. +describe('pasteEntry admits an mpt entry into Design Data only', () => { + const paramsText = readFileSync( + fileURLToPath(new URL('../test-integration/fixtures/workspace/params.sldd', import.meta.url)), + 'utf8', + ); + /** `text` with entry `name`'s first `_array_class` changed from `from` to `to`. */ + function reclass(text: string, name: string, from: string, to: string): string { + const at = text.indexOf(`"name": "${name}"`); + const cls = text.indexOf(`"_array_class": "${from}"`, at); + if (at < 0 || cls < at) throw new Error(`params.sldd spells no ${from} for ${name}`); + return text.slice(0, cls) + `"_array_class": "${to}"` + text.slice(cls + `"_array_class": "${from}"`.length); + } + const mptText = reclass( + reclass(paramsText, 'gravity', 'Simulink.Parameter', 'mpt.Parameter'), + 'sig1', + 'Simulink.Signal', + 'mpt.Signal', + ); + const load = (uri: string, text = mptText) => { + invalidate(uri); + return getModel(uri, 'params.sldd', text); + }; + const entryOf = (model: any, name: string) => + (model.children as any[]).flatMap((s: any) => s.children as any[]).find((e: any) => e.name === name); + + for (const [name, cls] of [['gravity', 'mpt.Parameter'], ['sig1', 'mpt.Signal']] as const) { + it(`pastes a copy of an ${cls} into Design Data, and the copy keeps its class`, () => { + const uri = `test://paste-${cls}.sldd`; + const model = load(uri); + const payload = entryOf(model, name).serialize() as Record; + expect(payload.value._array_class).toBe(cls); + + const { newText } = pasteEntry(mptText, model.getSection('design'), payload); + expect(isValidJson(newText)).toBe(true); + expect(newText.split(`"_array_class": "${cls}"`).length - 1, 'the copy writes the class').toBe(2); + + // Reopened, the one new entry is in Design Data, with the class it was copied with. + const before = new Set(rowNames(`${uri}#before`, mptText)); + const added = rowNames(`${uri}#after`, newText).filter((n) => !before.has(n)); + expect(added).toHaveLength(1); + const copy = entryOf(load(`${uri}#reread`, newText), added[0]); + expect(copy.className).toBe(cls); + expect(copy.parent.name).toBe('design'); + expect(siblingByteIdentical(mptText, newText, name)).toBe(true); + }); + + it(`still refuses a copy of an ${cls} in Architectural Data`, () => { + const model = load(`test://paste-${cls}-arch.sldd`); + const payload = entryOf(model, name).serialize() as Record; + expect(() => pasteEntry(mptText, model.getSection('arch'), payload)).toThrow( + `A "${cls}" entry is not allowed in Architectural Data.`, + ); + }); + } +}); + describe('deleteChild / addChild on a node that does not support it', () => { it('refuses to add a child to a scalar with an actionable message', () => { const uri = 'test://add-child-scalar-msg.sldd'; diff --git a/test/xmlStructuralEdit.test.ts b/test/xmlStructuralEdit.test.ts index 3973375..c48486e 100644 --- a/test/xmlStructuralEdit.test.ts +++ b/test/xmlStructuralEdit.test.ts @@ -367,3 +367,55 @@ describe('pasteEntryXml rebinds the pasted entry to the target section', () => { ); }); }); + +// C-180, the binary twin of structuralEdit.test.ts's mpt paste. A paste copies an mpt object +// MATLAB already wrote, so core admits an mpt.Parameter or mpt.Signal into Design Data, and +// nowhere else. The source is params.sldd's MATLAB-written gravity and sig1, re-classed in the +// chunk, so the payload is what a copy of a real mpt entry carries. +describe('pasteEntryXml admits an mpt entry into Design Data only', () => { + /** `xml` with entry `name`'s first `` changed to `to`. */ + function reclass(xml: string, name: string, from: string, to: string): string { + const at = xml.indexOf(`

${name}

`); + const tag = ``; + const el = xml.indexOf(tag, at); + if (at < 0 || el < at) throw new Error(`params.sldd spells no ${from} for ${name}`); + return xml.slice(0, el) + `` + xml.slice(el + tag.length); + } + const mptXml = reclass( + reclass(load('mem://xse-mpt-src').xml, 'gravity', 'Simulink.Parameter', 'mpt.Parameter'), + 'sig1', + 'Simulink.Signal', + 'mpt.Signal', + ); + const entryOf = (model: any, name: string) => + (model.children as any[]).flatMap((s: any) => s.children as any[]).find((e: any) => e.name === name); + + for (const [name, cls] of [['gravity', 'mpt.Parameter'], ['sig1', 'mpt.Signal']] as const) { + it(`pastes a copy of an ${cls} into Design Data, and the copy keeps its class`, () => { + const model = reparseModel(`mem://xse-mpt-${cls}`, mptXml); + const payload = entryOf(model, name).serialize() as Record; + expect(payload.value._array_class).toBe(cls); + + const { newText } = pasteEntryXml(mptXml, model.getSection('design'), payload); + expect(newText.split(``).length - 1, 'the copy writes the class').toBe(2); + + // Reopened, the one new entry is in Design Data, with the class it was copied with. + const before = new Set(entryNames(reparseModel(`mem://xse-mpt-${cls}-before`, mptXml))); + const reread = reparseModel(`mem://xse-mpt-${cls}-after`, newText); + const added = entryNames(reread).filter((n) => !before.has(n)); + expect(added).toHaveLength(1); + const copy = entryOf(reread, added[0]); + expect(copy.className).toBe(cls); + expect(copy.parent.name).toBe('design'); + expect(siblingIdentical(mptXml, newText, name)).toBe(true); + }); + + it(`still refuses a copy of an ${cls} in Architectural Data`, () => { + const model = reparseModel(`mem://xse-mpt-${cls}-arch`, mptXml); + const payload = entryOf(model, name).serialize() as Record; + expect(() => pasteEntryXml(mptXml, model.getSection('arch'), payload)).toThrow( + `A "${cls}" entry is not allowed in Architectural Data.`, + ); + }); + } +}); From 2bc4a31a12aa2b5b4e201c222982f9149f8fcd31 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 21:10:21 -0400 Subject: [PATCH 4/5] Stop a cell edit on one of two same-named entries landing on the first Core now refuses to resolve a node id that names two nodes: two entries with one name in one section, and every row inside them (F1-50, group E ruling of 2026-10-16). SlddModel.findNode fell back to its cached tree whenever the session answered null, and for the text editor that tree is the session's own, so an edit on either twin's row edited the first one. findNode now takes the session's null as final for a source the session holds. It still falls back to the cache for a core without findNodeById and for a source the session no longer holds. The parse-warning banner no longer counts core's new duplicate-id warning as a part that could not be read. When shared names are all there is, the headline says so; core's messages are listed below it as before. The forward-order undo test in binaryEntryScopedEdit now expects the refusal: the remove after the insert throws, rather than removing whichever twin was indexed last, and both entries are left standing. Tests: findNodeSharedName (findNode on a text dictionary with two K Parameters and two S structs), three banner cases in parseWarnings, and sharedNameEntries through the real SlddTextEditorProvider. --- src/host/SlddModel.ts | 29 ++-- src/host/parseWarnings.ts | 26 ++- .../suite/sharedNameEntries.test.ts | 148 ++++++++++++++++++ test/binaryEntryScopedEdit.test.ts | 14 +- test/findNodeSharedName.test.ts | 134 ++++++++++++++++ test/parseWarnings.test.ts | 38 +++++ 6 files changed, 370 insertions(+), 19 deletions(-) create mode 100644 test-integration/suite/sharedNameEntries.test.ts create mode 100644 test/findNodeSharedName.test.ts diff --git a/src/host/SlddModel.ts b/src/host/SlddModel.ts index cdefd03..484737d 100644 --- a/src/host/SlddModel.ts +++ b/src/host/SlddModel.ts @@ -232,16 +232,27 @@ export function findNode(uriString: string, nodeId: string): any | null { // model in DataModel under a prefixed srcId but NOT in this module's `cache`, // so gating on `cache.get(uriString)` here would wrongly drop their selections // (the Property Inspector would never render). Try the registry unconditionally. - // PRECONDITION (untested) for the `: null` arm: the pinned data-explorer-core - // always exports findNodeById on DataModel. The feature-detect exists because - // core is a git-pinned dependency bumped independently of this repo, so an older - // pin must degrade to the cache fallback below rather than throw on every - // selection; the `try` covers the same risk for a throwing implementation. + // The pinned data-explorer-core always exports findNodeById on DataModel. The + // feature-detect exists because core is a git-pinned dependency bumped + // independently of this repo, so an older pin must degrade to the cache fallback + // below rather than throw on every selection (test/findNodeSharedName.test.ts); + // the `try` covers the same risk for a throwing implementation. + // + // A null from the registry is FINAL when the registry holds this uri's source. Core + // answers null for an id that names two nodes (F1-50: two same-named entries in one + // section, and every row inside them), because it cannot tell which one a row meant. + // Every adder here registers under the uri and caches the very tree it registered, so + // flattening the cache would find the FIRST twin, the wrong-entry pick core refuses, + // and a cell edit, a child delete, a copy, a drag and the Property Inspector all act + // on what this returns. So the cache answers only what the registry cannot: an old + // pin without findNodeById, a lookup that threw, or a source it no longer holds. try { - const viaRegistry = (DataModel as any).findNodeById - ? (DataModel as any).findNodeById(nodeId) - : null; - if (viaRegistry) return viaRegistry; + const session = DataModel as any; + if (typeof session.findNodeById === 'function') { + const viaRegistry = session.findNodeById(nodeId); + if (viaRegistry) return viaRegistry; + if (session.hasDataSource?.(uriString)) return null; + } } catch { /* fall through */ } diff --git a/src/host/parseWarnings.ts b/src/host/parseWarnings.ts index cc41048..53cbeb8 100644 --- a/src/host/parseWarnings.ts +++ b/src/host/parseWarnings.ts @@ -33,6 +33,17 @@ import type { ParseWarning } from 'data-explorer-core'; /** The `code`s that describe the whole source rather than one piece of it. */ const SOURCE_LEVEL = new Set(['source-empty', 'source-unreadable']); +/** + * The `code` core's session adds at open for siblings that share a name, and so one node + * id (F1-50). Nothing was lost: both are read and both are shown, but neither can be edited + * or deleted by id until the file names them apart. So it is never counted as a part that + * could not be read, and never a reason to refuse the file. + */ +const SHARED_NAME = 'duplicate-id'; + +/** The headline when the only thing to say is that some names are shared. */ +const SHARED_NAME_HEADLINE = 'Some items in this file share a name, so they cannot be edited or deleted here.'; + /** * How many individual messages the banner spells out. A `.mat` with a truncated * record tail can report a warning per variable, and a banner tall enough to hold @@ -91,8 +102,10 @@ export function sourceWarnings(node: unknown): ParseWarning[] { * * The headline is a source-level warning's own message when there is one, because * core writes those as a whole sentence about the whole file ("…so this project - * reads as empty") and that is the larger fact; otherwise it counts the parts. The - * details are core's messages verbatim — each already names what it lost, so the + * reads as empty") and that is the larger fact; otherwise it counts the parts that + * could not be read. A shared name (`duplicate-id`) is not one of those: it gets a + * headline of its own when it is all there is, and otherwise only a line below. The + * details are core's messages verbatim — each already names what it is about, so the * `part` field is used to tell two warnings apart here and never rendered. */ export function warningBanner(warnings: ParseWarning[] | undefined): WarningBanner | undefined { @@ -115,11 +128,14 @@ export function warningBanner(warnings: ParseWarning[] | undefined): WarningBann const sourceLevel = unique.find((w) => SOURCE_LEVEL.has(w.code)); const rest = unique.filter((w) => w !== sourceLevel); + const lost = rest.filter((w) => w.code !== SHARED_NAME).length; const headline = sourceLevel ? sourceLevel.message - : rest.length === 1 - ? 'One part of this file could not be read, so what you see below is incomplete.' - : `${rest.length} parts of this file could not be read, so what you see below is incomplete.`; + : lost === 0 + ? SHARED_NAME_HEADLINE + : lost === 1 + ? 'One part of this file could not be read, so what you see below is incomplete.' + : `${lost} parts of this file could not be read, so what you see below is incomplete.`; const details = rest.slice(0, MAX_BANNER_DETAILS).map((w) => w.message); const hidden = rest.length - details.length; diff --git a/test-integration/suite/sharedNameEntries.test.ts b/test-integration/suite/sharedNameEntries.test.ts new file mode 100644 index 0000000..ac42c3a --- /dev/null +++ b/test-integration/suite/sharedNameEntries.test.ts @@ -0,0 +1,148 @@ +// Copyright 2026 The MathWorks, Inc. +// Two entries with one name in one section, through the real text editor (F1-50). +// +// The ruling (2026-10-16, group E): core refuses to edit or delete by id while an id names two +// nodes, and warns at open. A node id is a path, so two same-named entries in one section are one +// id, and the table cannot tell which of the two rows was clicked. Only a malformed file holds +// such a pair. The file still opens, and both entries are on screen. +// +// What the host owes the ruling, and what this suite reads off a real SlddTextEditorProvider: +// - the banner says the names are shared. It does not count the twins as parts that could not +// be read, since nothing was lost; +// - a cell edit on a twin is refused and the file is untouched. SlddModel.findNode used to fall +// back to its cached tree when the session answered null, and for this editor that tree IS +// the session's, so it handed the edit to the FIRST twin whichever row was clicked; +// - an entry with a name of its own still edits as before. +// +// The fixture is the workspace's params.sldd with a second `gravity` (a Simulink.Parameter, with +// its own uuid) right after the first. Each test edits its own copy in a temporary directory. +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { strFromU8, strToU8 } from 'fflate'; +import { SlddTextEditorProvider } from '../../src/host/SlddTextEditorProvider'; +import { chunkOf, ctx, fakePanel, isEntryRow, nameOf, rowNamed, settle, token, waitFor, wsUri } from './tools/hostHarness'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +const TWIN = 'gravity'; +const SHARED = 'Some items in this file share a name, so they cannot be edited or deleted here.'; +const CORE_SAYS = + "Two entries in Design Data are named 'gravity'. " + + 'Neither can be edited or deleted until the file gives one of them another name.'; + +/** params.sldd with a second `gravity` right after the first, under a uuid of its own. */ +function withTwin(original: string): string { + const doc = JSON.parse(original); + const entries: any[] = doc.__MW_TEXT_PARTS__['__MW_TEXT_PART__/data/chunk0'].__MW_TEXT_content.entries; + const at = entries.findIndex((e) => e.name === TWIN); + assert.ok(at >= 0, `the fixture holds ${TWIN}`); + const twin = JSON.parse(JSON.stringify(entries[at])); + twin.metadata.uuid = 'f1-50-second-gravity'; + entries.splice(at + 1, 0, twin); + return JSON.stringify(doc, null, '\t'); +} + +/** `uuid Value` for each entry the text holds under `name`, in file order. */ +const valuesOf = (text: string, name: string): string[] => + ((chunkOf(text).entries ?? []) as any[]) + .filter((e) => e.name === name) + .map((e) => `${e.metadata?.uuid} ${JSON.stringify(e.value?._elements?.[0]?._properties?.Value ?? e.value)}`); + +const errorsIn = (posts: any[]): string[] => posts.filter((m) => m.type === 'error').map((m) => String(m.message)); + +interface Opened { + doc: vscode.TextDocument; + view: ReturnType; + paint: any; +} + +suite('two entries with one name in one section, in the text editor', () => { + let dir: string | undefined; + let view: ReturnType | undefined; + let copies = 0; + + suiteSetup(async () => { + await vscode.extensions.getExtension('mathworks.simulink-data-explorer')?.activate(); + }); + + teardown(() => { + view?.close(); + view = undefined; + if (dir) rmSync(dir, { recursive: true, force: true }); + dir = undefined; + }); + + async function open(): Promise { + const original = strFromU8(await vscode.workspace.fs.readFile(wsUri('params.sldd'))); + dir = mkdtempSync(join(tmpdir(), 'dex-shared-name-')); + // A name of its own per test, so VS Code cannot hand back an earlier test's document. + const uri = vscode.Uri.file(join(dir, `shared_name_${++copies}.sldd`)); + await vscode.workspace.fs.writeFile(uri, strToU8(withTwin(original))); + const doc = await vscode.workspace.openTextDocument(uri); + view = fakePanel(); + await new SlddTextEditorProvider(ctx()).resolveCustomTextEditor(doc, view.panel, token()); + view.send({ type: 'ready' }); + await waitFor('the table paint', () => view!.posts.some((p) => p.type === 'setRows' || p.type === 'error')); + assert.deepStrictEqual(errorsIn(view.posts), [], 'the file opens'); + return { doc, view, paint: view.posts.find((p) => p.type === 'setRows') }; + } + + /** Send `msg`, and wait for the host to refuse it or to change the document. */ + async function answered(opened: Opened, msg: any): Promise<{ before: string; errors: string[] }> { + const before = opened.doc.getText(); + opened.view.posts.length = 0; + opened.view.send(msg); + const deadline = Date.now() + 8000; + while (opened.doc.getText() === before && errorsIn(opened.view.posts).length === 0 && Date.now() < deadline) { + await settle(20); + } + await settle(300); + return { before, errors: errorsIn(opened.view.posts) }; + } + + test('the banner says the names are shared, and counts nothing as unread', async () => { + const { paint } = await open(); + const twins = (paint.rows as any[]).filter((r) => isEntryRow(r) && nameOf(r) === TWIN); + assert.strictEqual(twins.length, 2, 'both entries are on screen'); + assert.ok(paint.warnings, 'the paint carries a banner'); + assert.strictEqual(paint.warnings.headline, SHARED); + assert.deepStrictEqual(paint.warnings.details, [CORE_SAYS], "core's message is below it"); + }).timeout(60000); + + test('a cell edit on a twin is refused, and the file is untouched', async () => { + const opened = await open(); + const row = rowNamed(opened.paint.rows, TWIN); + const { before, errors } = await answered(opened, { + type: 'edit', + rowId: row.ID, + columnId: 'Value', + oldValue: '9.81', + newValue: '1', + }); + assert.deepStrictEqual( + valuesOf(opened.doc.getText(), TWIN), + valuesOf(before, TWIN), + 'neither gravity was edited', + ); + assert.strictEqual(opened.doc.getText(), before, 'the document is untouched'); + assert.strictEqual(errors.length, 1, `one refusal: ${errors.join(' | ')}`); + }).timeout(60000); + + test('an entry with a name of its own still edits as before', async () => { + const opened = await open(); + const row = rowNamed(opened.paint.rows, 'scalarD'); + const { errors } = await answered(opened, { + type: 'edit', + rowId: row.ID, + columnId: 'Value', + oldValue: '3.14', + newValue: '42', + }); + assert.deepStrictEqual(errors, [], 'the edit is applied'); + const scalarD = ((chunkOf(opened.doc.getText()).entries ?? []) as any[]).find((e) => e.name === 'scalarD'); + assert.strictEqual(scalarD?.value, 42, 'and lands in the text'); + }).timeout(60000); +}); diff --git a/test/binaryEntryScopedEdit.test.ts b/test/binaryEntryScopedEdit.test.ts index 54f9896..c1c55bf 100644 --- a/test/binaryEntryScopedEdit.test.ts +++ b/test/binaryEntryScopedEdit.test.ts @@ -650,13 +650,17 @@ describe('binary .sldd entry ops — undo, redo, delete, paste', () => { it('does NOT undo correctly with the inverses in forward order', () => { // Pinned as the reason patchOfPairs reverses: inserting the source while the copy is - // still standing puts two entries under one id, and the remove that follows resolves - // to whichever was indexed last — the source it just restored. The copy survives in - // its place, unindexed, and the table shows an entry no edit can address. + // still standing puts two entries under one id, and the remove that follows has no + // one entry to resolve to. Core refuses an id that names two nodes (F1-50), so the + // remove throws rather than taking whichever twin was indexed last, and the caller + // falls back to a wide repaint from the text. Forward order is still wrong: it leaves + // both entries standing, under one id that nothing can address. const { d, model, section, srcId, pairs, srcUuid } = setUp('test://ops-reclaim-forward.sldd'); - applyEntryOps(model, pairs.map((p) => p.undo)); // NOT reversed - expect(gravities(section)).toEqual(['copy-uuid']); + expect(() => applyEntryOps(model, pairs.map((p) => p.undo))).toThrow(/No entry is indexed/); // NOT reversed + // The insert before the refused remove is not rolled back (applyEntryOps applies op by + // op): the source is back at its own index and the copy still stands after it. expect(srcUuid).not.toBe('copy-uuid'); + expect(gravities(section)).toEqual([srcUuid, 'copy-uuid']); expect((DataModel as any).findNodeById(srcId)).toBeNull(); d.dispose(); }); diff --git a/test/findNodeSharedName.test.ts b/test/findNodeSharedName.test.ts new file mode 100644 index 0000000..cbebca1 --- /dev/null +++ b/test/findNodeSharedName.test.ts @@ -0,0 +1,134 @@ +// Copyright 2026 The MathWorks, Inc. +// +// findNode and two entries with one name in one section (F1-50). +// +// A node id is a path, so two same-named siblings are one id, and so is every row inside them. +// Only a malformed file holds such a pair, and core keeps both as read. Since F1-50 its session +// refuses to resolve such an id: `DataModel.findNodeById` answers null, and so do its delete, +// edit and add-child forms. Which twin the user meant cannot be told from an id both rows carry. +// +// SlddModel.findNode used to fall back to this module's cached tree whenever the registry +// answered null. For the text editor that cache IS the registered tree, so the fallback handed +// back the FIRST twin, the very wrong-entry pick core refuses. A cell edit, a child delete, a +// copy, a drag and the Property Inspector all act on findNode's answer without asking the +// session again. So a registered source's null has to stay null. The fallback is kept for what +// it was for: a source the registry cannot answer about. +import { describe, it, expect, afterEach } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { DataModel } from 'data-explorer-core'; +import { getModel, findNode, invalidate } from '../src/host/SlddModel.js'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +const fixture = JSON.parse( + readFileSync(fileURLToPath(new URL('../test-integration/fixtures/workspace/params.sldd', import.meta.url)), 'utf8'), +); + +const entriesOf = (doc: any): any[] => doc.__MW_TEXT_PARTS__['__MW_TEXT_PART__/data/chunk0'].__MW_TEXT_content.entries; + +/** `name` under the given uuid: the same record the file holds, so the copy reads exactly as it does. */ +function copyOf(entryName: string, name: string, uuid: string): any { + const entry = structuredClone(entriesOf(fixture).find((e) => e.name === entryName)); + expect(entry, `the fixture holds ${entryName}`).toBeTruthy(); + return { ...entry, name, metadata: { ...entry.metadata, uuid } }; +} + +/** + * params.sldd with two Simulink.Parameter entries named K and two structs named S, all in + * Design Data, ahead of the fixture's own entries. Each twin carries its own uuid, as two + * entries MATLAB wrote would. + */ +function twinsText(): string { + const doc = structuredClone(fixture); + const entries = entriesOf(doc); + entries.unshift( + copyOf('gravity', 'K', 'twin-k-1'), + copyOf('gravity', 'K', 'twin-k-2'), + copyOf('myStruct', 'S', 'twin-s-1'), + copyOf('myStruct', 'S', 'twin-s-2'), + ); + return JSON.stringify(doc, null, '\t'); +} + +const design = (model: any): any => model.children.find((s: any) => s.name === 'design'); + +/** Which of `twins` a lookup answered, so a failure says which one rather than dumping the node. */ +const which = (node: any, twins: any[]): string | null => + node === null ? null : twins.indexOf(node) === 0 ? 'the first twin' : twins.indexOf(node) === 1 ? 'the second twin' : 'some other node'; +const named = (model: any, name: string): any[] => design(model).children.filter((e: any) => e.name === name); + +const opened: string[] = []; +function open(uri: string): any { + invalidate(uri); + opened.push(uri); + return getModel(uri, 'twins.sldd', twinsText()); +} + +afterEach(() => { + for (const uri of opened.splice(0)) { + invalidate(uri); + DataModel.removeDataSource(uri); + } +}); + +describe('findNode on a text .sldd whose section holds two entries with one name', () => { + it('answers null for the twins, and still resolves the entry that has its name to itself', () => { + const uri = 'test://find-node-twins-entry.sldd'; + const model = open(uri); + const ks = named(model, 'K'); + expect(ks).toHaveLength(2); + expect(ks[0].className).toBe('Simulink.Parameter'); + expect(ks[1].id).toBe(ks[0].id); // one id for both rows: the state the ruling is about + expect(ks[0].metadata.uuid).toBe('twin-k-1'); + + // Not the first twin (what the cache fallback handed back), and not the second. + expect(which(findNode(uri, ks[0].id), ks)).toBeNull(); + + // An entry whose name is its own resolves as before, from the same registered source. + const gravity = named(model, 'gravity')[0]; + expect(findNode(uri, gravity.id)).toBe(gravity); + }); + + it('answers null for a row inside a twin, which shares its id with the row in the other twin', () => { + const uri = 'test://find-node-twins-child.sldd'; + const model = open(uri); + const ss = named(model, 'S'); + expect(ss).toHaveLength(2); + const field = ss[0].children.find((c: any) => c.name === 'a'); + expect(field, 'S has a field a').toBeTruthy(); + expect(field.id).toBe(`${ss[0].id}/a`); + expect(ss[1].children.find((c: any) => c.name === 'a').id).toBe(field.id); + + const fields = ss.map((twin: any) => twin.children.find((c: any) => c.name === 'a')); + expect(which(findNode(uri, field.id), fields)).toBeNull(); + + // The same field of a struct that has its name to itself still resolves. + const own = named(model, 'myStruct')[0].children.find((c: any) => c.name === 'a'); + expect(findNode(uri, own.id)).toBe(own); + }); + + // The other half of the rule, which the change must keep: the cache answers when the registry + // cannot. Passes before the change too; it is here so that dropping the fallback whole, which + // would also pass the two tests above, fails. + it('still falls back to the cached tree when the source is not registered, or core has no findNodeById', () => { + const uri = 'test://find-node-twins-fallback.sldd'; + const model = open(uri); + const gravity = named(model, 'gravity')[0]; + + // An older core: no findNodeById on DataModel at all. + const dm = DataModel as any; + const lookup = dm.findNodeById; + dm.findNodeById = undefined; + try { + expect(findNode(uri, gravity.id)).toBe(gravity); + } finally { + dm.findNodeById = lookup; + } + + // A source the session no longer holds, while this module's cache still does. + DataModel.removeDataSource(uri); + expect(DataModel.hasDataSource(uri)).toBe(false); + expect(findNode(uri, gravity.id)).toBe(gravity); + }); +}); diff --git a/test/parseWarnings.test.ts b/test/parseWarnings.test.ts index 7259bf2..82b9513 100644 --- a/test/parseWarnings.test.ts +++ b/test/parseWarnings.test.ts @@ -147,3 +147,41 @@ describe('warningBanner', () => { expect(banner.headline).toContain(String(MAX_BANNER_DETAILS + 3)); }); }); + +// Two siblings with one name (F1-50). Core's session warns at open, with the code +// `duplicate-id`, because it refuses to edit or delete either of them by id. Nothing was +// lost: both are read and both are on screen. So the banner must not count them as parts +// that could not be read, and the file must still open. +describe('warningBanner and duplicate-id', () => { + const SHARED = 'Some items in this file share a name, so they cannot be edited or deleted here.'; + const twins = (part: string, where = 'Design Data'): ParseWarning => ({ + code: 'duplicate-id', + message: + `Two entries in ${where} are named '${part}'. ` + + 'Neither can be edited or deleted until the file gives one of them another name.', + part, + }); + + it('has a headline of its own when the only warnings are shared names, and core’s messages below it', () => { + const banner = warningBanner([twins('K'), twins('S')])!; + expect(banner.headline).toBe(SHARED); + expect(banner.details).toEqual([twins('K').message, twins('S').message]); + // And the file opens: a shared name is never a reason to refuse it. + expect(() => refuseIfUnreadable([twins('K')])).not.toThrow(); + }); + + it('counts only the parts that could not be read when both kinds are present', () => { + const banner = warningBanner([part('Skipped a part.', 'data/chunk1.xml'), twins('K'), twins('S')])!; + expect(banner.headline).toBe('One part of this file could not be read, so what you see below is incomplete.'); + // Every message is still listed: the shared names are worth saying, just not as a loss. + expect(banner.details).toEqual(['Skipped a part.', twins('K').message, twins('S').message]); + }); + + it('dedupes a repeated shared-name warning, and keeps one name shared in two sections apart', () => { + // One group reported twice is one line. One name doubled in Design Data and again in + // Other Data is two groups: `part` is the same and the message says where. + const banner = warningBanner([twins('K'), twins('K'), twins('K', 'Other Data')])!; + expect(banner.headline).toBe(SHARED); + expect(banner.details).toEqual([twins('K').message, twins('K', 'Other Data').message]); + }); +}); From f53f645fd70e9f9a8cc738d038f18e87f20c11bb Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 16 Oct 2026 21:34:32 -0400 Subject: [PATCH 5/5] Pin core v1.40.0, which carries the group E core halves v1.40.0 (f9acdf9) carries mathworks/data-explorer-core#124 (Signal Complexity and DimensionsMode options; Alignment editing), #125 (mpt entries admitted by paste and drop) and #127 (findNodeById and the duplicate-id warning), the core halves of C-030, C-086, C-180 and F1-50. It also carries #128 (MCOS numeric classes, per-element object classes). The lockfile changes only data-explorer-core: its root spec and its node_modules entry, which now resolves to f9acdf939aa726cb9ddd3162a6cac19fc4f4a5ca. With the pin: npm run verify passes 3422 of 3422 tests in 175 files, and check:leak is OK; npm run test:integration has 221 passing and 1 pending. --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 48ab800..bcc70f1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "BSD-3-Clause", "dependencies": { "@lit/context": "^1.1.6", - "data-explorer-core": "github:mathworks/data-explorer-core#v1.39.0", + "data-explorer-core": "github:mathworks/data-explorer-core#v1.40.0", "fast-xml-parser": "^5.8.0", "fflate": "^0.8.3", "highlight.js": "^11.11.1", @@ -3184,8 +3184,8 @@ } }, "node_modules/data-explorer-core": { - "version": "1.39.0", - "resolved": "git+ssh://git@github.com/mathworks/data-explorer-core.git#8e334082682eae7901cc07016f4fb90b3cdc7c73", + "version": "1.40.0", + "resolved": "git+ssh://git@github.com/mathworks/data-explorer-core.git#f9acdf939aa726cb9ddd3162a6cac19fc4f4a5ca", "license": "BSD-3-Clause", "dependencies": { "fast-xml-parser": "^5.8.0", diff --git a/package.json b/package.json index dfe4596..8dd6b9e 100644 --- a/package.json +++ b/package.json @@ -236,7 +236,7 @@ }, "dependencies": { "@lit/context": "^1.1.6", - "data-explorer-core": "github:mathworks/data-explorer-core#v1.39.0", + "data-explorer-core": "github:mathworks/data-explorer-core#v1.40.0", "fast-xml-parser": "^5.8.0", "fflate": "^0.8.3", "highlight.js": "^11.11.1",