6636-custom-editor: support for CustomEditor API

[danarad05]

Signed-off-by: Dan Arad dan.arad@sap.com

Mainly developed by @EstherPerelman.

What it does

Fix #6636

Adds a prototype of custom editors contributed by extensions with this functionality:

• Adds a new contribution point for custom editors.
• Adds API for registering a custom editor providers.
• Implements CustomEditor extension API - based on VSCode (excluding backup functionality not implemented in this PR).
• Adds CustomEditorWidget extending WebviewWidget containing a model reference to CustomEditorModel.
• Supports two CustomEditorModel implementations: CustomTextEditorModel for text documents and MainCustomEditorModel for binary documents.
• Registers openHandlers for CustomEditors.
• Adds openWith command for selecting which editor to use when openning a resource.
• Adds Undo/Redo functionality for CustomEditors.

CQ: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23017

How to test

1. Clone & build custom editor sample repo https://github.com/microsoft/vscode-extension-samples/tree/master/custom-editor-sample
2. Run as Hosted Plugin in theia
3. Open/Edit/Save pawdraw & cscratch file types under exampleFiles folder.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[caponetto]

Hi everyone,

I'm one of the developers of Kogito Tooling [1]. You can check our VS Code extensions at [2] and [3].

We have been using the custom webview editor API from VS Code since the proposed API
and we have also been helping them with feedback.

I was very thrilled to know that Theia will also support such an API, so I'd like to share some thoughts.

From the branch of this PR, I was able to build everything up and run our extensions very easily.

In fact, it was very impressive seeing our extensions working seamlessly in Theia without any changes.

I can confirm that operations like save, save as, save all, redo, undo, etc are working as expected.

However, I couldn't find the Revert File that you would normally find under the File menu in VS Code.
Am I missing something?

Another two things are related to the Source Control tab:

• When you open a file associated with a custom editor to see the diff, VS Code opens 2 custom editors.
  On Theia (this PR), however, I see text editors being opened for these files.
  Is it something that should be implemented in the future?
  I ask because seeing the differences between 2 files is better when using the custom editor instead of text.

• When I try to Discard Changes of an empty file associated with our custom editor, I see:

command.ts:300 Uncaught (in promise) Error: The command 'file.delete' cannot be executed. There are no active handlers available for the command. (args: [{"codeUri":{"$mid":1,"path":"/home/guilherme/dev/kiegroup/kogito-examples/dmn-quarkus-example/test.bpmn","scheme":"file"}}])

I'm not sure if this is something at our end though it doesn't reproduce on VS Code, but I'd like to let you know.

And that's all.

If you need any help testing this, please let me know.

Great job!

[1] https://github.com/kiegroup/kogito-tooling
[2] https://marketplace.visualstudio.com/items?itemName=redhat.vscode-extension-dmn-editor
[3] https://marketplace.visualstudio.com/items?itemName=redhat.vscode-extension-bpmn-editor

[marcdumais-work]

Dev-meeting entry added, to make committers aware of this PR.

[danarad05]

#8910 (comment)
Thanks @caponetto for reviewing and testing out the PR.

In regards to your comments:
1. I couldn't find the Revert File
Revert File is not implemented in this PR. Please open issue for that implementation if you want.

2. Theia opening 2 text editors and not custom editors
This will not be implemented in this PR.

3. When I try to Discard Changes of an empty file associated with our custom editor,
I tried myself and Discard changes seemed to work for me and I didn't get this error. Could you elaborate or add screen capture of how you reproduced this error?
Thanks

[azatsarynnyy]

I'm going to look at it this week.

[marcdumais-work]

@danarad05 Can you please resolve the conflicts and push a new version?

[danarad05]

@azatsarynnyy Thanks
@marcdumais-work Will do. Thanks

[azatsarynnyy]

I did some basic checks with the VS Code sample extension https://github.com/microsoft/vscode-extension-samples/tree/master/custom-editor-sample

Both of the contributed CustomEditor and CustomTextEditor are working as expected.
I plan to continue to review/test it more.

[danarad05]

@danarad05 Can you please resolve the conflicts and push a new version?

Done

[marcdumais-work]

@danarad05 @amiramw

Since SAP is an Eclipse Foundation member company, I think we do not need to register a CQ for this substantial contribution (> 1000 LoC). Can you please look at the official flowchart(eclipse legal process pdf) and confirm - I think this contribution would fit under the figure 2 case, correct? If so can you also answer the question on figure 7? :

Confirm that the Contribution:
1. Contains No Cryptography
2. Is Developed from Scratch (without
incorporating content from
elsewhere) – Contact the EMO if
you aren’t sure.
3. Is 100% Project License Code

Note: this CQ for substantial contributions is independent of any CQ we might need. e.g. for copied code or new, as of yet unapproved runtime dependencies.

