WireViz icon indicating copy to clipboard operation
WireViz copied to clipboard

[feature] Add tweak option per node

Open kvid opened this issue 1 year ago • 6 comments

I suggest adding a tweak option for each node (both cables and connectors). It has been on mind many times, but was now triggered again by @martinrieder, and I create a new issue to avoid cluttering the original issue where such a feature would be helpful.

@martinrieder wrote:

[...] My suggestion on this would take some universal approach: allow the tweak to be defined beneath any cable or connector. Their designators could then automatically be referenced, even for auto-generated ones.

I totally agree. It has been on my mind many times since I wrote my suggestion above, but I haven't given priority to diving into the graphviz package to find how to inject my tweak code for each node. Todays tweak code is very simple using search and replace on the total .gv text output from the graphviz package. To do it per node, it will probably require injecting my code earlier in the process, while each node is generated.

Note that it is possible to define multiple subgraphs with the same name. These will be merged by Graphviz, which allows appending designators to clusters.

That I haven't tried. That sounds like very useful in combination with tweak per node. I have been thinking of adding my own per node specifyer for being included into a common group, but what you describe might be a lower hanging fruit.

Originally posted by @kvid in https://github.com/wireviz/WireViz/issues/270#issuecomment-2116041655

kvid avatar May 18 '24 10:05 kvid

I wrote:

[...] To do it per node, it will probably require injecting my code earlier in the process, while each node is generated.

I just realized that I might have been making this problem more complicated than needed. It should be sufficient to accumulate tweaks from each node in the harness and join everything before executing all at once with the total .gv output as we do today.

We just need to find a proper placeholder for the node designator, and replace all usage of this placeholder within each node tweak with the real node designator before joining everything. Such a placeholder should be a short text that is highly unlikely someone wants literally in tweak texts - preferably something illegal to use in the .gv syntax. Otherwise, we can simply add an optional tweak:placeholder so the user must specify the placeholder text when needing one. This latter alternative is also backwards compatible.

Are there also other attributes of a cable or connector that would be useful to have placeholders for? I am mainly thinking about attributes that are not easily known by the YAML author.

kvid avatar May 18 '24 11:05 kvid

That sounds still rather complicated. The tweaks would be inserted inside the [ ] brackets before or after the label=<... > HTML table. It comes right after each designator node definition. Why would you want to do any search and replace for this?

Another way that should work, but I have not tested is simply using the same designator multiple times for each tweak. They should be merged by GraphViz in the same fashion that subgraphs are merged when they have identical names.

martinrieder avatar May 18 '24 21:05 martinrieder

@kvid Great to see this implemented! Now I understand how you intended to do that search & replace, though I still do not understand what the placeholder would be needed for. I think that it might make sense to have a prefix to an auto-generated designator though... What is the advantage of having a user-defined ID here?

Considering that we use prefixes instead of placeholders, then it could be used to implement the grouping through clusters. This could be a solution to #325 then, if the prefix itself could also be used literally without being replaced. Something like:

GROUP//ID

Because Graphviz has C-style comments, it would ignore everything after // as long as it is not an explicit string .

subgraph clusterGROUP//ID 
{ "GROUP//ID" }

Should work, though I cannot test it currently.

martinrieder avatar May 23 '24 09:05 martinrieder

That sounds still rather complicated. The tweaks would be inserted inside the [ ] brackets before or after the label=<... > HTML table. It comes right after each designator node definition. Why would you want to do any search and replace for this?

I meant that the tweaks would be applied after the calls to dot.node here https://github.com/wireviz/WireViz/blob/issue349tweak/src%2Fwireviz%2FHarness.py#L292-L298 and here https://github.com/wireviz/WireViz/blob/issue349tweak/src%2Fwireviz%2FHarness.py#L555-L561.

The way that you implemented it applies all the tweaks in a final step. This allows some debugging by comparing before and after tweaking, if needed.

martinrieder avatar May 23 '24 09:05 martinrieder

@kvid wrote:

Are there also other attributes of a cable or connector that would be useful to have placeholders for? I am mainly thinking about attributes that are not easily known by the YAML author.

Yes and no. I could think of placeholders for metadata like user, file and folder name that a user might like to add in a text label. Date and time stamps are also something useful, though I see that YAML might provide tags for this purpose. I guess that we should discuss this elsewhere.

Note that YAML tags could be used to generate random designators/placeholders!

martinrieder avatar May 23 '24 09:05 martinrieder

@martinrieder wrote:

@kvid Great to see this implemented! Now I understand how you intended to do that search & replace, though I still do not understand what the placeholder would be needed for.

Any presence of the placeholder text will be replaced with the node instance designator. The only reason for making the user select such a placeholder text instead of having a hard coded placeholder, is to avoid the edge case that such a hard coded text is needed literally in any tweak attribute.

See also my newly extended https://github.com/wireviz/WireViz/issues/270#issuecomment-1029535877 for a use case with example YAML input using a placeholder.

I think that it might make sense to have a prefix to an auto-generated designator though...

What do you need a prefix for?

What is the advantage of having a user-defined ID here?

See the upper part of this comment. The user is able to choose a placeholder that doesn't conflict with the other tweak text.

Considering that we use prefixes instead of placeholders, then it could be used to implement the grouping through clusters. This could be a solution to #325 then, if the prefix itself could also be used literally without being replaced. Something like:

GROUP//ID

Because Graphviz has C-style comments, it would ignore everything after // as long as it is not an explicit string.

subgraph clusterGROUP//ID 
{ "GROUP//ID" }

I still don't understand how this will work.

Should work, though I cannot test it currently.

kvid avatar May 24 '24 00:05 kvid