Skip to content

Render nested Action Text attachment content - #335

Open
nitrnitr wants to merge 1 commit into
basecamp:mainfrom
nitrnitr:fix/nested-action-text-attachment-content
Open

Render nested Action Text attachment content#335
nitrnitr wants to merge 1 commit into
basecamp:mainfrom
nitrnitr:fix/nested-action-text-attachment-content

Conversation

@nitrnitr

Copy link
Copy Markdown

What changed

Handle bare <action-text-attachment> elements whose HTML is stored in a content attribute.

HEY can nest this element inside a Trix HTML attachment. Previously, ToMarkdown treated it as a file attachment and rendered 📎 attachment, dropping the embedded text and links.

Unnamed elements with non-empty content are now rendered through the existing bounded embedded-HTML path. Named elements remain file attachments.

Added regression coverage for both the nested HTML case and filename precedence.

Testing

  • make check
  • make build

@github-actions github-actions Bot added the tests label Aug 26, 2026
@nitrnitr
nitrnitr marked this pull request as ready for review August 26, 2026 01:33
@nitrnitr
nitrnitr requested a review from a team as a code owner August 26, 2026 01:33
Copilot AI balanced review requested due to automatic review settings August 26, 2026 01:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Renders unnamed nested Action Text attachment HTML through the existing bounded Markdown path while preserving named file attachments.

Changes:

  • Added nested attachment-content rendering.
  • Added regression and filename-precedence tests.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/htmlutil/markdown.go Handles embedded Action Text attachment content.
internal/htmlutil/markdown_test.go Tests nested HTML and filename precedence.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/htmlutil/markdown.go Outdated
@nitrnitr
nitrnitr force-pushed the fix/nested-action-text-attachment-content branch from 2b6d4f3 to f33596e Compare August 26, 2026 01:56
@jeremy
jeremy force-pushed the fix/nested-action-text-attachment-content branch from f33596e to 6de1d33 Compare September 9, 2026 22:48
@jeremy

jeremy commented Sep 9, 2026

Copy link
Copy Markdown
Member

Rebased onto main as 6de1d33 so this applies cleanly again — #387 (decorative-image skipping) landed in the same three spots. The merge keeps #387's decorative check first in walkNode, findImages and the Markdown path, then the embedded-content branch from this PR, then the named-attachment fallback; renderActionTextAttachment is folded into main's actionTextAttachment rather than kept beside it. No behaviour change to what this PR does: an unnamed content attachment is rendered as embedded HTML, a named one stays a file attachment, and all five new tests pass as written. make check is green locally; the test workflow has not run on this PR yet (first-time-contributor approval), so the local run is the gate for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants