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

custom-editors: Fix the issue: Auto Save on Binary Custom Editor does not work correctly #11599

Merged
merged 3 commits into from
Sep 16, 2022
Merged

custom-editors: Fix the issue: Auto Save on Binary Custom Editor does not work correctly #11599

merged 3 commits into from
Sep 16, 2022

Conversation

safisa
Copy link
Contributor

@safisa safisa commented Aug 22, 2022

What it does

Fixes: #11460.
Fix an issue where auto save on custom editor does not work correctly.

How to test

  1. enable the auto-save
  2. Use the vscode cats example and open the binary file example.pawDraw
  3. do a change, and make sure that the file was saved
  4. make sure that the save is not called recursively, you can see it by adding a breakpoint in the custom-editors-main.ts file using dev-tools, on the method change of the MainCustomEditorModel class.

Review checklist

Reminder for reviewers

Signed-off-by: Safi Seid-Ahmad [email protected]

@safisa safisa changed the title Fix the issue: Auto Save on Custom Editor does not work correctly custom-editors: Fix the issue: Auto Save on Binary Custom Editor does not work correctly Aug 22, 2022
@safisa
Copy link
Contributor Author

safisa commented Aug 24, 2022

Hi @vince-fugnitto @msujew
Can you review this PR?

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Aug 24, 2022

Hi @vince-fugnitto @msujew Can you review this PR?

@safisa do you mind sharing the custom-editor-sample you used, I see the following locally (not able to actually draw):

Screen Shot 2022-08-24 at 1 42 05 PM

@vince-fugnitto vince-fugnitto added the custom-editor issues related to custom-editor functionality label Aug 24, 2022
@safisa
Copy link
Contributor Author

safisa commented Aug 25, 2022

Hi @vince-fugnitto

First, thank you for the review.
Actually, I am using a custom editor that I can't share here, since it requires many other dependencies. I also checked the vscode cats pawDraw example (this uses the binary custom editor), and there is an issue in the example since it uses API that is not supported in Theia (vscode.workspace.fs.isWritableFileSystem - there is already an opened issue on this).
I did some changes in the example by removing the isWritableFileSystem API, but still had an issue in loading the image in the pawDraw.js file...
Anyway, now you can use the attached updated example. When opening the example.pawDraw file, you will get a blank canvas with no image (image load issue!), and then start drawing, you will see that the file becomes dirty, and on save you will see that the file on disk is updated (check file timestamp or size), but when reopened you will start from blank again because of the image load issue.

If you enable the auto-save, and open and edit the file as above, before this PR, the save event will keep being invoked infinitely. you can check it by testing the timestamp of the file on disk, or putting a breakpoint in the pawDraw.js file inside the message event (getFileData) or in the Theia custom-editors-main.ts file, on the method change of the MainCustomEditorModel class.
After this PR fix the auto-save works as expected.
I need this fix since we are suing many custom editors in our App.

Thanks in advance.

cat-customs-0.0.1.vsix.zip

@vince-fugnitto
Copy link
Member

@safisa can you please fix the git history (seems to be issues after rebasing) and ping me when it's ready for another review?

@safisa
Copy link
Contributor Author

safisa commented Aug 30, 2022

Hi @vince-fugnitto
Fixed the git history, now there is only the 2 files I have changed

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed the bug on master and that it is correctly fixed by the updates 👍

CHANGELOG.md Outdated Show resolved Hide resolved
@safisa
Copy link
Contributor Author

safisa commented Sep 15, 2022

Hi @vince-fugnitto
This PR is already approved, is it going to be merged before the next Theia version?

@vince-fugnitto vince-fugnitto merged commit 3a5172a into eclipse-theia:master Sep 16, 2022
@vince-fugnitto vince-fugnitto added this to the 1.30.0 milestone Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
custom-editor issues related to custom-editor functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto Save on Binary Custom Editor does not work correctly
2 participants