From 4e93f70db384fc34e62de708ae42acda59d79285 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 11 Sep 2024 10:34:39 -0600 Subject: [PATCH 1/3] d2ir/imports: fix removing parent scope of nested import fields/edges --- d2compiler/compile_test.go | 4 +- d2ir/import.go | 11 ++-- .../TestCompile/vars-in-imports.exp.json | 51 ++++++++++++++----- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index f077bccd4e..662fe1062b 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -2979,8 +2979,8 @@ qa: { assertions: func(t *testing.T, g *d2graph.Graph) { tassert.Equal(t, "dev.env", g.Objects[1].AbsID()) tassert.Equal(t, "Dev Environment", g.Objects[1].Label.Value) - tassert.Equal(t, "qa.env", g.Objects[2].AbsID()) - tassert.Equal(t, "Qa Environment", g.Objects[2].Label.Value) + tassert.Equal(t, "qa.env", g.Objects[4].AbsID()) + tassert.Equal(t, "Qa Environment", g.Objects[4].Label.Value) }, }, { diff --git a/d2ir/import.go b/d2ir/import.go index c415cf1385..ef80d358cd 100644 --- a/d2ir/import.go +++ b/d2ir/import.go @@ -61,15 +61,18 @@ func (c *compiler) _import(imp *d2ast.Import) (Node, bool) { if !ok { return nil, false } - nilScopeMap(ir) if len(imp.IDA()) > 0 { f := ir.GetField(imp.IDA()...) if f == nil { c.errorf(imp, "import key %q doesn't exist inside import", imp.IDA()) return nil, false } + if f.Map() != nil { + nilScopeMap(f.Map()) + } return f, true } + nilScopeMap(ir) return ir, true } @@ -132,15 +135,9 @@ func nilScopeMap(n Node) { for _, r := range n.References { r.Context_.ScopeMap = nil } - if n.Map() != nil { - nilScopeMap(n.Map()) - } case *Field: for _, r := range n.References { r.Context_.ScopeMap = nil } - if n.Map() != nil { - nilScopeMap(n.Map()) - } } } diff --git a/testdata/d2compiler/TestCompile/vars-in-imports.exp.json b/testdata/d2compiler/TestCompile/vars-in-imports.exp.json index 05c63f5e36..fad97a8406 100644 --- a/testdata/d2compiler/TestCompile/vars-in-imports.exp.json +++ b/testdata/d2compiler/TestCompile/vars-in-imports.exp.json @@ -345,20 +345,20 @@ "zIndex": 0 }, { - "id": "env", - "id_val": "env", + "id": "vm", + "id_val": "vm", "references": [ { "key": { - "range": "d2/testdata/d2compiler/TestCompile/template.d2,0:0:0-0:3:3", + "range": "d2/testdata/d2compiler/TestCompile/template.d2,2:2:37-2:4:39", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/template.d2,0:0:0-0:3:3", + "range": "d2/testdata/d2compiler/TestCompile/template.d2,2:2:37-2:4:39", "value": [ { - "string": "env", - "raw_string": "env" + "string": "vm", + "raw_string": "vm" } ] } @@ -371,7 +371,7 @@ ], "attributes": { "label": { - "value": "Qa Environment" + "value": "My Virtual machine!" }, "labelDimensions": { "width": 0, @@ -390,20 +390,43 @@ "zIndex": 0 }, { - "id": "vm", - "id_val": "vm", + "id": "env", + "id_val": "env", + "attributes": { + "label": { + "value": "env" + }, + "labelDimensions": { + "width": 0, + "height": 0 + }, + "style": {}, + "near_key": null, + "shape": { + "value": "rectangle" + }, + "direction": { + "value": "" + }, + "constraint": null + }, + "zIndex": 0 + }, + { + "id": "env", + "id_val": "env", "references": [ { "key": { - "range": "d2/testdata/d2compiler/TestCompile/template.d2,2:2:37-2:4:39", + "range": "d2/testdata/d2compiler/TestCompile/template.d2,0:0:0-0:3:3", "path": [ { "unquoted_string": { - "range": "d2/testdata/d2compiler/TestCompile/template.d2,2:2:37-2:4:39", + "range": "d2/testdata/d2compiler/TestCompile/template.d2,0:0:0-0:3:3", "value": [ { - "string": "vm", - "raw_string": "vm" + "string": "env", + "raw_string": "env" } ] } @@ -416,7 +439,7 @@ ], "attributes": { "label": { - "value": "My Virtual machine!" + "value": "Qa Environment" }, "labelDimensions": { "width": 0, From 7c967954e0508f56bffa78e496cdff6ec8b45f56 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 11 Sep 2024 11:05:04 -0600 Subject: [PATCH 2/3] d2sequence: ensure order for child boards --- d2graph/seqdiagram.go | 4 +- d2layouts/d2sequence/sequence_diagram.go | 9 ++ e2etests-cli/main_test.go | 24 ++++ .../TestCLI_E2E/sequence-layer/index.exp.svg | 95 +++++++++++++++ .../TestCLI_E2E/sequence-layer/seq.exp.svg | 109 ++++++++++++++++++ 5 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 e2etests-cli/testdata/TestCLI_E2E/sequence-layer/index.exp.svg create mode 100644 e2etests-cli/testdata/TestCLI_E2E/sequence-layer/seq.exp.svg diff --git a/d2graph/seqdiagram.go b/d2graph/seqdiagram.go index 3cd2cf171a..472a1d3ed3 100644 --- a/d2graph/seqdiagram.go +++ b/d2graph/seqdiagram.go @@ -1,6 +1,8 @@ package d2graph -import "oss.terrastruct.com/d2/d2target" +import ( + "oss.terrastruct.com/d2/d2target" +) func (obj *Object) IsSequenceDiagram() bool { return obj != nil && obj.Shape.Value == d2target.ShapeSequenceDiagram diff --git a/d2layouts/d2sequence/sequence_diagram.go b/d2layouts/d2sequence/sequence_diagram.go index e5ec4d4d72..4b85cbe131 100644 --- a/d2layouts/d2sequence/sequence_diagram.go +++ b/d2layouts/d2sequence/sequence_diagram.go @@ -1,9 +1,11 @@ package d2sequence import ( + "cmp" "errors" "fmt" "math" + "slices" "sort" "strconv" "strings" @@ -76,6 +78,13 @@ func newSequenceDiagram(objects []*d2graph.Object, messages []*d2graph.Edge) (*s var actors []*d2graph.Object var groups []*d2graph.Object + slices.SortFunc(objects, func(a, b *d2graph.Object) int { + return cmp.Compare(getObjEarliestLineNum(a), getObjEarliestLineNum(b)) + }) + slices.SortFunc(messages, func(a, b *d2graph.Edge) int { + return cmp.Compare(getEdgeEarliestLineNum(a), getEdgeEarliestLineNum(b)) + }) + for _, obj := range objects { if obj.IsSequenceDiagramGroup() { queue := []*d2graph.Object{obj} diff --git a/e2etests-cli/main_test.go b/e2etests-cli/main_test.go index d586299121..273112a7fc 100644 --- a/e2etests-cli/main_test.go +++ b/e2etests-cli/main_test.go @@ -94,6 +94,30 @@ func TestCLI_E2E(t *testing.T) { assert.TestdataDir(t, filepath.Join(dir, "layer-link")) }, }, + { + name: "sequence-layer", + run: func(t *testing.T, ctx context.Context, dir string, env *xos.Env) { + writeFile(t, dir, "index.d2", `k; layers: { seq: @seq.d2 }`) + writeFile(t, dir, "seq.d2", `shape: sequence_diagram +a: me +b: github.com/terrastruct/d2 + +a -> b: issue about a bug +a."some note about the bug" + +if i'm right: { + a <- b: fix +} + +if i'm wrong: { + a <- b: nah, intended +}`) + err := runTestMain(t, ctx, dir, env, "index.d2") + assert.Success(t, err) + + assert.TestdataDir(t, filepath.Join(dir, "index")) + }, + }, { // Skip the empty base board so the animation doesn't show blank for 1400ms name: "empty-base", diff --git a/e2etests-cli/testdata/TestCLI_E2E/sequence-layer/index.exp.svg b/e2etests-cli/testdata/TestCLI_E2E/sequence-layer/index.exp.svg new file mode 100644 index 0000000000..29a8a55da6 --- /dev/null +++ b/e2etests-cli/testdata/TestCLI_E2E/sequence-layer/index.exp.svg @@ -0,0 +1,95 @@ +k + + + diff --git a/e2etests-cli/testdata/TestCLI_E2E/sequence-layer/seq.exp.svg b/e2etests-cli/testdata/TestCLI_E2E/sequence-layer/seq.exp.svg new file mode 100644 index 0000000000..f8f8b973d1 --- /dev/null +++ b/e2etests-cli/testdata/TestCLI_E2E/sequence-layer/seq.exp.svg @@ -0,0 +1,109 @@ +megithub.com/terrastruct/d2if i'm rightif i'm wrong fixnah, intended issue about a bugsome note about the bug + + + + + + + + + + From 90dd854778d3f69f644226e60525eaf23facbaf1 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Wed, 11 Sep 2024 11:06:30 -0600 Subject: [PATCH 3/3] next --- ci/release/changelogs/next.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index db9486ee71..c8c1b0434c 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -17,6 +17,7 @@ - Sequence diagram: multi-line edge labels no longer can collide with other elements [#2049](https://github.com/terrastruct/d2/pull/2049) - Sequence diagram: long self-referential edge labels no longer can collide neighboring actors (or its own) lifeline edges [#2050](https://github.com/terrastruct/d2/pull/2050) +- Sequence diagram: fixes layout when sequence diagrams are in children boards (e.g. a layer) [#1692](https://github.com/terrastruct/d2/issues/1692) - Globs: An edge case was fixed where globs used in edges were creating nodes when it shouldn't have [#2051](https://github.com/terrastruct/d2/pull/2051) - Render: Multi-line class labels/headers are rendered correctly [#2057](https://github.com/terrastruct/d2/pull/2057) - CLI: Watch mode uses correct backlinks (`_` usages) [#2058](https://github.com/terrastruct/d2/pull/2058)