-
Notifications
You must be signed in to change notification settings - Fork 1
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
Enhance grammar to support attribute mapping from sources to target #45
Conversation
7735d88
to
43c0d44
Compare
- Provide mapping grammar for source objects, joins and target mapping - Add new element creation for mappings in toolbar and context menu - Adapt custom serializer for new attributes - Add example for mapping Refactorings: - Split grammar into multiple files - Replace id and id references with unquoted text - Fix auto completion in text files through completion lexer and parser - Enhance auto-completion for string, number, and boolean
43c0d44
to
f4dd129
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completed the other PR and rebased this branch onto latest main where I had to solve some conflicts. I think I did it the right way, but maybe you can double check.
I reviewed the code, where I put some comments on.
I also ran the tests and I see some Jest tests are failing. Could you have a look at these?
Also when I start CrossModel now the diagrams are not working anymore.
examples/mapping-example/ExampleCRM/relationships/Order_Customer.relationship.cm
Outdated
Show resolved
Hide resolved
- Only export root nodes and entity attributes for external reference - Properly check exported ids and local ids
- Workspace initialization was cancelled when a document was opened
@harmen-xb Thank you for your review! I addressed all your concerns in a follow up commit. |
@martin-fleck-at Thanks for the update, sorry it took a while for me to have time to look at it. Some observations: Everything works fine if I only have one workspace open, but when I first open the examples/yaml-example workspace and then the e2e-tests/src/resources/sample-workspace, in the yaml-example workspace things stop working for me. I think it depends in which order I open the workspaces which one stops working. In this example I first openend yaml-example, and it all worked fine. Then I opended e2e-tests/src/resources/sample-workspace in the same browser on a second tab and things stopped working for the yaml-example workspace, while it works fine in the sample-workspace. Examples within the yaml-example workspace:
When I close and re-open the perspectives the within that single workspace the problems remain. |
- Create one ModelServiceImpl per frontend connection - Improve connection reporting for server clients - Replace temporary workspace files with port commands -- Query command contributed by extension -- Extension forwards command execution to language server
@harmen-xb Good catch! I think that was a problem that was always present but with the new commit everything should work much more stabler. |
@martin-fleck-at Thanks for the update! It all works quite well, functionaly I don't see any issues, but I do get error message in the console while opening a workspace:
|
@harmen-xb Yeah, I saw that too but unfortunately it is a log statement that automatically happens in Theia, see https://github.com/eclipse-theia/theia/blob/c30a79f90d9393e392cabfebbce612695cbe0d40/packages/core/src/common/messaging/proxy-factory.ts#L177. Even though it re-throws the error and we catch and ignore it the log already happens. If it is a big issue, I can definitely see how to best fix it. |
It's not a big issue, it's just something I didn't see before. And the console was all nice and clean lately :). |
@harmen-xb If everything else works, could you please approve the PR so we can merge it or is there anything else I should have a look at? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes all look good. For the mentioned issue in the comment I will make an issue on repo level. Is there an issue in Theia already for this port error message?
@martin-fleck-at Yes, PR is merged. The port error message should be an issue on the Theia repo I assume? |
@harmen-xb Yes, that is correct. |
@harmen-xb I opened a ticket on the Theia side for it: eclipse-theia/theia#13189 |
Refactorings:
Please note this PR is based on #44 to avoid conflicts but only the last commit serves the functionality described above. I'd suggest to merge the other PR first and then this one after a rebase.