From 21f531dff9dd6463232044055f17f5866db234b6 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Mon, 5 Dec 2022 11:10:45 -0800 Subject: [PATCH 1/9] add darge newline test --- e2etests/regression_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/e2etests/regression_test.go b/e2etests/regression_test.go index d2ec37c80..4db509e55 100644 --- a/e2etests/regression_test.go +++ b/e2etests/regression_test.go @@ -5,7 +5,12 @@ import ( ) func testRegression(t *testing.T) { - tcs := []testCase{} + tcs := []testCase{ + { + name: "dagre_id_with_newline", + script: `ninety\nnine`, + }, + } runa(t, tcs) } From 8659af5d576fe5808e90c5e8338bd70daac0ac2b Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Mon, 5 Dec 2022 11:13:19 -0800 Subject: [PATCH 2/9] fix unmarshalling id with newlines from dagre --- d2layouts/d2dagrelayout/layout.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/d2layouts/d2dagrelayout/layout.go b/d2layouts/d2dagrelayout/layout.go index 3b6885bf8..c984dc2e2 100644 --- a/d2layouts/d2dagrelayout/layout.go +++ b/d2layouts/d2dagrelayout/layout.go @@ -136,6 +136,9 @@ func Layout(ctx context.Context, g *d2graph.Graph) (err error) { if err := json.Unmarshal([]byte(val.String()), &dn); err != nil { return err } + // ID "ninety\nnine" is set in dagre with backticks: g.setNode(`"ninety\nnine"`, ...) + // but unmarshal converts \n into an actual newline, so we need to replace these to get the AbsID string and lookup the node + dn.ID = strings.ReplaceAll(dn.ID, "\n", "\\n") if debugJS { log.Debug(ctx, "graph", slog.F("json", dn)) } From e83eb998cc2e8f8dd82e7600f3ebc6b254263d45 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Mon, 5 Dec 2022 11:13:41 -0800 Subject: [PATCH 3/9] update tests --- .../dagre/board.exp.json | 45 +++++++++++++++++++ .../dagre/sketch.exp.svg | 24 ++++++++++ .../dagre_id_with_newline/elk/board.exp.json | 45 +++++++++++++++++++ .../dagre_id_with_newline/elk/sketch.exp.svg | 24 ++++++++++ 4 files changed, 138 insertions(+) create mode 100644 e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json create mode 100644 e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg create mode 100644 e2etests/testdata/regression/dagre_id_with_newline/elk/board.exp.json create mode 100644 e2etests/testdata/regression/dagre_id_with_newline/elk/sketch.exp.svg diff --git a/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json b/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json new file mode 100644 index 000000000..4366386d6 --- /dev/null +++ b/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json @@ -0,0 +1,45 @@ +{ + "name": "", + "shapes": [ + { + "id": "\"ninety\\nnine\"", + "type": "", + "pos": { + "x": 0, + "y": 0 + }, + "width": 151, + "height": 142, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "ninety\nnine", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 51, + "labelHeight": 42, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + } + ], + "connections": [] +} diff --git a/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg b/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg new file mode 100644 index 000000000..c9c9cf7da --- /dev/null +++ b/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg @@ -0,0 +1,24 @@ + +ninetynine \ No newline at end of file diff --git a/e2etests/testdata/regression/dagre_id_with_newline/elk/board.exp.json b/e2etests/testdata/regression/dagre_id_with_newline/elk/board.exp.json new file mode 100644 index 000000000..560f31355 --- /dev/null +++ b/e2etests/testdata/regression/dagre_id_with_newline/elk/board.exp.json @@ -0,0 +1,45 @@ +{ + "name": "", + "shapes": [ + { + "id": "\"ninety\\nnine\"", + "type": "", + "pos": { + "x": 12, + "y": 12 + }, + "width": 151, + "height": 142, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "ninety\nnine", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 51, + "labelHeight": 42, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + } + ], + "connections": [] +} diff --git a/e2etests/testdata/regression/dagre_id_with_newline/elk/sketch.exp.svg b/e2etests/testdata/regression/dagre_id_with_newline/elk/sketch.exp.svg new file mode 100644 index 000000000..c967c4201 --- /dev/null +++ b/e2etests/testdata/regression/dagre_id_with_newline/elk/sketch.exp.svg @@ -0,0 +1,24 @@ + +ninetynine \ No newline at end of file From 53bceb85786feee6f974ca99b2d78b7132218a87 Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Mon, 5 Dec 2022 11:40:21 -0800 Subject: [PATCH 4/9] fix dagre layout with \r in IDs --- d2compiler/compile_test.go | 11 +++++++++++ d2layouts/d2dagrelayout/layout.go | 9 +++++++-- e2etests/regression_test.go | 8 ++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index fa5f12f07..0855902f0 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1351,6 +1351,17 @@ y -> x.style b`, g.Objects[0].Attributes.Label.Value) }, }, + { + name: "unescaped_id_cr", + + text: `b\rb`, + assertions: func(t *testing.T, g *d2graph.Graph) { + if len(g.Objects) != 1 { + t.Fatal(g.Objects) + } + assert.String(t, "b\rb", g.Objects[0].Attributes.Label.Value) + }, + }, { name: "class_style", diff --git a/d2layouts/d2dagrelayout/layout.go b/d2layouts/d2dagrelayout/layout.go index c984dc2e2..cd2e44d21 100644 --- a/d2layouts/d2dagrelayout/layout.go +++ b/d2layouts/d2dagrelayout/layout.go @@ -259,15 +259,20 @@ func setGraphAttrs(attrs dagreGraphAttrs) string { ) } +func escapeID(id string) string { + return strings.ReplaceAll(id, "\r", "\\r") +} + func generateAddNodeLine(id string, width, height int) string { + id = escapeID(id) return fmt.Sprintf("g.setNode(`%s`, { id: `%s`, width: %d, height: %d });\n", id, id, width, height) } func generateAddParentLine(childID, parentID string) string { - return fmt.Sprintf("g.setParent(`%s`, `%s`);\n", childID, parentID) + return fmt.Sprintf("g.setParent(`%s`, `%s`);\n", escapeID(childID), escapeID(parentID)) } func generateAddEdgeLine(fromID, toID, edgeID string) string { // in dagre v is from, w is to, name is to uniquely identify - return fmt.Sprintf("g.setEdge({v:`%s`, w:`%s`, name:`%s` });\n", fromID, toID, edgeID) + return fmt.Sprintf("g.setEdge({v:`%s`, w:`%s`, name:`%s` });\n", escapeID(fromID), escapeID(toID), escapeID(edgeID)) } diff --git a/e2etests/regression_test.go b/e2etests/regression_test.go index 4db509e55..fc0f6c7c7 100644 --- a/e2etests/regression_test.go +++ b/e2etests/regression_test.go @@ -7,8 +7,12 @@ import ( func testRegression(t *testing.T) { tcs := []testCase{ { - name: "dagre_id_with_newline", - script: `ninety\nnine`, + name: "dagre_id_with_newline", + script: ` +ninety\nnine +eighty\reight +seventy\r\nseven +`, }, } From a9f0d5978b593e6f7039e54b9b6638c484b9abdc Mon Sep 17 00:00:00 2001 From: Gavin Nishizawa Date: Mon, 5 Dec 2022 11:41:35 -0800 Subject: [PATCH 5/9] update tests --- .../dagre/board.exp.json | 78 ++++++++++++++ .../dagre/sketch.exp.svg | 4 +- .../dagre_id_with_newline/elk/board.exp.json | 80 +++++++++++++- .../dagre_id_with_newline/elk/sketch.exp.svg | 4 +- .../TestCompile/unescaped_id_cr.exp.json | 102 ++++++++++++++++++ 5 files changed, 263 insertions(+), 5 deletions(-) create mode 100644 testdata/d2compiler/TestCompile/unescaped_id_cr.exp.json diff --git a/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json b/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json index 4366386d6..be2cc9969 100644 --- a/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json +++ b/e2etests/testdata/regression/dagre_id_with_newline/dagre/board.exp.json @@ -39,6 +39,84 @@ "labelPosition": "INSIDE_MIDDLE_CENTER", "zIndex": 0, "level": 1 + }, + { + "id": "eighty\reight", + "type": "", + "pos": { + "x": 211, + "y": 8 + }, + "width": 151, + "height": 126, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "eighty\reight", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 51, + "labelHeight": 26, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 + }, + { + "id": "\"seventy\r\\nseven\"", + "type": "", + "pos": { + "x": 422, + "y": 0 + }, + "width": 162, + "height": 142, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "fields": null, + "methods": null, + "columns": null, + "label": "seventy\r\nseven", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 62, + "labelHeight": 42, + "labelPosition": "INSIDE_MIDDLE_CENTER", + "zIndex": 0, + "level": 1 } ], "connections": [] diff --git a/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg b/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg index c9c9cf7da..eeb82e050 100644 --- a/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg +++ b/e2etests/testdata/regression/dagre_id_with_newline/dagre/sketch.exp.svg @@ -2,7 +2,7 @@