search

diagrams / diagrams-graphviz

Graph layout and drawing with GraphViz and diagrams
BSD 3-Clause "New" or "Revised" License
9 stars 4 forks source link

Allow for drawing of vertices over edges. #5

Closed GregorySchwartz closed 6 years ago

GregorySchwartz commented 6 years ago

Adds a new function to draw vertices over edges rather than the opposite, and corrects the types for drawGraph and getGraph.

byorgey commented 6 years ago

Thanks for the PR! I agree the behavior with drawing edges over vertices or vice versa needs to be configurable. I merged this since it's a reasonable first cut, but I wonder if we can talk about ways to reduce the code duplication. e.g. maybe we could split out a function which produces not just a diagram, but a pair of lists of diagrams (representing the vertices and edges)? Then for convenience functions like drawGraph can just put them all on top of each other, but if you want more control you can layer them yourself. What do you think?

GregorySchwartz commented 6 years ago

Can you think of a use case where only the vertices or edges would be needed that wouldn't be addressed with the functions to draw the edge path or draw the vertex diagram? If so then splitting out a function is good, but if not then I don't see why it can't be a edges on top == True switch (or the like).

byorgey commented 6 years ago

Maybe not, I was just brainstorming. So right, another way to do it would be to have drawGraph' take a Bool saying whether edges should be on top, and then redefine drawGraph to just call drawGraph' True.

GregorySchwartz commented 6 years ago

Although I said to use a switch, I always hesitate with a Bool though, because drawGraph' True isn't very descriptive. I have no problem with a Bool, but is it possible to have a good documentation of that type, like type EdgesOnTop = Bool or even a data GraphLayering = EdgesOnTop | VerticesOnTop or something like that.

byorgey commented 6 years ago

Ah, good point. data GraphLayering = EdgesOnTop | VerticesOnTop makes sense.