Skip to content

Add configurable D2 SVG output with bundling support - #719

Open
mnowotnik wants to merge 18 commits into
kovetskiy:masterfrom
mnowotnik:feature/svg-support
Open

Add configurable D2 SVG output with bundling support#719
mnowotnik wants to merge 18 commits into
kovetskiy:masterfrom
mnowotnik:feature/svg-support

Conversation

@mnowotnik

@mnowotnik mnowotnik commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Changes:

  • d2/d2.go: add SVG bundling via imgbundler, scale-aware dimensions, and adapter logger
  • renderer/fencedcodeblock.go: pass file path and scale through the SVG code path
  • markdown/markdown.go: wire the new fenced-code renderer signature
  • util/flags.go, util/cli.go, types/types.go: introduce d2-output config and propagate it to MarkConfig
  • d2/d2_test.go: cover the SVG path and checksum expectations
  • README.md: document the new d2-output option
  • go.mod/go.sum: pull in indirect deps needed by SVG bundling

Why:

  • Allow uploading original D2 SVGs with bundled assets and respect scaling to avoid manual resizing.

Changes:
- d2/d2.go: add SVG bundling via imgbundler, scale-aware dimensions, and adapter logger
- renderer/fencedcodeblock.go: pass file path, bundle flag, and scale through SVG code path
- markdown/markdown.go: wire new fenced-code renderer signature
- util/flags.go, util/cli.go, types/types.go: introduce d2-output/d2-bundle config and propagate to MarkConfig
- d2/d2_test.go: cover SVG path and checksum expectations
- README.md: document new d2-output and d2-bundle options
- go.mod/go.sum: pull in new indirect deps
- main.go: remove stray blank line

Why:
- Allow uploading original D2 SVGs with optional bundled assets and respect scaling to avoid manual resizing.
@mnowotnik

Copy link
Copy Markdown
Contributor Author

Tested manually on Confluence. Although, d2-scale has much bigger impact on this new svg output, because it takes dimensions to send to Confluence straight from svg rendered by d2 lib instead of from headless chrome. I think this is correct behaviour as far as defaults go, but may be surprising to users.

@mnowotnik

Copy link
Copy Markdown
Contributor Author

@mrueg Could you take a look? Pretty straightforward change. And Mark gets SVG with animations and other features.

Comment thread d2/d2.go Outdated
)

