Repository navigation
feat(view): import renames, note text and picture sources from draw.io - #1695
Merged
Merged
Conversation
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 <hanhuijun@gmail.com>
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 <hanhuijun@gmail.com>
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 <hanhuijun@gmail.com>
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 <hanhuijun@gmail.com>
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 <element> to <name>" above its diff. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
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 <hanhuijun@gmail.com>
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 <hanhuijun@gmail.com>
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 <hanhuijun@gmail.com>
…ources Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
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 <hanhuijun@gmail.com>
…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 <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
The draw.io round trip (#1679) imported what a user moves, resizes, folds, reroutes, recolours and
resizes the page to. This makes three more kinds of edit made in diagrams.net / draw.io Desktop
import back into the model, through the same baseline diff and the same model-edit path:
fqn-bearing edge whose edited label equals thewriter's own label with exactly the declared name replaced by one valid SysML name (an identifier
or a quoted
'…'name, judged by the lexer) becomesedit.Rename(target, newName), so thedeclaration and every reference across documents move — including the view's own
expose,aboutandLayoutnames. The writer records where it put each name (drawioNamespans on theobject,
labeller.headName,portView.pinLabel,edge.Label == edge.Name); nothing re-parsesthe label text. The target is the qualified path the import already uses for that cell's
Layout(Loop::sensor::waterOutfor a member drawn under an inheritor), and resolves to thedeclaration, not the usage; the docs state this. Renames are ordered after the import's
Layout/Route/Style/Note/Pictureedits so those still resolve against the old names,and a file that moves and renames the same element applies cleanly (one
Rename, oneLayout).SetNotewith the new text and the geometry it has (text and move together are still one
SetNote).html=1text is decoded as the inverse of what the writer emits (<br>,<div>, ,entities); multi-line text round-trips
\nexactly, and the model-edit writer now escapescontrol characters in string literals so the applied note is valid SysML.
image=source differs from what thewriter emits for
Picture.LocationbecomesSetPicturewith the new location: a relativepath, an absolute path or a
file:URL, each naming a readable image the renderer acceptsunder the view's directory (stored relative to it); a source outside that directory is
refused before anything is read. Geometry and
altare kept.Everything else is reported, one line per cell, never imported (exact messages below). Nothing
creates or deletes elements; the request shapes of LSP
opensysml/importDiagram, MCPimport_diagramand VS Codeopensysml.importDiagramare unchanged (the result's edit carries anewNameon aRename),editors/vscode/src/webviewis untouched.The importer's behaviour was verified against a file saved by draw.io Desktop 32.6.1 under Xvfb:
editing an
<object>cell rewrites onlylabel(novaluefallback;
for a line break,html=0kept), editing a note rewrites themxCellvalue. That compressed file is the fixtureinternal/ir/view/testdata/layout-desktop-rename.drawio(1.6 KiB), read byTestReadDrawioCompressedDesktopRenameAndNote.Reported, not imported
: Type, multiplicity, parameter line, compartment row)<cell>: its label changed beyond the name "<old>"; a keyword, type, multiplicity, parameter or compartment edit is not imported<cell>: renamed to "<new>", which <is a keyword / is not an identifier / is an unterminated quoted name / is not a single name>, so the rename is not imported<cell>: its label was edited, but the cell shows no declared name of its own (its name is synthesized or the label carries other text), so nothing is renamed and the label is not imported<cell>: the view draws it twice and the copies disagree ("<label>" and "<label>"), so no rename is importedCheckRenamemessage, answered as the import'srefusededit:<element> cannot be renamed to "<name>": that name already means <element> where <reference> reads it(rename_conflict.go, unchanged)note <id> ("<first line>") was emptied; a Note with no text is not a Note, so the note is left as it wasdata:URIpicture <id> ("<alt or location>"): its image was changed to embedded image data, which no Picture location names (a Picture names a file), so the picture keeps its filepicture <id> (…): its image was changed to <url>, a URL no drawing tool reads, so the picture keeps its filefile:URL with a host or no path… changed to <src>, a file URL naming no local file, …file:URL outside the view's directory (absolute,../, or any absolute path when the document has no directory)… changed to <src>, a file outside the view's directory, which the import does not read… changed to "<location>", a file that does not read/"<location>", which is not an image/"<location>", which is not drawn: <err>/no imageWhat changed in #1679's matcher
diffNotereads the note's text frommxCell.value(where draw.io keeps it) through theHTML decoder, and refuses empty text; before it compared geometry only.
diffPicturecompares theimage=source as well as geometry; before it compared geometry only.diffLabel); renames are collected andresolved after the matcher finishes (
resolveRenames) so duplicate drawn copies can be checkedagainst each other by their declaration's FQN and appended last, deepest path first, so a
member renamed together with its owner is still reached by the path the file names it with.
The pairing itself (by
fqn, ordinal, ends, position) is unchanged.targetNameis reused for the rename target, so inherited-member paths stay the usage path.edit.Model.renameableresolves anOpRenamestatedDeclaredInanother document in thatdocument (the import states each rename's origin document, so a view renames what it draws from
any document of the workspace, the declaration splice written there as reference splices already
were) and falls back to
renameTargetwhen the path is a usage path (owner member lookup, aslayoutElementalready does);stringLiteralescapes\n,\r,\t.lexer.NameProblemis the one rule for "a valid declared name";edit/validate.gouses it too.Specification basis
Tooling only: no language semantics change. The rename reuses the existing model-edit
renameand its conflict rules.
How it was verified
internal/ir/view:drawio_import_rename_test.go,drawio_import_note_picture_test.go,TestReadDrawioCompressedDesktopRenameAndNote(real Desktop 32.6.1 compressed file); theuntouched-export invariant over every draw.io golden on both rendering paths still holds.
internal/check/edit: rename a part def, a port (connection ends follow), an element referencedfrom another document, to a quoted name, to a conflicting name (refused), keyword/type/
multiplicity edits (reported), one of two drawn copies (reported), rename + move; note text
single/multi-line/HTML-encoded, note + move, emptied note; picture relative/absolute/file URL,
data URI, remote/missing/non-image and outside-the-view's-directory refusals; an owner and its
member renamed in one import; a view in one document renaming a usage and a definition declared
in another (both tests fail without their fix). The random-mutation property test
(
TestDrawioRoundTrip) gainedrename,note textandnote emptiedmutations, eachapplied and re-exported.
cmd/sysml,internal/frontend/lsp: dry-run diff and result shape.MCP stdio exchange (real
sysml-mcp -root ., two documents, dry run then apply)The documents after the apply:
The same import from the CLI,
sysml plant.sysml site.sysml -import-diagram loop-edited.drawio -dry-run:Gate
go test ./...with Go's default 10 m per-package timeout, run on an 8-core VM with every packagein parallel, timed out in three packages —
cmd/sysml(still runningTestExploreTablesTheSpacecraftRaceWithinItsBudget, untouched here),internal/check/edit(
TestDrawioRoundTrip, now 35 goldens × 10 mutations) andtests/codegen(untouched) — and passedeverywhere else (121
ok). The three re-run alone with the CI timeout (make testuses-timeout 45m):For scale,
internal/check/edit'sTestDrawioRoundTripondevelopmeasured 635 s against 886 shere under the same load; the added cost is the
renamemutation (≈95 s over 35 goldens), whichapplies the rename and re-exports.
After the review-round fixes (head
d525aa040: cross-document rename target, member-before-ownerorder, picture source confined to the view's directory, MCP tool description):
make build,go vet ./...,gofmt -l .(empty),make lint,make docs,scripts/changelog.py check,make man-checkall pass again; the packages touched re-ran clean —go test ./internal/check/edit -run Drawio(314 s),./internal/ir/view -run Drawio(30 s),./internal/frontend/lsp,./internal/frontend/mcp/...,./cmd/sysml -run 'ImportDiagram|Drawio|Usage|Man'.Review rounds two and three (head
d220bf8dc): the new comments were cut to two lines, and thepicture-directory check now resolves symlinks — the directory and the named file through
filepath.EvalSymlinks— for the containment test only, so a link under the view's directory to afile outside it is refused before anything is read while a link to a file under it imports as the
link the user named (
link.png→ outside is refused,inlink.png→other.pngimports atinlink.png; both asserted inTestDrawioImportReportsPictureSourcesItCannotName, which failswithout the change).
go vet,gofmt -l .,make lintandgo test ./internal/ir/view -run 'Picture|Drawio'pass; the full CI tier ond220bf8dcis green (31 checks).After merging
develop(the sharedDrawioImporterof #1694;drawio_test.goresolved onto theimporter-per-golden shape, the two added test files likewise):
make build,go vet ./...,gofmt -l .(empty),make lint,make docs,scripts/changelog.py check,make man-check,npm testandgo test -timeout 45m ./...all pass;go test -race -pgo=off -count=1 ./internal/check/edit=ok … internal/check/edit 213.619s(run alongside the lint/docs gate on an8-core VM; 162.9 s alone), the whole package well inside the CircleCI budget #1694 restored.
Checklist
make testandmake lintpass locallychanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build)F4,K5) in the body, docs, or changelogLink to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/e8b647f232d940628fb882686e2b78c1
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/e8b647f232d940628fb882686e2b78c1?variant=devin
Requested by: @HuiJun