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..4ca37fe055 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) { @@ -170,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/cmd/sysml/usage.go b/cmd/sysml/usage.go index 391699d065..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, 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 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 dddf1b3855..7d4ebd8ab9 100644 --- a/docs/guide/08-editors.md +++ b/docs/guide/08-editors.md @@ -258,10 +258,14 @@ 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 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/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..56f946ff98 100644 --- a/docs/project/view-rendering-forms.md +++ b/docs/project/view-rendering-forms.md @@ -1562,7 +1562,20 @@ 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, 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 — 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 +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 +1584,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, 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. ## Limiting what a view draws diff --git a/docs/reference/lsp.md b/docs/reference/lsp.md index 573c78e961..d74f1f265b 100644 --- a/docs/reference/lsp.md +++ b/docs/reference/lsp.md @@ -678,15 +678,21 @@ 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 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`. ```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/docs/reference/sysml-mcp.md b/docs/reference/sysml-mcp.md index 63bd3a7f2b..bdd3f927b5 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, 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 touches the disk only with `"apply": true`. The session tools that write change the session's diff --git a/internal/check/edit/drawio.go b/internal/check/edit/drawio.go index 9e689948e6..9a974514c2 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).DeclaredIn(e.Origin.Doc) 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_note_picture_test.go b/internal/check/edit/drawio_note_picture_test.go new file mode 100644 index 0000000000..eaa563f97f --- /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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, redrawn, reoptions), redrawn.View, 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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, rendering, options), rendering.View, 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_rename_test.go b/internal/check/edit/drawio_rename_test.go new file mode 100644 index 0000000000..114e2f0b35 --- /dev/null +++ b/internal/check/edit/drawio_rename_test.go @@ -0,0 +1,232 @@ +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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, rendering, options), rendering.View, 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) + } +} + +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, drawioImporter(t, rendering, options), rendering.View, 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, drawioImporter(t, rendering, options), rendering.View, 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/drawio_test.go b/internal/check/edit/drawio_test.go index c1b1ddb9a2..301d28354d 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,22 @@ 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: "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) { @@ -253,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) @@ -264,6 +298,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*||/>)`) @@ -326,9 +400,15 @@ func TestDrawioRoundTrip(t *testing.T) { if _, imp := drawioImport(t, importer, rendering.View, 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, importer, rendering.View, drawioMutate(t, document, c.ID, mut.edit)) + _, imp := drawioImport(t, importer, rendering.View, mutate(mut, c.ID)) return imp } } @@ -338,7 +418,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, importer, rendering.View, edited) if mut.notice != "" { if len(imp.Edits) != 0 || len(imp.Notices) != 1 || !strings.Contains(imp.Notices[0], mut.notice) { @@ -370,6 +450,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) @@ -388,10 +471,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, drawioImporter(t, redrawn, reoptions), rendering.View, 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..a89d7ae9fb 100644 --- a/internal/check/edit/edit.go +++ b/internal/check/edit/edit.go @@ -939,7 +939,7 @@ func (m Model) splicesFor(i int, op Operation) ([]splice, error) { } return deletes, nil } - sym, err := m.target(i, op) + sym, err := m.renameable(i, op) if err != nil { return nil, err } 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/check/edit/rename.go b/internal/check/edit/rename.go index 10698cd898..34d5585f2e 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 @@ -44,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() { @@ -113,3 +119,52 @@ func (m Model) renameOccurrences(r *resolve.Resolver, sym *symbols.Symbol, ident sortReferrers(elsewhere) return out, elsewhere } + +// 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 + 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, +// 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]), "") + 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() && member.DocName != op.DeclarationDoc { + 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..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 @@ -111,18 +112,25 @@ 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 { 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 } - 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/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/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 == "" { 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(" 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 +// differs — a copy left as it was still shows the old name — and names it. +func (m *drawioMatcher) copyDisagrees(r drawioRename, byCell map[string]drawioRename) (string, bool) { + base := m.base.Cell(r.base) + for i := range m.base.Cells { + other := &m.base.Cells[i] + if other.ID == base.ID || other.FQN != base.FQN || other.Kind != base.Kind || m.paired[other.ID] == nil { + continue + } + if copied, ok := byCell[other.ID]; ok { + if copied.edit.NewName != r.edit.NewName { + return copied.edit.NewName, true + } + continue + } + if name, ok := m.w.names[other.ID]; ok { + return other.Label[name.Offset : name.Offset+name.Len], true + } + return other.Label, true + } + return "", false +} + +// drawioLabelText is a cell's label as plain text: the label as written when +// the cell is plain text, as the writer leaves every cell; the HTML draw.io +// writes once a user turns formatted text on decoded back to the lines and +// characters the writer emitted. +func drawioLabelText(cell *DrawioCell) string { + 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(text) +} + +// drawioHTMLText decodes draw.io's formatted-text HTML to plain text: a +// `
` 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 { @@ -561,6 +711,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 @@ -596,34 +750,128 @@ 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}}) +} + +// 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) + 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 || 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) { + 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 + } + 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) + } + 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..2e8ec39c43 --- /dev/null +++ b/internal/ir/view/drawio_import_note_picture_test.go @@ -0,0 +1,241 @@ +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) + } + 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"}, + "dotted path": {"sub/../other.png", "other.png"}, + } { + 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, 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"}, + "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)) + 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) + } + }) + } + // 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) + } + // 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") + }) + 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 +} 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/ir/view/testdata/layout-desktop-rename.drawio b/internal/ir/view/testdata/layout-desktop-rename.drawio new file mode 100644 index 0000000000..e6910cafee --- /dev/null +++ b/internal/ir/view/testdata/layout-desktop-rename.drawio @@ -0,0 +1 @@ +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 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 "" +} 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..62fd6bb96f 100644 --- a/packaging/man/man1/sysml.1 +++ b/packaging/man/man1/sysml.1 @@ -376,8 +376,12 @@ 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 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