-
-
Notifications
You must be signed in to change notification settings - Fork 15
Feat: Add plantuml server rendering #603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
'$plantumlServerUrl', | ||
'%guides.graphs.plantuml_server%', | ||
) | ||
->alias(DiagramRenderer::class, PlantumlServerRenderer::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now this is not configurable, the configuration options are there, but I need to add a service locator that allows you to register any class.
@@ -54,7 +54,7 @@ public function loadExtensionConfig(string $extension, array $config): void | |||
|
|||
$extensionAlias = $this->registeredExtensions[$extensionFqcn] ?? false; | |||
if (!$extensionAlias) { | |||
$this->registerExtension(new $extensionFqcn()); | |||
$this->registerExtension(new $extensionFqcn(), $config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wouterj I think you missed this when implementing the extension config? Otherwise only default extensions can be configured, not the ones configured in the guides.xml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right 👍
I think I originally intended this without the early return on the next line (so we "register" the extension without config, and then add extra config). But this works as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is a good start!
@jaapio should we merge this? Further open issues can be adressed afterwards |
This patch makes is possible to load configuration for extensions that are not loaded by default. This gives us the option to add configuration to the graph extension. By now this extension supports the options to use an remote plantuml server to render uml diagrams.
5d822c3
to
4857cfe
Compare
This patch makes is possible to load configuration for extensions that are not loaded by default. This gives us the option to add configuration to the graph extension.
By now this extension supports the options to use an remote plantuml server to render uml diagrams.