From ae380571595d3fde1f597dc8ff3d1e9d0873d943 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:58:46 +0000 Subject: [PATCH 01/12] feat(view): import renamed elements from a draw.io file A matched node, port or edge whose label differs from the writer's own label by exactly its declared name imports as a Rename through the model-edit path, ordered after the import's other edits; a usage path reaching an inherited member renames the declaration it resolves to. Label edits beyond the name, invalid names, refused renames and drawn copies that disagree are reported, not imported. Co-Authored-By: jason.han --- .../unreleased/drawio-import-rename.added.md | 1 + cmd/sysml/import_diagram.go | 7 +- internal/check/edit/drawio.go | 32 ++++ internal/check/edit/drawio_rename_test.go | 165 ++++++++++++++++++ internal/check/edit/drawio_test.go | 75 +++++++- internal/check/edit/edit.go | 3 + internal/check/edit/rename.go | 35 ++++ internal/check/edit/validate.go | 17 +- internal/frontend/lsp/importdiagram.go | 9 +- internal/ir/view/drawio.go | 72 ++++++-- internal/ir/view/drawio_import_diff.go | 145 ++++++++++++++- internal/ir/view/drawio_import_rename_test.go | 141 +++++++++++++++ internal/ir/view/label.go | 15 ++ internal/syntax/lexer/name.go | 28 +++ 14 files changed, 706 insertions(+), 39 deletions(-) create mode 100644 changes/unreleased/drawio-import-rename.added.md create mode 100644 internal/check/edit/drawio_rename_test.go create mode 100644 internal/ir/view/drawio_import_rename_test.go create mode 100644 internal/syntax/lexer/name.go diff --git a/changes/unreleased/drawio-import-rename.added.md b/changes/unreleased/drawio-import-rename.added.md new file mode 100644 index 0000000000..ef4ed1057a --- /dev/null +++ b/changes/unreleased/drawio-import-rename.added.md @@ -0,0 +1 @@ +- `sysml -import-diagram`, the LSP `opensysml/importDiagram` request, the MCP `import_diagram` tool and the VS Code *Import Diagram* command read three more kinds of edit back from a draw.io file: an element renamed by retyping its cell's text (applied as a rename, so its declaration and every reference move, through the path a `Layout` of the cell takes), a note whose text was edited, and a picture whose image source was changed to a file the model can name. A label edited beyond its name, a name SysML does not accept, a rename the model refuses, copies of one element renamed differently, an emptied note and a picture turned into embedded data are reported and left alone. diff --git a/cmd/sysml/import_diagram.go b/cmd/sysml/import_diagram.go index 914b5d1ee2..b31048d63e 100644 --- a/cmd/sysml/import_diagram.go +++ b/cmd/sysml/import_diagram.go @@ -44,14 +44,15 @@ func runImportDiagram(files []string) error { if err != nil { return fmt.Errorf("%s: %w", importDiagramPath, err) } + doc := drawioImportDocument(imp, files) + result, err := modeledit.ImportDrawio(imp, false, func(ops []modeledit.Operation) (*model.EditResult, error) { + return sess.ApplyModelEdit(doc, ops) + }) reportDrawioImport(imp) if len(imp.Edits) == 0 { fmt.Fprintf(os.Stderr, "%s: nothing differs from the rendering of %s; nothing to import\n", importDiagramPath, file.View) return nil } - ops := modeledit.DrawioOperations(imp, false) - doc := drawioImportDocument(imp, files) - result, err := sess.ApplyModelEdit(doc, ops) if err != nil { var refusal *modeledit.Error if errors.As(err, &refusal) { diff --git a/internal/check/edit/drawio.go b/internal/check/edit/drawio.go index 9e689948e6..f9daae3b9e 100644 --- a/internal/check/edit/drawio.go +++ b/internal/check/edit/drawio.go @@ -1,6 +1,9 @@ package edit import ( + "errors" + "fmt" + "github.com/Open-MBEE/OpenSysML/internal/ir/view" ) @@ -30,6 +33,8 @@ func DrawioOperations(imp *view.DrawioImport, inline bool) []Operation { op = SetNoteAt(e.Origin.Span, e.Note).DeclaredIn(e.Origin.Doc) case view.DrawioEditPicture: op = SetPictureAt(e.Origin.Span, e.Picture).DeclaredIn(e.Origin.Doc) + case view.DrawioEditRename: + op = Rename(e.FQN, e.NewName) default: continue } @@ -46,3 +51,30 @@ func drawioTargeted(e view.DrawioEdit, named, declared Operation) Operation { } return declared.DeclaredIn(e.Origin.Doc) } + +// ImportDrawio applies an import's edits through apply, the editor of the +// document the view is declared in. A rename the editor refuses — the name is +// taken, a reference would be captured — is reported in the import's notices +// and the other edits are applied again, so one refused rename holds none of +// them back; any other refusal is the caller's. The import's edits are left +// as what was applied. +func ImportDrawio[R any](imp *view.DrawioImport, inline bool, apply func([]Operation) (R, error)) (R, error) { + for { + var none R + if len(imp.Edits) == 0 { + return none, nil + } + result, err := apply(DrawioOperations(imp, inline)) + var refusal *Error + if !errors.As(err, &refusal) { + return result, err + } + i := refusal.OperationIndex + if i < 0 || i >= len(imp.Edits) || imp.Edits[i].Kind != view.DrawioEditRename { + return result, err + } + e := imp.Edits[i] + imp.Notices = append(imp.Notices, fmt.Sprintf("%s: not renamed to %q: %s", e.FQN, e.NewName, refusal.Message)) + imp.Edits = append(imp.Edits[:i:i], imp.Edits[i+1:]...) + } +} diff --git a/internal/check/edit/drawio_rename_test.go b/internal/check/edit/drawio_rename_test.go new file mode 100644 index 0000000000..9fe028a3ba --- /dev/null +++ b/internal/check/edit/drawio_rename_test.go @@ -0,0 +1,165 @@ +package edit + +import ( + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/ir/view" +) + +const drawioLoop = `package Plant { + port def FluidPort; + part def Pump { + port outlet : FluidPort; + } + part def Tank { + port inlet : FluidPort; + } + part def Loop { + part pump : Pump; + part tank : Tank; + connect pump.outlet to tank.inlet; + } + view loopView { + expose Loop::*; + } +} +` + +// drawioCellOf is the exported cell of the element named, by its kind. +func drawioCellOf(t *testing.T, file *view.DrawioFile, kind, fqn string) *view.DrawioCell { + t.Helper() + cell := drawioFirst(file, func(c *view.DrawioCell) bool { return c.Kind == kind && c.FQN == fqn }) + if cell == nil { + t.Fatalf("no %s %s among the cells", kind, fqn) + } + return cell +} + +func drawioRenameOf(t *testing.T, imp *view.DrawioImport) view.DrawioEdit { + t.Helper() + if len(imp.Edits) != 1 || imp.Edits[0].Kind != view.DrawioEditRename { + t.Fatalf("edits %+v notices %q, want one Rename", imp.Edits, imp.Notices) + } + return imp.Edits[0] +} + +func TestDrawioImportRenamesAPortAndItsConnectionEnds(t *testing.T) { + m, rendering, options := drawioModel(t, drawioLoop, "Plant::loopView") + document, exported := drawioExport(t, rendering, options) + port := drawioCellOf(t, exported, "port", "Plant::Pump::outlet") + edited := drawioRelabel(t, document, port.ID, func(label string) string { return strings.Replace(label, "outlet", "discharge", 1) }) + _, imp := drawioImport(t, rendering, options, edited) + if e := drawioRenameOf(t, imp); e.FQN != "Plant::Loop::pump::outlet" || e.NewName != "discharge" { + t.Fatalf("rename = %s to %q, want the port by the path a Layout of it takes", e.FQN, e.NewName) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + content := string(res.Content) + if !strings.Contains(content, "port discharge : FluidPort;") || !strings.Contains(content, "connect pump.discharge to tank.inlet;") || strings.Contains(content, "outlet") { + t.Errorf("renamed model:\n%s", content) + } + _, redrawn, reoptions := drawioModel(t, content, "Plant::loopView") + _, re := drawioExport(t, redrawn, reoptions) + if again := drawioCellOf(t, re, "port", "Plant::Pump::discharge"); again.Label != strings.Replace(port.Label, "outlet", "discharge", 1) { + t.Errorf("re-exported label %q", again.Label) + } +} + +func TestDrawioImportRenameRewritesAnotherDocument(t *testing.T) { + const other = "package Other {\n\tpart p : Plant::Pump;\n}\n" + m := loadEditableWorkspace(t, "model.sysml", drawioLoop, map[string]string{"other.sysml": other}) + m, rendering, options := drawioModelOf(t, m, "Plant::loopView") + document, exported := drawioExport(t, rendering, options) + pump := drawioCellOf(t, exported, "part", "Plant::Loop::pump") + _, imp := drawioImport(t, rendering, options, drawioRelabel(t, document, pump.ID, func(label string) string { return label + "2" })) + if e := drawioRenameOf(t, imp); e.FQN != "Plant::Loop::pump" || e.NewName != "pump2" { + t.Fatalf("rename = %+v", e) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + if content := string(res.Content); !strings.Contains(content, "part pump2 : Pump;") || !strings.Contains(content, "connect pump2.outlet") { + t.Errorf("renamed model:\n%s", content) + } + if len(res.Others) != 0 { + t.Errorf("a usage nothing else names rewrote %+v", res.Others) + } + // The definition the other document types its part with. + _, rendering, options = drawioModelOf(t, loadEditableWorkspace(t, "model.sysml", strings.Replace(drawioLoop, "expose Loop::*;", "expose Plant::Pump;", 1), map[string]string{"other.sysml": other}), "Plant::loopView") + document, exported = drawioExport(t, rendering, options) + def := drawioCellOf(t, exported, "part def", "Plant::Pump") + _, imp = drawioImport(t, rendering, options, drawioRelabel(t, document, def.ID, func(label string) string { return strings.Replace(label, "Pump", "Motor", 1) })) + m = loadEditableWorkspace(t, "model.sysml", strings.Replace(drawioLoop, "expose Loop::*;", "expose Plant::Pump;", 1), map[string]string{"other.sysml": other}) + res, err = Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + if content := string(res.Content); !strings.Contains(content, "part def Motor {") || !strings.Contains(content, "part pump : Motor;") { + t.Errorf("renamed model:\n%s", content) + } + if len(res.Others) != 1 || res.Others[0].Name != "other.sysml" || !strings.Contains(string(res.Others[0].Content), "part p : Plant::Motor;") { + t.Errorf("other documents: %+v", res.Others) + } +} + +func TestDrawioImportReportsARenameTheEditorRefuses(t *testing.T) { + m, rendering, options := drawioModel(t, drawioLoop, "Plant::loopView") + document, exported := drawioExport(t, rendering, options) + pump := drawioCellOf(t, exported, "part", "Plant::Loop::pump") + edited := drawioRelabel(t, document, pump.ID, func(label string) string { return strings.Replace(label, "pump", "tank", 1) }) + edited = drawioMutate(t, edited, pump.ID, func(cell string) string { return drawioShift(cell, "x", 77) }) + _, imp := drawioImport(t, rendering, options, edited) + if _, err := Apply(m, DrawioOperations(imp, false)); err == nil { + t.Fatal("renaming pump to tank applied") + } + applied := 0 + res, err := ImportDrawio(imp, false, func(ops []Operation) (*Result, error) { + applied++ + return Apply(m, ops) + }) + if err != nil || res == nil || applied != 2 { + t.Fatalf("result %v, error %v after %d applications; want the Layout applied once the rename was dropped", res, err, applied) + } + if len(imp.Notices) != 1 || !strings.Contains(imp.Notices[0], `Plant::Loop::pump: not renamed to "tank": `) || !strings.Contains(imp.Notices[0], "tank") { + t.Errorf("notices %q", imp.Notices) + } + for _, e := range imp.Edits { + if e.Kind == view.DrawioEditRename { + t.Errorf("the refused rename is still among the edits: %+v", e) + } + } + if content := string(res.Content); !strings.Contains(content, "part pump : Pump;") || !strings.Contains(content, "Layout about Plant::Loop::pump") { + t.Errorf("model after the import:\n%s", content) + } +} + +func TestDrawioImportRenamesAndMovesOneElement(t *testing.T) { + m, rendering, options := drawioModel(t, drawioLoop, "Plant::loopView") + document, exported := drawioExport(t, rendering, options) + pump := drawioCellOf(t, exported, "part", "Plant::Loop::pump") + edited := drawioRelabel(t, document, pump.ID, func(label string) string { return label + "X" }) + edited = drawioMutate(t, edited, pump.ID, func(cell string) string { return drawioShift(cell, "x", 77) }) + editedFile, imp := drawioImport(t, rendering, options, edited) + if n := len(imp.Edits); n < 2 || imp.Edits[n-1].Kind != view.DrawioEditRename || imp.Edits[n-1].NewName != "pumpX" { + t.Fatalf("edits %+v, want Layouts then the Rename", imp.Edits) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + content := string(res.Content) + if !strings.Contains(content, "part pumpX : Pump;") || !strings.Contains(content, "Layout about Plant::Loop::pumpX") || strings.Contains(content, "Loop::pump ") { + t.Errorf("model after the import:\n%s", content) + } + _, redrawn, reoptions := drawioModel(t, content, "Plant::loopView") + _, re := drawioExport(t, redrawn, reoptions) + was := editedFile.Cell(pump.ID) + again := drawioCellOf(t, re, "part", "Plant::Loop::pumpX") + if again.Label != was.Label || !drawioSameBox(again.Geometry, was.Geometry) { + t.Errorf("re-exported %q at %+v, edited %q at %+v", again.Label, again.Geometry, was.Label, was.Geometry) + } +} diff --git a/internal/check/edit/drawio_test.go b/internal/check/edit/drawio_test.go index 63ff8fef02..d94c60b0b0 100644 --- a/internal/check/edit/drawio_test.go +++ b/internal/check/edit/drawio_test.go @@ -1,6 +1,7 @@ package edit import ( + "html" "os" "path/filepath" "regexp" @@ -45,7 +46,12 @@ func drawioCases(t *testing.T) []drawioCase { // each cell's declaration. func drawioModel(t *testing.T, content, viewName string) (Model, *view.Rendering, view.Options) { t.Helper() - m := loadContent(t, "model.sysml", content) + return drawioModelOf(t, loadContent(t, "model.sysml", content), viewName) +} + +// drawioModelOf renders the view of a loaded model as the workspace does. +func drawioModelOf(t *testing.T, m Model, viewName string) (Model, *view.Rendering, view.Options) { + t.Helper() r, sem := m.resolver() viewSym, err := m.viewNamed(0, viewName, "", viewName+" is no view") if err != nil { @@ -81,6 +87,8 @@ type drawioMutation struct { // the mutation of a cell, for a pick that depends on what the import makes of it. pick func(file *view.DrawioFile, try func(*view.DrawioCell) *view.DrawioImport) *view.DrawioCell edit func(cell string) string + // relabel edits the object's label instead, the text a user retypes in draw.io. + relabel func(label string) string // kind is the edit expected, and check the cell of the re-export against the // edited one; a notice instead names what the import reports and leaves alone. kind view.DrawioEditKind @@ -203,6 +211,13 @@ func drawioReroute(cell string) string { } var drawioMutations = []drawioMutation{ + {name: "rename", pick: drawioRenameable, kind: view.DrawioEditRename, + relabel: func(label string) string { return label + "X" }, + check: func(t *testing.T, edited, re *view.DrawioCell) { + if edited.Label != re.Label { + t.Errorf("relabelled %q, re-exported as %q", edited.Label, re.Label) + } + }}, {name: "move", pick: drawioLeaf, edit: func(cell string) string { return drawioShift(cell, "x", 77) }, kind: view.DrawioEditLayout, check: func(t *testing.T, edited, re *view.DrawioCell) { @@ -264,6 +279,46 @@ func drawioShift(cell, attr string, delta float64) string { } // drawioMutate rewrites the of the cell id in the document. +// drawioRenameable is the first cell whose label, with text appended, imports +// as a rename: one whose label ends with the element's own name. The view and +// its owners are left alone, as the round trip renders the view by name. +func drawioRenameable(f *view.DrawioFile, try func(*view.DrawioCell) *view.DrawioImport) *view.DrawioCell { + return drawioFirst(f, func(c *view.DrawioCell) bool { + if c.FQN == "" || c.Label == "" || f.View == c.FQN || strings.HasPrefix(f.View, c.FQN+"::") { + return false + } + imp := try(c) + return len(imp.Edits) == 1 && imp.Edits[0].Kind == view.DrawioEditRename && len(imp.Notices) == 0 + }) +} + +// drawioRelabel rewrites the label attribute of the object cell id, the one +// attribute draw.io rewrites when a user edits the text of a cell. +func drawioRelabel(t *testing.T, document, id string, relabel func(label string) string) string { + t.Helper() + re := regexp.MustCompile(`= 0 { + renamed.FQN = cell.FQN[:i+2] + newName + } else { + renamed.FQN = newName + } + return &renamed +} + func drawioMutate(t *testing.T, document, id string, edit func(cell string) string) string { t.Helper() re := regexp.MustCompile(`(?s)]*>\s*()\s*||/>)`) @@ -315,9 +370,15 @@ func TestDrawioRoundTrip(t *testing.T) { if _, imp := drawioImport(t, rendering, options, document); len(imp.Edits)+len(imp.Unmatched)+len(imp.Deleted)+len(imp.Unknown)+len(imp.Notices) != 0 { t.Fatalf("untouched export: %+v", imp) } + mutate := func(mut drawioMutation, id string) string { + if mut.relabel != nil { + return drawioRelabel(t, document, id, mut.relabel) + } + return drawioMutate(t, document, id, mut.edit) + } try := func(mut drawioMutation) func(c *view.DrawioCell) *view.DrawioImport { return func(c *view.DrawioCell) *view.DrawioImport { - _, imp := drawioImport(t, rendering, options, drawioMutate(t, document, c.ID, mut.edit)) + _, imp := drawioImport(t, rendering, options, mutate(mut, c.ID)) return imp } } @@ -327,7 +388,7 @@ func TestDrawioRoundTrip(t *testing.T) { continue } t.Run(mut.name, func(t *testing.T) { - edited := drawioMutate(t, document, cell.ID, mut.edit) + edited := mutate(mut, cell.ID) editedFile, imp := drawioImport(t, rendering, options, edited) if mut.notice != "" { if len(imp.Edits) != 0 || len(imp.Notices) != 1 || !strings.Contains(imp.Notices[0], mut.notice) { @@ -359,6 +420,9 @@ func TestDrawioRoundTrip(t *testing.T) { _, re := drawioExport(t, redrawn, reoptions) for _, e := range imp.Edits { was := editedFile.Cell(e.Cell) + if e.Kind == view.DrawioEditRename { + was = drawioRenamed(was, e.NewName) + } again := drawioSameCell(editedFile, string(content), was, re, string(res.Content)) if again == nil { t.Fatalf("the re-export has no cell for %s, %s %s", e.Cell, was.Kind, was.FQN) @@ -377,10 +441,11 @@ func TestDrawioRoundTrip(t *testing.T) { } } } - // The edited cells are where the file has them; unplaced neighbours may have flowed around them. + // The edited cells are where the file has them; unplaced neighbours may have + // flowed around them, and a renamed node is sized to its longer name. if _, imp := drawioImport(t, redrawn, reoptions, edited); len(imp.Edits) != 0 { for _, e := range imp.Edits { - if e.Cell == cell.ID { + if e.Cell == cell.ID && (mut.kind != view.DrawioEditRename || e.Kind == view.DrawioEditRename) { t.Errorf("the edited file still imports %+v against the edited model", e) } } diff --git a/internal/check/edit/edit.go b/internal/check/edit/edit.go index 5e7227824f..9ddef68253 100644 --- a/internal/check/edit/edit.go +++ b/internal/check/edit/edit.go @@ -940,6 +940,9 @@ func (m Model) splicesFor(i int, op Operation) ([]splice, error) { return deletes, nil } sym, err := m.target(i, op) + if err != nil && op.Kind == OpRename { + sym, err = m.renameTarget(i, op, err) + } if err != nil { return nil, err } diff --git a/internal/check/edit/rename.go b/internal/check/edit/rename.go index 10698cd898..5949291d3d 100644 --- a/internal/check/edit/rename.go +++ b/internal/check/edit/rename.go @@ -1,12 +1,14 @@ package edit import ( + "errors" "fmt" "sort" "github.com/Open-MBEE/OpenSysML/internal/semantic/resolve" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" + "github.com/Open-MBEE/OpenSysML/internal/syntax/source" ) // renameSplices are the byte ranges a rename rewrites: the declaration's name @@ -113,3 +115,36 @@ func (m Model) renameOccurrences(r *resolve.Resolver, sym *symbols.Symbol, ident sortReferrers(elsewhere) return out, elsewhere } + +// renameTarget is the declaration a rename names when no declaration answers +// to its target: a qualified path reaching an inherited member through the +// owner a rendering draws it under, as a Layout about that member states it, +// renames the declaration the member resolves to. The path's own failure +// stands when it leads nowhere. +func (m Model) renameTarget(i int, op Operation, err error) (*symbols.Symbol, error) { + var refusal *Error + if !errors.As(err, &refusal) || refusal.Failure != FailureUnknownTarget || op.Target == "" { + return nil, err + } + names, ok := source.QualifiedNameSegments(op.Target) + if !ok || len(names) < 2 { + return nil, err + } + owner, ownerErr := m.declaredOnceIn(i, source.QualifiedNameOf(names[:len(names)-1]), op.DeclarationDoc) + if ownerErr != nil { + return nil, err + } + _, sem := m.resolver() + member, ok := sem.LookupMember(owner, names[len(names)-1]) + if !ok { + return nil, err + } + if member.DocName != m.Source.Name() { + return nil, &Error{ + Failure: FailureUnknownTarget, + OperationIndex: i, + Message: fmt.Sprintf("%q is declared in %s, not in this model's source", op.Target, docLabel(member.DocName)), + } + } + return member, nil +} diff --git a/internal/check/edit/validate.go b/internal/check/edit/validate.go index 5e99fb5e22..a2ed5fde9c 100644 --- a/internal/check/edit/validate.go +++ b/internal/check/edit/validate.go @@ -57,21 +57,8 @@ func checkName(i int, name string) error { Message: fmt.Sprintf("new name %q %s", name, reason), } } - if name == "" { - return refuse("is empty") - } - if source.IsKeyword(name) { - return refuse("is a keyword") - } - lx := lexer.New(source.New("", []byte(name))) - tok := lx.Next() - switch { - case tok.Kind != lexer.Identifier && tok.Kind != lexer.UnrestrictedName: - return refuse("is not an identifier") - case tok.Unterminated: - return refuse("is an unterminated quoted name") - case tok.Span.Len != len(name): - return refuse("is not a single name") + if problem := lexer.NameProblem(name); problem != "" { + return refuse(problem) } return nil } diff --git a/internal/frontend/lsp/importdiagram.go b/internal/frontend/lsp/importdiagram.go index 36436ce88d..423385b81d 100644 --- a/internal/frontend/lsp/importdiagram.go +++ b/internal/frontend/lsp/importdiagram.go @@ -111,6 +111,14 @@ func (s *Server) ImportDiagram(params *ImportDiagramParams) (*ImportDiagramResul if err != nil { return nil, err } + var version int + var ok bool + result, err := modeledit.ImportDrawio(imp, params.Inline, func(ops []modeledit.Operation) (*model.EditResult, error) { + var result *model.EditResult + var err error + result, version, ok, err = s.ws.ApplyEdit(name, ops, snapshot.Documents()) + return result, err + }) out := &ImportDiagramResult{View: file.View, DryRun: params.DryRun, Edits: []ImportDiagramEdit{}, Unmatched: imp.Unmatched, Deleted: imp.Deleted, Notices: imp.Notices} for _, cell := range imp.Unknown { @@ -122,7 +130,6 @@ func (s *Server) ImportDiagram(params *ImportDiagramParams) (*ImportDiagramResul if len(imp.Edits) == 0 { return out, nil } - result, version, ok, err := s.ws.ApplyEdit(name, modeledit.DrawioOperations(imp, params.Inline), snapshot.Documents()) if !ok { return nil, fmt.Errorf("%s: no such document", name) } diff --git a/internal/ir/view/drawio.go b/internal/ir/view/drawio.go index 04061dc490..ad07d42056 100644 --- a/internal/ir/view/drawio.go +++ b/internal/ir/view/drawio.go @@ -55,6 +55,7 @@ func (r *Rendering) drawioPrepare(options Options) (*Rendering, *drawioWriter, e w := &drawioWriter{ fills: familyFills{palette: options.Palette, tree: r.Kind == KindTree}, labels: labelsOf(r.Roots, false, nil, r.Shown), + names: map[string]drawioName{}, ports: r.portView(options.Ports), links: options.Links, view: r.View, @@ -92,17 +93,18 @@ type drawioWriter struct { used map[string]map[string]bool links Links view string - boxes map[string]drawioBox // node ID -> its box in canvas coordinates - unrouted bool // this build has no router; draw.io routes on opening - cells map[string]bool // the cell IDs written, the ends an edge may attach to - portBoxes map[string]drawioBox // port cell ID -> its box in canvas coordinates - routed map[int]bool // the edges (by index) the router gave their route - unroutable []int // the edges (by index) the router found no path for - inward map[int]drawioEnds // per edge, the ends that lead into the port's owner - fqns map[string]bool // the qualified names written on cells so far - edgeIDs map[[2]string]string // edge ends -> its cell ID, the anchor an edge note joins - grid int // nodes and notes laid on the grid for want of a position - extent drawioBox // the bounds of everything placed + boxes map[string]drawioBox // node ID -> its box in canvas coordinates + unrouted bool // this build has no router; draw.io routes on opening + cells map[string]bool // the cell IDs written, the ends an edge may attach to + portBoxes map[string]drawioBox // port cell ID -> its box in canvas coordinates + routed map[int]bool // the edges (by index) the router gave their route + unroutable []int // the edges (by index) the router found no path for + inward map[int]drawioEnds // per edge, the ends that lead into the port's owner + fqns map[string]bool // the qualified names written on cells so far + names map[string]drawioName // cell ID -> where its label shows the declared name + edgeIDs map[[2]string]string // edge ends -> its cell ID, the anchor an edge note joins + grid int // nodes and notes laid on the grid for want of a position + extent drawioBox // the bounds of everything placed b strings.Builder } @@ -516,6 +518,13 @@ func (w *drawioWriter) attr(name, value string) { // drawioObject are the attributes a semantic cell's carries. type drawioObject struct { id, label, fqn, kind, link string + name drawioName +} + +// drawioName is where a cell's label shows the declared name of its element, +// the token a rename of the cell rewrites; Len 0 when the label shows none. +type drawioName struct { + Offset, Len int } // openObject writes the wrapper of a semantic cell; the view is every @@ -533,9 +542,44 @@ func (w *drawioWriter) openObject(indent string, o drawioObject) { if o.link != "" { w.attr("link", o.link) } + if o.name.Len > 0 && w.names != nil { + w.names[o.id] = o.name + } w.b.WriteString(">\n") } +// nodeName locates shown(node)'s token in the label headerLines joins: after +// the keyword line and the head's usage prefix, as the labeller places it. +func (w *drawioWriter) nodeName(node *Node) drawioName { + before, name := w.labels.headName(node) + if name == "" { + return drawioName{} + } + offset := len(before) + if keyword := w.labels.keyword(node); keyword != "" { + offset += len(keyword) + 1 + } + return drawioName{Offset: offset, Len: len(name)} +} + +// drawioPortName locates a port's name in its pin label, after the usage prefix +// displayName writes; a synthesized name is drawn as none. +func drawioPortName(port Port) drawioName { + if port.Name == "" || port.NameSynthesized { + return drawioName{} + } + return drawioName{Offset: len(usagePrefix(port.Inherited)), Len: len(port.Name)} +} + +// drawioEdgeName is an edge's whole label when that is its name alone; a label +// carrying a trigger, guard or pins shows no name to rename. +func drawioEdgeName(edge Edge) drawioName { + if edge.Name == "" || edge.Label != edge.Name { + return drawioName{} + } + return drawioName{Len: len(edge.Label)} +} + // link is a declaration's site, `file:line`, or the link template's URL when // one is set; nothing for an origin with no source. func (w *drawioWriter) link(origin Origin) string { @@ -607,7 +651,7 @@ func (w *drawioWriter) writeNode(node *Node, parent, parentFQN string) { } compartments := w.compartments(node) header := w.headerHeight(node) - w.openObject(" ", drawioObject{id: node.ID, label: strings.Join(w.headerLines(node), "\n"), fqn: fqn, kind: node.Kind, link: w.link(node.Origin)}) + w.openObject(" ", drawioObject{id: node.ID, label: strings.Join(w.headerLines(node), "\n"), fqn: fqn, kind: node.Kind, link: w.link(node.Origin), name: w.nodeName(node)}) w.b.WriteString(" 0)) w.b.WriteString(` vertex="1"`) @@ -669,7 +713,7 @@ func (w *drawioWriter) writePorts(node *Node, fqn string) { if portFQN == "" && fqn != "" && port.Name != "" && !port.NameSynthesized { portFQN = fqn + "::" + port.Name } - w.openObject(" ", drawioObject{id: id, label: w.ports.pinLabel(port), fqn: portFQN, kind: drawioPortKind(port), link: w.link(port.Origin)}) + w.openObject(" ", drawioObject{id: id, label: w.ports.pinLabel(port), fqn: portFQN, kind: drawioPortKind(port), link: w.link(port.Origin), name: drawioPortName(port)}) w.b.WriteString(" ` is a line break, a `
` or `

