nidi3 / graphviz-java

Use graphviz with pure java
Apache License 2.0
937 stars 107 forks source link

Parser does not honour cluster naming convention #112

Closed Gwaptiva closed 5 years ago

Gwaptiva commented 5 years ago

First off, a great thank you for a wonderful tool. I may not be a fan of the builder-style API, (probably too dumb to use it), so I build a string to feed the parser, and then manipulate the resultant object when I have to.

Here a suggestion for a small improvement. Not a bug, not critical either.

Parsing a DOT file/string like digraph L { subgraph cluster_root { .... } }

The parser correctly creates me a MutableGraph with a nested MutableGraph, but the nested graph's isCluster method teturns false.

I realise that clusters aren't a DOT/Graphviz standard, but they are listed in the language spec as a convention. Would be nice if the parser honoured this.

Thanks,

Jaap

nidi3 commented 5 years ago

A little thing, but a bug never the less. It's fixed in version 0.8.7.

Gwaptiva commented 5 years ago

I'm sure you like low hanging fruits as much as the next one. Thanks for the quick turnaround

Gwaptiva commented 5 years ago

Hmm, looking at your solution: by stripping out the 'cluster_' from the name, will the Graphviz engine still recognise it as a cluster? or does that ignore it anyway?

nidi3 commented 5 years ago

It will be added before feeding it to graphviz: https://github.com/nidi3/graphviz-java/blob/ed139d24830c3f75cd1353a3a6d0f45a13e4da9b/graphviz-java/src/main/java/guru/nidi/graphviz/model/Serializer.java#L82-L84