-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add href attribute to connectors, cables, and additional_bom_items #168
base: dev
Are you sure you want to change the base?
Conversation
I like this feature! |
IMHO, this is a good candidate for merging soon, after #163, or before, if that one requires additional discussion. There shouldn't be any conflicts between the two. |
|
Take your time with this, it's not critical.
Would it make sense to turn this around? #164 was made before the idea of a separate Example from #164 (alternatives to # Example 7: Crossover Cable
connectors:
X1:
type: '<a href="$URL">Stewart Connector SS-37000-002</a>'
subtype: male
pinlabels: [DA+,DA-,DB+,DC+,DC-,DB-,DD+,DD-] # pincount is implicit in pinout
url: https://www.digikey.com/product-detail/en/stewart-connector/SS-37000-002/380-1098-ND/1033365
X2:
type: '<a href="$URL">Stewart Connector SS-37000-002</a>'
subtype: male
pinlabels: [DB+,DB-,DA+,DD+,DD-,DA-,DC+,DC-]
url: https://www.digikey.com/product-detail/en/stewart-connector/SS-37000-002/380-1098-ND/1033365 TBH, having a separate The HTML BOM could have a column that just shows the text If someone really needs the full URL visibly listed as text inside the BOM, a new global harness parameter could take care of that; something like |
If desired, referencing multiple URLs could be achieved by allowing I see this as an edge case, however. |
FYI, GraphViz HTML supports links inside tables, not as separate
I would be OK for including clickable links within the GraphViz output, under the condition that they are discrete, and don't look ridiculous when, for example, printing the diagram, rendering them useless. So, having some discrete 🔗 icon or similar in the corner is OK; having a separate cell with the word 'Link` in it is not. Just leaving it here as an additional idea. [Edit] Sorry if you feel this should go in a separate issue/PR... let me know. |
It seems that Graphviz then uses the href value from a previous edge!
I noticed you mentioned this PR in #189 (comment), but it is still marked as draft. So I'm not sure if you want me to take a closer look already, or if you're still working on it :) Either way, simply change the draft status and drop a comment when it's ready! No pressure, just wanting to avoid any misunderstanding. |
I've edited my comment you are referring to and made it more clear that I don't expect any review + merge-in before I complete this draft and mark it ready for review. It's still easy to rebase this branch onto the current |
Can you elaborate on this? Do you mean something like an additional attribute to enable/disable "smart" splitting of URLs in to lines? If breaking apart long URLs is the only thing blocking this PR, I would suggest just printing URLs in one line for now, and tackling the issue in a separate PR. In addition, please consider the following ideas. Bear in mind that they are not 100% thought out, it's just a spontaneous thought. Idea 1 Idea 1.1 |
What I have in mind, is whether it would be better to let the user choose between text that might contain linebreaks that he wants to be translated to Either this choice can be a global option as in #158, or maybe per attribute like this: mpn:
html: '<a href="...">00893-11</a><br/>(temporary out of stock)'
pn:
plain: "43-456-700\n(limited local stock)" The concept above also open up for more than one alternative per attribute, e.g. both HTML alternative and non-HTML alternative, and even one alternative for each of the outputs (gv, html, tsv, etc.) if that could be useful for anyone. Another way, could be to create a rule for an automatic choice, saying text containing any tag
Long URLs in the BOM is a separate issue that can be postponed into a separate issue as you suggest.
Alternatively a global option.
Also possible. I have to think about this one. |
Implement second method in #165