` opens a new line after the first, +// other tags are dropped, and entities read as the characters they name. +func drawioHTMLText(text string) string { + var b strings.Builder + for i := 0; i < len(text); { + if text[i] != '<' { + b.WriteByte(text[i]) + i++ + continue + } + end := strings.IndexByte(text[i:], '>') + if end < 0 { + b.WriteString(text[i:]) + break + } + tag := strings.ToLower(text[i+1 : i+end]) + name, _, _ := strings.Cut(strings.Trim(tag, "/ "), " ") + closing := strings.HasPrefix(tag, "/") + if name == "br" || (name == "div" || name == "p") && !closing && b.Len() > 0 { + b.WriteByte('\n') + } + i += end + 1 + } + return strings.ReplaceAll(html.UnescapeString(b.String()), "\u00a0", " ") +} + // originOf is the declaration behind a baseline cell id: a port's or a node's. func (m *drawioMatcher) originOf(id string) (Origin, bool) { if port, ok := m.ports[id]; ok { @@ -533,6 +672,10 @@ func (m *drawioMatcher) diffEdge(cell, base *DrawioCell, key DrawioEdgeKey) { out.Kind, out.Route, out.Style = DrawioEditStyle, nil, style m.out.Edits = append(m.out.Edits, out) } + if cell.Label != base.Label { + out.FQN = base.FQN + m.diffLabel(cell, base, out) + } } // plainCell handles a cell without an element: a note, a picture, an end label diff --git a/internal/ir/view/drawio_import_rename_test.go b/internal/ir/view/drawio_import_rename_test.go new file mode 100644 index 0000000000..cd1e3d8da8 --- /dev/null +++ b/internal/ir/view/drawio_import_rename_test.go @@ -0,0 +1,141 @@ +package view + +import ( + "html" + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// setLabel rewrites the label attribute of the object cell id, the only +// attribute draw.io desktop rewrites when a user edits a cell's text. +func setLabel(t *testing.T, document, id, label string) string { + t.Helper() + re := regexp.MustCompile(`( 1 { + before += l.text(source.QualifiedNameOf(segments[:len(segments)-1]) + "::") + name = segments[len(segments)-1] + } + return before, l.text(name) +} + // usagePrefix is the notation's prefix before a shown usage name: `^` for a // usage the owner it is drawn under does not declare (SysML v2 §8.2.3.6 // `el-prefix`), nothing for one the owner declares. diff --git a/internal/syntax/lexer/name.go b/internal/syntax/lexer/name.go new file mode 100644 index 0000000000..2b67e4e5ba --- /dev/null +++ b/internal/syntax/lexer/name.go @@ -0,0 +1,28 @@ +package lexer + +import "github.com/Open-MBEE/OpenSysML/internal/syntax/source" + +// NotASingleName is NameProblem's answer to text that starts with a name +// but carries more: a type, a multiplicity, a second word. +const NotASingleName = "is not a single name" + +// NameProblem says why text is not one declared name — a basic identifier or +// a quoted unrestricted name, and not a keyword — or "" when it is one. +func NameProblem(name string) string { + if name == "" { + return "is empty" + } + if source.IsKeyword(name) { + return "is a keyword" + } + tok := New(source.New("", []byte(name))).Next() + switch { + case tok.Kind != Identifier && tok.Kind != UnrestrictedName: + return "is not an identifier" + case tok.Unterminated: + return "is an unterminated quoted name" + case tok.Span.Len != len(name): + return NotASingleName + } + return "" +} From e6df9a9abd99793945fedf9c107ce2e6f0528215 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:11:50 +0000 Subject: [PATCH 02/12] feat(view): import note text and picture source edits from draw.io A note box whose text was retyped imports as a SetNote with its text, decoded from draw.io's formatted form when the note was formatted, and together with a move as one SetNote; an emptied note is reported. A picture whose image was changed to a file the model can locate imports as a SetPicture naming it relative to the view's directory; embedded data, a URL and a file that does not read as an image are reported. String literals the edits write spell line breaks and tabs as escapes. Co-Authored-By: jason.han --- .../check/edit/drawio_note_picture_test.go | 123 ++++++++++ internal/check/edit/drawio_test.go | 19 ++ internal/check/edit/layout.go | 3 + internal/ir/view/drawio_import_diff.go | 96 ++++++-- .../view/drawio_import_note_picture_test.go | 218 ++++++++++++++++++ 5 files changed, 446 insertions(+), 13 deletions(-) create mode 100644 internal/check/edit/drawio_note_picture_test.go create mode 100644 internal/ir/view/drawio_import_note_picture_test.go diff --git a/internal/check/edit/drawio_note_picture_test.go b/internal/check/edit/drawio_note_picture_test.go new file mode 100644 index 0000000000..157b1a6790 --- /dev/null +++ b/internal/check/edit/drawio_note_picture_test.go @@ -0,0 +1,123 @@ +package edit + +import ( + "bytes" + "encoding/base64" + "image" + "image/png" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/ir/view" +) + +const drawioAnnotated = `package Plant { + private import Views::*; + private import StandardViewDefinitions::*; + private import DiagramLayout::*; + + part def Pump; + part def Loop { + part pump : Pump; + } + view loopView { + expose Loop::*; + render asInterconnectionDiagram; + metadata Layout about Loop::pump { x = 100; y = 100; width = 120; height = 60; } + metadata Note about Loop::pump { text = "runs"; x = 300; y = 100; width = 100; height = 40; } + @Picture { location = "local.png"; x = 0; y = 0; width = 40; height = 30; alt = "the site"; } + } +} +` + +// drawioAnnotatedModel loads drawioAnnotated from a directory holding local.png +// and other.png, so its Picture reads and a diagram may point at the other file. +func drawioAnnotatedModel(t *testing.T) (Model, *view.Rendering, view.Options, string) { + t.Helper() + dir := t.TempDir() + for name, side := range map[string]int{"local.png": 2, "other.png": 3} { + var data bytes.Buffer + if err := png.Encode(&data, image.NewRGBA(image.Rect(0, 0, side, side))); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), data.Bytes(), 0o600); err != nil { + t.Fatal(err) + } + } + m, rendering, options := drawioModelOf(t, loadContent(t, filepath.Join(dir, "model.sysml"), drawioAnnotated), "Plant::loopView") + return m, rendering, options, dir +} + +func TestDrawioImportAppliesANoteText(t *testing.T) { + m, rendering, options, dir := drawioAnnotatedModel(t) + document, _ := drawioExport(t, rendering, options) + for _, tc := range []struct{ value, text, literal string }{ + {"Checked weekly", "Checked weekly", `"Checked weekly"`}, + {"Checked weekly by the "operator"", "Checked weekly\nby the \"operator\"", `"Checked weekly\nby the \"operator\""`}, + } { + edited := drawioMutate(t, document, "note0", func(cell string) string { return drawioSetValue(cell, tc.value) }) + _, imp := drawioImport(t, rendering, options, edited) + if len(imp.Edits) != 1 || imp.Edits[0].Kind != view.DrawioEditNote || imp.Edits[0].Note.Text != tc.text || imp.Edits[0].Note.X != 300 { + t.Fatalf("edits = %+v", imp.Edits) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + want := `metadata Note about Loop::pump { text = ` + tc.literal + `; x = 300; y = 100; width = 100; height = 40; }` + if !strings.Contains(string(res.Content), want) { + t.Errorf("applied:\n%s\nwant %s", res.Content, want) + } + _, redrawn, reoptions := drawioModelOf(t, loadContent(t, filepath.Join(dir, "model.sysml"), string(res.Content)), "Plant::loopView") + _, re := drawioExport(t, redrawn, reoptions) + if note := re.Cell("note0"); note == nil || note.Value != tc.text { + t.Errorf("re-exported note = %+v, want text %q", note, tc.text) + } + if _, again := drawioImport(t, redrawn, reoptions, edited); len(again.Edits) != 0 || len(again.Notices) != 0 { + t.Errorf("the edited file still imports %+v %q", again.Edits, again.Notices) + } + } +} + +func TestDrawioImportAppliesAPictureSource(t *testing.T) { + m, rendering, options, dir := drawioAnnotatedModel(t) + document, exported := drawioExport(t, rendering, options) + if exported.Cell("picture0") == nil { + t.Fatalf("no picture drawn:\n%s", document) + } + other, err := os.ReadFile(filepath.Join(dir, "other.png")) + if err != nil { + t.Fatal(err) + } + edited := drawioMutate(t, document, "picture0", func(cell string) string { + return regexp.MustCompile(`image=[^;"]*;`).ReplaceAllLiteralString(cell, "image=file://"+filepath.ToSlash(filepath.Join(dir, "other.png"))+";") + }) + _, imp := drawioImport(t, rendering, options, edited) + if len(imp.Edits) != 1 || imp.Edits[0].Kind != view.DrawioEditPicture || imp.Edits[0].Picture.Location != "other.png" || imp.Edits[0].Picture.Alt != "the site" { + t.Fatalf("edits = %+v notices %q", imp.Edits, imp.Notices) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + want := `@Picture { location = "other.png"; x = 0; y = 0; width = 40; height = 30; alt = "the site"; }` + if !strings.Contains(string(res.Content), want) { + t.Errorf("applied:\n%s\nwant %s", res.Content, want) + } + _, redrawn, reoptions := drawioModelOf(t, loadContent(t, filepath.Join(dir, "model.sysml"), string(res.Content)), "Plant::loopView") + _, re := drawioExport(t, redrawn, reoptions) + if picture := re.Cell("picture0"); picture == nil || picture.Style["image"] != "data:image/png,"+base64.StdEncoding.EncodeToString(other) { + t.Errorf("re-exported picture = %+v, want the other file's data", picture) + } + + // Embedded data is reported and the picture keeps its file. + edited = drawioMutate(t, document, "picture0", func(cell string) string { + return regexp.MustCompile(`image=[^;"]*;`).ReplaceAllLiteralString(cell, "image=data:image/png,"+base64.StdEncoding.EncodeToString(other)+";") + }) + if _, imp := drawioImport(t, rendering, options, edited); len(imp.Edits) != 0 || len(imp.Notices) != 1 || !strings.Contains(imp.Notices[0], "embedded image data") { + t.Errorf("edits %+v notices %q", imp.Edits, imp.Notices) + } +} diff --git a/internal/check/edit/drawio_test.go b/internal/check/edit/drawio_test.go index d94c60b0b0..c5300ae464 100644 --- a/internal/check/edit/drawio_test.go +++ b/internal/check/edit/drawio_test.go @@ -218,6 +218,15 @@ var drawioMutations = []drawioMutation{ t.Errorf("relabelled %q, re-exported as %q", edited.Label, re.Label) } }}, + {name: "note text", pick: drawioNote, kind: view.DrawioEditNote, + edit: func(cell string) string { return drawioSetValue(cell, "Checked weekly by hand") }, + check: func(t *testing.T, edited, re *view.DrawioCell) { + if re.Value != edited.Value || edited.Value != "Checked weekly\nby hand" { + t.Errorf("reworded to %q, re-exported as %q", edited.Value, re.Value) + } + }}, + {name: "note emptied", pick: drawioNote, notice: "was emptied", + edit: func(cell string) string { return drawioSetValue(cell, "") }}, {name: "move", pick: drawioLeaf, edit: func(cell string) string { return drawioShift(cell, "x", 77) }, kind: view.DrawioEditLayout, check: func(t *testing.T, edited, re *view.DrawioCell) { @@ -268,6 +277,16 @@ var drawioMutations = []drawioMutation{ }}, } +// drawioNote is the first note box the file draws. +func drawioNote(f *view.DrawioFile, _ func(*view.DrawioCell) *view.DrawioImport) *view.DrawioCell { + return drawioFirst(f, func(c *view.DrawioCell) bool { return !c.Edge && c.FQN == "" && strings.HasPrefix(c.ID, "note") }) +} + +// drawioSetValue sets the cell's value attribute, XML-escaped as given. +func drawioSetValue(cell, value string) string { + return regexp.MustCompile(`value="[^"]*"`).ReplaceAllLiteralString(cell, `value="`+value+`"`) +} + // drawioShift adds delta to one attribute of the cell's geometry. func drawioShift(cell, attr string, delta float64) string { m := regexp.MustCompile(`]*\b` + attr + `="([^"]*)"`).FindStringSubmatchIndex(cell) diff --git a/internal/check/edit/layout.go b/internal/check/edit/layout.go index 2af632216a..74a735cddb 100644 --- a/internal/check/edit/layout.go +++ b/internal/check/edit/layout.go @@ -288,6 +288,9 @@ func realLiteral(v float64) string { func stringLiteral(s string) string { s = strings.ReplaceAll(s, `\`, `\\`) s = strings.ReplaceAll(s, `"`, `\"`) + s = strings.ReplaceAll(s, "\r", `\r`) + s = strings.ReplaceAll(s, "\n", `\n`) + s = strings.ReplaceAll(s, "\t", `\t`) return `"` + s + `"` } diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 4be4b1cbb4..5cbd9c8f5c 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -2,8 +2,12 @@ package view import ( "fmt" + "github.com/Open-MBEE/OpenSysML/internal/ir/imagefile" "html" "math" + "net/url" + "os" + "path/filepath" "sort" "strconv" "strings" @@ -471,10 +475,15 @@ func (m *drawioMatcher) copyDisagrees(r drawioRename, byCell map[string]drawioRe // writes once a user turns formatted text on decoded back to the lines and // characters the writer emitted. func drawioLabelText(cell *DrawioCell) string { - if cell.Style["html"] != "1" { - return cell.Label + return drawioText(cell.Label, cell.Style) +} + +// drawioText is the plain text of a cell's text, decoded when its style formats it. +func drawioText(text string, style map[string]string) string { + if style["html"] != "1" { + return text } - return drawioHTMLText(cell.Label) + return drawioHTMLText(text) } // drawioHTMLText decodes draw.io's formatted-text HTML to plain text: a @@ -711,34 +720,95 @@ func (m *drawioMatcher) noteText(anchor *DrawioCell) string { return anchor.Source } -// diffNote imports a moved, resized or reworded note box. +// diffNote imports a moved, resized or reworded note box; a note emptied of +// its text is reported, as a Note with no text is not a Note. func (m *drawioMatcher) diffNote(cell, base *DrawioCell) { i, err := strconv.Atoi(strings.TrimPrefix(cell.ID, "note")) if err != nil || i < 0 || i >= len(m.norm.Notes) { return } - if sameBox(boxOf(cell), boxOf(base)) && cell.Value == base.Value { + text, box := drawioText(cell.Value, cell.Style), boxOf(cell) + if sameBox(box, boxOf(base)) && text == base.Value { return } note := m.norm.Notes[i] - box := boxOf(cell) - m.out.Edits = append(m.out.Edits, DrawioEdit{Kind: DrawioEditNote, Cell: cell.ID, Label: cell.Value, Origin: note.Origin, Index: i, - Note: &semantics.Note{Text: cell.Value, X: box.X, Y: box.Y, Width: box.Width, Height: box.Height, HasSize: true}}) + if strings.TrimSpace(text) == "" { + m.out.Notices = append(m.out.Notices, fmt.Sprintf("note %s (%s) was emptied; a Note with no text is not a Note, so the note is left as it was", cell.ID, strconv.Quote(firstLine(base.Value)))) + return + } + m.out.Edits = append(m.out.Edits, DrawioEdit{Kind: DrawioEditNote, Cell: cell.ID, Label: text, Origin: note.Origin, Index: i, + Note: &semantics.Note{Text: text, X: box.X, Y: box.Y, Width: box.Width, Height: box.Height, HasSize: true}}) } -// diffPicture imports a moved or resized picture; its bytes stay the file's. +// diffPicture imports a moved or resized picture, or one whose image was changed +// to a file the model can locate; embedded data or a URL is reported, since no +// Picture location names them, and the picture keeps its file. func (m *drawioMatcher) diffPicture(cell, base *DrawioCell) { i, err := strconv.Atoi(strings.TrimPrefix(cell.ID, "picture")) if err != nil || i < 0 || i >= len(m.norm.Pictures) { return } - if sameBox(boxOf(cell), boxOf(base)) && cell.Value == base.Value { + picture, box := m.norm.Pictures[i], boxOf(cell) + location, changed := picture.Location, cell.Style["image"] != base.Style["image"] + if changed { + if replaced, problem := drawioPictureLocation(cell.Style["image"], picture.Dir); problem != "" { + m.out.Notices = append(m.out.Notices, fmt.Sprintf("picture %s (%s): its image was changed to %s, so the picture keeps its file", cell.ID, picture.Location, problem)) + changed = false + } else { + location = replaced + } + } + if !changed && sameBox(box, boxOf(base)) && cell.Value == base.Value { return } - picture := m.norm.Pictures[i] - box := boxOf(cell) m.out.Edits = append(m.out.Edits, DrawioEdit{Kind: DrawioEditPicture, Cell: cell.ID, Label: cell.Value, Origin: picture.Origin, Index: i, - Picture: &semantics.Picture{Location: picture.Location, X: box.X, Y: box.Y, Width: box.Width, Height: box.Height, Alt: cell.Value, Above: picture.Above}}) + Picture: &semantics.Picture{Location: location, X: box.X, Y: box.Y, Width: box.Width, Height: box.Height, Alt: cell.Value, Above: picture.Above}}) +} + +// drawioPictureLocation is the Picture location a draw.io image source names: +// a path, kept relative to the view's directory dir as the Picture states it, +// or a file URL made one. Embedded data, a URL no drawing tool reads, and a +// file that does not read as an image are a problem, worded for a notice. +func drawioPictureLocation(src, dir string) (location, problem string) { + src = strings.TrimSpace(src) + switch { + case src == "": + return "", "no image" + case strings.HasPrefix(strings.ToLower(src), "data:"): + return "", "embedded image data, which no Picture location names (a Picture names a file)" + case strings.HasPrefix(strings.ToLower(src), "file:"): + u, err := url.Parse(src) + if err != nil || u.Path == "" || (u.Host != "" && u.Host != "localhost") { + return "", fmt.Sprintf("%s, a file URL naming no local file", src) + } + location = filepath.FromSlash(u.Path) + case RemotePictureLocation(src): + return "", fmt.Sprintf("%s, a URL no drawing tool reads", src) + default: + location = src + } + if dir != "" && filepath.IsAbs(location) { + if rel, err := filepath.Rel(dir, location); err == nil && !strings.HasPrefix(rel, "..") { + location = filepath.ToSlash(rel) + } + } + data, err := os.ReadFile(Picture{Location: location, Dir: dir}.Path()) // #nosec G304 -- the user named the file in the diagram. + if err != nil { + return "", fmt.Sprintf("%q, a file that does not read", location) + } + if imagefile.ContentType(data) == "" { + return "", fmt.Sprintf("%q, which is not an image", location) + } + if err := CheckPicture(location, data); err != nil { + return "", fmt.Sprintf("%q, which is not drawn: %v", location, err) + } + return location, "" +} + +// firstLine is the first line of text, for a notice naming a note. +func firstLine(text string) string { + line, _, _ := strings.Cut(text, "\n") + return line } // canvas imports a changed page size as the view's Canvas. diff --git a/internal/ir/view/drawio_import_note_picture_test.go b/internal/ir/view/drawio_import_note_picture_test.go new file mode 100644 index 0000000000..94d18c5261 --- /dev/null +++ b/internal/ir/view/drawio_import_note_picture_test.go @@ -0,0 +1,218 @@ +package view + +import ( + "bytes" + "image" + "image/png" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" +) + +// setCellAttr sets the attribute name of the id to value, XML-escaped as given. +func setCellAttr(t *testing.T, document, id, name, value string) string { + t.Helper() + re := regexp.MustCompile(`(]*? ` + name + `=")[^"]*(")`) + if !re.MatchString(document) { + t.Fatalf("no %s on cell %s", name, id) + } + return re.ReplaceAllString(document, "${1}"+strings.ReplaceAll(value, "$", "$$")+"${2}") +} + +// setImage replaces the image= source in the style of the cell id. +func setImage(t *testing.T, document, id, src string) string { + t.Helper() + whole, _ := drawioCellXML(t, document, id) + re := regexp.MustCompile(`image=[^;"]*;`) + if !re.MatchString(whole) { + t.Fatalf("no image on cell %s", id) + } + return strings.Replace(document, whole, re.ReplaceAllLiteralString(whole, "image="+src+";"), 1) +} + +func drawioNoteCase(t *testing.T) (golden string, noteText string, noteY float64) { + t.Helper() + raw, err := os.ReadFile(filepath.Join("testdata", layoutCase.name+".drawio.golden")) + if err != nil { + t.Fatal(err) + } + file, err := ReadDrawio(raw, "") + if err != nil { + t.Fatal(err) + } + note := file.Cell("note0") + if note == nil || note.Value == "" { + t.Fatal("the golden draws no note0") + } + return string(raw), note.Value, note.Geometry.Y +} + +func TestDrawioImportRewordedNoteIsANote(t *testing.T) { + golden, _, y := drawioNoteCase(t) + for name, tc := range map[string]struct{ value, html, want string }{ + "single line": {"Checked weekly", "", "Checked weekly"}, + "multi line": {"Checked weekly by the operator", "", "Checked weekly\nby the operator"}, + "formatted": {"Checked&nbsp;weekly<br>by the &lt;operator&gt;", "1", "Checked weekly\nby the "}, + } { + t.Run(name, func(t *testing.T) { + document := setCellAttr(t, golden, "note0", "value", tc.value) + if tc.html != "" { + document = strings.Replace(document, `shape=note;html=0;`, `shape=note;html=`+tc.html+`;`, 1) + } + imp := drawioGoldenImport(t, layoutCase, document) + edit := onlyEdit(t, imp, DrawioEditNote) + if edit.Note == nil || edit.Note.Text != tc.want || edit.Note.Y != y || edit.Index != 0 || edit.Origin.Doc == "" { + t.Errorf("note = %+v at %+v", edit.Note, edit.Origin) + } + if len(imp.Notices) != 0 { + t.Errorf("notices = %q", imp.Notices) + } + }) + } +} + +func TestDrawioImportRewordedAndMovedNoteIsOneNote(t *testing.T) { + golden, _, _ := drawioNoteCase(t) + document := mutateCell(t, setCellAttr(t, golden, "note0", "value", "Moved and reworded"), "note0", func(cell string) string { + return setGeometryAttr(t, cell, "y", "333") + }) + edit := onlyEdit(t, drawioGoldenImport(t, layoutCase, document), DrawioEditNote) + if edit.Note == nil || edit.Note.Text != "Moved and reworded" || edit.Note.Y != 333 { + t.Errorf("note = %+v", edit.Note) + } +} + +func TestDrawioImportReportsAnEmptiedNote(t *testing.T) { + golden, text, _ := drawioNoteCase(t) + for _, value := range []string{"", " ", "<br>"} { + document := setCellAttr(t, golden, "note0", "value", value) + if value == "<br>" { + document = strings.Replace(document, `shape=note;html=0;`, `shape=note;html=1;`, 1) + } + imp := drawioGoldenImport(t, layoutCase, document) + want := "note note0 (" + strconv.Quote(firstLine(text)) + ") was emptied; a Note with no text is not a Note, so the note is left as it was" + if len(imp.Edits) != 0 || len(imp.Notices) != 1 || imp.Notices[0] != want { + t.Errorf("value %q: edits %+v notices %q, want %q", value, imp.Edits, imp.Notices, want) + } + } +} + +// drawioPicturedRendering renders two pictures of local.png under dir, beside +// other.png, which the diagram may point a picture at instead. +func drawioPicturedRendering(t *testing.T) (*Rendering, string, string) { + t.Helper() + dir := t.TempDir() + for name, side := range map[string]int{"local.png": 2, "other.png": 3} { + var data bytes.Buffer + if err := png.Encode(&data, image.NewRGBA(image.Rect(0, 0, side, side))); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), data.Bytes(), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("not an image"), 0o600); err != nil { + t.Fatal(err) + } + rendering := &Rendering{View: "Views::pictured", Kind: KindTree, Roots: []*Node{{ID: "n", Kind: "part", Name: "part"}}, + Pictures: []Picture{ + {Location: "local.png", Dir: dir, X: 10, Y: 20, Width: 40, Height: 30, Alt: "the site"}, + {Location: "local.png", Dir: dir, X: 60, Y: 20, Width: 40, Height: 30, Above: true}, + }} + exported, err := rendering.Drawio() + if err != nil { + t.Fatal(err) + } + return rendering, exported, dir +} + +func drawioPicturedImport(t *testing.T, rendering *Rendering, document string) *DrawioImport { + t.Helper() + file, err := ReadDrawio([]byte(document), "") + if err != nil { + t.Fatal(err) + } + imp, err := rendering.ImportDrawio(file, Options{}) + if err != nil { + t.Fatal(err) + } + return imp +} + +func TestDrawioImportChangedPictureSourceIsAPicture(t *testing.T) { + rendering, exported, dir := drawioPicturedRendering(t) + if imp := drawioPicturedImport(t, rendering, exported); len(imp.Edits) != 0 || len(imp.Notices) != 0 { + t.Fatalf("untouched: edits %+v notices %q", imp.Edits, imp.Notices) + } + outside := filepath.Join(filepath.Dir(dir), filepath.Base(dir)+"-outside.png") + if err := os.WriteFile(outside, mustReadFile(t, filepath.Join(dir, "other.png")), 0o600); err != nil { + t.Fatal(err) + } + defer os.Remove(outside) + for name, tc := range map[string]struct{ src, location string }{ + "relative path": {"other.png", "other.png"}, + "absolute path": {filepath.Join(dir, "other.png"), "other.png"}, + "file URL": {"file://" + filepath.ToSlash(filepath.Join(dir, "other.png")), "other.png"}, + "outside dir": {"file://" + filepath.ToSlash(outside), outside}, + } { + t.Run(name, func(t *testing.T) { + imp := drawioPicturedImport(t, rendering, setImage(t, exported, "picture0", tc.src)) + edit := onlyEdit(t, imp, DrawioEditPicture) + if edit.Picture == nil || edit.Picture.Location != tc.location || edit.Picture.X != 10 || edit.Picture.Alt != "the site" || edit.Index != 0 { + t.Errorf("picture = %+v", edit.Picture) + } + if len(imp.Notices) != 0 { + t.Errorf("notices = %q", imp.Notices) + } + }) + } + t.Run("source and move", func(t *testing.T) { + document := mutateCell(t, setImage(t, exported, "picture1", "other.png"), "picture1", func(cell string) string { + return setGeometryAttr(t, cell, "x", "715") + }) + edit := onlyEdit(t, drawioPicturedImport(t, rendering, document), DrawioEditPicture) + if edit.Picture == nil || edit.Picture.Location != "other.png" || edit.Picture.X != 715 || !edit.Picture.Above { + t.Errorf("picture = %+v", edit.Picture) + } + }) +} + +func TestDrawioImportReportsPictureSourcesItCannotName(t *testing.T) { + rendering, exported, _ := drawioPicturedRendering(t) + for name, tc := range map[string]struct{ src, problem string }{ + "data URI": {"data:image/png,iVBORw0KGgo=", "embedded image data, which no Picture location names (a Picture names a file)"}, + "remote URL": {"https://example.org/bench.png", "https://example.org/bench.png, a URL no drawing tool reads"}, + "missing": {"missing.png", `"missing.png", a file that does not read`}, + "not image": {"notes.txt", `"notes.txt", which is not an image`}, + "no image": {"", "no image"}, + } { + t.Run(name, func(t *testing.T) { + imp := drawioPicturedImport(t, rendering, setImage(t, exported, "picture0", tc.src)) + want := "picture picture0 (local.png): its image was changed to " + tc.problem + if len(imp.Edits) != 0 || len(imp.Notices) != 1 || !strings.HasPrefix(imp.Notices[0], want) || !strings.HasSuffix(imp.Notices[0], ", so the picture keeps its file") { + t.Errorf("edits %+v notices %q, want %q", imp.Edits, imp.Notices, want) + } + }) + } + // Moved as well: the move imports with the picture's own file, the source is reported. + document := mutateCell(t, setImage(t, exported, "picture0", "data:image/png,iVBORw0KGgo="), "picture0", func(cell string) string { + return setGeometryAttr(t, cell, "x", "15") + }) + imp := drawioPicturedImport(t, rendering, document) + edit := onlyEdit(t, imp, DrawioEditPicture) + if edit.Picture == nil || edit.Picture.Location != "local.png" || edit.Picture.X != 15 || len(imp.Notices) != 1 { + t.Errorf("picture = %+v notices %q", edit.Picture, imp.Notices) + } +} + +func mustReadFile(t *testing.T, path string) []byte { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return data +} From 4ec39dfecd6d9402cc054af5147d59d76114771b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:15:24 +0000 Subject: [PATCH 03/12] test(view): read a rename and a note retyped in draw.io desktop 32.6.1 The compressed fixture is the layout golden as desktop saved it after the pump's text and the note's were retyped: only the object's label changes, the line break is and html=0 stays, so the import sees one rename and one note text and nothing else. Co-Authored-By: jason.han --- internal/ir/view/drawio_read_test.go | 48 +++++++++++++++++++ .../testdata/layout-desktop-rename.drawio | 1 + 2 files changed, 49 insertions(+) create mode 100644 internal/ir/view/testdata/layout-desktop-rename.drawio diff --git a/internal/ir/view/drawio_read_test.go b/internal/ir/view/drawio_read_test.go index ed997713dd..f45ec297bb 100644 --- a/internal/ir/view/drawio_read_test.go +++ b/internal/ir/view/drawio_read_test.go @@ -154,3 +154,51 @@ func TestReadDrawioKeepsUnknownCellsApart(t *testing.T) { t.Errorf("the root cells are listed") } } + +// The renamed fixture is the layout golden as draw.io desktop 32.6.1 saved it +// (File › Properties › Compressed) after the pump's text was retyped as +// "«part»⏎pumpX" and the note's as "Checked weekly⏎by hand": desktop rewrites +// only the object's label attribute, with the line break as and html=0 +// kept, so the import sees one rename and one note text. +func TestReadDrawioCompressedDesktopRenameAndNote(t *testing.T) { + data, err := os.ReadFile(filepath.Join("testdata", "layout-desktop-rename.drawio")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `compressed="true"`) || strings.Contains(string(data), "7Zlbc+o2EIB/DY/N2DK3PgKB04fTmUzTafsq7AWryJIrixD667uyZSzfQiDONNMpk8lYq9Vt99u1FkbBKnn9pmga/ywj4CPiRa+j4HFEyCyY438jOBeC6YwUgr1iUSHyK8Ez+xus0LPSI4sgqylqKblmaV0YSiEg1DUZVUqe6mo7yeurpnQPLcFzSHlb+juLdGylxPOqjp+A7WO79LzsSGipbAVZTCN5ckTBehSslJS6eEpeV8CN7Uq7FOM2Pb2XjSkQumOA3P5p7EE8TrfoklxhtCKjxRKHGHkEO9NeLkdkOiIBGjxYfpcytYb6SxRjnjg18y/wr+o9MGG34UyW97wwODkDf8NmVoxOOQ0hMoJCkzNxKDQ5PcujfsjOWYIbXfiTQqE8qaisdbGCe2xriUyfS6dlJ5bg6thaxtrM+WgOd4qZhucUd4GCE9KKsp0U+tkO9LFNOdubg4c4OygUvIDSDHFY2A4tzbBM47EtrGPc2zKUnNM0Y9vLTNYcOBxenX3aY3wDmYBWZ1SJHXr8qQ2XU8VaMLERY6eZWJ5sQPkz26aW9P1l6goGfLCmK5sWj/eg0oQkPSbpH1coKTyOit28DEXKrEGK3ybFegb6w0Z4LYCUPIooH5JDoULr6/z8V5C6ilDCosgstNwxzleSS5UvGmw26/nyMadLyQM4PcXH4rqhCePG+QvFKC8ZtvvzO5i+i8Rg0gJxMq5x6E9rHP5I+jG0S/2ClFGxx4011pr2rlXORjnaUlANS+OZrMX25RT34j5ZI1gcdD/XT4bm/MnVdMiWA5I9b4EdLIT/cCUTinYqrEju49XB2SXJtHMDPcmMaSY7kf7eUCiy49UI2EqtZXJ3krSkKeBUs5f6i/rkvqRrvHoPk6uEPkkmdDXkhzriZbOcQO52meVgUBb7Uy+Gz+E9mbfS+6TMS0gDUHKFzH8rx95F2NTrgMmr0XRL+vvAW3iyZuLNrPSr8XT+5Ch+UlLy207HrESuZiXyf1b6D2Sld6Sd7Jim/Nxk0JZmxheDpZ+gTiJ0IAhRq7Jr1wzyqELoeMfWaDUzlRcqIfOyog6wAyuIaGHKzkq1uM2VxWPQdb/z8LPZXMjD4mIPVfC0YswhsJO2XpRwZ/TsKKQGrKyftLFXrzVmjZK0qd4sTZolbEN/8ub0+FDs9z6GWwWzkBq8fhKc6jGmaeFq/Y4KMrOZaeykGA473V874ngm9tgeV7mG8qNdfRVDeECyMGkAHEw0Xd7+2zznUIyru2707Vv2dFbLRZdS0vqDTG9/qfWYPVhQEcZS2QVujs3Kd2/HpeOsiGbx5XrRDsuOUOsx4rtj7BaTvHHur0Ji4TG4k7aOK5RXv0JNG+lifPsXGT2mHYC2tjOGpo10G/FTaCNfn7bQ5D2zRwVZdlQwVI7zg/rXFg3ogvl8KBMPQF37pjwwdfDRe8QtJgm+PnU7BYOR1vh+jJTgfYA0bFY/EBRXoOpXlmD9Dw== \ No newline at end of file From 0ffeaa6dde95e33058438ceb5b8db2d7b01405fb Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:19:42 +0000 Subject: [PATCH 04/12] docs(view): say what the draw.io import renames, retypes and reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The -import-diagram usage, the import_diagram catalog row and manual pages, the MCP reference, the rendering-forms and layout-annotation notes and the editors guide name the three new kinds of edit — a retyped name, a retyped note, a re-pointed picture — the rule a rename follows (the writer's label with only the name changed; the path a Layout would use, so an inherited member renames its declaration) and each case that is reported rather than imported. Co-Authored-By: jason.han --- cmd/sysml/usage.go | 2 +- docs/guide/08-editors.md | 11 ++++++---- docs/project/diagram-layout-annotations.md | 5 ++++- docs/project/view-rendering-forms.md | 24 ++++++++++++++++++++-- docs/reference/sysml-mcp.md | 14 ++++++++----- internal/frontend/mcp/catalog/catalog.go | 2 +- packaging/man/man1/sysml-mcp.1 | 6 ++++-- packaging/man/man1/sysml.1 | 7 +++++-- 8 files changed, 53 insertions(+), 18 deletions(-) diff --git a/cmd/sysml/usage.go b/cmd/sysml/usage.go index 391699d065..d5699c8b73 100644 --- a/cmd/sysml/usage.go +++ b/cmd/sysml/usage.go @@ -739,7 +739,7 @@ func registerFlags(fs *flag.FlagSet) { fs.StringVar(&renderView, "render", "", "Render this view of the model instead of running it, in the form its render member states; # renders every file loaded and #: one element, kind being tree, interconnection, state, action, case, mixed, sequence, table or matrix, without a declared view") fs.StringVar(&renderAllDir, "render-all", "", "Render every declared view into this directory") - fs.StringVar(&importDiagramPath, "import-diagram", "", "Import this diagrams.net (draw.io) file, written by -render-form drawio and edited in draw.io, back into the model: what moved, resized, folded, re-routed, recoloured or re-sized the page becomes DiagramLayout metadata in the view's body, written through the model editor; cells the rendering lacks are reported, never created or deleted") + fs.StringVar(&importDiagramPath, "import-diagram", "", "Import this diagrams.net (draw.io) file, written by -render-form drawio and edited in draw.io, back into the model: what moved, resized, folded, re-routed, recoloured or re-sized the page becomes DiagramLayout metadata in the view's body, a retyped name renames the element and its references, a retyped note or a picture pointed at another file updates the annotation, all written through the model editor; cells the rendering lacks, a label edited beyond its name, an invalid or conflicting name, an emptied note and a picture source no file carries are reported, never created, deleted or guessed") fs.StringVar(&importView, "view", "", "The view -import-diagram imports, when the file has several pages; default the one page's view") fs.BoolVar(&dryRun, "dry-run", false, "With -import-diagram, print the planned edits and a unified diff of each file they change, writing nothing") fs.StringVar(&graphsSubject, "graphs", "", "Write the lowered graph of this action or state machine, and of every behavior it performs, as the canonical graphs:1 JSON an external analysis engine is sent, to stdout or -output") diff --git a/docs/guide/08-editors.md b/docs/guide/08-editors.md index dddf1b3855..9a8881254b 100644 --- a/docs/guide/08-editors.md +++ b/docs/guide/08-editors.md @@ -258,10 +258,13 @@ alone (Ctrl+Alt toggles them when `editor.inlayHints.enabl `SysML: Import layout from draw.io…` (beside *Export Diagram* in the title bar's `…` menu and the right-click menu) reads a `.drawio` file that *Export Diagram* wrote and you edited in draw.io desktop, the diagrams.net editor or the VS Code draw.io extension, and writes what you moved, -resized, collapsed, re-routed or recoloured — and a changed page size or a moved note — back into -the model as `DiagramLayout` metadata in the view's body, exactly as dragging a node in the -diagram panel does, in one undoable edit. Cells that name no element, elements the rendering no -longer has and cells you deleted are reported, not created or deleted. The same import is +resized, collapsed, re-routed or recoloured — and a changed page size, a moved or retyped note, a +picture pointed at another image file — back into the model as `DiagramLayout` metadata in the +view's body, exactly as dragging a node in the diagram panel does, in one undoable edit; a node, +port or edge whose text you retyped with only its name changed renames the element and every +reference to it. Cells that name no element, elements the rendering no longer has and cells you +deleted are reported, not created or deleted, as are a label edited beyond its name, a name that +is not valid or already taken, an emptied note and a picture given embedded data. The same import is `sysml model.sysml -import-diagram view.drawio -dry-run` on the command line, which prints the planned edits and a diff first. diff --git a/docs/project/diagram-layout-annotations.md b/docs/project/diagram-layout-annotations.md index 46eec49c20..d2d3a0145a 100644 --- a/docs/project/diagram-layout-annotations.md +++ b/docs/project/diagram-layout-annotations.md @@ -378,7 +378,10 @@ exactly the bytes it produced before; the existing rendering goldens pin that. `import_diagram` tool or the VS Code *Import layout from draw.io…* command, which diff it against a fresh export of the same view and write what moved as `setLayout`, `setRoute`, `setStyle`, `setCanvas`, `setNote` and `setPicture` operations — the same edits the panel - writes, view-local by default ([the round trip](view-rendering-forms.md#round-trip)). + writes, view-local by default — a retyped note or a re-pointed picture as a `setNote` or + `setPicture` with the new text or location, and a cell retyped with only its name changed as + a `rename` of the element the cell's qualified path resolves to, so a member drawn under an + inheritor renames its declaration ([the round trip](view-rendering-forms.md#round-trip)). - **Editors.** The LSP fields are what a graphical client (the VS Code diagram panel, a SysON-style editor) reads to place nodes where the model says. Writing positions back is `opensysml/applyModelEdit` with `setLayout`, `setRoute`, `setCanvas`, `setStyle`, diff --git a/docs/project/view-rendering-forms.md b/docs/project/view-rendering-forms.md index d3357e33ae..6b98e227e7 100644 --- a/docs/project/view-rendering-forms.md +++ b/docs/project/view-rendering-forms.md @@ -1562,7 +1562,18 @@ drag in the diagram panel is: a moved, resized or collapsed node is a `Layout` ( coordinates, as the notation reads them; a container the model did not place is pinned with its children so a child moved inside it lands where the file has it), a re-routed edge a `Route` (a straightened one clears it), a recoloured cell a `Style`, a changed page size a `Canvas`, a moved -note or picture a `Note` or `Picture`. Nodes and ports are matched by the `fqn` the writer stamps, +or retyped note a `Note` with the text the file has (line breaks kept; a note formatted in draw.io +is decoded from the HTML it saves), a moved picture or one pointed at another file a `Picture` +whose location names that file as the writer would, relative to the view's document (a path, an +absolute path or a `file:` URL under that directory). A node, port or named edge whose text was +retyped is a rename when the new text is the writer's own label with exactly the declared name +replaced by one SysML name — an identifier or a quoted `'…'` name, as the lexer reads it. The +rename goes through the model editor's `rename`, so the declaration and every reference to it move, +across documents and in the view's own `expose`, `about` and layout lines; its target is the +qualified path the import gives a `Layout` of the same cell, so a member drawn under an inheritor +(`Loop::sensor::waterOut`) renames the declaration that path resolves to, not a usage of it. A +cell moved and retyped at once imports as one `Layout` and one rename, the rename applied last so +the layout still resolves. Nodes and ports are matched by the `fqn` the writer stamps, edges by their ends, kind and order, notes and pictures by position; positions within half a pixel are unchanged, so an untouched export imports nothing and the routes the writer computed itself are never written back unless the user moved them. The compressed form draw.io desktop @@ -1571,7 +1582,16 @@ view named. What the import cannot carry is reported, not written: cells naming (nothing is created), elements the rendering lacks (renamed or gone), cells the file lost (nothing is deleted), a port recoloured (ports are drawn by direction), a note re-anchored, a label moved, and an edge re-routed that no `Route` steers yet, such as a typing edge drawn beside -its membership edge. `-dry-run` prints the planned edits and a unified diff and writes nothing. +its membership edge. So is a retyping the rename rule does not cover, each with a one-line reason +naming the cell: a label edited beyond its name (a keyword, `: Type`, multiplicity, parameter +line or compartment row), a new name that is not one SysML name (empty, a keyword, two words, an +unterminated quote), a name an existing member already holds (the rename check's own message), a +cell whose name is synthesized and so has none to rename, and an element the view draws twice +(an inherited member in a tree) whose copies disagree. A note emptied is reported, since a `Note` +with no text is not one, and so is a picture given embedded `data:` content, a URL, or a file +that does not read as an image — no `Picture` location names those. Retyping a name to the same +text imports nothing and reports nothing. `-dry-run` prints the planned edits and a unified diff +of every document they change and writes nothing. ## Limiting what a view draws diff --git a/docs/reference/sysml-mcp.md b/docs/reference/sysml-mcp.md index 63bd3a7f2b..b19119cdfe 100644 --- a/docs/reference/sysml-mcp.md +++ b/docs/reference/sysml-mcp.md @@ -260,7 +260,7 @@ without this page. | `documents` | List the document definitions (doc def) declared across the workspace, which render_document renders to Markdown. | no | | `format` | Format a SysML v2 / KerML document, or only the lines of a range, in the canonical style. | yes | | `hover` | Explain the element at a position: its kind, qualified name, type, value and documentation, as Markdown. | no | -| `import_diagram` | Import a draw.io file render_view wrote with form drawio and a user edited, as DiagramLayout annotations in the view's body; a dry run until apply. | yes | +| `import_diagram` | Import a draw.io file render_view wrote with form drawio and a user edited: moves, routes, styles, the page, note text and picture sources as DiagramLayout annotations in the view's body, a retyped name as a rename; the rest is reported; a dry run until apply. | yes | | `inlay_hints` | List the inferred types and values the editor would show inline for the features of a document or range. | no | | `open_document` | Hold a document open with the given text, as an editor buffer the server analyzes instead of the file on disk, so text can be checked (diagnostics, hover, render) before it is written. | yes | | `references` | Find every place in the workspace that refers to the element at a position. | no | @@ -357,11 +357,15 @@ without this page. - **Round-tripping a diagram** is `render_view` with `form: "drawio"`, an edit in draw.io, then `import_diagram` with the edited file (`file` on the host or `content` as text): the tool renders the view again, diffs the file against that export and answers the `DiagramLayout` - annotations the differences amount to — `Layout`, `Route`, `Style`, `Canvas`, `Note`, - `Picture` — as the text edits `apply_model_edit` would make, per document. Call it first + annotations the differences amount to — `Layout`, `Route`, `Style`, `Canvas`, `Note` (a moved + or retyped note), `Picture` (a moved picture or one pointed at another image file) — and, for a + cell whose text was retyped with only its name changed, the `rename` of that element and every + reference to it, as the text edits `apply_model_edit` would make, per document. Call it first without `apply` to read the plan and what it reports rather than imports (cells naming no - element, elements the rendering lacks, deleted cells, and changes no annotation carries yet, - such as a re-routed typing edge), then with `"apply": true` to write the files. + element, elements the rendering lacks, deleted cells, a label edited beyond its name, a name + that is not one or that an existing member holds, an emptied note, a picture given embedded + data or a URL, and changes no annotation carries yet, such as a re-routed typing edge), then + with `"apply": true` to write the files. - **Writing** happens only on request: a tool marked as writing (`format`, `rename`, `apply_model_edit`, `session_save`, ...) returns the edit or the file it would write and touches the disk only with `"apply": true`. The session tools that write change the session's diff --git a/internal/frontend/mcp/catalog/catalog.go b/internal/frontend/mcp/catalog/catalog.go index 7c37f409d6..cb8bb0e0d5 100644 --- a/internal/frontend/mcp/catalog/catalog.go +++ b/internal/frontend/mcp/catalog/catalog.go @@ -73,7 +73,7 @@ var Tools = []Entry{ {Name: "documents", Group: GroupLSP, Summary: "List the document definitions declared across the workspace."}, {Name: "format", Group: GroupLSP, Summary: "Format a document, or a range of it, in the canonical style; write the file with apply."}, {Name: "hover", Group: GroupLSP, Summary: "Explain the element at a position: kind, qualified name, type, value and documentation."}, - {Name: "import_diagram", Group: GroupLSP, Flags: []string{"import-diagram", "view", "dry-run"}, Summary: "Import a draw.io file render_view wrote with form drawio and a user edited, as DiagramLayout annotations in the view's body; a dry run until apply."}, + {Name: "import_diagram", Group: GroupLSP, Flags: []string{"import-diagram", "view", "dry-run"}, Summary: "Import a draw.io file render_view wrote with form drawio and a user edited: moves, routes, styles, the page, note text and picture sources as DiagramLayout annotations in the view's body, a retyped name as a rename; the rest is reported; a dry run until apply."}, {Name: "inlay_hints", Group: GroupLSP, Summary: "List the inferred types and values an editor would show inline for a document or range."}, {Name: "open_document", Group: GroupLSP, Summary: "Hold a document open with the given text, analyzed instead of the file on disk."}, {Name: "references", Group: GroupLSP, Summary: "Find every place in the workspace that refers to the element at a position."}, diff --git a/packaging/man/man1/sysml-mcp.1 b/packaging/man/man1/sysml-mcp.1 index 5757449688..38ee8a8e6d 100644 --- a/packaging/man/man1/sysml-mcp.1 +++ b/packaging/man/man1/sysml-mcp.1 @@ -261,8 +261,10 @@ documentation. .TP .B import_diagram lsp: Import a draw.io file render_view wrote with form drawio and a user -edited, as DiagramLayout annotations in the view's body; a dry run until apply -(sysml \-import\-diagram, sysml \-view, sysml \-dry\-run). +edited: moves, routes, styles, the page, note text and picture sources as +DiagramLayout annotations in the view's body, a retyped name as a rename; the +rest is reported; a dry run until apply (sysml \-import\-diagram, sysml +\-view, sysml \-dry\-run). .TP .B inlay_hints lsp: List the inferred types and values an editor would show inline for a diff --git a/packaging/man/man1/sysml.1 b/packaging/man/man1/sysml.1 index edf18c609c..c1c0b9fce0 100644 --- a/packaging/man/man1/sysml.1 +++ b/packaging/man/man1/sysml.1 @@ -376,8 +376,11 @@ Render every declared view into this directory Import this diagrams.net (draw.io) file, written by \-render\-form drawio and edited in draw.io, back into the model: what moved, resized, folded, re\-routed, recoloured or re\-sized the page becomes DiagramLayout metadata in -the view's body, written through the model editor; cells the rendering lacks -are reported, never created or deleted +the view's body, a retyped name renames the element and its references, a +retyped note or a picture pointed at another file updates the annotation, all +written through the model editor; cells the rendering lacks, a label edited +beyond its name, an invalid or conflicting name, an emptied note and a picture +source no file carries are reported, never created, deleted or guessed .TP .BR \-view " \fIview\fP" The view \-import\-diagram imports, when the file has several pages; default From de0d1e4a28307b42c881c408e4625ac4ba924e54 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:30:58 +0000 Subject: [PATCH 05/12] feat(lsp): name the new name a draw.io rename imports The importDiagram result's edit carries newName on a Rename, so an MCP host or editor sees what the element becomes, and the CLI dry run prints the rename as "Rename to " above its diff. Co-Authored-By: jason.han --- cmd/sysml/import_diagram.go | 4 ++++ docs/reference/lsp.md | 13 +++++++++---- internal/frontend/lsp/importdiagram.go | 11 ++++++----- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/cmd/sysml/import_diagram.go b/cmd/sysml/import_diagram.go index b31048d63e..4ca37fe055 100644 --- a/cmd/sysml/import_diagram.go +++ b/cmd/sysml/import_diagram.go @@ -171,6 +171,10 @@ func stageEdited(target string, content []byte, perm fs.FileMode) (string, error // reportDrawioImport lists the edits an import plans and what it leaves alone. func reportDrawioImport(imp *view.DrawioImport) { for _, e := range imp.Edits { + if e.Kind == view.DrawioEditRename { + fmt.Fprintf(os.Stdout, "%s %s to %s\n", e.Kind, drawioEditTarget(e), e.NewName) + continue + } fmt.Fprintf(os.Stdout, "%s about %s\n", e.Kind, drawioEditTarget(e)) } for _, name := range imp.Unmatched { diff --git a/docs/reference/lsp.md b/docs/reference/lsp.md index 573c78e961..200b9015c4 100644 --- a/docs/reference/lsp.md +++ b/docs/reference/lsp.md @@ -678,15 +678,20 @@ edited in draw.io, as the `DiagramLayout` edits `opensysml/applyModelEdit` would The server renders the view, exports it again as the writer would, and compares the file with that export cell by cell: only what differs becomes an edit, matched to the model by the `fqn` each cell carries (edges by their ends and kind, notes and pictures by position). The result lists -the planned `edits` (`kind`, `target`, `label`, `cell`), what is reported rather than imported — -`unmatched` elements the rendering lacks, `unknown` cells naming no element, `deleted` cells, and -`notices` for changes no annotation carries yet — and either `edit`, a `WorkspaceEdit` the client +the planned `edits` (`kind`, `target`, `label`, `cell`, and `newName` on a `Rename`, which a cell +retyped with only its name changed amounts to; a retyped note is a `Note`, a picture pointed at +another file a `Picture`), what is reported rather than imported — `unmatched` elements the +rendering lacks, `unknown` cells naming no element, `deleted` cells, and `notices` for changes no +annotation carries yet and retypings the rename rule does not cover (a label edited beyond its +name, an invalid or taken name, an emptied note, a picture given embedded data) — and either +`edit`, a `WorkspaceEdit` the client applies as it applies `opensysml/applyModelEdit`'s, or `refused`, the planned edit the model editor refused and why. An untouched export answers no edits and no `edit`. ```json { "view": "PlantViews::loopView", "dryRun": true, "version": 3, - "edits": [{ "kind": "Layout", "target": "Plant::Loop::pump", "label": "«part»\npump : Pump", "cell": "n2" }], + "edits": [{ "kind": "Layout", "target": "Plant::Loop::pump", "label": "«part»\npump : Pump", "cell": "n2" }, + { "kind": "Rename", "target": "Plant::Loop::tank", "newName": "reservoir", "label": "«part»\nreservoir : Tank", "cell": "n3" }], "edit": { "documentChanges": [ … ] } } ``` diff --git a/internal/frontend/lsp/importdiagram.go b/internal/frontend/lsp/importdiagram.go index 423385b81d..360fc987a9 100644 --- a/internal/frontend/lsp/importdiagram.go +++ b/internal/frontend/lsp/importdiagram.go @@ -34,10 +34,11 @@ type ImportDiagramParams struct { // ImportDiagramEdit is one edit the import plans: the annotation kind, the // element it is about, and the file cell it comes from. type ImportDiagramEdit struct { - Kind string `json:"kind"` - Target string `json:"target,omitempty"` - Label string `json:"label,omitempty"` - Cell string `json:"cell"` + Kind string `json:"kind"` + Target string `json:"target,omitempty"` + NewName string `json:"newName,omitempty"` + Label string `json:"label,omitempty"` + Cell string `json:"cell"` } // ImportDiagramResult is what the import plans and what it leaves alone, and @@ -125,7 +126,7 @@ func (s *Server) ImportDiagram(params *ImportDiagramParams) (*ImportDiagramResul out.Unknown = append(out.Unknown, cell.ID) } for _, e := range imp.Edits { - out.Edits = append(out.Edits, ImportDiagramEdit{Kind: e.Kind.String(), Target: importTarget(e), Label: e.Label, Cell: e.Cell}) + out.Edits = append(out.Edits, ImportDiagramEdit{Kind: e.Kind.String(), Target: importTarget(e), NewName: e.NewName, Label: e.Label, Cell: e.Cell}) } if len(imp.Edits) == 0 { return out, nil From e669898e2a4191937589161fe1c76d8170948257 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:11:59 +0000 Subject: [PATCH 06/12] fix(edit): rename what a view draws from another document A draw.io rename is applied through the view's document, and the editor only renamed a declaration of that document, so an element the view draws from another document was reported, not renamed. A rename stated DeclaredIn the declaring document now reaches it: the declaration splice rewrites that document as reference splices already did, and the import states the cell's origin document. The owner path of an inherited member is resolved in any document, as a Layout's is. Also tests a member renamed together with its owner in one import. Co-Authored-By: jason.han --- docs/project/view-rendering-forms.md | 10 ++-- internal/check/edit/drawio.go | 2 +- internal/check/edit/drawio_rename_test.go | 67 +++++++++++++++++++++++ internal/check/edit/edit.go | 5 +- internal/check/edit/rename.go | 29 +++++++++- 5 files changed, 101 insertions(+), 12 deletions(-) diff --git a/docs/project/view-rendering-forms.md b/docs/project/view-rendering-forms.md index 6b98e227e7..56f946ff98 100644 --- a/docs/project/view-rendering-forms.md +++ b/docs/project/view-rendering-forms.md @@ -1565,11 +1565,13 @@ straightened one clears it), a recoloured cell a `Style`, a changed page size a or retyped note a `Note` with the text the file has (line breaks kept; a note formatted in draw.io is decoded from the HTML it saves), a moved picture or one pointed at another file a `Picture` whose location names that file as the writer would, relative to the view's document (a path, an -absolute path or a `file:` URL under that directory). A node, port or named edge whose text was +absolute path or a `file:` URL, each under that directory: the import reads no file outside it). A node, port or named edge whose text was retyped is a rename when the new text is the writer's own label with exactly the declared name replaced by one SysML name — an identifier or a quoted `'…'` name, as the lexer reads it. The rename goes through the model editor's `rename`, so the declaration and every reference to it move, -across documents and in the view's own `expose`, `about` and layout lines; its target is the +across documents — the declaration may be in a document other than the view's — and in the view's +own `expose`, `about` and layout lines; a member renamed together with its owner is renamed first, +by the path the file still names it with; its target is the qualified path the import gives a `Layout` of the same cell, so a member drawn under an inheritor (`Loop::sensor::waterOut`) renames the declaration that path resolves to, not a usage of it. A cell moved and retyped at once imports as one `Layout` and one rename, the rename applied last so @@ -1588,8 +1590,8 @@ line or compartment row), a new name that is not one SysML name (empty, a keywor unterminated quote), a name an existing member already holds (the rename check's own message), a cell whose name is synthesized and so has none to rename, and an element the view draws twice (an inherited member in a tree) whose copies disagree. A note emptied is reported, since a `Note` -with no text is not one, and so is a picture given embedded `data:` content, a URL, or a file -that does not read as an image — no `Picture` location names those. Retyping a name to the same +with no text is not one, and so is a picture given embedded `data:` content, a URL, a file outside +the view's directory, or a file that does not read as an image — no `Picture` location names those. Retyping a name to the same text imports nothing and reports nothing. `-dry-run` prints the planned edits and a unified diff of every document they change and writes nothing. diff --git a/internal/check/edit/drawio.go b/internal/check/edit/drawio.go index f9daae3b9e..9a974514c2 100644 --- a/internal/check/edit/drawio.go +++ b/internal/check/edit/drawio.go @@ -34,7 +34,7 @@ func DrawioOperations(imp *view.DrawioImport, inline bool) []Operation { case view.DrawioEditPicture: op = SetPictureAt(e.Origin.Span, e.Picture).DeclaredIn(e.Origin.Doc) case view.DrawioEditRename: - op = Rename(e.FQN, e.NewName) + op = Rename(e.FQN, e.NewName).DeclaredIn(e.Origin.Doc) default: continue } diff --git a/internal/check/edit/drawio_rename_test.go b/internal/check/edit/drawio_rename_test.go index 9fe028a3ba..02ac0761fd 100644 --- a/internal/check/edit/drawio_rename_test.go +++ b/internal/check/edit/drawio_rename_test.go @@ -163,3 +163,70 @@ func TestDrawioImportRenamesAndMovesOneElement(t *testing.T) { t.Errorf("re-exported %q at %+v, edited %q at %+v", again.Label, again.Geometry, was.Label, was.Geometry) } } + +func TestDrawioImportRenamesAnOwnerAndItsMemberTogether(t *testing.T) { + m, rendering, options := drawioModel(t, strings.Replace(drawioLoop, "expose Loop::*;", "expose Loop::**;", 1), "Plant::loopView") + document, exported := drawioExport(t, rendering, options) + loop, pump := drawioCellOf(t, exported, "part def", "Plant::Loop"), drawioCellOf(t, exported, "part", "Plant::Loop::pump") + edited := drawioRelabel(t, document, loop.ID, func(label string) string { return strings.Replace(label, "Loop", "Circuit", 1) }) + edited = drawioRelabel(t, edited, pump.ID, func(label string) string { return strings.Replace(label, "pump", "motor", 1) }) + _, imp := drawioImport(t, rendering, options, edited) + if len(imp.Edits) != 2 || imp.Edits[0].FQN != "Plant::Loop::pump" || imp.Edits[0].NewName != "motor" || imp.Edits[1].FQN != "Plant::Loop" || imp.Edits[1].NewName != "Circuit" { + t.Fatalf("edits %+v notices %q, want the member's rename before its owner's", imp.Edits, imp.Notices) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + content := string(res.Content) + for _, want := range []string{"part def Circuit {", "part motor : Pump;", "connect motor.outlet to tank.inlet;", "expose Circuit::**;"} { + if !strings.Contains(content, want) { + t.Errorf("renamed model lacks %q:\n%s", want, content) + } + } + if strings.Contains(content, "Loop") || strings.Contains(content, "pump") { + t.Errorf("old names survive:\n%s", content) + } +} + +func TestDrawioImportRenamesWhatAnotherDocumentDeclares(t *testing.T) { + const views = "package Views {\n\tview loopView {\n\t\texpose Plant::Loop::*;\n\t\texpose Plant::Pump;\n\t}\n}\n" + plant := strings.Replace(drawioLoop, "\tview loopView {\n\t\texpose Loop::*;\n\t}\n", "", 1) + if plant == drawioLoop { + t.Fatal("the view was not cut from the plant") + } + for name, tc := range map[string]struct { + kind, fqn, from, to string + want []string + }{ + "usage": {"part", "Plant::Loop::pump", "pump", "pump2", []string{"part pump2 : Pump;", "connect pump2.outlet to tank.inlet;"}}, + "definition": {"part def", "Plant::Pump", "Pump", "Motor", []string{"part def Motor {", "part pump : Motor;", "expose Plant::Motor;"}}, + } { + t.Run(name, func(t *testing.T) { + m := loadEditableWorkspace(t, "views.sysml", views, map[string]string{"plant.sysml": plant}) + m, rendering, options := drawioModelOf(t, m, "Views::loopView") + document, exported := drawioExport(t, rendering, options) + cell := drawioCellOf(t, exported, tc.kind, tc.fqn) + _, imp := drawioImport(t, rendering, options, drawioRelabel(t, document, cell.ID, func(label string) string { return strings.Replace(label, tc.from, tc.to, 1) })) + if e := drawioRenameOf(t, imp); e.FQN != tc.fqn || e.NewName != tc.to || e.Origin.Doc != "plant.sysml" { + t.Fatalf("rename = %+v", e) + } + res, err := Apply(m, DrawioOperations(imp, false)) + if err != nil { + t.Fatal(err) + } + if len(res.Others) != 1 || res.Others[0].Name != "plant.sysml" { + t.Fatalf("other documents: %+v", res.Others) + } + content := string(res.Content) + string(res.Others[0].Content) + for _, want := range tc.want { + if !strings.Contains(content, want) { + t.Errorf("renamed documents lack %q:\nviews.sysml:\n%s\nplant.sysml:\n%s", want, res.Content, res.Others[0].Content) + } + } + if len(imp.Notices) != 0 { + t.Errorf("notices %q", imp.Notices) + } + }) + } +} diff --git a/internal/check/edit/edit.go b/internal/check/edit/edit.go index 9ddef68253..a89d7ae9fb 100644 --- a/internal/check/edit/edit.go +++ b/internal/check/edit/edit.go @@ -939,10 +939,7 @@ func (m Model) splicesFor(i int, op Operation) ([]splice, error) { } return deletes, nil } - sym, err := m.target(i, op) - if err != nil && op.Kind == OpRename { - sym, err = m.renameTarget(i, op, err) - } + sym, err := m.renameable(i, op) if err != nil { return nil, err } diff --git a/internal/check/edit/rename.go b/internal/check/edit/rename.go index 5949291d3d..6942a6c60d 100644 --- a/internal/check/edit/rename.go +++ b/internal/check/edit/rename.go @@ -46,7 +46,11 @@ func (m Model) renameSplices(i int, op Operation, sym *symbols.Symbol) ([]splice return nil, e } out := make([]splice, 0, len(occurrences)+1) - out = append(out, splice{span: ident.NameSpan, text: op.NewName, opIndex: i, target: op.Target}) + declaration := splice{span: ident.NameSpan, text: op.NewName, opIndex: i, target: op.Target} + if sym.DocName != m.Source.Name() { + declaration.doc = sym.DocName + } + out = append(out, declaration) for _, occ := range occurrences { sp := splice{span: occ.Span(), text: op.NewName, opIndex: i, target: op.Target} if occ.doc != m.Source.Name() { @@ -116,6 +120,25 @@ func (m Model) renameOccurrences(r *resolve.Resolver, sym *symbols.Symbol, ident return out, elsewhere } +// renameable is the declaration an operation renames: one of this model's own +// document, or, for a rename stated DeclaredIn another document, the one +// declared there — a view's import renames what the view draws from any +// document of the workspace, and the rename rewrites that document as it does +// the references of any other. +func (m Model) renameable(i int, op Operation) (*symbols.Symbol, error) { + var sym *symbols.Symbol + var err error + if op.Kind == OpRename && op.DeclarationDoc != "" { + sym, err = m.element(i, op) + } else { + sym, err = m.target(i, op) + } + if err != nil && op.Kind == OpRename { + sym, err = m.renameTarget(i, op, err) + } + return sym, err +} + // renameTarget is the declaration a rename names when no declaration answers // to its target: a qualified path reaching an inherited member through the // owner a rendering draws it under, as a Layout about that member states it, @@ -130,7 +153,7 @@ func (m Model) renameTarget(i int, op Operation, err error) (*symbols.Symbol, er if !ok || len(names) < 2 { return nil, err } - owner, ownerErr := m.declaredOnceIn(i, source.QualifiedNameOf(names[:len(names)-1]), op.DeclarationDoc) + owner, ownerErr := m.declaredOnceIn(i, source.QualifiedNameOf(names[:len(names)-1]), "") if ownerErr != nil { return nil, err } @@ -139,7 +162,7 @@ func (m Model) renameTarget(i int, op Operation, err error) (*symbols.Symbol, er if !ok { return nil, err } - if member.DocName != m.Source.Name() { + if member.DocName != m.Source.Name() && member.DocName != op.DeclarationDoc { return nil, &Error{ Failure: FailureUnknownTarget, OperationIndex: i, From 428ec29b62c1e535951a01880fd64f210802ce65 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:12:59 +0000 Subject: [PATCH 07/12] fix(view): rename a member before its owner in one import Renames of an owner and of one of its members in the same import were applied owner first, after which the member's path in the file no longer resolved and its rename was refused. The imported renames are now ordered deepest path first, after the one-per-element and disagreeing-copy rules have chosen which copy's rename to keep. Co-Authored-By: jason.han --- internal/ir/view/drawio_import_diff.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 5cbd9c8f5c..61ccdc8001 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -425,14 +425,16 @@ type drawioRename struct { // resolveRenames appends the renames after every other edit — the edit batch // applies in order, so the Layout, Route and Style edits are written against -// the names the file still has and the rename then moves them too — keeping -// one rename per element; copies of an element the view draws twice that +// the names the file still has and the rename then moves them too — deepest +// path first, so a member renamed along with its owner is still reached by the +// path the file names it with — keeping one rename per element; copies of an element the view draws twice that // disagree are reported and none of them is imported. func (m *drawioMatcher) resolveRenames() { byCell, seen := map[string]drawioRename{}, map[string]bool{} for _, r := range m.renames { byCell[r.base] = r } + var kept []DrawioEdit for _, r := range m.renames { if declared := m.base.Cell(r.base).FQN; seen[declared] { continue @@ -443,8 +445,12 @@ func (m *drawioMatcher) resolveRenames() { m.out.Notices = append(m.out.Notices, fmt.Sprintf("%s: the view draws it twice and the copies disagree (%q and %q), so no rename is imported", r.edit.FQN, r.edit.NewName, other)) continue } - m.out.Edits = append(m.out.Edits, r.edit) + kept = append(kept, r.edit) } + sort.SliceStable(kept, func(a, b int) bool { + return strings.Count(kept[a].FQN, "::") > strings.Count(kept[b].FQN, "::") + }) + m.out.Edits = append(m.out.Edits, kept...) } // copyDisagrees finds another drawn copy of a renamed element whose new name From 10d035720cc7499a135414ce8a18ffe9546ae04d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:12:59 +0000 Subject: [PATCH 08/12] fix(view): read a picture source only under the view's directory An edited image source naming an absolute path or file URL outside the view's directory was read to check it is an image, and a Picture keeping the absolute location was imported. Such a source is now refused before any read, with a notice, and the accepted sources are stated relative to the view's directory, as the writer states them. Co-Authored-By: jason.han --- cmd/sysml/usage.go | 2 +- docs/guide/08-editors.md | 3 +- docs/reference/lsp.md | 3 +- docs/reference/sysml-mcp.md | 2 +- internal/ir/view/drawio_import_diff.go | 36 +++++++++++++++---- .../view/drawio_import_note_picture_test.go | 30 +++++++++------- packaging/man/man1/sysml.1 | 3 +- 7 files changed, 56 insertions(+), 23 deletions(-) diff --git a/cmd/sysml/usage.go b/cmd/sysml/usage.go index d5699c8b73..eed94ab8d8 100644 --- a/cmd/sysml/usage.go +++ b/cmd/sysml/usage.go @@ -739,7 +739,7 @@ func registerFlags(fs *flag.FlagSet) { fs.StringVar(&renderView, "render", "", "Render this view of the model instead of running it, in the form its render member states; # renders every file loaded and #: one element, kind being tree, interconnection, state, action, case, mixed, sequence, table or matrix, without a declared view") fs.StringVar(&renderAllDir, "render-all", "", "Render every declared view into this directory") - fs.StringVar(&importDiagramPath, "import-diagram", "", "Import this diagrams.net (draw.io) file, written by -render-form drawio and edited in draw.io, back into the model: what moved, resized, folded, re-routed, recoloured or re-sized the page becomes DiagramLayout metadata in the view's body, a retyped name renames the element and its references, a retyped note or a picture pointed at another file updates the annotation, all written through the model editor; cells the rendering lacks, a label edited beyond its name, an invalid or conflicting name, an emptied note and a picture source no file carries are reported, never created, deleted or guessed") + fs.StringVar(&importDiagramPath, "import-diagram", "", "Import this diagrams.net (draw.io) file, written by -render-form drawio and edited in draw.io, back into the model: what moved, resized, folded, re-routed, recoloured or re-sized the page becomes DiagramLayout metadata in the view's body, a retyped name renames the element and its references, a retyped note or a picture pointed at another file updates the annotation, all written through the model editor; cells the rendering lacks, a label edited beyond its name, an invalid or conflicting name, an emptied note and a picture source no file under the view's directory carries are reported, never created, deleted or guessed") fs.StringVar(&importView, "view", "", "The view -import-diagram imports, when the file has several pages; default the one page's view") fs.BoolVar(&dryRun, "dry-run", false, "With -import-diagram, print the planned edits and a unified diff of each file they change, writing nothing") fs.StringVar(&graphsSubject, "graphs", "", "Write the lowered graph of this action or state machine, and of every behavior it performs, as the canonical graphs:1 JSON an external analysis engine is sent, to stdout or -output") diff --git a/docs/guide/08-editors.md b/docs/guide/08-editors.md index 9a8881254b..7d4ebd8ab9 100644 --- a/docs/guide/08-editors.md +++ b/docs/guide/08-editors.md @@ -264,7 +264,8 @@ view's body, exactly as dragging a node in the diagram panel does, in one undoab port or edge whose text you retyped with only its name changed renames the element and every reference to it. Cells that name no element, elements the rendering no longer has and cells you deleted are reported, not created or deleted, as are a label edited beyond its name, a name that -is not valid or already taken, an emptied note and a picture given embedded data. The same import is +is not valid or already taken, an emptied note and a picture given embedded data or a file outside +the view's directory. The same import is `sysml model.sysml -import-diagram view.drawio -dry-run` on the command line, which prints the planned edits and a diff first. diff --git a/docs/reference/lsp.md b/docs/reference/lsp.md index 200b9015c4..d74f1f265b 100644 --- a/docs/reference/lsp.md +++ b/docs/reference/lsp.md @@ -683,7 +683,8 @@ retyped with only its name changed amounts to; a retyped note is a `Note`, a pic another file a `Picture`), what is reported rather than imported — `unmatched` elements the rendering lacks, `unknown` cells naming no element, `deleted` cells, and `notices` for changes no annotation carries yet and retypings the rename rule does not cover (a label edited beyond its -name, an invalid or taken name, an emptied note, a picture given embedded data) — and either +name, an invalid or taken name, an emptied note, a picture given embedded data or a file outside +the view's directory) — and either `edit`, a `WorkspaceEdit` the client applies as it applies `opensysml/applyModelEdit`'s, or `refused`, the planned edit the model editor refused and why. An untouched export answers no edits and no `edit`. diff --git a/docs/reference/sysml-mcp.md b/docs/reference/sysml-mcp.md index b19119cdfe..bdd3f927b5 100644 --- a/docs/reference/sysml-mcp.md +++ b/docs/reference/sysml-mcp.md @@ -364,7 +364,7 @@ without this page. without `apply` to read the plan and what it reports rather than imports (cells naming no element, elements the rendering lacks, deleted cells, a label edited beyond its name, a name that is not one or that an existing member holds, an emptied note, a picture given embedded - data or a URL, and changes no annotation carries yet, such as a re-routed typing edge), then + data, a URL or a file outside the view's directory, and changes no annotation carries yet, such as a re-routed typing edge), then with `"apply": true` to write the files. - **Writing** happens only on request: a tool marked as writing (`format`, `rename`, `apply_model_edit`, `session_save`, ...) returns the edit or the file it would write and diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 61ccdc8001..5118a9b682 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -771,10 +771,34 @@ func (m *drawioMatcher) diffPicture(cell, base *DrawioCell) { Picture: &semantics.Picture{Location: location, X: box.X, Y: box.Y, Width: box.Width, Height: box.Height, Alt: cell.Value, Above: picture.Above}}) } +// drawioPictureWithin is location as a Picture under the view's directory dir +// states it, relative to dir; a file outside dir — by an absolute path, a +// `..` or a document with no directory — is not one, and is not read. +func drawioPictureWithin(location, dir string) (string, bool) { + if dir == "" { + clean := filepath.ToSlash(filepath.Clean(filepath.FromSlash(location))) + return location, !filepath.IsAbs(location) && clean != ".." && !strings.HasPrefix(clean, "../") + } + base, err := filepath.Abs(dir) + if err != nil { + return "", false + } + path, err := filepath.Abs(Picture{Location: location, Dir: dir}.Path()) + if err != nil { + return "", false + } + rel, err := filepath.Rel(base, path) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", false + } + return filepath.ToSlash(rel), true +} + // drawioPictureLocation is the Picture location a draw.io image source names: // a path, kept relative to the view's directory dir as the Picture states it, -// or a file URL made one. Embedded data, a URL no drawing tool reads, and a -// file that does not read as an image are a problem, worded for a notice. +// or a file URL made one. Embedded data, a URL no drawing tool reads, a file +// outside dir and one that does not read as an image are a problem, worded for +// a notice. func drawioPictureLocation(src, dir string) (location, problem string) { src = strings.TrimSpace(src) switch { @@ -793,11 +817,11 @@ func drawioPictureLocation(src, dir string) (location, problem string) { default: location = src } - if dir != "" && filepath.IsAbs(location) { - if rel, err := filepath.Rel(dir, location); err == nil && !strings.HasPrefix(rel, "..") { - location = filepath.ToSlash(rel) - } + within, ok := drawioPictureWithin(location, dir) + if !ok { + return "", fmt.Sprintf("%s, a file outside the view's directory, which the import does not read", src) } + location = within data, err := os.ReadFile(Picture{Location: location, Dir: dir}.Path()) // #nosec G304 -- the user named the file in the diagram. if err != nil { return "", fmt.Sprintf("%q, a file that does not read", location) diff --git a/internal/ir/view/drawio_import_note_picture_test.go b/internal/ir/view/drawio_import_note_picture_test.go index 94d18c5261..23f840017c 100644 --- a/internal/ir/view/drawio_import_note_picture_test.go +++ b/internal/ir/view/drawio_import_note_picture_test.go @@ -147,16 +147,11 @@ func TestDrawioImportChangedPictureSourceIsAPicture(t *testing.T) { if imp := drawioPicturedImport(t, rendering, exported); len(imp.Edits) != 0 || len(imp.Notices) != 0 { t.Fatalf("untouched: edits %+v notices %q", imp.Edits, imp.Notices) } - outside := filepath.Join(filepath.Dir(dir), filepath.Base(dir)+"-outside.png") - if err := os.WriteFile(outside, mustReadFile(t, filepath.Join(dir, "other.png")), 0o600); err != nil { - t.Fatal(err) - } - defer os.Remove(outside) for name, tc := range map[string]struct{ src, location string }{ "relative path": {"other.png", "other.png"}, "absolute path": {filepath.Join(dir, "other.png"), "other.png"}, "file URL": {"file://" + filepath.ToSlash(filepath.Join(dir, "other.png")), "other.png"}, - "outside dir": {"file://" + filepath.ToSlash(outside), outside}, + "dotted path": {"sub/../other.png", "other.png"}, } { t.Run(name, func(t *testing.T) { imp := drawioPicturedImport(t, rendering, setImage(t, exported, "picture0", tc.src)) @@ -181,13 +176,24 @@ func TestDrawioImportChangedPictureSourceIsAPicture(t *testing.T) { } func TestDrawioImportReportsPictureSourcesItCannotName(t *testing.T) { - rendering, exported, _ := drawioPicturedRendering(t) + rendering, exported, dir := drawioPicturedRendering(t) + outside := filepath.Join(filepath.Dir(dir), filepath.Base(dir)+"-outside.png") + if err := os.WriteFile(outside, mustReadFile(t, filepath.Join(dir, "other.png")), 0o600); err != nil { + t.Fatal(err) + } + defer os.Remove(outside) + outsideURL := "file://" + filepath.ToSlash(outside) + escape := "../" + filepath.Base(outside) for name, tc := range map[string]struct{ src, problem string }{ - "data URI": {"data:image/png,iVBORw0KGgo=", "embedded image data, which no Picture location names (a Picture names a file)"}, - "remote URL": {"https://example.org/bench.png", "https://example.org/bench.png, a URL no drawing tool reads"}, - "missing": {"missing.png", `"missing.png", a file that does not read`}, - "not image": {"notes.txt", `"notes.txt", which is not an image`}, - "no image": {"", "no image"}, + "data URI": {"data:image/png,iVBORw0KGgo=", "embedded image data, which no Picture location names (a Picture names a file)"}, + "remote URL": {"https://example.org/bench.png", "https://example.org/bench.png, a URL no drawing tool reads"}, + "outside dir": {outside, outside + ", a file outside the view's directory, which the import does not read"}, + "outside URL": {outsideURL, outsideURL + ", a file outside the view's directory, which the import does not read"}, + "escaping path": {escape, escape + ", a file outside the view's directory, which the import does not read"}, + "system file": {"/etc/hostname", "/etc/hostname, a file outside the view's directory, which the import does not read"}, + "missing": {"missing.png", `"missing.png", a file that does not read`}, + "not image": {"notes.txt", `"notes.txt", which is not an image`}, + "no image": {"", "no image"}, } { t.Run(name, func(t *testing.T) { imp := drawioPicturedImport(t, rendering, setImage(t, exported, "picture0", tc.src)) diff --git a/packaging/man/man1/sysml.1 b/packaging/man/man1/sysml.1 index c1c0b9fce0..62fd6bb96f 100644 --- a/packaging/man/man1/sysml.1 +++ b/packaging/man/man1/sysml.1 @@ -380,7 +380,8 @@ the view's body, a retyped name renames the element and its references, a retyped note or a picture pointed at another file updates the annotation, all written through the model editor; cells the rendering lacks, a label edited beyond its name, an invalid or conflicting name, an emptied note and a picture -source no file carries are reported, never created, deleted or guessed +source no file under the view's directory carries are reported, never created, +deleted or guessed .TP .BR \-view " \fIview\fP" The view \-import\-diagram imports, when the file has several pages; default From d525aa040f8c9cbb9989e0007b3178579ecc7c16 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:12:59 +0000 Subject: [PATCH 09/12] docs(mcp): describe import_diagram's renames, note text and picture sources Co-Authored-By: jason.han --- internal/frontend/mcp/lsptools/importing.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/frontend/mcp/lsptools/importing.go b/internal/frontend/mcp/lsptools/importing.go index cf7b5c13c9..90cdc10e9d 100644 --- a/internal/frontend/mcp/lsptools/importing.go +++ b/internal/frontend/mcp/lsptools/importing.go @@ -41,9 +41,11 @@ type ImportDiagramResult struct { func registerImporting(s *mcp.Server, h *Host) { mcp.AddTool(s, &mcp.Tool{ Name: "import_diagram", - Description: "Import a draw.io (diagrams.net) file that render_view wrote with form drawio and a user edited in draw.io: what moved, resized, folded, re-routed or recoloured, and the page size, " + - "become DiagramLayout annotations (Layout, Route, Style, Canvas, Note, Picture) in the view's body, applied as apply_model_edit applies its operations. " + - "Cells the rendering lacks, cells naming no element and deleted cells are reported, never created or deleted. Answers the planned edits and text changes; writes the files only with apply=true.", + Description: "Import a draw.io (diagrams.net) file that render_view wrote with form drawio and a user edited in draw.io: what moved, resized, folded, re-routed or recoloured, the page size, a note's text and a picture's file " + + "become DiagramLayout annotations (Layout, Route, Style, Canvas, Note, Picture) in the view's body, and a name retyped in a node, port or edge label becomes a rename of the element, " + + "declaration and references alike, applied as apply_model_edit applies its operations. Cells the rendering lacks, cells naming no element, deleted cells, a label edited beyond its name, " + + "an invalid or taken name, an emptied note and a picture source no file under the view's directory names are reported, never created, deleted or guessed. " + + "Answers the planned edits and text changes; writes the files only with apply=true.", Annotations: toolkit.Mutating("Import diagram", false), }, writing(h, func(ctx context.Context, _ *mcp.CallToolRequest, in ImportDiagramInput) (*mcp.CallToolResult, ImportDiagramResult, error) { if in.File == "" && in.Content == "" { From 1b16cbb20a5124e0d55d2768d9f4422aee12d1cb Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:20:33 +0000 Subject: [PATCH 10/12] docs(view): keep the rename and picture comments to two lines Co-Authored-By: jason.han --- internal/check/edit/rename.go | 7 ++----- internal/ir/view/drawio_import_diff.go | 15 ++++----------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/internal/check/edit/rename.go b/internal/check/edit/rename.go index 6942a6c60d..34d5585f2e 100644 --- a/internal/check/edit/rename.go +++ b/internal/check/edit/rename.go @@ -120,11 +120,8 @@ func (m Model) renameOccurrences(r *resolve.Resolver, sym *symbols.Symbol, ident return out, elsewhere } -// renameable is the declaration an operation renames: one of this model's own -// document, or, for a rename stated DeclaredIn another document, the one -// declared there — a view's import renames what the view draws from any -// document of the workspace, and the rename rewrites that document as it does -// the references of any other. +// renameable is the declaration an operation renames: in this model's document, +// or in the one a rename is stated DeclaredIn (a view renames what it draws from any document). func (m Model) renameable(i int, op Operation) (*symbols.Symbol, error) { var sym *symbols.Symbol var err error diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 5118a9b682..c44735f563 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -423,12 +423,8 @@ type drawioRename struct { base string } -// resolveRenames appends the renames after every other edit — the edit batch -// applies in order, so the Layout, Route and Style edits are written against -// the names the file still has and the rename then moves them too — deepest -// path first, so a member renamed along with its owner is still reached by the -// path the file names it with — keeping one rename per element; copies of an element the view draws twice that -// disagree are reported and none of them is imported. +// resolveRenames appends the renames last (the earlier edits still resolve by the old names), deepest +// path first (a member before its owner), one per element; copies that disagree are reported, not imported. func (m *drawioMatcher) resolveRenames() { byCell, seen := map[string]drawioRename{}, map[string]bool{} for _, r := range m.renames { @@ -794,11 +790,8 @@ func drawioPictureWithin(location, dir string) (string, bool) { return filepath.ToSlash(rel), true } -// drawioPictureLocation is the Picture location a draw.io image source names: -// a path, kept relative to the view's directory dir as the Picture states it, -// or a file URL made one. Embedded data, a URL no drawing tool reads, a file -// outside dir and one that does not read as an image are a problem, worded for -// a notice. +// drawioPictureLocation is the Picture location (a path relative to the view's directory dir) a draw.io +// image source names; embedded data, a URL, a file outside dir or a non-image is a problem, worded for a notice. func drawioPictureLocation(src, dir string) (location, problem string) { src = strings.TrimSpace(src) switch { From 91358424e120aa7f4d88ec5cd4054c900d722fbc Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:20:33 +0000 Subject: [PATCH 11/12] fix(view): do not follow a picture symlink out of the view's directory The directory check compared paths lexically, so a symlink under the view's directory to a file outside it passed and the file was read. Both the directory and the named file are resolved through their symlinks before the check; a file that does not exist keeps its path and is still reported as one that does not read. Co-Authored-By: jason.han --- internal/ir/view/drawio_import_diff.go | 13 +++++++++---- internal/ir/view/drawio_import_note_picture_test.go | 9 +++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index c44735f563..410e3897d4 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -767,15 +767,17 @@ func (m *drawioMatcher) diffPicture(cell, base *DrawioCell) { Picture: &semantics.Picture{Location: location, X: box.X, Y: box.Y, Width: box.Width, Height: box.Height, Alt: cell.Value, Above: picture.Above}}) } -// drawioPictureWithin is location as a Picture under the view's directory dir -// states it, relative to dir; a file outside dir — by an absolute path, a -// `..` or a document with no directory — is not one, and is not read. +// drawioPictureWithin is location relative to the view's directory dir, or false for a file outside +// dir (an absolute path, a `..`, a symlink out of it, or any absolute path when there is no dir). func drawioPictureWithin(location, dir string) (string, bool) { if dir == "" { clean := filepath.ToSlash(filepath.Clean(filepath.FromSlash(location))) return location, !filepath.IsAbs(location) && clean != ".." && !strings.HasPrefix(clean, "../") } - base, err := filepath.Abs(dir) + base, err := filepath.EvalSymlinks(dir) + if err == nil { + base, err = filepath.Abs(base) + } if err != nil { return "", false } @@ -783,6 +785,9 @@ func drawioPictureWithin(location, dir string) (string, bool) { if err != nil { return "", false } + if real, err := filepath.EvalSymlinks(path); err == nil { + path = real + } rel, err := filepath.Rel(base, path) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return "", false diff --git a/internal/ir/view/drawio_import_note_picture_test.go b/internal/ir/view/drawio_import_note_picture_test.go index 23f840017c..727e75eb7a 100644 --- a/internal/ir/view/drawio_import_note_picture_test.go +++ b/internal/ir/view/drawio_import_note_picture_test.go @@ -203,6 +203,15 @@ func TestDrawioImportReportsPictureSourcesItCannotName(t *testing.T) { } }) } + // A symlink under the directory to a file outside it is followed before the check, not after. + link := filepath.Join(dir, "link.png") + if err := os.Symlink(outside, link); err != nil { + t.Skipf("no symlink here: %v", err) + } + linked := drawioPicturedImport(t, rendering, setImage(t, exported, "picture0", "link.png")) + if want := "picture picture0 (local.png): its image was changed to link.png, a file outside the view's directory, which the import does not read, so the picture keeps its file"; len(linked.Edits) != 0 || len(linked.Notices) != 1 || linked.Notices[0] != want { + t.Errorf("symlink out: edits %+v notices %q, want %q", linked.Edits, linked.Notices, want) + } // Moved as well: the move imports with the picture's own file, the source is reported. document := mutateCell(t, setImage(t, exported, "picture0", "data:image/png,iVBORw0KGgo="), "picture0", func(cell string) string { return setGeometryAttr(t, cell, "x", "15") From d220bf8dc8213c35c1ed74c939c420410908c6f0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:24:38 +0000 Subject: [PATCH 12/12] fix(view): keep the symlink a picture source names, checking what it reaches The symlink check stored the file a link reaches as the picture's location, so a link under the view's directory was lost on import. The lexical path is kept and the resolved path is checked for containment only; a link under the directory to a file under it imports as the link. Co-Authored-By: jason.han --- internal/ir/view/drawio_import_diff.go | 23 ++++++++++++------- .../view/drawio_import_note_picture_test.go | 8 +++++++ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/ir/view/drawio_import_diff.go b/internal/ir/view/drawio_import_diff.go index 410e3897d4..3da1c94c33 100644 --- a/internal/ir/view/drawio_import_diff.go +++ b/internal/ir/view/drawio_import_diff.go @@ -774,10 +774,7 @@ func drawioPictureWithin(location, dir string) (string, bool) { clean := filepath.ToSlash(filepath.Clean(filepath.FromSlash(location))) return location, !filepath.IsAbs(location) && clean != ".." && !strings.HasPrefix(clean, "../") } - base, err := filepath.EvalSymlinks(dir) - if err == nil { - base, err = filepath.Abs(base) - } + base, err := filepath.Abs(dir) if err != nil { return "", false } @@ -785,16 +782,26 @@ func drawioPictureWithin(location, dir string) (string, bool) { if err != nil { return "", false } - if real, err := filepath.EvalSymlinks(path); err == nil { - path = real - } rel, err := filepath.Rel(base, path) - if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + if err != nil || drawioOutside(rel) { return "", false } + // The file a symlink reaches must be under dir too; the location kept is the link the user named. + if realBase, err := filepath.EvalSymlinks(base); err == nil { + if real, err := filepath.EvalSymlinks(path); err == nil { + if realRel, err := filepath.Rel(realBase, real); err != nil || drawioOutside(realRel) { + return "", false + } + } + } return filepath.ToSlash(rel), true } +// drawioOutside is whether a relative path leaves the directory it is relative to. +func drawioOutside(rel string) bool { + return rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + // drawioPictureLocation is the Picture location (a path relative to the view's directory dir) a draw.io // image source names; embedded data, a URL, a file outside dir or a non-image is a problem, worded for a notice. func drawioPictureLocation(src, dir string) (location, problem string) { diff --git a/internal/ir/view/drawio_import_note_picture_test.go b/internal/ir/view/drawio_import_note_picture_test.go index 727e75eb7a..2e8ec39c43 100644 --- a/internal/ir/view/drawio_import_note_picture_test.go +++ b/internal/ir/view/drawio_import_note_picture_test.go @@ -212,6 +212,14 @@ func TestDrawioImportReportsPictureSourcesItCannotName(t *testing.T) { if want := "picture picture0 (local.png): its image was changed to link.png, a file outside the view's directory, which the import does not read, so the picture keeps its file"; len(linked.Edits) != 0 || len(linked.Notices) != 1 || linked.Notices[0] != want { t.Errorf("symlink out: edits %+v notices %q, want %q", linked.Edits, linked.Notices, want) } + // A symlink under the directory to a file under it imports as the link the user named. + if err := os.Symlink(filepath.Join(dir, "other.png"), filepath.Join(dir, "inlink.png")); err != nil { + t.Fatal(err) + } + kept := drawioPicturedImport(t, rendering, setImage(t, exported, "picture0", "inlink.png")) + if len(kept.Edits) != 1 || kept.Edits[0].Kind != DrawioEditPicture || kept.Edits[0].Picture.Location != "inlink.png" || len(kept.Notices) != 0 { + t.Errorf("symlink in: edits %+v notices %q, want one Picture at inlink.png", kept.Edits, kept.Notices) + } // Moved as well: the move imports with the picture's own file, the source is reported. document := mutateCell(t, setImage(t, exported, "picture0", "data:image/png,iVBORw0KGgo="), "picture0", func(cell string) string { return setGeometryAttr(t, cell, "x", "15")