Support pcb note elements in footprint generation#15
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.
ℹ️ 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
| for (const noteText of noteTexts) { | ||
| const anchorPosition = noteText.anchor_position ?? { x: 0, y: 0 } | ||
| const anchorAlignment = noteText.anchor_alignment ?? "center" | ||
| const font = noteText.font ?? "tscircuit2024" | ||
| const fontSize = noteText.font_size ?? 0 | ||
| const colorAttr = noteText.color ? ` color="${noteText.color}"` : "" | ||
|
|
||
| const rawText = String(noteText.text ?? "") | ||
| const escapedText = rawText.replace(/"/g, '\\"') | ||
|
|
||
| elementStrings.push( | ||
| `<pcbnotetext pcbX={${anchorPosition.x}} pcbY={${anchorPosition.y}} anchorAlignment="${anchorAlignment}" font="${font}" fontSize={${fontSize}} text="${escapedText}"${colorAttr} />`, |
There was a problem hiding this comment.
The conversion logic escapes only double quotes before writing noteText.text into the text="…" attribute, but values such as "A<B" or "A&B" are not escaped. When a note contains </>/&, the generated TSX will be syntactically invalid because the attribute value terminates prematurely or introduces an entity. Consider encoding the string with JSON.stringify or replacing these characters so note texts containing comparison symbols or ampersands still generate valid JSX.
Useful? React with 👍 / 👎.
| if (noteDimension.text !== undefined) { | ||
| const escapedText = String(noteDimension.text).replace(/"/g, '\\"') | ||
| attrs.push(`text="${escapedText}"`) |
There was a problem hiding this comment.
The dimension loop performs the same quote-only escaping for noteDimension.text and embeds it directly into text="…". If a dimension label contains characters like <, > or &, the generated JSX becomes invalid and convertCircuitJsonToTscircuit will emit code that fails to compile. The label should be fully escaped (e.g., via JSON.stringify or manual replacement of <, >, &) before being injected.
Useful? React with 👍 / 👎.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const rawText = String(noteText.text ?? "") | ||
| const escapedText = rawText.replace(/"/g, '\\"') |
There was a problem hiding this comment.
Backslashes in text are not escaped, which will break the generated TSX syntax. If the text contains a backslash (e.g., "test\value"), the generated code will have text="test\value" where the backslash could escape the closing quote or create invalid escape sequences.
Fix by escaping backslashes before quotes:
const rawText = String(noteText.text ?? "")
const escapedText = rawText.replace(/\\/g, '\\\\').replace(/"/g, '\\"')| const rawText = String(noteText.text ?? "") | |
| const escapedText = rawText.replace(/"/g, '\\"') | |
| const rawText = String(noteText.text ?? "") | |
| const escapedText = rawText.replace(/\\/g, '\\\\').replace(/"/g, '\\"') |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.
| } | ||
|
|
||
| if (noteDimension.text !== undefined) { | ||
| const escapedText = String(noteDimension.text).replace(/"/g, '\\"') |
There was a problem hiding this comment.
Same backslash escaping issue as noteText. Backslashes in dimension text are not escaped before embedding in the generated TSX string.
Fix by escaping backslashes before quotes:
const escapedText = String(noteDimension.text).replace(/\\/g, '\\\\').replace(/"/g, '\\"')| const escapedText = String(noteDimension.text).replace(/"/g, '\\"') | |
| const escapedText = String(noteDimension.text).replace(/\\/g, '\\\\').replace(/"/g, '\\"') |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_b_68f069c999e0832eaa2dbe021e4ff8c9