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.

Sign up for GitHub

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

Jump to bottom

Various output channel-related fixes #8476

Merged
merged 2 commits into from
Sep 15, 2020
Merged

Various output channel-related fixes #8476

merged 2 commits into from
Sep 15, 2020

Conversation

kittaakos
Copy link
Contributor

What it does

  • Clear the text modification queue when disposing the output channel,
  • Exposed private OutputChannel properties with protected visibility.
  • From now on, downstreams can customize the output channel instance creation with a template method.
  • Removed the superfluous channel caching for VS Code extensions. Instead of erroneously keeping an extra cache of output channels for the extensions, we delegate into Theia's singleton channel manager via the command API. (Closes [output][vscode][plugin] Output channels are not cleaned up after closing the channel #8122)

How to test

  • This is where it gets tricky, I am way behind with the Theia versions. I still use @theia/languages.

Review checklist

Reminder for reviewers

Akos Kitta added 2 commits September 7, 2020 09:22
Clients can customize channel creation.
bfd24a1 
 - Fixed text-model is disposed error when
starting/restarting LS.
 - Exposed the channel properties with protected visibility.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Removed superfluous channel caching for extensions
d3e7922 
Instead of keeping an extra cache of channels for VS Code extensions,
we delegate the channel  manager one via commands.

Closes #8122

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos
Copy link
Contributor Author

@akosyakov, please take a look and provide some upfront feedback. Thanks!

@kittaakos kittaakos marked this pull request as ready for review September 7, 2020 14:56
@vince-fugnitto vince-fugnitto added the output issues related to the output label Sep 8, 2020
@kittaakos
Copy link
Contributor Author

@benoitf, as the original committer of the output-channel-registry-main.ts module, please review.

benoitf
benoitf approved these changes Sep 11, 2020
View reviewed changes
Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

sorry @kittaakos I'm not the original author https://github.com/theia-demo-plugins/wiptheia/blame/b9a2ee833b201ae0c07f4245ab75060be02f1d6c/packages/plugin-ext/src/main/browser/output-channel-registry-main.ts

but plugin's output is there
image

@kittaakos
Copy link
Contributor Author

sorry @kittaakos I'm not the original author https://github.com/theia-demo-plugins/wiptheia/blame/b9a2ee833b201ae0c07f4245ab75060be02f1d6c/packages/plugin-ext/src/main/browser/output-channel-registry-main.ts

Uhh, sorry for the confusion, @benoitf. GH has tricked me.

@benoitf, as the original committer

output

Thank you for looking into it. 👍

@kittaakos
Copy link
Contributor Author

I am going to leave it open until next week, then merge it if there is no objection.

@kittaakos kittaakos merged commit 525b4e6 into master Sep 15, 2020
@kittaakos kittaakos deleted the fix-text-model-disposed branch September 15, 2020 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benoitf benoitf benoitf approved these changes

@akosyakov akosyakov Awaiting requested review from akosyakov

@vince-fugnitto vince-fugnitto Awaiting requested review from vince-fugnitto

Assignees
No one assigned
Labels
output issues related to the output
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[output][vscode][plugin] Output channels are not cleaned up after closing the channel
3 participants
@kittaakos @benoitf @vince-fugnitto