-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
bugfix/fix-kroki-fetch-diagram-attribute-in-js-version #453
base: master
Are you sure you want to change the base?
bugfix/fix-kroki-fetch-diagram-attribute-in-js-version #453
Conversation
…es the disk cache file if available
I believe that
👍🏻
No worries. |
Thank you the sample document would be: = Title
:safe: unsafe
:data-uri:
:allow-uri-read:
:kroki-fetch-diagram:
== Subtitle
[plantuml]
----
@startuml
Bob -> Alice : Request
Alice -> Bob : Response
@enduml
---- I am using the latest VSCode and AsciiDoc extension. The only setting applied is enabling kroki. if I replace |
I don't think it will fix this issue. spawn(process.argv[0], ["-e", execString]) require('child_process').execSync(`"${process.execPath}" ...`) It works relatively well in a Node environment, since I haven't try Enrik Berkhan solution but I believe it's the only way to fix this issue. |
Thank you for your detailed description. But after the changes in this pull request I was able to retrieve diagram files and have them embedded in the final HTML output and the export is single standalone HTML file again. Currently I am using these modifications on my local VSCode AsciiDoc plugin. Before these changes, I needed to export using the Ruby version of AsciiDoctor to have an standalone HTML file. |
Interesting... I will give it a try 👍🏻 |
I can confirm that The issue is that the file is written at the root of the repository but we try to load the image from the "output directory". Then it's working! Anyway, what we should do is embed images as data URI if As a next step, we should try to find a solution to make it work in preview environment (such as VS code) when |
Thank you As a matter of fact I always set the
So I get best performance in preview. |
I think if I don't set the
Oh, I didn't know that data-uri is not working in VSCode. Because I did use it before to embed external images inside final HTML export and if I didn't use it, the result HTML just keep a URL to its location |
Reading local files from VS code is not supported by Asciidoctor.js since we need to use a |
I have merged your changes in #454 thanks again for the fix! |
Thank you so much. I read your final merged code to learn more about the internal functionality of the system and also how to write the required tests. I have only two questions.
Because I saw that no file is ever created and so every time all the diagrams should be fetched again, which really takes quite some times on big documents that normally contains no changes in most of diagrams. I tested the behavior with the Ruby version and it also creates fetched images in the I also test it with my original pull request in VSCode environment:
Thanks |
…th if no dataUri is requested for diagram fetching
If it is of any help, I also updated my code to remove extra parameter in save function and also return the full file path of the file in case of no |
We support both but in VS code
The above document used to throw an error in VS code so we've decided to forcibly disable
No, we are reading the content and converting it to base64 in memory.
When using |
Yes its is correct that
I also think that is correct, but what about when Thank you |
Yes, in my exprience: This is the sample file (test.ad):
and the command to run is: asciidoctor -r asciidoctor-diagram test.ad This creates a picture file: (diag-plantuml-md5-98ade79bc4f3c15dbb15304e0097be58.png) and a HTML file with the image data base64 encoded in it. |
I also tested the asciidoctor-kroki directly (instead of asciidoctor-diagram) and the result is:
This is the sample file (test.ad):
and the command to run: asciidoctor -r asciidoctor-kroki test.ad |
It makes more sense to write files on disk when |
Yes it looks like that "Asciidoctor Diagram" does that with only |
Sorry I lost track of this pull request. Can we close it or do we still need to do something? |
Hello, Sorry I thought you might be busy, so I didn't bother anymore But I see that after the changes made here, it has conflicts with current codes |
Fixes the kroki-fetch-diagram attribute in NodeJS Version and also use the disk cache file if available