asciidoctor / asciidoctor-kroki

Asciidoctor.js extension to convert diagrams to images using Kroki!
https://kroki.io/
MIT License
147 stars 50 forks source link

"invalid group specifier name" on MacOS Safari #225

Closed romkavt closed 3 years ago

romkavt commented 3 years ago

Hello!

asciidoctor-kroki.js version 0.12 On Safari 14.0.3 with MacOS 10.15.7 (Catalina) the PlantUML diagrams doesn't render with the following error:

[Error] SyntaxError: Invalid regular expression: invalid group specifier name
    (anonymous function) (asciidoctor-kroki.js:18656)

Thanks for help in advance

ggrossetie commented 3 years ago

Safari does not support lookbehind in regular expressions (sigh): https://caniuse.com/js-regexp-lookbehind

https://github.com/Mogztter/asciidoctor-kroki/blob/05623c9ea0bbf29c11bad848ee529f8694b9f2f3/src/preprocess.js#L106

To accommodate Safari we should rewrite this regular expression to avoid using lookbehind. It might require to do additional checks.

ggrossetie commented 3 years ago

@romkavt The line number does not correspond but the error tells me that this is related to a regular expression. Do you have a more complete stacktrace ?

secdim commented 3 years ago

@Mogztter can we get this fix for Safari merged in?

ggrossetie commented 3 years ago

@secdim If you could confirm that https://github.com/Mogztter/asciidoctor-kroki/pull/228 actually resolves this issue in Safari that would be helpful. I don't have Safari so I can't make sure that my fix is sufficient.

romkavt commented 3 years ago

@secdim If you could confirm that #228 actually resolves this issue in Safari that would be helpful. I don't have Safari so I can't make sure that my fix is sufficient.

I merged this 'by hands' in our installation and I confirm the fix works well on Mac OS Safari.

ggrossetie commented 3 years ago

Thanks for confirming, I will merge the pull request!

secdim commented 3 years ago

@secdim If you could confirm that #228 actually resolves this issue in Safari that would be helpful. I don't have Safari so I can't make sure that my fix is sufficient.

Yes, I have tested it and it fixes Safari issue.

ggrossetie commented 3 years ago

@secdim Thanks for confirming, I will release a new version today.

ggrossetie commented 3 years ago

Version https://github.com/Mogztter/asciidoctor-kroki/releases/tag/v0.14.0 has been released 🎉