[azatsarynnyy]

Also, I've checked it with Red Hat BPMN/DMN extensions, mentioned in #8910 (comment) - all the basic functionality works well 👍

[vince-fugnitto]

I have a few general comments regarding the code (I have not yet tested the functionality). As mentioned by Marc (https://github.com/eclipse-theia/theia/pull/8910/files#r561122885) we will need a CQ which makes sure to cover all the copied code before it can be accepted 👍 (I believe Amiram is handling it).

As for the pull-request itself, it is quite a significant change (~3000 lines) and it would be good to better document the functionality in both the commit message (which is currently empty), and the pull-request description.

[amiramw]

@danarad05 @amiramw

Since SAP is an Eclipse Foundation member company, I think we do not need to register a CQ for this substantial contribution (> 1000 LoC). Can you please look at the official flowchart(eclipse legal process pdf) and confirm - I think this contribution would fit under the figure 2 case, correct? If so can you also answer the question on figure 7? :

Confirm that the Contribution:
1. Contains No Cryptography
2. Is Developed from Scratch (without
incorporating content from
elsewhere) – Contact the EMO if
you aren’t sure.
3. Is 100% Project License Code

Note: this CQ for substantial contributions is independent of any CQ we might need. e.g. for copied code or new, as of yet unapproved runtime dependencies.

@marcdumais-work this is also my understanding.

We confirm that this contribution does not contain Cryptography, is developed from scratch and 100% project license code.
We will create CQ for the vs code inspired code.

[jsrosman]

Thanks, Good to see the Custom editor API will be available in master branch soon as well. We will use the API in our current project and using the https://github.com/danarad05/theia/tree/6636-custom-editor branch in the meantime.

[azatsarynnyy]

I reviewed the code - all my notes were addressed.
Also, I tested the functionality in #8910 (review) and #8910 (review). Everything works as expected.
There're some missing parts of CustomEditor API that aren't implemented in this PR, but I'd say it's okay to implement them later, as needed.

[danarad05]

Thank you @azatsarynnyy

@marechal-p, @vince-fugnitto - addresssed your issues - please review again. Thanks

[vince-fugnitto]

I performed another round of review, please be sure to address my comment about the commit title and message: #8910 (review)

In addition, please add a link to the CQ for this pull-request in the pull-request description so that committers are able to track the state of the copied code.

[danarad05]

I performed another round of review, please be sure to address my comment about the commit title and message: #8910 (review)

@vince-fugnitto updated commit message and PR description. Also fixed all your review items. Please review again. Thanks.

[marcdumais-work]

@amiramw Thanks for creating the CQ. For your information, ATM the CQ is waiting for your action. You added the copied code as atachment, then you need to "return" the CQ to the IP team by clicking button "issues addressed, return CQ to IPTeam".

Until you do, no one will look at it. Thanks!

[pgirishibm]

@danarad05 Would these changes help with the jupyter notebook support in eclipse che? We are currently seeing the below error when we try the Jupyter extension from https://marketplace.visualstudio.com/items?itemName=ms-toolsai.jupyter in eclipse che.

When do we expect the above PR to be merged?

[danarad05]

@pgirishibm

@danarad05 Would these changes help with the jupyter notebook support in eclipse che?

I think they might yes, but you should check with them.

When do we expect the above PR to be merged?

hopefully next version.

[danarad05]

@vince-fugnitto Thank for latest review. Fixed 2 of them and please see comment on third #8910 (comment).
Thanks.

[jsrosman]

We validated the new API in 3 different custom editors. That is working fine as expected. When can we expect a merge to master? @danarad05 do we require more approvers and approvals?

Thanks @jsrosman for reviewing. Yes we do need more reviewers/approvers. Are you a reviewer in Theia?

Thanks you @danarad05. We are just consumers of the API, we are not contributing to Theia right now, so no approval rights now. But we are looking forward to an formal Theia release containing the CustomEditor API because we have some dependencies now as we are expecting this API will be in as soon there are enough approvals. Please go ahead.

[vince-fugnitto]

I verified the functionality of the cat-draw custom-editor and it works very well (both browser and electron) 👍

[danarad05]

@RomanNikitenko
Thanks for review. resolved your convs. Please check again. Thanks

@vince-fugnitto
Please see reply in #8910 (comment). Other issues were resolved

[danarad05]

@RomanNikitenko @vince-fugnitto completed latest review changes. Please check again. Thanks

[vince-fugnitto]

The changes look good to me, thank you for your effort and patience @danarad05 👍
I'll let @RomanNikitenko review as well if he'd like.

[RomanNikitenko]

My comments were addressed, but I didn't test the changes.
I do not want to hold the PR and there are 2 approval, so Dan please merge the changes at will.
Thank you!

[amiramw]

Merged. Thanks eveyone for the review!