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.

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

Add imports #1371

Merged
merged 27 commits into from
Jun 7, 2023
Merged

Add imports #1371

merged 27 commits into from
Jun 7, 2023

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Jun 5, 2023

See commits.

Works as so:

index.d2:

x: @x

x.d2:

label: meow
shape: circle

TODO:

  • Correctly import into fields by handling the scenarios/steps overlay.
    - And links.
  • Add a test for every kind of import.

Closes #670

Includes rewriting @file.d2 to @file
Works as so:

index.d2:
```d2
x: @x
```

x.d2:
```d2
label: meow
shape: circle
```

TODO:

- [ ] Correctly import into fields by handling the scenarios/steps overlay.
      - And links.
- [ ] Test every kind of import.
@nhooyr nhooyr requested review from alixander and a team June 5, 2023 06:33
@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 5, 2023

Spread imports work too like:

x: {
  ...@x
}

where x.d2 is the filename.

And @x.d2 imports are rewritten automatically to @x.

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

sweet, did a first pass, will wait for tests and do another. i'm going to start writing docs

d2compiler/compile.go Show resolved Hide resolved
d2format/format_test.go Show resolved Hide resolved
d2ir/compile.go Show resolved Hide resolved
d2ir/import.go Outdated Show resolved Hide resolved
d2ir/import.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

i have

./static/d2/imports-normal.d2:

x: @imports-normal-x.d2

./static/d2/imports-normal-x.d2:

x: {
  shape: circle
}

But when I run

d2 ./static/d2/imports-normal.d2

it fails:
Screen Shot 2023-06-05 at 12 28 34 PM

d2compiler/compile.go Show resolved Hide resolved
@alixander
Copy link
Collaborator

alixander commented Jun 6, 2023

Correctly import into fields by handling the scenarios/steps overlay.

does that mean it can't render x and include the nested layers?

x.d2: "layers: { y: @...y}"
y.d2: "hi; layers: { z: @...z}"
z.d2: "z"

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

should we make usages of @ as start of keys to be compiler errors? seems confusing

@hello
@hello -> yo

maybe something like, @hello is not a valid import. Did you mean "...@hello"?

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

i mistyped an import spread and got a panic

x: @...imports-normal-x.d2
x -> y
_normal: exec: d2 --dark-theme=200 -l dagre -c --pad 0 ./static/d2/imports-normal.d2 ./static/img/generated/imports-normal.svg2
_normal: panic: runtime error: invalid memory address or nil pointer dereference
_normal: [signal SIGSEGV: segmentation violation code=0x2 addr=0x40 pc=0x1027ebf64]
_normal:
_normal: goroutine 34 [running]:
_normal: oss.terrastruct.com/d2/d2parser.(*parser).parseImport(0x140004be4c8, 0x0)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:1713 +0x194
_normal: oss.terrastruct.com/d2/d2parser.(*parser).parseValue(0x140004be4c8)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:1586 +0x998
_normal: oss.terrastruct.com/d2/d2parser.(*parser).parseMapKeyValue(0x140004be4c8, 0x14000098d00)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:715 +0x3ac
_normal: oss.terrastruct.com/d2/d2parser.(*parser).parseMapKey(0x140004be4c8)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:685 +0x984
_normal: oss.terrastruct.com/d2/d2parser.(*parser).parseMapNode(0x140004be4c8, 0x78)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:484 +0x65c
_normal: oss.terrastruct.com/d2/d2parser.(*parser).parseMap(0x140004be4c8, 0x1)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:392 +0x1f0
_normal: oss.terrastruct.com/d2/d2parser.Parse({0x14000574201?, 0x3c?}, {0x103e09c20?, 0x1400078e3e0?}, 0x1400078e3e0?)
_normal:        /Users/alexanderwang/dev/d2/d2parser/parse.go:52 +0xa0
_normal: oss.terrastruct.com/d2/d2compiler.Compile({0x14000574200?, 0x3c?}, {0x103e09c20?, 0x1400078e3e0?}, 0x14000130db0?)
_normal:        /Users/alexanderwang/dev/d2/d2compiler/compile.go:53 +0x148
_normal: oss.terrastruct.com/d2/d2lib.Compile({0x103e11528, 0x140008a09f0}, {0x14000036990, 0x22}, 0x2?)
_normal:        /Users/alexanderwang/dev/d2/d2lib/d2.go:43 +0xd0
_normal: oss.terrastruct.com/d2/d2cli.compile({0x103e11528, 0x140008a09f0}, 0x1400016cc40, {0x103e132a8, 0x10461aec0}, {0x0, 0x0, 0x1, 0x0, 0x14000852010, ...}, ...)
_normal:        /Users/alexanderwang/dev/d2/d2cli/main.go:336 +0x1fc
_normal: oss.terrastruct.com/d2/d2cli.Run({0x103e114b8?, 0x14000602000?}, 0x1400016cc40)
_normal:        /Users/alexanderwang/dev/d2/d2cli/main.go:297 +0x1c04
_normal: oss.terrastruct.com/util-go/xmain.(*State).Main.func1()
_normal:        /Users/alexanderwang/go/pkg/mod/oss.terrastruct.com/util-go@v0.0.0-20230604222829-11c3c60fec14/xmain/xmain.go:105 +0x68
_normal: created by oss.terrastruct.com/util-go/xmain.(*State).Main
_normal:        /Users/alexanderwang/go/pkg/mod/oss.terrastruct.com/util-go@v0.0.0-20230604222829-11c3c60fec14/xmain/xmain.go:103 +0x110
_normal$: failure (0h0m0s)
Screen Shot 2023-06-06 at 1 48 30 PM

