ocamlgraph icon indicating copy to clipboard operation
ocamlgraph copied to clipboard

Improve the (graphviz) dot parser

Open arbipher opened this issue 2 years ago • 0 comments

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).

arbipher avatar Nov 26 '22 04:11 arbipher