Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

render: support width/height on shapes #498

Merged
merged 11 commits into from
Dec 29, 2022
Merged

render: support width/height on shapes #498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
17e4ae0
add shape_set_width_height test
gavin-ts Dec 22, 2022
6cb72cb
add unnamed_class_table_code regression test
gavin-ts Dec 28, 2022
5d4059e
support setting width/height on non-image shapes
gavin-ts Dec 22, 2022
e45f78b
changelog
gavin-ts Dec 22, 2022
73d1a66
cleanup
gavin-ts Dec 28, 2022
2f4446e
add tests with only width or height set
gavin-ts Dec 29, 2022
8384d96
refactor SetDimensions
gavin-ts Dec 29, 2022
5509621
update tests
gavin-ts Dec 29, 2022
f418e06
cleanup
gavin-ts Dec 29, 2022
fbf48fa
compile error for circle/square shapes with different width and height
gavin-ts Dec 29, 2022
ae4e636
add compiler error for using width or height on a container
gavin-ts Dec 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ci/release/changelogs/next.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

- Tooltips can be set on shapes. See [https://d2lang.com/tour/tooltips](https://d2lang.com/tour/interactive). [#548](https://github.com/terrastruct/d2/pull/548)
- Links can be set on shapes. See [https://d2lang.com/tour/tooltips](https://d2lang.com/tour/interactive). [#548](https://github.com/terrastruct/d2/pull/548)
- The `width` and `height` attributes are no longer restricted to images and can be applied to non-container shapes. [#498](https://github.com/terrastruct/d2/pull/498)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-container shapes

Maybe, non-container objects as there're no such thing as container shapes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wanted to call every "node" a shape, as object is pretty all-encompassing. but i'm not 100% sure about that route

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, from a changeling perspective, shape reminds me of shape keyword and not the objects themselves

Copy link
Contributor Author

@gavin-ts gavin-ts Dec 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2022-12-29 at 12 07 15 PM

from https://d2lang.com/tour/containers :

server
# Declares a shape inside of another shape
server.process


#### Improvements 🧹

#### Bugfixes ⛑️

- Restricts where `near` key constant values can be used, with good error messages, instead of erroring (e.g. setting `near: top-center` on a container would cause bad layouts or error). [#538](https://github.com/terrastruct/d2/pull/538)
- Fixes rendering classes and tables with empty headers. [#498](https://github.com/terrastruct/d2/pull/498)
27 changes: 19 additions & 8 deletions d2compiler/compile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,15 +797,18 @@ func (c *compiler) validateKey(obj *d2graph.Object, m *d2ast.Map, mk *d2ast.Key)
return
}

if reserved == "" && obj.Attributes.Shape.Value == d2target.ShapeImage {
c.errorf(mk.Range.Start, mk.Range.End, "image shapes cannot have children.")
}
switch strings.ToLower(obj.Attributes.Shape.Value) {
case d2target.ShapeImage:
if reserved == "" {
c.errorf(mk.Range.Start, mk.Range.End, "image shapes cannot have children.")
}
case d2target.ShapeCircle, d2target.ShapeSquare:
checkEqual := (reserved == "width" && obj.Attributes.Height != nil) ||
(reserved == "height" && obj.Attributes.Width != nil)

if reserved == "width" && obj.Attributes.Shape.Value != d2target.ShapeImage {
c.errorf(mk.Range.Start, mk.Range.End, "width is only applicable to image shapes.")
}
if reserved == "height" && obj.Attributes.Shape.Value != d2target.ShapeImage {
c.errorf(mk.Range.Start, mk.Range.End, "height is only applicable to image shapes.")
if checkEqual && obj.Attributes.Width.Value != obj.Attributes.Height.Value {
c.errorf(mk.Range.Start, mk.Range.End, fmt.Sprintf("width and height must be equal for %s shapes", obj.Attributes.Shape.Value))
}
}

in := d2target.IsShape(obj.Attributes.Shape.Value)
Expand All @@ -830,6 +833,14 @@ func (c *compiler) validateKey(obj *d2graph.Object, m *d2ast.Map, mk *d2ast.Key)
return
}

switch strings.ToLower(obj.Attributes.Shape.Value) {
case d2target.ShapeSQLTable, d2target.ShapeClass:
default:
if len(obj.Children) > 0 && (reserved == "width" || reserved == "height") {
c.errorf(mk.Range.Start, mk.Range.End, fmt.Sprintf("%s cannot be used on container: %s", reserved, obj.AbsID()))
}
}

if len(mk.Edges) > 0 {
return
}
Expand Down
115 changes: 113 additions & 2 deletions d2compiler/compile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,119 @@ x: {
height: 230
}
`,
expErr: `d2/testdata/d2compiler/TestCompile/dimensions_on_nonimage.d2:3:2: width is only applicable to image shapes.
d2/testdata/d2compiler/TestCompile/dimensions_on_nonimage.d2:4:2: height is only applicable to image shapes.
assertions: func(t *testing.T, g *d2graph.Graph) {
if len(g.Objects) != 1 {
t.Fatalf("expected 1 objects: %#v", g.Objects)
}
if g.Objects[0].ID != "hey" {
t.Fatalf("expected g.Objects[0].ID to be 'hey': %#v", g.Objects[0])
}
if g.Objects[0].Attributes.Shape.Value != d2target.ShapeHexagon {
t.Fatalf("expected g.Objects[0].Attributes.Shape.Value to be hexagon: %#v", g.Objects[0].Attributes.Shape.Value)
}
if g.Objects[0].Attributes.Width.Value != "200" {
t.Fatalf("expected g.Objects[0].Attributes.Width.Value to be 200: %#v", g.Objects[0].Attributes.Width.Value)
}
if g.Objects[0].Attributes.Height.Value != "230" {
t.Fatalf("expected g.Objects[0].Attributes.Height.Value to be 230: %#v", g.Objects[0].Attributes.Height.Value)
}
},
},
{
name: "equal_dimensions_on_circle",

text: `hey: "" {
shape: circle
width: 200
height: 230
}
`,
expErr: `d2/testdata/d2compiler/TestCompile/equal_dimensions_on_circle.d2:3:2: width and height must be equal for circle shapes
d2/testdata/d2compiler/TestCompile/equal_dimensions_on_circle.d2:4:2: width and height must be equal for circle shapes
`,
},
{
name: "single_dimension_on_circle",

text: `hey: "" {
shape: circle
height: 230
}
`,
assertions: func(t *testing.T, g *d2graph.Graph) {
if len(g.Objects) != 1 {
t.Fatalf("expected 1 objects: %#v", g.Objects)
}
if g.Objects[0].ID != "hey" {
t.Fatalf("expected ID to be 'hey': %#v", g.Objects[0])
}
if g.Objects[0].Attributes.Shape.Value != d2target.ShapeCircle {
t.Fatalf("expected Attributes.Shape.Value to be circle: %#v", g.Objects[0].Attributes.Shape.Value)
}
if g.Objects[0].Attributes.Width != nil {
t.Fatalf("expected Attributes.Width to be nil: %#v", g.Objects[0].Attributes.Width)
}
if g.Objects[0].Attributes.Height == nil {
t.Fatalf("Attributes.Height is nil")
}
},
},
{
name: "no_dimensions_on_containers",

text: `
containers: {
circle container: {
shape: circle
width: 512

diamond: {
shape: diamond
width: 128
height: 64
}
}
diamond container: {
shape: diamond
width: 512
height: 256

circle: {
shape: circle
width: 128
}
}
oval container: {
shape: oval
width: 512
height: 256

hexagon: {
shape: hexagon
width: 128
height: 64
}
}
hexagon container: {
shape: hexagon
width: 512
height: 256

oval: {
shape: oval
width: 128
height: 64
}
}
}
`,
expErr: `d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:5:3: width cannot be used on container: containers.circle container
d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:15:3: width cannot be used on container: containers.diamond container
d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:16:3: height cannot be used on container: containers.diamond container
d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:25:3: width cannot be used on container: containers.oval container
d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:26:3: height cannot be used on container: containers.oval container
d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:36:3: width cannot be used on container: containers.hexagon container
d2/testdata/d2compiler/TestCompile/no_dimensions_on_containers.d2:37:3: height cannot be used on container: containers.hexagon container
`,
},
{
Expand Down
Loading