Pull in 4 bug fixes from upstream wireviz/WireViz#1
Conversation
Without this, SVG connector/cable images embedded as base64 data URIs emit "image/svg" instead of the correct "image/svg+xml" and browsers fail to render them. Cherry-picked from wireviz#443 (originally by Michael Wagner / @TheMDev) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…iz#473) Custom HTML templates referenced by name in metadata.template.name were only resolved against the output directory and the built-in templates dir. When --output-dir pointed elsewhere, a template living next to the input YAML wouldn't be found. Threads the input file path through parse() → Harness → wv_html so the source directory is searched first, then the output directory, then the built-in templates. Cherry-picked from wireviz#473 (originally by Andrew Cassidy / @ddi-acassidy, addressing wireviz#472) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wireviz#495) A hex RGB color specifier (7 chars) tripped the heuristic that a string longer than 2 chars meant a multi-colored wire, causing single hex colors to be rendered with extra padding. Use get_color_hex() to count actual color components instead of relying on the spec string length. Cherry-picked from wireviz#495 (originally by @kvid, fixing wireviz#494) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…viz#439) Three root causes in the loop-handling path: 1. Loop-only connectors silently dropped — a connector declared with loops: but never referenced in any connection set was treated as an unused template and never instantiated. Now auto-instantiated with an Info: log noting the promotion. 2. All loops forced onto a single side. The previous create_graph picked one side per connector from ports_left/ports_right and emitted every loop edge there. Two-sided connectors got stray arcs on the wrong side, and a dead-end "No side for loops" exception could fire on loop-only pin pairs. Each pin now records its activated sides in a new Connector.pin_sides dict, and Connector.resolve_loops() picks per-loop endpoints by intersecting the two pins' known sides. Disjoint sides emit a cross-connector loop; loop-only pairs fall back to an existing port column; default is RIGHT. 3. Loop edge ports referenced pin numbers instead of pin positions, matching the cable-edge convention only by accident when pin numbers happened to equal positions. On connectors with explicit non-sequential pins: lists, GraphViz silently routed the loop arcs over the connector node centre. Adds three regression-witness YAML examples (loopback_bug, loopback_nonseq, loopback_return) and their rendered artifacts regenerated against this environment's graphviz. Cherry-picked from wireviz#496 (originally by Mark Ruvald Pedersen / @eisbaw) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the YAML → Harness → GraphViz pipeline, entry points (wv_cli.py / wireviz.parse() / build_examples.py), the core model modules, and the relationship to the planned wireviz-gui sibling repo that will consume parse() programmatically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for auto-instantiating connectors that contain only loopback wires, even if they are not explicitly referenced in the connection set. It also improves the resolution of loopback wire sides by considering existing cable connections, and adds support for resolving custom HTML templates relative to the YAML source file. My feedback highlights a potential issue in the auto-instantiation logic where it may incorrectly instantiate a template that has already been used elsewhere in the harness.
| for template_name, attribs in template_connectors.items(): | ||
| if template_name in harness.connectors: | ||
| continue |
There was a problem hiding this comment.
The auto-instantiation logic for loop-only connectors is too broad. It currently checks if the template_name is in harness.connectors, but it doesn't check if the template was used to instantiate other connectors (e.g., using the Template.Designator syntax). If a template with loops is used for one or more instances, this logic will still auto-instantiate the template itself as a separate floating connector, which is likely not intended. It should also check if the template name exists in designators_and_templates.values() to ensure it wasn't already used as a template for other components.
| for template_name, attribs in template_connectors.items(): | |
| if template_name in harness.connectors: | |
| continue | |
| for template_name, attribs in template_connectors.items(): | |
| if template_name in harness.connectors or template_name in designators_and_templates.values(): | |
| continue |
…Designator The auto-instantiation pass added in 222e671 (port of upstream PR wireviz#496) guarded against re-instantiating a template by checking `template_name in harness.connectors`, but that only catches the case where the template was used as a literal designator. When a template is used via the `Template.Designator` syntax (e.g. `DSub.X1`), the harness holds the *instance* (X1), not the template name (DSub) — so the guard missed the case and the template would get a phantom floating connector named after the template carrying duplicate loop edges. Also skip when `template_name` appears in `designators_and_templates.values()`, which is the canonical record of templates that have been bound via the T.X syntax during connection-set processing. Verified against examples/loopback_bug.yml (loop-only connector still auto-instantiates correctly) and a regression case where a template with loops is referenced as `DSub.X1` / `DSub.X2` (no phantom DSub connector generated, no Info: auto-instantiating log). Addresses gemini-code-assist review feedback on #1 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Heads-up to the original PR authors whose work is being incorporated here: @TheMDev (upstream #443 — SVG MIME type) Background: We use WireViz to document Classic Mini Cooper wiring harnesses at Classic Mini DIY, and we're building out a GUI on top of it. Upstream Each commit in this PR retains attribution to your original work and links back to your upstream PR. Generated example artifacts were rebuilt against our environment (graphviz 14.1.4) where applicable, but no semantic changes to your logic. If you'd prefer different attribution wording or notice anything off, please flag it. Thanks for the work — it's actively useful to us. |
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>
|
Appreciate the ping, your project looks quite interesting and I'll definitely be looking into it further! |
|
@TheMDev Thanks! Happy to take any contributions if you have ideas. I am working on building comprehensive tests for this repo first. Then I am going to start fresh on making a GUI version of this tool in a second repo. |
3 participants
Summary
Cherry-picks 4 bug fixes that have been sitting in open PRs against the abandoned upstream
wireviz/WireVizrepo, plus adds aCLAUDE.mdto orient future Claude Code sessions in this fork.image/svginstead ofimage/svg+xml), so browsers fail to render them.svgembed.pymetadata.template.nameweren't found when--output-dirpointed away from the YAML source. Now searches the source directory first, then output dir, then built-in templates.Harness.py,wireviz.py,wv_cli.py,wv_html.py#ff0000) tripped the multi-color heuristic (string length > 2) and rendered single-color wires with extra padding. Now counts actual color components viaget_color_hex().Harness.pypins:lists).DataClasses.py,Harness.py,wireviz.py+ 3 new regression-witness YAML examplesVerification
build_examples.pyruns cleanly across all 14 examples + 8 tutorials + 2 demos with the fixes applied..gv,.bom.tsv) are byte-identical to baseline → no behavior change on any existing example.examples/loopback_*.ymlregression witnesses render correctly; their artifacts in this PR were regenerated against this environment's graphviz (14.1.4) rather than carrying over the upstream PR author's environment.Notes
loopback_*.ymlfiles are outsidebuild_examples.py's sweep (it only scansex*/tutorial*/demo*prefixes). Follow-up: rename toex15–ex17or add a fourth group to the script.Test plan
Create Examplespasses across Python 3.7–3.12loopback_bug.svg,loopback_nonseq.svg,loopback_return.svgrender correctly in a browser🤖 Generated with Claude Code