diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9feefc85..e4beb9701 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,7 +1,7 @@ name: ci on: [push, pull_request] concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} cancel-in-progress: true jobs: diff --git a/ci/release/template/scripts/lib.sh b/ci/release/template/scripts/lib.sh index 699795908..2ee814abf 100644 --- a/ci/release/template/scripts/lib.sh +++ b/ci/release/template/scripts/lib.sh @@ -437,10 +437,10 @@ ensure_prefix() { if [ -n "${PREFIX-}" ]; then return fi - # The reason for checking whether bin is writable is that on macOS you have /usr/local + # The reason for checking whether lib is writable is that on macOS you have /usr/local # owned by root but you don't need root to write to its subdirectories which is all we # need to do. - if ! is_writable_dir "/usr/local/bin"; then + if ! is_writable_dir "/usr/local/lib"; then # This also handles M1 Mac's which do not allow modifications to /usr/local even # with sudo. PREFIX=$HOME/.local @@ -453,10 +453,10 @@ ensure_prefix_sh_c() { ensure_prefix sh_c="sh_c" - # The reason for checking whether bin is writable is that on macOS you have /usr/local + # The reason for checking whether lib is writable is that on macOS you have /usr/local # owned by root but you don't need root to write to its subdirectories which is all we # need to do. - if ! is_writable_dir "$PREFIX/bin"; then + if ! is_writable_dir "$PREFIX/lib"; then sh_c="sudo_sh_c" fi } diff --git a/ci/sub b/ci/sub index b1ec0a8d4..8ac704818 160000 --- a/ci/sub +++ b/ci/sub @@ -1 +1 @@ -Subproject commit b1ec0a8d430a62b7556211ed8bcd7b6e41e2362c +Subproject commit 8ac704818b5d7ab519e4b87caf5eb79716493709 diff --git a/d2graph/d2graph.go b/d2graph/d2graph.go index 222149eab..8fb930384 100644 --- a/d2graph/d2graph.go +++ b/d2graph/d2graph.go @@ -738,6 +738,9 @@ func (obj *Object) GetLabelSize(mtexts []*d2target.MText, ruler *textmeasure.Rul } if dims == nil { + if obj.Text().Text == "" { + return d2target.NewTextDimensions(0, 0), nil + } if shapeType == d2target.ShapeImage { dims = d2target.NewTextDimensions(0, 0) } else { @@ -759,7 +762,7 @@ func (obj *Object) GetDefaultSize(mtexts []*d2target.MText, ruler *textmeasure.R return d2target.NewTextDimensions(128, 128), nil case d2target.ShapeClass: - maxWidth := labelDims.Width + maxWidth := go2.Max(12, labelDims.Width) for _, f := range obj.Class.Fields { fdims := GetTextDimensions(mtexts, ruler, f.Text(), go2.Pointer(d2fonts.SourceCodePro)) @@ -795,7 +798,7 @@ func (obj *Object) GetDefaultSize(mtexts []*d2target.MText, ruler *textmeasure.R rowHeight := GetTextDimensions(mtexts, ruler, anyRowText, go2.Pointer(d2fonts.SourceCodePro)).Height + 20 dims.Height = rowHeight * (len(obj.Class.Fields) + len(obj.Class.Methods) + 2) } else { - dims.Height = labelDims.Height + dims.Height = go2.Max(12, labelDims.Height) } case d2target.ShapeSQLTable: @@ -835,10 +838,10 @@ func (obj *Object) GetDefaultSize(mtexts []*d2target.MText, ruler *textmeasure.R } // The rows get padded a little due to header font being larger than row font - dims.Height = labelDims.Height * (len(obj.SQLTable.Columns) + 1) + dims.Height = go2.Max(12, labelDims.Height*(len(obj.SQLTable.Columns)+1)) headerWidth := d2target.HeaderPadding + labelDims.Width + d2target.HeaderPadding rowsWidth := d2target.NamePadding + maxNameWidth + d2target.TypePadding + maxTypeWidth + d2target.TypePadding + constraintWidth - dims.Width = go2.Max(headerWidth, rowsWidth) + dims.Width = go2.Max(12, go2.Max(headerWidth, rowsWidth)) } return &dims, nil diff --git a/docs/flow.d2 b/docs/flow.d2 index 41c60d079..24cd5f31f 100644 --- a/docs/flow.d2 +++ b/docs/flow.d2 @@ -1,8 +1,10 @@ -inputFile \ - -> d2parser\ - -> d2ast\ - -> d2compiler\ - -> d2graph\ - -> d2layouts/d2dagrelayout\ - -> d2exporter\ - -> d2target +# How we actually want it formatted (d2 fmt removes line continuations currently): +# inputFile \ +# -> d2parser\ +# -> d2ast\ +# -> d2compiler\ +# -> d2graph\ +# -> d2layouts/d2dagrelayout\ +# -> d2exporter\ +# -> d2target +inputFile -> d2parser -> d2ast -> d2compiler -> d2graph -> d2layouts/d2dagrelayout -> d2exporter -> d2target diff --git a/e2etests/e2e_test.go b/e2etests/e2e_test.go index ac6ab2cab..749c22d22 100644 --- a/e2etests/e2e_test.go +++ b/e2etests/e2e_test.go @@ -37,6 +37,7 @@ func TestE2E(t *testing.T) { t.Run("stable", testStable) t.Run("regression", testRegression) t.Run("todo", testTodo) + t.Run("measured", testMeasured) } func testSanity(t *testing.T) { @@ -73,6 +74,7 @@ a -> c type testCase struct { name string script string + mtexts []*d2target.MText assertions func(t *testing.T, diagram *d2target.Diagram) skip bool } @@ -118,10 +120,14 @@ func run(t *testing.T, tc testCase) { ctx = log.WithTB(ctx, t, nil) ctx = log.Leveled(ctx, slog.LevelDebug) - ruler, err := textmeasure.NewRuler() - trequire.Nil(t, err) + var ruler *textmeasure.Ruler + var err error + if tc.mtexts == nil { + ruler, err = textmeasure.NewRuler() + trequire.Nil(t, err) - serde(t, tc, ruler) + serde(t, tc, ruler) + } layoutsTested := []string{"dagre", "elk"} @@ -130,12 +136,17 @@ func run(t *testing.T, tc testCase) { if layoutName == "dagre" { layout = d2dagrelayout.DefaultLayout } else if layoutName == "elk" { + // If measured texts exists, we are specifically exercising text measurements, no need to run on both layouts + if tc.mtexts != nil { + continue + } layout = d2elklayout.DefaultLayout } diagram, _, err := d2lib.Compile(ctx, tc.script, &d2lib.CompileOptions{ - Ruler: ruler, - ThemeID: 0, - Layout: layout, + Ruler: ruler, + MeasuredTexts: tc.mtexts, + ThemeID: 0, + Layout: layout, }) trequire.Nil(t, err) diff --git a/e2etests/measured_test.go b/e2etests/measured_test.go new file mode 100644 index 000000000..777fda482 --- /dev/null +++ b/e2etests/measured_test.go @@ -0,0 +1,34 @@ +package e2etests + +import ( + _ "embed" + "testing" + + "oss.terrastruct.com/d2/d2target" +) + +// testMeasured exercises the code paths that provide pre-measured texts +func testMeasured(t *testing.T) { + tcs := []testCase{ + { + name: "empty-shape", + mtexts: []*d2target.MText{}, + script: `a: "" +`, + }, + { + name: "empty-class", + mtexts: []*d2target.MText{}, + script: `a: "" { shape: class } +`, + }, + { + name: "empty-sql_table", + mtexts: []*d2target.MText{}, + script: `a: "" { shape: sql_table } +`, + }, + } + + runa(t, tcs) +} diff --git a/e2etests/testdata/measured/empty-class/dagre/board.exp.json b/e2etests/testdata/measured/empty-class/dagre/board.exp.json new file mode 100644 index 000000000..1f92e5b45 --- /dev/null +++ b/e2etests/testdata/measured/empty-class/dagre/board.exp.json @@ -0,0 +1,49 @@ +{ + "name": "", + "fontFamily": "SourceSansPro", + "shapes": [ + { + "id": "a", + "type": "class", + "pos": { + "x": 0, + "y": 0 + }, + "width": 112, + "height": 12, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#0A0F25", + "stroke": "#FFFFFF", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "", + "fontSize": 20, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": false, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "zIndex": 0, + "level": 1, + "primaryAccentColor": "#0D32B2", + "secondaryAccentColor": "#4A6FF3", + "neutralAccentColor": "#676C7E" + } + ], + "connections": [] +} diff --git a/e2etests/testdata/measured/empty-class/dagre/sketch.exp.svg b/e2etests/testdata/measured/empty-class/dagre/sketch.exp.svg new file mode 100644 index 000000000..830b5caa3 --- /dev/null +++ b/e2etests/testdata/measured/empty-class/dagre/sketch.exp.svg @@ -0,0 +1,45 @@ + + + + + \ No newline at end of file diff --git a/e2etests/testdata/measured/empty-shape/dagre/board.exp.json b/e2etests/testdata/measured/empty-shape/dagre/board.exp.json new file mode 100644 index 000000000..bf4da9d37 --- /dev/null +++ b/e2etests/testdata/measured/empty-shape/dagre/board.exp.json @@ -0,0 +1,46 @@ +{ + "name": "", + "fontFamily": "SourceSansPro", + "shapes": [ + { + "id": "a", + "type": "", + "pos": { + "x": 0, + "y": 0 + }, + "width": 100, + "height": 100, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#F7F8FE", + "stroke": "#0D32B2", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "", + "fontSize": 16, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "zIndex": 0, + "level": 1 + } + ], + "connections": [] +} diff --git a/e2etests/testdata/measured/empty-shape/dagre/sketch.exp.svg b/e2etests/testdata/measured/empty-shape/dagre/sketch.exp.svg new file mode 100644 index 000000000..1acdbb250 --- /dev/null +++ b/e2etests/testdata/measured/empty-shape/dagre/sketch.exp.svg @@ -0,0 +1,45 @@ + + + + + \ No newline at end of file diff --git a/e2etests/testdata/measured/empty-sql_table/dagre/board.exp.json b/e2etests/testdata/measured/empty-sql_table/dagre/board.exp.json new file mode 100644 index 000000000..dcd887dd3 --- /dev/null +++ b/e2etests/testdata/measured/empty-sql_table/dagre/board.exp.json @@ -0,0 +1,49 @@ +{ + "name": "", + "fontFamily": "SourceSansPro", + "shapes": [ + { + "id": "a", + "type": "sql_table", + "pos": { + "x": 0, + "y": 0 + }, + "width": 50, + "height": 12, + "opacity": 1, + "strokeDash": 0, + "strokeWidth": 2, + "borderRadius": 0, + "fill": "#0A0F25", + "stroke": "#FFFFFF", + "shadow": false, + "3d": false, + "multiple": false, + "tooltip": "", + "link": "", + "icon": null, + "iconPosition": "", + "blend": false, + "fields": null, + "methods": null, + "columns": null, + "label": "", + "fontSize": 20, + "fontFamily": "DEFAULT", + "language": "", + "color": "#0A0F25", + "italic": false, + "bold": true, + "underline": false, + "labelWidth": 0, + "labelHeight": 0, + "zIndex": 0, + "level": 1, + "primaryAccentColor": "#0D32B2", + "secondaryAccentColor": "#4A6FF3", + "neutralAccentColor": "#676C7E" + } + ], + "connections": [] +} diff --git a/e2etests/testdata/measured/empty-sql_table/dagre/sketch.exp.svg b/e2etests/testdata/measured/empty-sql_table/dagre/sketch.exp.svg new file mode 100644 index 000000000..69d917f3c --- /dev/null +++ b/e2etests/testdata/measured/empty-sql_table/dagre/sketch.exp.svg @@ -0,0 +1,45 @@ + + + + + \ No newline at end of file diff --git a/go.mod b/go.mod index 162dfe0ce..b69950701 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 gonum.org/v1/plot v0.12.0 nhooyr.io/websocket v1.8.7 - oss.terrastruct.com/util-go v0.0.0-20230124223326-041fa96d3331 + oss.terrastruct.com/util-go v0.0.0-20230124232704-39c2226d2b5e ) require ( diff --git a/go.sum b/go.sum index 5fc6eee70..63a8ea258 100644 --- a/go.sum +++ b/go.sum @@ -266,6 +266,6 @@ honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= nhooyr.io/websocket v1.8.7 h1:usjR2uOr/zjjkVMy0lW+PPohFok7PCow5sDjLgX4P4g= nhooyr.io/websocket v1.8.7/go.mod h1:B70DZP8IakI65RVQ51MsWP/8jndNma26DVA/nFSCgW0= -oss.terrastruct.com/util-go v0.0.0-20230124223326-041fa96d3331 h1:5B6+mum6eq9bOtAkHK15T6rvPgtijxu/v165uEAqd7I= -oss.terrastruct.com/util-go v0.0.0-20230124223326-041fa96d3331/go.mod h1:Fwy72FDIOOM4K8F96ScXkxHHppR1CPfUyo9+x9c1PBU= +oss.terrastruct.com/util-go v0.0.0-20230124232704-39c2226d2b5e h1:pCKxiUFOLQamCtmyMZsM3Hc8MuKVpDg/4VunhVOVW/4= +oss.terrastruct.com/util-go v0.0.0-20230124232704-39c2226d2b5e/go.mod h1:Fwy72FDIOOM4K8F96ScXkxHHppR1CPfUyo9+x9c1PBU= rsc.io/pdf v0.1.1 h1:k1MczvYDUvJBe93bYd7wrZLLUEcLZAuF824/I4e5Xr4= diff --git a/install.sh b/install.sh index 026f423c9..04721fd93 100755 --- a/install.sh +++ b/install.sh @@ -573,10 +573,10 @@ ensure_prefix() { if [ -n "${PREFIX-}" ]; then return fi - # The reason for checking whether bin is writable is that on macOS you have /usr/local + # The reason for checking whether lib is writable is that on macOS you have /usr/local # owned by root but you don't need root to write to its subdirectories which is all we # need to do. - if ! is_writable_dir "/usr/local/bin"; then + if ! is_writable_dir "/usr/local/lib"; then # This also handles M1 Mac's which do not allow modifications to /usr/local even # with sudo. PREFIX=$HOME/.local @@ -589,10 +589,10 @@ ensure_prefix_sh_c() { ensure_prefix sh_c="sh_c" - # The reason for checking whether bin is writable is that on macOS you have /usr/local + # The reason for checking whether lib is writable is that on macOS you have /usr/local # owned by root but you don't need root to write to its subdirectories which is all we # need to do. - if ! is_writable_dir "$PREFIX/bin"; then + if ! is_writable_dir "$PREFIX/lib"; then sh_c="sudo_sh_c" fi }