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

replace setModelMarkers with onDidChangeMarkers in onValidate useEffect #244

Conversation

a-sam-randall
Copy link
Contributor

Hi @suren-atoyan and team,
I have been using this library and I ran into issue #182. I was able to fix this issue by using monaco.editor.onDidChangeMarkers so I have replaced the onValidate in this library with my fix.

Please let me know if you have any questions or would like to me to make any changes.

@suren-atoyan
Copy link
Owner

Hello @a-sam-randall

thank you for your support!

I remember there was a well-known issue with onDidChangeMarkers, I'll check it and come back to you

@suren-atoyan
Copy link
Owner

ah, I see, the onDidChangeMarkers comes with version 0.22.0. Before, we had only getModelMarkers as a "normal" way to get the markers. And there were issues with that (actually they are not issues, we just call getModelMarker in the onDidModelChange - at the point when the new markers are not generated yet, due to their async nature). That's why we had to use some tricks and backflips to active it. Now, it's possible to handle it with onDidChangeMarkers, I'll examine it and your code and let you know. Thank you! 🙂

@a-sam-randall
Copy link
Contributor Author

@suren-atoyan have you had a chance to review this pull request yet?

@suren-atoyan
Copy link
Owner

I'll do it today; sorry for late the answer

@suren-atoyan
Copy link
Owner

@a-sam-randall could you please fix the merge conflict that I can merge the PR

@a-sam-randall
Copy link
Contributor Author

@suren-atoyan the conflict has been resolved.

@suren-atoyan suren-atoyan merged commit 5a30800 into suren-atoyan:master Aug 7, 2021
@suren-atoyan
Copy link
Owner

@a-sam-randall I've checked and tested your changes; it's almost ready to be released as a new version. However, there is a small question; could you check this?

@suren-atoyan
Copy link
Owner

@a-sam-randall it's already here. Please check the latest version - v4.2.2

Thank you for your support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants