add NodeID class, used by quote and quote_edge

[bayersglassey-zesty]

Draft PR for now; the tests pass (including the new ones), but I didn't update the docs, and the docstring on NodeID could probably use some tightening up!

[bayersglassey-zesty]

@xflr6 If there is no interest in this approach, shall I close the PR?

[xflr6]

Thanks for the proposal and sorry for the delay.

I still think that that allowing singleton tuples to triples would be a good way to introduce this new feature. Maybe we can discuss why you disprefer it? E.g., I don't think it's nice to require the user to use a custom class.

It's a bit more explicit, and allows you to pass "fully-qualified" node IDs (including port + compass) to node(), which wasn't previously possible. (And is also totally useless, because node statements ignore ports and compasses. But it's nice to be consistent.)

If compass and port in node statements are ignored, I am not sure why we would expose this possibility to the user.

[xflr6]

Can you clarify why we would apply escaping here, rather than doing all of the conversion in one place?

[bayersglassey-zesty]

No, I don't recall.
It looks like escape returns an instance of NoHtml, not actually an instance of str.
So perhaps...
...hmmm...
No, I have no idea. :) Maybe it's better to do escaping elsewhere, then!

[xflr6]

Did you consider creating a list and using ':'.join()?

[bayersglassey-zesty]

No, I find this equally readable. Obviously either way is equivalent, so I wouldn't mind if it were switched to the other way.

[xflr6]

Do we need to include these details in the documentation for the user?

[bayersglassey-zesty]

No, I added all these details because I'm not 100% sure of myself, thus the "as far as I can tell, ...".
If this PR was to be accepted, I'd want to figure out whether that was definitely the case, and remove much of this comment (leaving behind a smaller, stronger statement -- just this: "The port and compass are ignored, so those 3 node statements are equivalent.")

Or whatever. Anyway, I tend to leave long comments in code, but it would be fine to leave them out instead.

[bayersglassey-zesty]

I still think that that allowing singleton tuples to triples would be a good way to introduce this new feature.

This PR is from some time ago, but I believe the reason I went with a custom class was because then I can control its __str__.
That is, when I use the graphviz library, I usually hold node IDs in strings. However, unless you remember to escape the strings before storing them, they can end up getting "mangled" when quote is applied to them later on.
So I thought that it might be nicer, as a user of the library, to store explicit NodeId objects instead:

graph = Digraph()

node_ids = {}

# Generate node ids, add nodes to graph
for thing in list_of_stuff:
    node_id = NodeId(...build unique id from thing...)
    node_ids[thing] = node_id
    graph.node(node_id)

# Add edges to graph
for thing in list_of_stuff:
    for linked_thing in thing.links:
        id1 = node_ids[thing]
        id2 = node_ids[linked_thing]
        graph.edge(id1, id2)

...that's the pattern I usually use, except usually I don't have a NodeId class, just raw strings.
...but I guess I could just use escape in place of NodeId. 🤔

So okay, maybe we don't need NodeId or even tuples; it's enough to let user know about escape.
But supporting tuple instead of raw string would be vaguely more user-friendly.