Fix #1578: shape: cycle#2771
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b362a193af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return theme.Colors.AB5 | ||
| default: | ||
| return "" | ||
| func GetTheme(id int) (*themes.Theme, error) { |
There was a problem hiding this comment.
This replacement leaves d2themes exporting only GetTheme, but repo-wide usages still construct d2themes.Theme/ColorPalette and call ResolveThemeColor (for example in d2themes/d2themescatalog and d2renderers/d2svg). I also checked under lib and there is no lib/themes package to back this import, so any normal build after this commit fails before the shape: cycle fix can be used.
Useful? React with 👍 / 👎.
| "oss.terrastruct.com/d2/lib/parser" | ||
| ) | ||
|
|
||
| func Parse(s string) (*ast.Board, error) { |
There was a problem hiding this comment.
This adds a second d2parser.Parse with a different signature while d2parser/parse.go already defines Parse(path string, r io.Reader, opts *ParseOptions). Go does not overload functions, and existing callers throughout the repo use the three-argument API, so the d2parser package cannot compile with this file present.
Useful? React with 👍 / 👎.
| "oss.terrastruct.com/d2/lib/shell" | ||
| ) | ||
|
|
||
| func Run(args []string) error { |
There was a problem hiding this comment.
d2cli/main.go already exports Run(ctx context.Context, ms *xmain.State), and this new Run(args []string) is in the same package. Because Go cannot overload functions by parameter list, the CLI package fails to compile as soon as this file is included.
Useful? React with 👍 / 👎.
1 participant
Closes #1578
This change resolves the behavior described in the linked issue.
Happy to adjust the approach or address any review feedback.