[bug] hide_disconnected_pins when nothing is connected

[egradman]

Hi,

When I have hide_disconnected_pins set on a connector that has no connections, I get a syntax error from dot. Looks like an empty table isn't supported.

I realize that this is a super edge-case, but the error message wasn't helpful enough to track down the problem, and it took me a long time to figure out what was going on. I generate some pretty complicated wireviz documents with jinja2, so sometimes I encounter weird edge cases like this!

Love the project. Thanks so much for making it!

[formatc1702]

Thanks for the input. I am currently refactoring the entire graph generation code, so I will keep this edge case in mind.

Is there a way to see some of the diagrams you are generating, out of sheer curiosity? Feel free to post them in #143.
I've also noticed your obsidian-wireviz repo.. Never used Obsidian myself, but nice to know that WireViz will be supported :-)

[egradman]

thanks! Here's a dropbox directory containing an example of how I'm combining jinja2 and wireviz. I probably overstated its complexity :) Also, I'm certain I'm not using WireViz in the manner in which you intended! https://www.dropbox.com/sh/9pooo26ypwdjmci/AACWMr0FgoHUQTcLXsLp-s_Ja?dl=0 As far as obsidian-wireviz is concerned, I'll probably poke at that project again next weekend. I totally rely on [mermaid]( https://mermaid-js.github.io/mermaid/#/) in obsidian, and being able to inline quick wireviz diagrams into project notes will be really useful to me!

…

On Mon, Nov 15, 2021 at 9:09 AM Daniel Rojas ***@***.***> wrote: Thanks for the input. I am currently refactoring the entire graph generation code, so I will keep this edge case in mind. Is there a way to see some of the diagrams you are generating, out of sheer curiosity? I've also noticed your obsidian-wireviz <https://github.com/egradman/obsidian-wireviz> repo.. Never used Obsidian myself, but nice to know that WireViz will be supported :-) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#257 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZFWOTGKKHQOBLHWK2M3DUME5GHANCNFSM5H5SDYUA> .

-- *Eric Gradman* CTO & Mad Inventor *P:* 213-268-6253 | *W:* twobitcircus.com <http://www.twobitcircus.com/> *TW:* @egradman <http://www.twitter.com/egradman> *TWO BIT CIRCUS* Engineering Entertainment Inventors | Developers | Performers

[kvid]

Another user has just reported the same bug, and I linked the duplicate to this issue. As both bug reports use quite complicated YAML input examples to demonstrate the same bug, I suggest here a much simpler YAML input to provoke the same error:

connectors:
  X1:
    pincount: 2
    hide_disconnected_pins: true

I got this error when running WireViz 0.3.2 with the input above (in Python 3.7.5 and Graphviz 2.40.1 at Ubuntu 18.04):

Error: syntax error in line 15
...
  </table> ...
in label of node X1
Traceback (most recent call last):
  File "wireviz.py", line 270, in <module>
    main()
  File "wireviz.py", line 266, in main
    parse(yaml_input, file_out=file_out)
  File "wireviz.py", line 195, in parse
    harness.output(filename=file_out, fmt=('png', 'svg'), view=False)
  File "../wireviz/Harness.py", line 437, in output
    graph.render(filename=filename, view=view, cleanup=cleanup)
  File "/home/kvid/src/formatc1702/WireViz/wv-env/lib/python3.7/site-packages/graphviz/files.py", line 209, in render
    quiet=quiet)
  File "/home/kvid/src/formatc1702/WireViz/wv-env/lib/python3.7/site-packages/graphviz/backend.py", line 221, in render
    run(cmd, capture_output=True, cwd=cwd, check=True, quiet=quiet)
  File "/home/kvid/src/formatc1702/WireViz/wv-env/lib/python3.7/site-packages/graphviz/backend.py", line 184, in run
    output=out, stderr=err)
graphviz.backend.CalledProcessError: Command '['dot', '-Tpng', '-O', 'single_connector_hide']' returned non-zero exit status 1. [stderr: b'Error: syntax error in line 15 \n... \n  </table> ...\nin label of node X1\n']

[martinrieder]

There are a few other syntax options for show_*, but only a single one for hide_*. This seems counter-intuitive and might be confusing some users. I would like to argue that the option hide_disconnected_pins should be dropped in favor of a new syntax show_disconnected, which would apply to either connectors or wires or both.

The latter part adds new functionality to wires though, so we might need to discuss this in a new issue. It depends a bit on how this bug would be solved. There was also a similar bug related to loops in issue #263, which got solved by PR #264 by @kvid.

Note that I dropped the ending _pins intentionally to support #331 by @scottbouch. Refer to PR #5 for the original implementation by @elliotmr.

PS: I can confirm this bug. My use case involved using a YAML template that I applied to all connectors. I found this as good practice to show all pins while building the connections list and only hide them when finished.

[kvid]

It's a shame this bug has been present for so long without being fixed. It was not complicated at all. @egradman and @martinrieder - Please try my PR #375 and report here if this issue was fixed.

