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

Updated to monaco-vscode-api 1.79.2 / monaco-editor 0.39.0 #503

Merged
merged 5 commits into from
Jun 16, 2023

Conversation

kaisalmen
Copy link
Collaborator

@kaisalmen kaisalmen commented Jun 13, 2023

Still WIP.
Even without adding new services, I see this error now:
image

Is this already fixed by this: CodinGame/monaco-vscode-api#128 ?

@kaisalmen
Copy link
Collaborator Author

@CGNonofr I guess it makes sense to release a next version with every big monaco-vscode-api PR and test it here beforehand as well.

@CGNonofr
Copy link
Collaborator

Is this already fixed by this: CodinGame/monaco-vscode-api#128 ?

Probably not! I'll have a look

@CGNonofr
Copy link
Collaborator

Ok it's fixed in CodinGame/monaco-vscode-api@824f51b

@kaisalmen
Copy link
Collaborator Author

Ok it's fixed in CodinGame/monaco-vscode-api@824f51b

@CGNonofr thank you, I will test it later ..

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jun 14, 2023

First problem is gone. This error shows up if I add the markers service. Does it depend on something else? Here I added everything, but the new terminal service. Does this even make sense without using the view service? If I don't configure the markers and terminal service there is no error.
image

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jun 14, 2023

@CGNonofr FYI I just copied over your TerminalBackend from the demo and then I get two errors (from markers and terminal):
image

That's just a local test. The code is not pushed up.

@CGNonofr
Copy link
Collaborator

First problem is gone. This error shows up if I add the markers service. Does it depend on something else? n't configure the markers and terminal service there is no error.

I guess it won't work without the view services, but the markers service override just adds the Problems view so it has no point anyway

Here I added everything, but the new terminal service. Does this even make sense without using the view service?

No. The marker service override doesn't add any service for the moment, it just register the marker view

FYI I just copied over your TerminalBackend from the demo and then I get two errors (from markers and terminal):

THAT is weird, I'll have a look

@CGNonofr
Copy link
Collaborator

It's fixed and published as @codingame/monaco-vscode-api@1.79.3-next.3

@kaisalmen
Copy link
Collaborator Author

@CGNonofr markers service with views service enabled, but not "renderPanelPart" set. I guess this is expected:
image

@CGNonofr
Copy link
Collaborator

@CGNonofr markers service with views service enabled, but not "renderPanelPart" set. I guess this is expected: image

Is it an issue? what would you want?

@kaisalmen
Copy link
Collaborator Author

I wanted to know if that is your expectation, too. I think that this is the correct behaviour.

@kaisalmen
Copy link
Collaborator Author

If markers is configured without views enabled this now throws an error

@CGNonofr
Copy link
Collaborator

Maybe we can improve the doc but importing markers without the views make no sense

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jun 15, 2023

Maybe we can improve the doc but importing markers without the views make no sense

Yes, docs should be updated. I added this code: https://github.com/TypeFox/monaco-languageclient/blob/mva-1.79.2/packages/client/src/monaco-vscode-api-services.ts#L171-L191 to flag / produce errors on bad configuration.

@kaisalmen
Copy link
Collaborator Author

@CGNonofr 1.79.3-next.4 works fine. 👍
I can confirm that when I add a panel and use markers with views service there are no more errors.

@kaisalmen
Copy link
Collaborator Author

@CGNonofr are you ok with changes. I will update the version to 6.2.0 after merge.

@kaisalmen kaisalmen marked this pull request as ready for review June 16, 2023 15:02
@kaisalmen kaisalmen requested a review from CGNonofr as a code owner June 16, 2023 15:02
@kaisalmen
Copy link
Collaborator Author

Oh it was still a draft. Version change is in here already.

Updated CHANGELOG and README
@kaisalmen
Copy link
Collaborator Author

@CGNonofr you raised a thumb, but you have to approve. Thank you

@kaisalmen kaisalmen merged commit fb941be into main Jun 16, 2023
2 checks passed
@kaisalmen kaisalmen deleted the mva-1.79.2 branch June 16, 2023 15:17
@kaisalmen
Copy link
Collaborator Author

I went ahead. Want to publish the release. 😊

@CGNonofr
Copy link
Collaborator

It was still a draft at that time :)

@kaisalmen
Copy link
Collaborator Author

Sorry, I overlooked that 🙂

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.

2 participants