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

Remove dependencies pulled in when writing to output channels #8733

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

westbury
Copy link
Contributor

@westbury westbury commented Nov 8, 2020

What it does

The changes to add scroll lock to the output view added a large number of dependencies to OutputChannelManager. While the scroll locking is a great improvement and much appreciated, the extra dependencies make it not possible for extenders to send logging output to the Output view. An attempt to inject OutputChannelManager into MessageClient causes a circular dependency. OutputChannelManager depends on QuickPickService which depends on QuickOpenService (bound to MonacoQuickOpenService), which depends on KeyboardLayoutProvider (implemented by BrowserKeyboardLayoutProvider) which depends on ILogger.

The dependencies are caused by the command registrations. The dependencies can be removed by moving the command registrations to the same place where other output channel commands are registered. This also removes the command registration from 'common'.

How to test

Ensure that no behavior is changed. All commands are accessible exactly as before.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added extensibility issues to simplify ability to extend Theia output issues related to the output labels Nov 10, 2020
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

👍

@westbury westbury merged commit dbc2cc8 into master Nov 11, 2020
@github-actions github-actions bot added this to the 1.8.0 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extensibility issues to simplify ability to extend Theia output issues related to the output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants