-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Ruby: rename attribute env-idea… #198
Conversation
didn't change JavaScript elements, therefore I plead "not guilty" for the failing JavaScript builds. |
58f3c3b
to
aa135f4
Compare
rebased, checks are now green. |
Indeed, we should rename this attribute. |
This would change the semantics, as now several formats would need to be supported, including the feature that the extension would need to know which format is support by what backend, see the following code comment:
New proposal: I'd rather implement the This will not be a perfect fit for the IntelliJ plugin; on the other hand the IntelliJ platform is switching to JCEF that support SVGs properly. The automatic switching to PNG caused confusion in some places before. WDYT? |
Sounds good, less is more 👍
So your plan is to use |
aa135f4
to
fbaa6f3
Compare
Hi @Mogztter - I finally completed the rework for this. I'd be happy if you could have a look. And yes, I plan to use |
Looks good. The commit message should be something like: "Remove env-idea logic (use kroki-default-format instead)", right? I can update it when merging but I want to make sure that I fully understand the change. |
The env-idea attribute leaked from an early version of this library that was used in an IntelliJ environment,and doesn't make sense in the more general usage of this library. kroki-default-format has already been implemented in the JavaScript version of this library.
fbaa6f3
to
d40db8d
Compare
Yes, you're right. I updated the description, keeping the short "what" you suggested in the first line, and putting a longer "why" in the second part of the commit message. |
…to kroki-force-png as this attribute name is leaking an implementation details from an early version of this plugin.
In fact, this was only true for the JavaFX preview. The new JCEF preview supports SVG preview.