switch r.MarkConfig.D2Output {
case "svg":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does confluence finally have svg support?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confluence Cloud for sure. Using this branch of Mark with it right now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I now see that the SVG support is a debated issue. I attached screen recording with diagram from https://gitlab.com/HariSekhon/Diagrams-as-Code that I uploaded with this Mark branch.

Screen.Recording.2026-02-13.at.11.51.32.mov

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Supposedly Confluence Cloud support rendering of SVG attachments. Inline SVG still missing, but we're using attachments anyway.
https://jira.atlassian.com/browse/CONFCLOUD-1762

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks for checking!

Comment thread d2/d2.go Outdated
cacheImages := os.Getenv("IMG_CACHE") == "1"
logger := markSimpleLogger{}

out, err = imgbundler.BundleLocal(ctx, logger, inputPath, out, cacheImages)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What does BundleRemote and BundleLocal do, why do we need to call both?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's logic from d2cli/main.go:

	svg, bundleErr := imgbundler.BundleLocal(ctx, l, inputPath, svg, cacheImages)
	if bundle {
		var bundleErr2 error
		svg, bundleErr2 = imgbundler.BundleRemote(ctx, l, svg, cacheImages)
		bundleErr = multierr.Combine(bundleErr, bundleErr2)
	}

I think it first bundles local files and if bundle param is set, it additionally tries to bundle remote files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Put the local bundling outside flag conditional clause to mimic how d2 cli works.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is something we need to understand better, and need to make sure it does what we want and I'm not sure if I want bundleRemote here.

Copilot summing it up:

BundleLocal takes an SVG document (in) that may reference local image files (e.g., <image href="foo.png">) and rewrites those <image> elements so the href becomes a data URI containing the base64-encoded image bytes:

    It calls the shared bundle(...) implementation with isRemote = false.
    With isRemote=false, only <image href="..."> values that are not http/https URLs are considered eligible (see filterImageElements).
    The worker for each eligible image reads bytes from disk:
        It HTML-unescapes the href first.
        If inputPath != "-" and the href path is relative, it resolves it relative to filepath.Dir(inputPath).
        Then it os.ReadFile(...)s the image.
    It then determines a MIME type (sniffing if needed), base64-encodes the bytes, and replaces:
        from: <image href="some/path.png"
        to: <image href="data:image/png;base64,...."

So the output is still an SVG, but now it’s “self-contained” for local image references.
What BundleRemote does
Go

func BundleRemote(ctx context.Context, l simplelog.Logger, in []byte, cacheImages bool) ([]byte, error) {
	return bundle(ctx, l, "", in, true, cacheImages)
}

BundleRemote does the same kind of bundling, but for remote images referenced by http/https URLs:

    It calls bundle(...) with isRemote = true and an empty inputPath (since it shouldn’t resolve filesystem paths).
    With isRemote=true, filterImageElements selects only <image href="http(s)://..."> references (and skips anything already data:).
    Each worker fetches the image via httpGet(...):
        1 minute per request timeout, and a max body size of maxImageSize (~32 MiB).
        Requires HTTP 200.
        Uses the response Content-Type header if present; otherwise MIME type is sniffed.
    Then it base64-encodes the downloaded bytes and rewrites the <image href="..."> to a data: URI exactly like local bundling.

Shared behavior (both)

Both functions:

    Find <image href="..."> using imageRegex.
    Deduplicate by href so the same image is only fetched/read once (per call).
    Run up to 16 concurrent workers.
    Use a 5 minute overall timeout for the full bundling operation.
    Optionally cache results in the global imgCache (cacheImages=true) keyed by the original href string (so repeated calls can reuse already-encoded outputs).
``

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mrueg Without bundling it doesn't work in Confluence (if remote images are in the given SVG file) , since Confluence SVG engine, at this time, cannot resolve remote images.

Yes, remote bundle can timeout, that's expected.

Actually, bundling should be the default behaviour given Confluence constraints, the only contraindication being not making remote http calls for security reasons.

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

Adds configuration to render D2 fenced code blocks as either PNG (current behavior) or SVG, with optional SVG asset bundling and scale-aware dimensions for Confluence image macros.

Changes:

  • Introduce --d2-output (png|svg) and --d2-bundle CLI/TOML/env configuration and propagate through MarkConfig.
  • Extend the fenced code block renderer to route D2 rendering through PNG or SVG paths and pass source path/scale.
  • Add SVG rendering + bundling implementation in d2 package and expand tests + README docs.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
util/flags.go Adds d2-output and d2-bundle CLI flags with env/TOML sources.
util/cli.go Wires new D2 config values into types.MarkConfig.
types/types.go Extends MarkConfig with D2Output and D2Bundle.
renderer/fencedcodeblock.go Routes D2 fenced code blocks to PNG vs SVG processing and passes input path/bundle/scale.
markdown/markdown.go Updates renderer constructor call signature to include the document path.
d2/d2.go Refactors SVG rendering, adds SVG bundling, and parses SVG dimensions for scale-aware width/height.
d2/d2_test.go Adds coverage for the SVG path and loosens PNG dimension assertions.
README.md Documents new --d2-output and --d2-bundle flags.
go.mod Adds new indirect dependency entries after tidy.
go.sum Updates sums for new/updated indirect dependencies after tidy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread d2/d2.go
Comment thread d2/d2_test.go Outdated
Comment thread d2/d2_test.go
@mnowotnik

mnowotnik commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

@mrueg Fixed mermaid_test.go cause it's been failing on main also.

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

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread d2/d2.go Outdated
Comment thread d2/d2.go Outdated
Comment thread renderer/fencedcodeblock.go Outdated

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

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread util/flags.go Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread d2/d2_test.go Outdated
mnowotnik and others added 3 commits April 19, 2026 14:31
NotEmpty passes for "0", which contradicts the comment's intent to
ensure positive dimensions. Parse Width/Height as int64 and assert
> 0, mirroring the mermaid test pattern.

Co-authored-by: Claude <noreply@anthropic.com>
The flag's behavior diverged from its documentation in two ways: local
assets were inlined unconditionally, and the flag was a no-op for the
default --d2-output png path (PNG is rendered by snapshotting the SVG
in headless Chrome, which already resolves all references). Drop the
flag and always call BundleLocal+BundleRemote for SVG output to bring
mark in line with `d2 --bundle` semantics by default.
Changes:
Library and CLI:
- Add the root library entrypoint and tests, thin the CLI wrapper, and move the command binary entrypoint into cmd/mark (mark.go, mark_test.go, util/cli.go, cmd/mark/main.go)
Rendering and transformers:
- Merge the GitHub alerts transformer/renderer work and the related markdown, image, blockquote, paragraph, text, metadata, page, and stdlib updates with new comparison and image coverage (transformer/gh_alerts.go, renderer/gh_alerts_blockquote.go, markdown/markdown.go, renderer/image.go, markdown/transformer_comparison_test.go, renderer/image_test.go)
D2 and diagram support:
- Preserve the feature branch D2 SVG output path through the new config flow and renderer signatures, with matching flag, type, and test updates (d2/d2.go, d2/d2_test.go, util/flags.go, types/types.go, renderer/fencedcodeblock.go, markdown/markdown.go, util/cli.go)
Build and dependency sync:
- Bring the repository, workflows, release config, docs, and dependency set in line with master while keeping the branch changes buildable (go.mod, go.sum, .github/workflows/ci.yml, .github/workflows/goreleaser.yml, .goreleaser.yml, Dockerfile, Makefile, README.md)

Why?:
- Bring feature/svg-support up to date with master without losing the branch-specific D2 SVG work
- Keep the newer library split, renderer changes, and build/dependency updates together so the merge remains coherent and testable
@mnowotnik

Copy link
Copy Markdown
Contributor Author

@mrueg can you give some feedback on this one?

@mnowotnik
mnowotnik requested review from Copilot and mrueg May 15, 2026 12:12

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

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

Comment thread util/flags.go
Comment thread d2/d2.go Outdated
Comment thread renderer/fencedcodeblock.go
Comment thread d2/d2_test.go Outdated
Comment thread d2/d2.go

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

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

Comment thread d2/d2.go Outdated
Comment thread util/flags.go
Comment thread d2/d2.go
Comment thread d2/d2.go Outdated
Comment thread util/cli.go Outdated
Comment thread d2/d2_test.go Outdated
Changes:
D2 CLI validation:
- Reject invalid d2-output values during flag validation and cover accepted and rejected cases (util/flags.go, util/cli_test.go)
Review hardening:
- Warn when SVG dimensions cannot be derived and handle wrapped EOF correctly in the parser (d2/d2.go)
Renderer cleanup:
- Rename the local D2 attachment variable to avoid shadowing the attachment package (renderer/fencedcodeblock.go)

Why?:
- Resolve the substantive Copilot review findings without changing the existing SVG attachment contract
- Surface configuration and metadata issues earlier while keeping SVG rendering behavior stable

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

Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.

Comment thread README.md Outdated
Comment thread d2/d2.go
Comment thread d2/d2.go
Comment thread renderer/fencedcodeblock.go
Comment thread util/cli.go Outdated
Comment thread renderer/fencedcodeblock.go
Changes:
CLI normalization:
- Trim and lowercase d2-output at config construction time and reject non-positive d2-scale values in flag validation (util/cli.go, util/flags.go)
Test coverage:
- Add coverage for whitespace-padded d2-output, invalid d2-scale values, and shared D2 output normalization (util/cli_test.go)
Library fail-fast validation:
- Normalize and validate D2 config in exported mark entry points so programmatic callers fail before file reads or renderer traversal (mark.go, mark_test.go)

Why?:
- Resolve the latest substantive Copilot review items around inconsistent D2 input handling
- Keep CLI and library callers aligned so invalid D2 configuration is rejected predictably and early

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

Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (3)

mark.go:173

  • normalizeAndValidateConfig is called in both Run and ProcessFile, but Run invokes ProcessFile for each matched file, so the validation runs once up-front and then again for every processed file. The operation is idempotent so behavior is correct, but it's wasted work and obscures the invariant of which entry point is responsible for normalization. Consider validating only at the public entry points (and trusting the normalized struct internally), or have Run skip the call when it already validated.
func Run(config Config) error {
	var err error
	config, err = normalizeAndValidateConfig(config)
	if err != nil {
		return err
	}

	api := confluence.NewAPI(config.BaseURL, config.Username, config.Password, config.InsecureSkipTLSVerify)

	files, err := doublestar.FilepathGlob(config.Files)
	if err != nil {
		return err
	}

	if len(files) == 0 {
		msg := "no files matched"
		if config.CI {
			log.Warn().Msg(msg)
		} else {
			return errors.New(msg)
		}
	}

	var hasErrors bool
	for _, file := range files {
		log.Info().Msgf("processing %s", file)

		target, err := ProcessFile(file, api, config)
		if err != nil {
			if config.ContinueOnError {
				log.Error().Err(err).Msgf("processing %s", file)
				hasErrors = true
				continue
			}
			return err
		}

		if target != nil {
			log.Info().Msgf("page successfully updated: %s", api.BaseURL+target.Links.Full)
			if _, err := fmt.Fprintln(config.output(), api.BaseURL+target.Links.Full); err != nil {
				return err
			}
		}
	}

	if hasErrors {
		return fmt.Errorf("one or more files failed to process")
	}

	return nil
}

// ProcessFile processes a single markdown file and publishes it to Confluence.
// Returns nil for the page info when compile-only or dry-run mode is active.
func ProcessFile(file string, api *confluence.API, config Config) (*confluence.PageInfo, error) {
	var err error
	config, err = normalizeAndValidateConfig(config)
	if err != nil {
		return nil, err
	}

d2/d2.go:250

  • parseSVGDimensions walks until io.EOF and only returns a result when it finds an <svg> with parseable dimensions. If dec.Token() returns a non-EOF error mid-stream (e.g., malformed XML somewhere later in the document) after a perfectly good outer <svg> was already inspected but had only non-numeric width/height and no viewBox, the function returns that downstream error instead of the dimension parsing context recorded in parseErr. Consider returning parseErr (when non-nil) in preference to unrelated decoder errors so callers get the more actionable failure cause, mirroring the EOF branch.
	for {
		tok, err := dec.Token()
		if err != nil {
			if errors.Is(err, io.EOF) {
				if parseErr != nil {
					return nil, parseErr
				}
				return nil, fmt.Errorf("svg dimensions not found")
			}
			return nil, err
		}

d2/d2.go:162

  • When parseSVGDimensions returns an error, ProcessD2SVG logs a warning and continues with boxModel == nil, which sets Width and Height to empty strings. The fenced-code-block template then renders att.Width/att.Height as empty into ac:image attributes. It would be helpful to confirm the Confluence storage-format template tolerates empty width/height (or to set a reasonable fallback) so a parse failure doesn't silently produce broken image markup; at minimum, this fallback behavior is worth a code comment.
	boxModel, err := parseSVGDimensions(out)
	if err != nil {
		log.Warn().
			Err(err).
			Str("title", title).
			Str("input_path", inputPath).
			Msg("could not read svg dimensions; width and height metadata will be omitted")
	}

	checkSum, err := attachment.GetChecksum(bytes.NewReader(d2Diagram))
	if err != nil {
		return attachment.Attachment{}, err
	}

	if title == "" {
		title = checkSum
	}

	width := ""
	height := ""
	if boxModel != nil {
		width = formatSVGDimension(boxModel.width, scale)
		height = formatSVGDimension(boxModel.height, scale)
	}

Comment thread mark.go Outdated
Comment thread util/flags.go Outdated
Comment thread renderer/fencedcodeblock.go
Comment thread d2/d2.go
Comment thread README.md Outdated
Changes:
Validation alignment:
- Make CLI d2-scale validation match the shared mark package rule so non-positive scales are only rejected when the d2 feature is enabled, with updated coverage for both paths (util/flags.go, util/cli_test.go)
Renderer contract docs:
- Document the fenced code renderer path parameter and field as the markdown-source base for local D2 asset resolution, including the empty-path fallback behavior (renderer/fencedcodeblock.go)
README wording:
- Qualify the D2 SVG asset-bundling docs to note that remote bundling failures fail rendering instead of claiming it always succeeds (README.md)

Why?:
- Resolve the latest worthwhile Copilot follow-up comments without changing the public behavior beyond aligning CLI and library validation
- Make D2 path and remote bundling behavior clearer for callers and users

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

Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

mark.go:172

  • Run already calls normalizeAndValidateConfig and then passes the resulting config to ProcessFile, which calls normalizeAndValidateConfig again on the same already-normalized config. This is harmless today but creates an implicit assumption that validation is idempotent. Either skip validation in ProcessFile when called from Run (e.g. via an internal helper that takes an already-validated config), or document that ProcessFile is also a public entry point and the double-validation is intentional.
func ProcessFile(file string, api *confluence.API, config Config) (*confluence.PageInfo, error) {
	var err error
	config, err = normalizeAndValidateConfig(config)
	if err != nil {
		return nil, err
	}

Comment thread mark.go Outdated
Comment thread d2/d2.go
Comment thread d2/d2.go
Comment thread README.md
Changes:
Shared validation helper:
- Extract a single D2 normalization and validation helper in the mark package and reuse it from library and CLI entry points so rules and error wording stay aligned (mark.go, util/flags.go, util/cli.go)
Test alignment:
- Update CLI validation tests to assert the shared D2 validation behavior and shared error messages after the consolidation (util/cli_test.go)

Why?:
- Resolve the last worthwhile Copilot maintainability finding about duplicated D2 validation logic
- Reduce future drift risk between CLI parsing and programmatic Run/ProcessFile behavior

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

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Comment thread README.md
Comment thread util/cli.go Outdated
Comment thread d2/d2.go Outdated
Changes:
D2 SVG parsing:
- Support additional absolute CSS length units when reading SVG width and height, while preserving viewBox fallback behavior in d2/d2.go.
- Cover absolute-unit parsing and unsupported relative-unit fallback in d2/d2_test.go.
CLI validation flow:
- Remove redundant D2 config normalization from RunMark and let the existing flag check plus mark.Run validation own that path in util/cli.go.

Why?:
- Avoid dropping SVG dimensions if D2 emits supported non-px absolute units.
- Keep the CLI and library validation flow simpler and reduce duplicated validation sites.

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

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Comment thread d2/d2.go Outdated
Comment thread d2/d2.go
Comment thread mark.go
Changes:
D2 SVG handling:
- Compute SVG attachment checksums from the bundled SVG output instead of the source diagram, so attachment updates track asset changes in d2/d2.go.
- Parse SVG length units case-insensitively and keep the corrected point-unit conversion in d2/d2.go.
Test coverage:
- Assert SVG checksums against emitted file bytes and cover uppercase CSS units in d2/d2_test.go.
Validation flow:
- Route Run through an internal validated ProcessFile path so config normalization happens once per run while preserving public ProcessFile validation in mark.go.

Why?:
- Ensure changes-only attachment reuse does not miss bundled SVG asset drift.
- Make SVG dimension parsing more robust across tool output variations.
- Remove redundant per-file validation work without weakening the public API contract.
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.

3 participants