Add possibility to use custom graphviz implementation

[PavelTurk]

Fixes #2008

[PavelTurk]

I've checked with JPMS service. In module-info I added:

provides net.sourceforge.plantuml.dot.Graphviz with com.foo.TempGraphviz;

and my service provider was called.

[arnaudroques]

That's a nice move. I'm curious to see where this takes us :-)

However, I suggest that we work on a dedicated branch for this https://github.com/plantuml/plantuml/tree/custom_graphviz

Could you please update your pull request accordingly?

[PavelTurk]

Could you please update your pull request accordingly?

Done.

[arnaudroques]

com.foo.TempGraphviz;

Are you considering committing this (or other classes) to the codebase?

[PavelTurk]

Are you considering committing this (or other classes) to the codebase?

No, we will create our own project that uses graphviz-java, but it will be under ASL. The reason is that we don’t want to be tied to PlantUML and prefer to make releases whenever we need.

But that’s exactly the beauty of the solution I’m proposing. From now on, everyone will be able to use PlantUML with the specific version of Graphviz that suits them. Isn’t that wonderful and doesn’t it provide incredible flexibility to PlantUML?

For example, we are working on a program that draws graphs. Everyone who downloads the program should have the ability to use this feature. Of course, telling everyone to install a specific Graphviz version isn’t a great approach. We tried Smetana, but it has some unpleasant bugs. Today, I tested ELK and PlantUML/Vizjs. Vizjs threw a bunch of errors. ELK worked, but the results are still not as good as with Graphviz. That’s why we decided to use graphviz-java.

[arnaudroques]

No, we will create our own project that uses graphviz-java, but it will be under ASL.

Understood, no problem. Tell us when you will have an URL for this project!

The reason is that we don’t want to be tied to PlantUML and prefer to make releases whenever we need.

That makes perfect sense.

But that’s exactly the beauty of the solution I’m proposing. From now on, everyone will be able to use PlantUML with the specific version of Graphviz that suits them. Isn’t that wonderful and doesn’t it provide incredible flexibility to PlantUML?

Absolutely! We’re ready to merge your pull request into the main branch. However, I do have a concern. The interface net.sourceforge.plantuml.dot.Graphviz is not particularly well-designed at the moment. It is subject to changes in the future, which could potentially break your integration.

On a related note, we’ve been considering refactoring parts of our code to make it cleaner. Specifically, we might take advantage of java.util.ServiceLoader to improve some of our existing implementations.

For example, we are working on a program that draws graphs. Everyone who downloads the program should have the ability to use this feature. Of course, telling everyone to install a specific Graphviz version isn’t a great approach. We tried Smetana, but it has some unpleasant bugs. Today, I tested ELK and PlantUML/Vizjs. Vizjs threw a bunch of errors. ELK worked, but the results are still not as good as with Graphviz. That’s why we decided to use graphviz-java.

Thanks for sharing! Since we’re on the topic of Graphviz implementations, you might want to explore https://github.com/jamisonjiang/graph-support
It’s a minimalist Graphviz re-implementation in pure Java with almost no external dependencies. This might be worth considering for your use case.

[PavelTurk]

Understood, no problem. Tell us when you will have an URL for this project!

Ok. But I will do it firstly as simple as possible, trying to spend minimum time.The main thing is that it does what we need

Absolutely! We’re ready to merge your pull request into the main branch. However, I do have a concern. The interface net.sourceforge.plantuml.dot.Graphviz is not particularly well-designed at the moment. It is subject to changes in the future, which could potentially break your integration.

Yes, I've also noticed that there are some things to discuss, for example, graphviz244onWindows(), as I understand.
However, If you later modify this interface it is not problem and in Service world it can happen.This is the responsibility of the person who assembles the binary distribution.

Now, I am thinking - maybe it is better to load not Graphviz, but GraphvizFactory. I think it will provide more flexibility. I mean:

Iterator<GraphvizFactory> iterator = ServiceLoader.load(GraphvizFactory.class).iterator();
if (iterator.hasNext()) {
    Graphviz graphviz = iterator.next().create();
    Log.info("Using service provider " + graphviz.getClass().getName());
    return graphviz;
}

What do you think? I can make PR.

On a related note, we’ve been considering refactoring parts of our code to make it cleaner. Specifically, we might take advantage of java.util.ServiceLoader to improve some of our existing implementations.

I absolute don't know PlantUML from developer side (I am only PlantUML user), but in our projects we use JPMS and JPMS services. We separate API from IMPL on module level and it works fine. For example, see our small library - ansi4j

Thanks for sharing! Since we’re on the topic of Graphviz implementations, you might want to explore https://github.com/jamisonjiang/graph-support It’s a minimalist Graphviz re-implementation in pure Java with almost no external dependencies. This might be worth considering for your use case.

Thank you for your wonderful project. I like Smetana and I am sure that with time it will be as good as graphviz.

Thank you for your suggestion about graph-support. I know this project and even opened several issues. It is very good that new java libraries related to graph generation are emerging

[arnaudroques]

Now, I am thinking - maybe it is better to load not Graphviz, but GraphvizFactory. I think it will provide more flexibility. I mean:

Iterator<GraphvizFactory> iterator = ServiceLoader.load(GraphvizFactory.class).iterator();
if (iterator.hasNext()) {
    Graphviz graphviz = iterator.next().create();
    Log.info("Using service provider " + graphviz.getClass().getName());
    return graphviz;
}

What do you think? I can make PR.

Yes, this does seem like a more flexible and improved solution.
Please feel free to create a new PR or update the current one. We look forward to reviewing your contribution!

graphviz244onWindows()

This is quite outdated at this point and should probably be removed entirely.
We’ll handle the removal, but we’d prefer to wait for your PR first to streamline the process.