Skip to content

Ship stdin/stdout + 6 feature ports to master#10

Merged
SomethingNew71 merged 21 commits into
masterfrom
upstream-fixes
May 5, 2026
Merged

Ship stdin/stdout + 6 feature ports to master#10
SomethingNew71 merged 21 commits into
masterfrom
upstream-fixes

Conversation

@SomethingNew71
Copy link
Copy Markdown

Summary

Ships everything currently on `upstream-fixes` to `master`. Once the companion PR (`port-321-stdin-stdout` → `upstream-fixes`) is merged, this PR will carry the full set of upstream feature ports through to `master`.

What's included

Already on `upstream-fixes` (from PR #2)

  • stdin/stdout streaming (upstream #321) — pipe YAML in, get rendered SVG/PNG/etc. out

Coming via the companion PR

Merge order

Merge the companion port-321 → upstream-fixes PR first, then this one. (Or, if you prefer the shortcut: close this and merge port-321 → master directly. Same end state, fewer merge commits.)

Verification

All features verified end-to-end on the chained tree (see companion PR test plan).

🤖 Generated with Claude Code

SomethingNew71 and others added 20 commits May 4, 2026 19:21
Lets the CLI participate in unix pipelines:

  cat harness.yml | wireviz -f s -O - -    > harness.svg
  cat harness.yml | wireviz -f p -O - -    > harness.png
  wireviz -f h -O - harness.yml             > harness.html

Pass `-` as the input filename to read YAML from stdin, and pass `-` to
either `--output-dir` or `--output-name` to write the rendered output to
stdout. When writing to stdout exactly one format may be requested.

Two refactors enable this:

1. Harness.output() now computes every requested format in memory via
   `Harness._render()` (graph.pipe(format=...) + embed_svg_images on a
   string) instead of the previous `graph.render()` -> `.tmp.svg` ->
   embed_svg_images_file -> rename dance. The caller dispatches the
   resulting `{fmt: bytes|str}` dict to either files or stdout.

2. generate_html_output() now takes the SVG as a string and returns the
   HTML string rather than reading a tmp.svg from disk and writing the
   .html file itself. New optional `output_dir` / `output_name` /
   `png_b64` parameters preserve the `<!-- %filename% -->` and
   `<!-- %diagram_png_b64% -->` template placeholders in file mode and
   leave them empty/unresolved in stdout mode.

Library-level `print()` warnings in DataClasses.py, Harness.py,
wv_colors.py, wv_helper.py, and wireviz.py are routed to `sys.stderr`
so stdout stays clean of log noise when piped. CLI status banners in
wv_cli.py go to stderr for the same reason.

Also drops the now-unused `embed_svg_images_file` helper.

Ported to master from wireviz#321
(originally targeting `dev` by Guillaume Grossetie / @ggrossetie,
resolves wireviz#320). The PR was rebased / adapted to current master's
file_read_text/file_write_text helpers and current Harness.output
signature; the in-memory render dict and stdout dispatch are the
load-bearing additions.

Verification:
- All 14 examples + 8 tutorials + 2 demos rebuild without errors.
- Deterministic outputs (.gv, .bom.tsv) byte-identical to baseline ->
  no behavior change in file mode.
- Verified stdin->stdout for SVG (text) and PNG (binary).
- Multi-format-to-stdout correctly rejected with a clear error.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six fixes from gemini-code-assist's review of
#2:

* Harness.py: move ``import base64`` to the module-level imports rather
  than the function-local position inside ``_render()`` (PEP 8).

* Harness.py: ``output()`` and ``_render()`` now accept fmt as
  ``Union[str, Tuple[str, ...], List[str]]`` and normalize a bare
  string to a one-tuple before iterating. Previously a programmatic
  caller passing ``output_formats="svg"`` to ``wireviz.parse()`` would
  cause ``for f in fmt:`` to iterate over the characters ``s``, ``v``,
  ``g`` — a pre-existing latent bug that became reachable through this
  PR's refactor.

* Harness.py: when ``self.source_path`` is set, embed_svg_images now
  resolves relative <image src=...> paths against the YAML source's
  parent directory rather than ``Path.cwd()``. In normal use
  wireviz.parse() rewrites relative image paths to absolute during
  YAML parse, so this only matters for already-rendered Harness
  objects or when a tweak injects a post-parse relative path — but
  it's the conceptually correct base path.

* wireviz.py: ``raise ValueError`` instead of generic ``Exception`` for
  the "exactly one output format when writing to stdout" check —
  signals to programmatic callers that this is an argument-validity
  error, not an internal failure.

* wv_cli.py: ``raise click.UsageError`` instead of generic
  ``Exception`` for the same check on the CLI side. Click renders
  UsageError with a "Try 'wireviz -h' for help." footer instead of a
  Python traceback, which is the right UX.

Verified with build_examples.py: deterministic outputs (.gv, .bom.tsv)
remain byte-identical to the baseline; the ``output_formats="svg"``
programmatic path now produces a valid SVG; multi-format-to-stdout is
still rejected, now with a clean Click-formatted error.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add stdin/stdout streaming (port of upstream PR wireviz#321)
Lets users point WireViz at an explicit directory of HTML templates
when resolving a metadata.template.name reference. Useful for shared
branded chrome that lives outside both the YAML source tree and the
output tree.

CLI:
    wireviz -t ./brand-templates harness.yml

Programmatic:
    wireviz.parse(yaml_str, output_formats=("html",),
                  template_dir="./brand-templates", ...)

The new explicit path is searched first, before the implicit ones
already in place. Final lookup order:

    1. --template-dir / parse template_dir         (explicit)
    2. YAML source directory (source_path.parent)  (PR wireviz#473)
    3. output directory                            (existing)
    4. WireViz built-in templates                  (fallback)

Adapted from wireviz#444 (originally by
@tbornon-sts) — the upstream patch used inconsistent naming
(``templatedir`` on the kwarg, ``template_dir`` on the CLI option) and
included a leftover ``print("Test")`` debug statement; this port uses
``template_dir`` consistently and drops the debug line.

Verified against build_examples.py (deterministic outputs unchanged)
and against a manual case with a custom branded.html living only in
the -t directory: template resolves correctly, and absence of -t
produces a clean "was not found" error from smart_file_resolve.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Exposes the Graphviz "dpi" graph attribute as a top-level WireViz YAML
option. Useful for boosting the resolution of PNG output beyond the
graphviz default of 96 DPI:

    options:
      output_dpi: 192   # 2x default — 4x pixel area in the PNG

Defaults to 96.0, matching graphviz's own default for non-PostScript
renderers, so existing harnesses render at the same pixel dimensions
they always have.

Files:
* DataClasses.py — Options.output_dpi: Optional[float] = 96.0
* Harness.py — pass dpi=str(self.options.output_dpi) into the graph attr
* docs/syntax.md — documents the new option under "options"
* examples/*.gv, tutorial/*.gv — rebaselined: every .gv now carries
  ``dpi=96.0``. The .png / .svg / .html outputs are environment-
  dependent (graphviz version) and intentionally left untouched, per
  CONTRIBUTING.md's "owner will rebuild" policy. ex08.gv is also
  intentionally left as baseline because it still contains absolute
  image paths from the original maintainer's machine.

Verified:
* Default DPI: demo PNG renders at 428x195 (matches pre-PR baseline).
* output_dpi=192: same harness renders at 857x391 — exactly 2x linear
  scale, 4x pixel area, as expected.
* build_examples.py runs cleanly across all examples.

Ported from wireviz#379 (originally
targeting upstream `dev` by Tobias Falk / @tobiasfalk).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses gemini-code-assist feedback on
#3 — template_dir was
missing from the docstrings of Harness.output(), Harness._render(),
and wireviz.parse(). Expanded scope to also document the parameters
that earlier PRs in this chain added without docstring coverage:

* Harness.output(): Args section now describes filename (incl. None →
  stdout semantics), fmt (incl. str→tuple normalization), output_dir,
  output_name, and template_dir; view/cleanup are noted as kept for API
  compat.
* Harness._render(): Args + Returns sections describing fmt,
  output_dir, output_name, template_dir, and the bytes-vs-str
  per-format return contract.
* wireviz.parse(): source_path (added during PR #1's loopback fix and
  threaded through PR #2's stdin/stdout port) and template_dir (this
  PR) added to the Args section, with template-search-priority
  semantics spelled out.

No behavior change. Verified against build_examples.py: deterministic
outputs unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses gemini-code-assist feedback on
#4 — the prior
``dpi=str(self.options.output_dpi)`` would emit the literal string
``"None"`` if a user set ``output_dpi: null`` in YAML, which Graphviz
treats as an invalid value.

The reviewer's exact suggestion (``dpi=self.options.output_dpi`` —
relying on graphviz auto-coercion of numerics) doesn't quite work
either: the graphviz Python lib filters None but does NOT auto-convert
ints/floats to strings (``dpi=192`` raises
``TypeError: expected string or bytes-like object, got 'int'``).

So compromise: build the graph attr dict, conditionally include the
dpi key only when output_dpi is not None, and stringify it ourselves.
Verified:

* ``output_dpi: 96.0`` (default) — emits ``dpi=96.0`` as before; all
  example .gv baselines remain byte-identical.
* ``output_dpi: 192``        — emits ``dpi=192``; PNG renders at 2x
  scale (857x391 vs 428x195 default).
* ``output_dpi: null``       — no dpi attr emitted; PNG renders at
  Graphviz's renderer default (96 for PNG → matches default-scale).

Also updates the DataClasses.Options.output_dpi comment to document
the null-as-defer-to-graphviz semantic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add --template-dir CLI option (port of upstream PR wireviz#444)
Add output_dpi option (port of upstream PR wireviz#379)
…eam PR wireviz#234)

Renders now embed the source YAML in PNG output as a zlib-compressed
iTXt chunk under the key ``wireviz:yaml``. The CLI auto-detects ``.png``
inputs and pulls the YAML back out, so a single PNG file is enough to
re-render or edit a harness — no sidecar .yml needed.

The headline workflow:

    wireviz harness.yml             # produces harness.png with yaml inside
    wireviz harness.png             # round-trips: extract YAML, re-render

This is the load-bearing capability for the upcoming wireviz-gui:
drag a PNG into the editor and recover the source. Without it, every
PNG in the wild is an opaque artifact divorced from its model.

API surface:
* wireviz.parse() gains ``embed_yaml: bool = True``. The default
  embeds; pass False to render plain PNGs without source-bearing
  metadata.
* Harness.output() / _render() gain ``yaml_source: Optional[str]``.
  When non-None and PNG is in the requested formats, the rendered PNG
  bytes are post-processed through PIL to attach the iTXt chunk.
* New module-level helpers in Harness.py:
  - PNG_YAML_CHUNK_KEY = "wireviz:yaml"
  - _embed_yaml_in_png(png_bytes, yaml_source) -> bytes
  - read_yaml_from_png(png_path) -> Optional[str]
* CLI ``--no-embed-yaml`` flag opts out of embedding when desired
  (e.g. before sharing a diagram externally without source).

Implementation notes:
* The chunk uses ``iTXt`` (international text, zip-compressed) rather
  than ``zTXt`` so unicode YAML round-trips cleanly. Key prefix
  ``wireviz:`` namespaces the chunk against PNG software-defined
  keywords.
* When parse() is called with a Dict input, we yaml.safe_dump it back
  for embedding — round-trip-readable, but without the original
  comments or formatting (those don't survive the dict-conversion
  step regardless of embedding).
* build_examples.py opts out (``embed_yaml=False``) so the regression
  baseline PNGs stay deterministic.

Adapted from wireviz#234 (originally
by @jacobian91, targeting upstream ``dev``). The 2021-era PR was
heavily bit-rotted — argparse, the old parse_cmdline / parse_file
layer, conceal-input enum — only the load-bearing idea (zTXT/iTXt
embed in PNG, .png input recovery) was preserved. Reworked against
current master's click CLI, the in-memory render dict from PR wireviz#321
stdin/stdout, and threaded source_path / template_dir from earlier
PRs in this chain.

Verified:
* round-trip: harness.yml → harness.png → re-extract → identical YAML
* --no-embed-yaml produces a PNG without the chunk (verified via PIL)
* ``wireviz harness.png`` on a chunk-less PNG raises a clean
  click.UsageError
* build_examples.py runs cleanly; .gv and .bom.tsv byte-identical to
  baseline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…wireviz#492)

Resolves to the key of the most recently added entry in
``metadata.revisions``. Useful in branded HTML chrome to surface a
"current revision" badge without expanding to the full
``<!-- %revisions_N_key% -->`` indexed form.

Example template fragment:

    <span class="rev">Rev <!-- %revision% --></span>

Adapted from wireviz#492 (originally
by @ishaid, targeting upstream ``dev``). The upstream patch was
against ``wv_output.py`` (a ``dev``-only renaming of ``wv_html.py``);
this port lives in master's ``wv_html.py``. Helper renamed from
``_get_latest_revision`` to ``_latest_revision`` and tightened to
return ``""`` for missing/None/empty revisions instead of raising.

Documents the new placeholder in templates/README.md.

Verified:
* Direct unit test: ``_latest_revision({"revisions": {"A": ..., "B": ...,
  "C": ...}})`` returns ``"C"``.
* Empty/missing/None ``revisions`` returns ``""``.
* build_examples.py: deterministic outputs (.gv, .bom.tsv) byte-
  identical to baseline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…iew feedback)

Addresses gemini-code-assist feedback on
#6 — the prior
``str(list(revisions)[-1])`` form returned only the last *character*
when ``revisions:`` was a string scalar (e.g. ``revisions: v1.0`` →
``"0"``), and would raise ``TypeError`` on a non-iterable scalar like
an integer.

Now branches on type:
* dict / list → last key/element (preserves prior behavior)
* str / int / float / any other non-None scalar → str(value)
* None / empty container / missing → ""

Verified against all six shapes:

  {'revisions': {'A': ..., 'B': ..., 'C': ...}} -> 'C'
  {'revisions': ['A', 'B', 'C']}                -> 'C'
  {'revisions': 'v1.2'}                         -> 'v1.2'
  {'revisions': 42}                             -> '42'
  {'revisions': None}                           -> ''
  {'revisions': {}}                             -> ''

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…stream PR wireviz#357)

Connectors and cables can now carry their own ``tweak:`` block with the
same ``override`` / ``append`` shape as the global one. The harness
folds per-node tweaks into the global tweak at instantiation, with an
optional placeholder substring rewritten to the node's actual
designator — making it practical to author a single tweak template and
apply it to many components.

Example:

    tweak:
      placeholder: "@@"

    connectors:
      X1:
        pinlabels: [A, B]
        tweak:
          append:
            - "@@_extra [color=red, style=dashed];"

renders X1's per-node tweak as ``X1_extra [color=red, style=dashed];``
in the .gv source. The same connector definition reused for X2 would
produce ``X2_extra ...``.

Placeholder semantics:
* Per-node ``tweak.placeholder`` overrides the global ``tweak.placeholder``.
* An empty string at the per-node level explicitly opts out of
  substitution for that node.
* ``None`` (the default) falls back to the global placeholder.
* When neither is set, no substitution happens — bare strings are
  appended/overridden as-written.

Implementation:

* DataClasses.py — ``Tweak`` gains ``placeholder: Optional[str] = None``.
  ``Connector`` and ``Cable`` gain ``tweak: Optional[Tweak] = None``,
  with ``__post_init__`` coercing a dict literal into a Tweak instance.
* Harness.py — new ``Harness._extend_tweak(node)`` method, called from
  ``add_connector()`` and ``add_cable()``, performs the placeholder
  substitution and merges into ``self.tweak``. Raises ``ValueError`` if
  two nodes contribute conflicting overrides for the same key.
* docs/syntax.md — documents per-connector / per-cable tweak fields and
  the new placeholder semantics.

Adapted from wireviz#357 (originally by
@kvid, targeting upstream ``dev``). Renamed ``extend_tweak`` to
``_extend_tweak`` to mark it private; otherwise faithful to the
original logic. ``make_list`` is already in master's wv_bom so no
helper backport needed.

Verified:
* Smoke test with placeholder ``@@`` and per-connector + per-cable
  ``append:`` blocks produces ``X1_extra``, ``W1_label``,
  ``cable W1`` in the rendered .gv (the @@'s are substituted).
* build_examples.py: deterministic outputs (.gv, .bom.tsv) byte-
  identical to baseline (no existing example uses the new syntax, so
  no new substitutions fire).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ack)

Addresses gemini-code-assist feedback on
#7:

* The ``rph`` lambda would raise ``AttributeError`` when called with a
  None value, which is a legitimate case in YAML when an override
  deletes a key (``key: null``). Now passes non-strings through
  unchanged so substitution is a no-op for None / numeric / bool values.

* ``s_override[ident] = s_dict or None`` would collapse an empty
  per-ident override dict to None, which Harness.create_graph()
  doesn't expect (it iterates ``override.items()`` expecting
  dict-shaped values). Always store the dict, even when empty —
  ``self.tweak.override = s_override or None`` already handles the
  outer "no overrides at all" case.

Verified: per-connector override with ``key: null`` now renders without
the prior AttributeError. build_examples.py deterministic outputs still
byte-identical.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the ``pdf`` output format that's been a TODO stub since
v0.4.1. Pipes the graph through Graphviz's PDF renderer
(``graph.pipe(format="pdf")``) and dispatches the bytes the same way
PNG goes — to a file in normal mode, to ``sys.stdout.buffer`` in
stdout mode.

Usage:

    wireviz -f P harness.yml                 # produces harness.pdf
    cat harness.yml | wireviz -f P -O - -    # stdout, binary

CLI flag changes:
* ``"P": "pdf"`` un-commented in ``format_codes`` (use ``-f P``)
* "PDF output is not yet supported" stderr warning removed from
  Harness.output()

Adapted from wireviz#367 (originally
by @tobiasfalk, targeting upstream ``dev``). The upstream patch went
through the old ``graph.render()`` + temp-file path — this port uses
the in-memory ``graph.pipe()`` wired up by the stdin/stdout refactor
(PR wireviz#321), so PDF works in both file mode AND stdout mode without
extra plumbing.

Verified:
* ``wireviz -f P harness.yml`` produces a valid PDF (file 1.7).
* ``cat harness.yml | wireviz -f P -O - -`` writes valid PDF to stdout.
* build_examples.py: deterministic outputs (.gv, .bom.tsv) byte-
  identical (no example .yml has been switched to request PDF
  rendering — keeping that out of the regression baseline since PDF
  bytes from graphviz vary by version).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eedback)

Addresses gemini-code-assist feedback on
#8 — the prior docstring
claimed PDF includes "the diagram and (depending on the template) the
BOM", which was carried over from upstream's never-completed PDF stub
plan.

The actual implementation in this PR pipes the graph through
Graphviz's PDF renderer (graph.pipe(format="pdf")), which produces a
diagram-only PDF with no embedded BOM table. That matches the PNG/SVG
behavior and is the right scope for a fork that already exposes
HTML+SVG embed for richer output.

Embedding the BOM in PDF would require a full document-composition
step (PIL or reportlab) that's well outside the scope of "implement
the missing format flag" — and HTML output exists for users who want
diagram + BOM in one artifact.

No code change; docstring only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Embed YAML source in PNG output (port of upstream PR wireviz#234)
Add <!-- %revision% --> HTML placeholder (port of upstream PR wireviz#492)
Per-connector / per-cable tweak with name placeholder (port of upstream PR wireviz#357)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the output generation logic to support piping to stdout and reading from stdin. It introduces an internal _render method in the Harness class to produce in-memory representations of various formats, replaces standard print statements with sys.stderr.write for warnings and errors, and updates the CLI to handle the new input/output streams. Feedback includes correcting a Data URI format in the HTML output, using more specific exception types in the CLI, and ensuring relative image paths can be resolved when using stdin.

Comment thread src/wireviz/Harness.py
# rendered in this same call; otherwise let the template
# fall back to reading {output_dir}/{output_name}.png.
png_b64 = (
f"data:image/png;base64, {base64.b64encode(png_bytes).decode('utf-8')}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The Data URI for the base64-encoded PNG contains an extra space after the comma. According to RFC 2397, the data follows the comma immediately. While most browsers are lenient and might strip the whitespace, it is better to adhere to the standard to ensure compatibility with all parsers and avoid potential decoding issues.

Suggested change
f"data:image/png;base64, {base64.b64encode(png_bytes).decode('utf-8')}"
f"data:image/png;base64,{base64.b64encode(png_bytes).decode('utf-8')}"

Comment thread src/wireviz/wv_cli.py
if fmt not in output_formats:
output_formats.append(fmt)
else:
raise Exception(f"Unknown output format: {code}")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using a generic Exception for user-facing errors is discouraged. Since this is a click application and other parts of the code already use click.UsageError (e.g., line 112), it is better to use it here as well for consistency and better error reporting to the user.

Suggested change
raise Exception(f"Unknown output format: {code}")
raise click.UsageError(f"Unknown output format: {code}")

Comment thread src/wireviz/wv_cli.py
for file in filepaths:
if str(file) == "-":
yaml_input = prepend_input + sys.stdin.read()
image_paths = set()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When reading from stdin, the image_paths set is initialized as empty. This means any relative image paths in the YAML input will fail to resolve because there is no base directory to search in. Initializing it with the current working directory ensures that relative paths are resolved correctly, consistent with how file-based input works (where the file's parent directory is used).

Suggested change
image_paths = set()
image_paths = {Path.cwd()}

Bring 6 upstream PR ports onto upstream-fixes
@SomethingNew71
Copy link
Copy Markdown
Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces significant enhancements to WireViz, including the ability to embed YAML source code within PNG outputs for round-trip editing, support for stdin/stdout piping, and a per-node tweak system with placeholder substitution. Additionally, it adds an output_dpi configuration, refactors output rendering for in-memory processing, and migrates logging to stderr. Review feedback suggests clarifying error messages for tweak conflicts, ensuring RFC 2397 compliance for data URIs, and fixing a bug where the source_path is not automatically populated from input files as documented.

Comment thread src/wireviz/Harness.py
Comment on lines +174 to +176
raise ValueError(
f"{node.name}.tweak.override.{ident}.{k} conflicts with another"
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The error message for conflicting tweak overrides is somewhat vague. It would be more helpful to the user if it explicitly stated which value is being conflicted with and what the new value is.

Suggested change
raise ValueError(
f"{node.name}.tweak.override.{ident}.{k} conflicts with another"
)
if k in s_dict and v != s_dict[k]:
raise ValueError(
f"{node.name}.tweak.override.{ident}.{k} conflicts with existing value '{s_dict[k]}' (new value: '{v}')"
)

Comment thread src/wireviz/Harness.py
# rendered in this same call; otherwise let the template
# fall back to reading {output_dir}/{output_name}.png.
png_b64 = (
f"data:image/png;base64, {base64.b64encode(png_bytes).decode('utf-8')}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The data URI for the embedded PNG contains a leading space after the comma (, ). According to RFC 2397, the syntax for a data URI is data:[<mediatype>][;base64],<data>. While most modern browsers are lenient and will ignore the space, it is technically non-standard and should be removed for better compatibility and adherence to the specification.

Suggested change
f"data:image/png;base64, {base64.b64encode(png_bytes).decode('utf-8')}"
f"data:image/png;base64,{base64.b64encode(png_bytes).decode('utf-8')}"

Comment thread src/wireviz/wireviz.py
raise Exception("No output formats or return types specified")

yaml_data, yaml_file = _get_yaml_data_and_path(inp)
yaml_data, yaml_file, yaml_str = _get_yaml_data_and_path(inp)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The docstring for the source_path parameter (lines 81-86) states that it is filled in automatically when inp is a Path. However, the current implementation does not actually perform this auto-fill, which can lead to issues with relative path resolution for templates and images if the caller doesn't explicitly provide it.

    yaml_data, yaml_file, yaml_str = _get_yaml_data_and_path(inp)
    if source_path is None:
        source_path = yaml_file

SomethingNew71 added a commit that referenced this pull request May 5, 2026
Five gemini-code-assist comments on
#10:

* Data URI leading-space stripped in three places (svgembed.py
  data_URI_base64 and embed_svg_images, plus Harness._render's PNG
  b64 inline). The prior ``data:image/png;base64, <b64>`` form had a
  stray space after the comma which violates RFC 2397; most browsers
  tolerate it but stricter parsers (some PDF/EPUB tooling, lint tools)
  reject it. All three call sites now emit the canonical
  ``data:image/png;base64,<b64>``.

* wv_cli.py: ``raise Exception("Unknown output format: ...")`` →
  ``raise click.UsageError(...)``. Matches the multi-format-stdout
  check elsewhere in the file and gives users the clean
  ``Try 'wireviz -h' for help.\nError: ...`` UX instead of a Python
  traceback.

* wv_cli.py: stdin mode now defaults ``image_paths`` to
  ``{Path.cwd()}`` instead of an empty set, so relative
  ``image: src:`` references in piped YAML resolve against the
  invocation directory the same way file-based input resolves
  against the YAML's parent.

* Harness._extend_tweak: tweak override conflict error now reports
  both the existing and new values
  (``"X1.tweak.override.X1.color: new value 'blue' conflicts with
  existing 'red'"``) instead of the prior vague ``"conflicts with
  another"``.

* wireviz.parse(): ``source_path`` now auto-fills from ``yaml_file``
  when the input was a Path. The docstring already promised this
  behavior; the code didn't actually do it. Confirmed via
  ``parse(Path('examples/demo01.yml'), output_formats=('svg',), ...)``
  succeeding without explicit source_path.

Verified against build_examples.py: deterministic outputs (.gv,
.bom.tsv) byte-identical to baseline. Tweak conflict YAML correctly
raises the new error. Bad CLI format flag now renders Click's
UsageError formatting. Data URIs in re-rendered ex08.svg / .html now
emit ``data:image/png;base64,iVB...`` (no space).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SomethingNew71 SomethingNew71 merged commit 6d5c9d0 into master May 5, 2026
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.

1 participant