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

Log service override #251

Merged
merged 25 commits into from
Nov 20, 2023
Merged

Log service override #251

merged 25 commits into from
Nov 20, 2023

Conversation

CGNonofr
Copy link
Contributor

Extract it from OutputService

also a few related fixes/changes

Loïc Mangeonjean added 25 commits November 17, 2023 12:32
…a file on the MkdirpOnWriteInMemoryFileSystemProvider
when the editor part is restored, it will try to restore the test.js from the previous session and the file doesn't exist yet
@CompuIves
Copy link
Collaborator

Nice PR! Does this also allow us to use more exports from VSCode directly?

Copy link
Collaborator

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

Couple of minor questions!

src/service-override/environment.ts Show resolved Hide resolved
src/service-override/extensions.ts Show resolved Hide resolved
@CGNonofr
Copy link
Contributor Author

Nice PR! Does this also allow us to use more exports from VSCode directly?

Missed that comment!

What do you mean?

@CompuIves
Copy link
Collaborator

Not entirely certain, I thought something had changed in Rollup exports, but I think I was wrong. Approved it now as it looks good!

@CGNonofr
Copy link
Contributor Author

The difference in the rollup build is that previously, service-overrides that were imported by services.ts as default were included in monaco-vscode-api directly. Now their code is in their dedicated package and monaco-vscode-api has them as dependencies

@CGNonofr
Copy link
Contributor Author

What I was trying to fix is that, as soon as a file is used at 2 different places, it was included in monaco-vscode-api to not be duplicated in the 2 sub-libraries including it.

The problem with that is if a service-override A was including another one B, the A service-override was considered used at 2 different places and included in monaco-vscode-api.

Now, B has A as dependency and monaco-vscode-api should be lighter

@CGNonofr CGNonofr merged commit ed9045e into main Nov 20, 2023
1 check passed
@CGNonofr CGNonofr deleted the fix-logs branch November 20, 2023 16:02
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