@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 6, 2023

@alixander yea i ran into that panic too fixing.

@alixander
Copy link
Collaborator

alixander commented Jun 6, 2023

this should probably also be a compiler error. it's an invalid import spread

x: ...@imports-normal-x
x -> y

but comes out rendered like
Screen Shot 2023-06-06 at 1 55 33 PM

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

did i misunderstand something? why does management show up as a box?

Screen Shot 2023-06-06 at 2 06 55 PM

@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 6, 2023

what if we also rewrite @./x to @x?

This would require involved changes to the parser so we can make an issue for later but not something I want to do now.

Panic fixed.

should we make usages of @ as start of keys to be compiler errors? seems confusing

Done.

#1371 (comment)

this should probably also be a compiler error. it's an invalid import spread

Unfortunately there's no easy way to detect that. Just a downside of having unquoted strings.

did i misunderstand something? why does management show up as a box?

Almost certainly a bug.

does that mean it can't render x and include the nested layers?

Not sure yet what the ramifications are. My point there was different. Let me fix it first before we dig into it.

@alixander
Copy link
Collaborator

Not sure yet what the ramifications are. My point there was different. Let me fix it first before we dig into it.

i'm thinking of this use case:

Screen Shot 2023-06-06 at 2 15 53 PM

@alixander
Copy link
Collaborator

alixander commented Jun 6, 2023

Unfortunately there's no easy way to detect that. Just a downside of having unquoted strings.

hmm why wouldn't it be adding it to parseKey like you did for the other? all valid imports would get transformed in the ir

@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 6, 2023

i'm thinking of this use case:

That will work once my omission is fixed.

hmm why wouldn't it be adding it to parseKey like you did for the other?

Yea nvm I guess I could actually. Actually even ./ I should be able to fix.

@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 6, 2023

all valid imports would get transformed in the ir

Not sure what you mean by that.

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

i tried doing a relative import and got this err

serviceA -> serviceB
serviceB.link: layers.serviceB
layers: {
  serviceB: @../d2/imports-nested-serviceB.d2
}
Screen Shot 2023-06-06 at 3 28 15 PM
_bespoke: err: failed to compile: /Users/alexanderwang/dev/d2-docs/static/bespoke-d2/imports-nested.d2:4:14: unexpected text after import

@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 6, 2023

Ah right I'll have to add a fix for that too.

@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 6, 2023

@alixander fixed that too.

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

it's still failing.

_bespoke: err: failed to compile: /Users/alexanderwang/dev/d2-docs/static/d2/imports-nested-serviceB.d2:13:9: failed to import "../d2/imports-nested-data.d2": open Users/alexanderwang/dev/d2/imports-nested-data.d2: no such file or directory
  • i'm in ~/dev/d2-docs
  • i'm rendering ~/dev/d2-docs/static/bespoke-d2/imports-nested.d2
  • that imports ../d2/imports-nested-data.d2

the CLI is interpreting that relative to the calling directory, when it should be relative to the file the import is from

should include a cli test that has the d2 files not in the same directory as the calling directory

@alixander alixander merged commit 202ffa2 into terrastruct:master Jun 7, 2023
@nhooyr nhooyr deleted the imports branch June 8, 2023 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: ability to import d2 files and reuse common components
2 participants