PS: I agree with @martinrieder that his suggestion about renaming the connector attribute deserves a new issue.

[martinrieder]

Fine, the option is called hide_disconnected_pins, so you are hiding the pinhtml table. This produces different results for "simple" connectors, since these are treated separately. It is a bit edgy, but it works. Thanks for providing a fix!

connectors:
  X1:
    style: simple
    hide_disconnected_pins: true
connections:
  - - X1

connectors:
  X1:
    pincount: 1
    show_name: false
    show_pincount: false
    hide_disconnected_pins: true
connections:
  - - X1

Note: If the connector is not referenced in the connections section, then it will not be shown at all, accompanied by a warning on the CLI.

[kvid]

@martinrieder wrote:

Fine, the option is called hide_disconnected_pins, so you are hiding the pinhtml table.

I only hide this table when it's empty to avoid the Graphviz error.

This produces different results for "simple" connectors, since these are treated separately.

• Please provide a YAML input with only a "simple" connector that produces different results with this fix than with v0.4 to prove your statement.
• Simple connectors doesn't show the pin table, so my fix doesn't affect simple connectors - they produce the same output as with v0.4.

It is a bit edgy, but it works.

I chose to not output an empty table that produced an error. Another choice could be to insert a dummy cell in empty tables, but that would increase the space of the connector node. Is that better? Please suggest alternatives.

Thanks for providing a fix!

connectors:
  X1:
    style: simple
    hide_disconnected_pins: true
connections:
  - - X1

• The hide_disconnected_pins: true has no effect on a simple connector because it doesn't show the pin table at all.
• The collapsed size of this connector node is due to an empty type attribute. If you add type: " " or some other non-empty text, you can see that the node increases in size.
• All these variations produce the same output with my fix as with v0.4.

connectors:
  X1:
    pincount: 1
    show_name: false
    show_pincount: false
    hide_disconnected_pins: true
connections:
  - - X1

This does not collapse in size due to some white-space in the otherwise empty cell in the connector table. I can remove that white-space, but It'll require code changes several places in the code instead of only at one single place. Another way to make the output of a simple connector more like a connector with all pins hidden, is to insert the similar white-space in the simple connector case, but I haven't tested that alternative for side-effects.

I don't want to make too much out of this fix because PR #251 will change all this code very soon anyway.

Note: If the connector is not referenced in the connections section, then it will not be shown at all, accompanied by a warning on the CLI.

That is as designed. See the syntax description about this.

[martinrieder]

This produces different results for "simple" connectors, since these are treated separately.

• Please provide a YAML input with only a "simple" connector that produces different results with this fix than with v0.4 to prove your statement.

I did exactly this, but the collapsed simple connector is easy to miss on a white background:

I chose to not output an empty table that produced an error. Another choice could be to insert a dummy cell in empty tables, but that would increase the space of the connector node. Is that better? Please suggest alternatives.
[...]

• The collapsed size of this connector node is due to an empty type attribute. If you add type: " " or some other non-empty text, you can see that the node increases in size.

There is need to have a dummy table: It would be less confusing if the whitespace was internally set as a default. If users wanted to override this, they would have to define none explicitly.

Another way to make the output of a simple connector more like a connector with all pins hidden, is to insert the similar white-space in the simple connector case, but I haven't tested that alternative for side-effects.

I am thinking the same, though it would need to be solved in another issue. There are some more important things to tackle than a "missing whitespace". ;-)

connectors:
  X1:
    pincount: 1
    show_name: false
    show_pincount: false
    hide_disconnected_pins: true
connections:
  - - X1

This does not collapse in size due to some white-space in the otherwise empty cell in the connector table. I can remove that white-space, but It'll require code changes several places in the code instead of only at one single place.

I do not see any benefit in this. Please leave it as is.

I don't want to make too much out of this fix because PR #251 will change all this code very soon anyway.

I am being pedantic, so please don't get me wrong. I just wanted to highlight the corner case by providing this example.

It is a bit edgy, but it works.

I meant to say: The described bug is fixed. End of story. ^^

[kvid]

We seem to agree that the reported bug is fixed, so I will not add more to my fix now. However, I just want you @martinrieder to understand that my fix has absolutely no effect on simple connectors. If you try that YAML input example with WireViz v0.4, you will see an identical collapsed output as with my fixed version. You can also remove the hide_disconnected_pins: true for simple connectors without changing the output in any way.

[martinrieder]

This produces different results for "simple" connectors, since these are treated separately.

• Please provide a YAML input with only a "simple" connector that produces different results with this fix than with v0.4 to prove your statement.

I did exactly this, but the collapsed simple connector is easy to miss on a white background:

@kvid sorry for the confusion, but we are on the same page. Comparing before and after fixing only has no effect on the simple connectors, of course.
This was a misunderstanding because I did not clearly state what is different. I meant to only compare simple vs. standard connectors.
That is out-of-scope for the reported bug, so I took the idea to insert some whitespace by default (for simple connectors) over here: #223 (comment)