ocamlgraph
ocamlgraph copied to clipboard
Improve the (graphviz) dot parser
The dot parser src/dot.ml contains small glitches, and I am happy to fix them if it sounds good to you.
My starting point was to add support for Equal of id * id
defined in dot_ast.mli but not handled in the dot parser. e.g. label="diagram_label"
bgcolor=red
bgcolor=lightgrey
label="cluster_1"
in this examples.
digraph D {
label="diagram_label";
bgcolor=red;
node [color=green];
edge [penwidth=3];
subgraph cluster_1 {
bgcolor=lightgrey;
label="cluster_1";
}
}
These graph attributes
are ignored by the current parser. It makes it difficult to provide in the val graph_attributes: t -> DotAttributes.graph list
that is used in module DotOutput = Graphviz.Dot(Display)
.
This Equal
binds attributes to the wrapping top-graph / subgraph. Then I realized there is no data structures to hold these attributes, which takes a subgraph as key.
In #129, I added such a structure graph_hash
and followed the approach of the existing code, using Map
internally and converting to a list before returning. (Is it necessary?)
I wonder how much change is welcomed to the existing code. Then I can change my PR, or just modify on my own branches. I understand the subtlety comes from graphviz contains nodes, edges, and (sub)graphs while graph in OCamlGraph contains only nodes and edges.
Here are my other questions:
Q1. val get_subgraph : V.t -> DotAttributes.subgraph option
(in Graphviz.Dot.X
) is not well documented. I can make one from the result of my parse_all
but it still looks tedious to me.
Q2. subgraph doesn't have to be named as cluster_<id>
but it's assumed by print_nested_subgraphs
(line 529). (but to accommodate this, I have to believe the subgraph must follow this convention in my #129
Q3. For node and edge attributes in dot file, a subgraph inherits attributes from its parent graph, however, changes inside a subgraph won't affect the nodes after the subgraph. (See my example dot file. I think the current implementation in the dot parser is incorrect because it uses one mutable node_attr
. (An immutable map is easier to handle this case in the scenario).