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

KOGITO-616: DMN Tooling produce single line source #3073

Merged
merged 2 commits into from Dec 20, 2019

Conversation

manstis
Copy link
Member

@manstis manstis commented Dec 13, 2019

See https://issues.redhat.com/browse/KOGITO-616

@tiagobento I'm up for suggestions how we can avoid the duplication when running the DMN kogito editor in the kogito-tooling wrapper.. Is there anyway I can check if your cachedXsltProcessor is available/defined from JavaScript and re-use it?

@jomarko
Copy link

jomarko commented Dec 13, 2019

Tests? One showing number of "\n" is not zero would made me happier.

@manstis
Copy link
Member Author

manstis commented Dec 13, 2019

Hi @jomarko I've added the test requirement to https://issues.redhat.com/browse/KOGITO-406

The change on this PR is to a JavaScript file that I cannot easily (simply) test with a regular unit test.

It needs a the same environment as the integration tests provided by #3069

@tiagobento
Copy link
Contributor

@manstis @jomarko Hi guys! Have you seen that? We actually do a very similar thing inside kogito-tooling infrastructure.

https://github.com/kiegroup/kogito-tooling/blob/master/packages/kie-bc-editors/src/DefaultXmlFormatter.ts

@manstis
Copy link
Member Author

manstis commented Dec 13, 2019

Yes I know @tiagobento that is where this was lifted from!

However we want to format the XML before it gets to the kogito wrapper as it is useful for our tests. So, when we're running the client-side DMN editor outside of VSCode or GitHub. I was asking if you know how I can - through JS - discover whether cachedXsltProcessor is defined from within the DMN editor itself.. If it is defined I can assume the code is running in the kogito wrapper and hence do not format it - as it if then formatted by the kogito wrapper. If it is undefined I can assume the code is running naked and perform the formatting.

@tiagobento
Copy link
Contributor

@manstis Oh sorry, I totally overlooked the description. Well, since the Resource Content API PR was merged, we have means to expose JS properties/methods to the editors. I think we could provide a global "formatXml" method, or something like that, so that the editor uses what we (kogito-tooling) provide.

The problem is that is just that when you're testing the editor on the -testing or the -runtime modules, you'll not have kogito-tooling providing you anything, so I don't see how we could avoid duplication in this scenario :(

@manstis
Copy link
Member Author

manstis commented Dec 13, 2019

@tiagobento I know we can do little to avoid the duplication of code.. unless we remove formatting from the kogito wrapper and move it solely to the editors themselves. However I wanted to remove duplicate processing; i.e. the editor converts String->DOM->XLST->String and the same is then done by the kogito wrapper.

@tiagobento
Copy link
Contributor

@manstis Right. In that case, I think you could create a flag in your standalone -testing webapp and check if it's set before formatting. When the editor is running inside VSCode, no one will have set that flag, so you can skip your formatting and let kogito-tooling do it. WDYT?

@tiagobento
Copy link
Contributor

@manstis Actually, now that I think about it, you can create a JS file with the formatting method and add that to the index.html of your -testing/-runtime webapps. If the method exists, you use it and format the XML. If we never fetch this JS file inside kogito-toolin (by not adding it to the resources list on the GwtRoutes file), this method will never be available. WDYT?

Copy link

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

One question, even it is do not merge state

@manstis
Copy link
Member Author

manstis commented Dec 17, 2019

@jomarko @tiagobento PR updated to move XML formatting to a new file that is only included in -webapp-kogito-runtime. @jomarko I've not included the file in -webapp-kogito-runtime so accessing the XML from gwtEditorBeans.get("DMNDiagramEditor").get().getContent(); will return unformatted XML (as this is handled by @tiagobento kogito wrapper).

Copy link

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

Took longer time to build vs code extension. but works fine. Thank you

@tiagobento
Copy link
Contributor

@jomarko Why the sad face? :(

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento @jomarko emoji is a "confused face".. I don't think my explanation was very clear!

@manstis manstis merged commit 7252e0a into kiegroup:master Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants