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

Introduce terminal contributions and make links one #175921

Merged
merged 14 commits into from
Mar 3, 2023
Merged

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Mar 2, 2023

This change introduces the concept of terminal contributions (that's right, contrib/terminalContrib/!) which are modeled after editor contributions with the main goal being to simplify and reduce the size of TerminalService and TerminalInstance. This is a natural extensions of the work done in #175785 and #175790, just contributions make managing the lifecycle of objects attached to instances easier.

One neat feature of the new terminal contributions is that imported from contrib/terminalContrib is disallowed in the main terminal code, to prevent accidental changes that go against the new architecture.

Before:

  • TerminalInstance owns a TerminalLinkManager
  • TerminalService owns link providers
  • TerminalService registers link providers on TerminalInstances
  • MainThreadTerminalService registers a link provider to TerminalService

After:

  • TerminalInstance doesn't know about links
  • TerminalLinkContribution owns TerminalLinkManager
  • TerminalLinkProviderService owns link providers
  • TerminalLinkContribution pulls link providers from TerminalLinkProviderService and registers them on TerminalLinkManager
  • MainThreadTerminalService registers a link provider to TerminalLinkProviderService

@Tyriar Tyriar added this to the March 2023 milestone Mar 2, 2023
@Tyriar Tyriar self-assigned this Mar 2, 2023
@Tyriar Tyriar requested a review from meganrogge March 2, 2023 19:06
@Tyriar Tyriar marked this pull request as ready for review March 2, 2023 19:06
@Tyriar Tyriar marked this pull request as draft March 2, 2023 20:36
@Tyriar
Copy link
Member Author

Tyriar commented Mar 2, 2023

Need to work out hygiene/layering problems

@Tyriar Tyriar marked this pull request as ready for review March 2, 2023 22:20
Copy link
Contributor

@meganrogge meganrogge left a comment

Choose a reason for hiding this comment

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

👏🏼

@Tyriar Tyriar merged commit c1b38af into main Mar 3, 2023
@Tyriar Tyriar deleted the tyriar/links_contrib branch March 3, 2023 15:42
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants