Skip to content
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

fix c4plantuml url #121

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arukiidou
Copy link
Contributor

No description provided.

Signed-off-by: junya koyama <[email protected]>
@@ -90,7 +90,8 @@ document.addEventListener('DOMContentLoaded', function () {
var diagramType = selectDiagramElement.value
var source = diagramSourceElement.value
if (diagramType && source && source.trim() !== '') {
var urlPath = diagramType + '/svg/' + btoa(pako.deflate(textEncode(source), { level: 9, to: 'string' }))
var urlDiagramType = diagramType === 'c4plantuml' ? 'plantuml' : diagramType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, both URLs are working. Any reason why you want to use plantuml instead of c4plantuml?

Copy link
Contributor Author

@arukiidou arukiidou May 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include keywords such as !include <C4/C4_Context> are
available in plantuml-stdlib and not limited to C4.

example:!include <awslib/AWSCommon>


Using the c4plantuml may mislead users into believing that they cannot use plantuml-stdlib other than c4plantuml.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see... I think we should discuss this change in https://github.com/yuzutech/kroki
Since C4 is now part of PlantUML, we might consider removing/deprecating the c4plantuml endpoint. As a first step, we could update the documentation to mention that it's recommended to use plantuml endpoint (and then we could indeed use /plantuml in the try section when C4 PlantUML is selected).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants